Details

    Description

      When building MariaDB 10.0.14 using clang 3.3 on FreeBSD amd64, there's a compile error in storage/connect/filamap.h

      107: virtual int GetNextPos(void)

      {return (int)Fpos + Nrec;}

      error: cast from pointer to smaller type 'int' loses information

      int is a 32-bit type this will not fit a memory address of a 64-bit platform. This should probably be handled using a uintptr_t type which should be available on all platforms.

      Attachments

        1. patch-storage_connect_array.cpp
          0.4 kB
          Bernard Spil
        2. patch-storage_connect_filamap.cpp
          0.8 kB
          Bernard Spil
        3. patch-storage_connect_filamap.h
          0.6 kB
          Bernard Spil

        Issue Links

          Activity

            spil Bernard Spil created issue -
            spil Bernard Spil made changes -
            Field Original Value New Value
            Attachment patch-storage_connect_filamap.cpp [ 34410 ]
            spil Bernard Spil made changes -
            Attachment patch-storage_connect_filamap.h [ 34409 ]
            spil Bernard Spil added a comment -

            Patches that enable build on FreeBSD 10.0 clang 3.3

            spil Bernard Spil added a comment - Patches that enable build on FreeBSD 10.0 clang 3.3
            spil Bernard Spil made changes -
            Attachment patch-storage_connect_filamap.h [ 34411 ]
            Attachment patch-storage_connect_filamap.cpp [ 34412 ]
            spil Bernard Spil added a comment -

            And a patch for
            storage/connect/array.cpp:132:26: �error: cast from pointer to smaller type 'int' loses information�

            spil Bernard Spil added a comment - And a patch for storage/connect/array.cpp:132:26: �error: cast from pointer to smaller type 'int' loses information�
            spil Bernard Spil made changes -
            Attachment patch-storage_connect_array.cpp [ 34413 ]
            elenst Elena Stepanova made changes -
            Fix Version/s 10.0 [ 16000 ]
            Assignee Olivier Bertrand [ bertrandop ]
            Labels connect-engine

            This function was wrongly defined but fortunately never used. However, it was fixed (just in case...) as:

            virtual int   GetNextPos(void) {return GetPos() + 1;}

            bertrandop Olivier Bertrand added a comment - This function was wrongly defined but fortunately never used. However, it was fixed (just in case...) as: virtual int GetNextPos(void) {return GetPos() + 1;}
            bertrandop Olivier Bertrand made changes -
            Fix Version/s 10.0.15 [ 17300 ]
            Resolution Fixed [ 1 ]
            Status Open [ 1 ] Closed [ 6 ]
            spil Bernard Spil added a comment -

            Great!
            I attached a patch for connect_array as well which is unrelated to the filamap compile issue I encountered. Would you want me to open another bug for this or can you check this as well?

            spil Bernard Spil added a comment - Great! I attached a patch for connect_array as well which is unrelated to the filamap compile issue I encountered. Would you want me to open another bug for this or can you check this as well?

            Sorry, I closed this before reading your last comments. About your patches:

            in filamap.h:
            Your patches on InitDelete are incorrect. These are virtual functions that must have the same signature as the base class one.

            in filamap.cpp:
            If there is a problem when the function is still defined as InitDelete(PGLOBAL g, int fpos, int spos) I suggest the following patches:

               Fpos = Memory + (uintptr_t)fpos;
               Mempos = Memory + (uintptr_t)spos;

            and the same casting in MPXFAM::InitDelete. This is not necessary with the MS or GCC compilers. Let me know whether it is with clang.

            In array.cpp:
            This patch is also incorrect and would make the program fail or even crash!
            Indeed, the parmp->Value variable is a void pointer but to avoid having to allocate an integer and make it point on it I choosed to directly put the integer value in it. This is tricky and not really correct but was done to save memory allocation.
            It works with MS and GCC that flag the casting (int)parmp->Value as a warning. With clang, the problem is to extract an integer from a pointer. Perhaps (int)(uintptr_t)parmp->Value would work? Let me know whether you have a solution for this. If not, I shall have to redesign the whole thing (perhaps using another stucture with a union in it)

            bertrandop Olivier Bertrand added a comment - Sorry, I closed this before reading your last comments. About your patches: in filamap.h: Your patches on InitDelete are incorrect. These are virtual functions that must have the same signature as the base class one. in filamap.cpp: If there is a problem when the function is still defined as InitDelete(PGLOBAL g, int fpos, int spos) I suggest the following patches: Fpos = Memory + (uintptr_t)fpos; Mempos = Memory + (uintptr_t)spos; and the same casting in MPXFAM::InitDelete. This is not necessary with the MS or GCC compilers. Let me know whether it is with clang. In array.cpp: This patch is also incorrect and would make the program fail or even crash! Indeed, the parmp->Value variable is a void pointer but to avoid having to allocate an integer and make it point on it I choosed to directly put the integer value in it. This is tricky and not really correct but was done to save memory allocation. It works with MS and GCC that flag the casting (int)parmp->Value as a warning. With clang, the problem is to extract an integer from a pointer. Perhaps (int)(uintptr_t)parmp->Value would work? Let me know whether you have a solution for this. If not, I shall have to redesign the whole thing (perhaps using another stucture with a union in it)

            Reopened until you tell me whether my last suggestions work with clang

            bertrandop Olivier Bertrand added a comment - Reopened until you tell me whether my last suggestions work with clang
            bertrandop Olivier Bertrand made changes -
            Resolution Fixed [ 1 ]
            Status Closed [ 6 ] Stalled [ 10000 ]
            spil Bernard Spil made changes -
            Attachment patch-storage_connect_array.cpp [ 34413 ]
            spil Bernard Spil made changes -
            Attachment patch-storage_connect_filamap.cpp [ 34412 ]
            spil Bernard Spil made changes -
            Attachment patch-storage_connect_filamap.h [ 34411 ]
            spil Bernard Spil added a comment -

            Thanks for your guidance Olivier Please find revised patches attached, Mariadb-10.0.14 builds on FreeBSD 10.0 with these patches.

            spil Bernard Spil added a comment - Thanks for your guidance Olivier Please find revised patches attached, Mariadb-10.0.14 builds on FreeBSD 10.0 with these patches.
            spil Bernard Spil made changes -
            Attachment patch-storage_connect_filamap.h [ 34415 ]
            Attachment patch-storage_connect_filamap.cpp [ 34416 ]
            Attachment patch-storage_connect_array.cpp [ 34417 ]

            I applied the new patches successfully on Windows. However, on Linux the GCC compiling failed saying that uintptr_t was undeclared.
            Therefore I added:

            #include <stdint.h>         // for uintprt_h

            into the Unix declare section.
            Is this ok with clang on FreeBSD?

            bertrandop Olivier Bertrand added a comment - I applied the new patches successfully on Windows. However, on Linux the GCC compiling failed saying that uintptr_t was undeclared. Therefore I added: #include <stdint.h> // for uintprt_h into the Unix declare section. Is this ok with clang on FreeBSD?
            spil Bernard Spil added a comment -

            Hi Olivier,
            This is OK without adding an include for stdint.h anywhere in the storage/connect/ code
            Built, packaged and deployed the server without a problem using the attached patches (sorry for the DOS CRLF)

            spil Bernard Spil added a comment - Hi Olivier, This is OK without adding an include for stdint.h anywhere in the storage/connect/ code Built, packaged and deployed the server without a problem using the attached patches (sorry for the DOS CRLF)

            This should fix all compiler issues. However, to do it in a cleaner way, I did not exactly apply the above patches:

            In the InitDelete functions I changed the cast from uintptr_t to ptrdiff_t, which seems more appropriate and does not require including stdint.h.

            About the array making, I have added an union in the PARM structure:

            typedef struct _parm {
              union {
                void *Value;
                int   Intval;
                }; // end union
              short Type, Domain;
              PPARM Next;
              } PARM;

            This makes possible to suppress all the funny casting that were done when creating the structure (in TXTFAM::AddListValue) and retrieving the integer value from it (in MakeValueArray)

            bertrandop Olivier Bertrand added a comment - This should fix all compiler issues. However, to do it in a cleaner way, I did not exactly apply the above patches: In the InitDelete functions I changed the cast from uintptr_t to ptrdiff_t, which seems more appropriate and does not require including stdint.h. About the array making, I have added an union in the PARM structure: typedef struct _parm { union { void *Value; int Intval; }; // end union short Type, Domain; PPARM Next; } PARM; This makes possible to suppress all the funny casting that were done when creating the structure (in TXTFAM::AddListValue) and retrieving the integer value from it (in MakeValueArray)
            bertrandop Olivier Bertrand made changes -
            Resolution Fixed [ 1 ]
            Status Stalled [ 10000 ] Closed [ 6 ]
            serg Sergei Golubchik made changes -
            Fix Version/s 10.0 [ 16000 ]
            bertrandop Olivier Bertrand made changes -
            ratzpo Rasmus Johansson (Inactive) made changes -
            Workflow MariaDB v2 [ 55044 ] MariaDB v3 [ 65142 ]
            serg Sergei Golubchik made changes -
            Workflow MariaDB v3 [ 65142 ] MariaDB v4 [ 148266 ]

            People

              bertrandop Olivier Bertrand
              spil Bernard Spil
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Git Integration

                  Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.