Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lib: fix AIX build issues #14464

Closed
wants to merge 8 commits into from
Closed

lib: fix AIX build issues #14464

wants to merge 8 commits into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Aug 8, 2024

  • memdebug: replace keyword malloc with __malloc__ to
    not interfere with envs where malloc is redefined. Also apply
    the fix to alloc_size.
    Fixes:

    lib/memdebug.h:107:13: warning: unknown attribute 'vec_malloc' ignored [-Wunknown-attributes]
    CURL_EXTERN ALLOC_FUNC FILE *curl_dbg_fdopen(int filedes, const char *mode,
                ^~~~~~~~~~
    lib/memdebug.h:37:37: note: expanded from macro 'ALLOC_FUNC'
    # define ALLOC_FUNC __attribute__((malloc))
                                       ^~~~~~
    /usr/include/stdlib.h:753:16: note: expanded from macro 'malloc'
    #define malloc vec_malloc
                   ^~~~~~~~~~
    
  • memdebug: always undef before defining.
    Also do this for the rest of functions redefined in the same block.
    Avoids warning on AIX:

    lib/memdebug.h:117:9: warning: 'malloc' macro redefined [-Wmacro-redefined]
    #define malloc(size) curl_dbg_malloc(size, __LINE__, __FILE__)
            ^
    /usr/include/stdlib.h:753:9: note: previous definition is here
    #define malloc vec_malloc
            ^
    
  • easy: fix -Wformat warning on AIX by adding a cast.

    lib/easy.c:608:47: warning: format specifies type 'int' but the argument has type 'long' [-Wformat]
    "%" CURL_FORMAT_SOCKET_T ")", fds[i].fd);
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~
    
  • if2ip: silence compiler warning inside AIX system header.

    /lib/if2ip.c:219:19: warning: signed shift result (0x80000000) sets the sign bit of the shift expression's type ('int') and becomes negative [-Wshift-sign-overflow]
    if(ioctl(dummy, SIOCGIFADDR, &req) < 0) {
                    ^~~~~~~~~~~
    /usr/include/sys/ioctl.h:401:26: note: expanded from macro 'SIOCGIFADDR'
    #define SIOCGIFADDR (int)_IOWR('i',33, struct oifreq) /* get ifnet address */
                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
    /usr/include/sys/ioctl.h:174:23: note: expanded from macro '_IOWR'
    #define _IOWR(x,y,t) (IOC_INOUT|((sizeof(t)&IOCPARM_MASK)<<16)|(x<<8)|y)
                          ^~~~~~~~~
    /usr/include/sys/ioctl.h:168:20: note: expanded from macro 'IOC_INOUT'
    #define IOC_INOUT (IOC_IN|IOC_OUT)
                       ^~~~~~
    /usr/include/sys/ioctl.h:167:28: note: expanded from macro 'IOC_IN'
    #define IOC_IN (0x40000000<<1) /* copy in parameters */
                    ~~~~~~~~~~^ ~
    

Ref: https://curl.se/dev/log.cgi?id=20240808180420-3809007
Assisted-by: Dan Fandrich
Closes #14464

```
lib/memdebug.h:107:13: warning: unknown attribute 'vec_malloc' ignored [-Wunknown-attributes]
CURL_EXTERN ALLOC_FUNC FILE *curl_dbg_fdopen(int filedes, const char *mode,
            ^~~~~~~~~~
lib/memdebug.h:37:37: note: expanded from macro 'ALLOC_FUNC'
                                   ^~~~~~
```
Ref: https://curl.se/dev/log.cgi?id=20240808180420-3809007
Avoiding warning on AIX:
```
lib/memdebug.h:117:9: warning: 'malloc' macro redefined [-Wmacro-redefined]
        ^
/usr/include/stdlib.h:753:9: note: previous definition is here
        ^
```
Ref: https://curl.se/dev/log.cgi?id=20240808180420-3809007
@dfandrich
Copy link
Contributor

This fixes the compile issue when building with ibm-clang, but I found a gcc on this machine and it doesn't need this workaround. The clang case set a _LINUX_SOURCE_COMPAT macros which triggers creation of the malloc et. al. macros that cause the problem. But, whatever the reason for _LINUX_SOURCE_COMPAT being set, this PR does fix things there. I'm still running a few more compiles to check it out further.

```
lib/easy.c:608:47: warning: format specifies type 'int' but the argument has type 'long' [-Wformat]
"%" CURL_FORMAT_SOCKET_T ")", fds[i].fd);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~
```
Ref: https://curl.se/dev/log.cgi?id=20240808180420-3809007
```
lib/if2ip.c:219:19: warning: signed shift result (0x80000000) sets the sign bit of the shift expression's type ('int') and becomes negative [-Wshift-sign-overflow]
if(ioctl(dummy, SIOCGIFADDR, &req) < 0) {
                ^~~~~~~~~~~
/usr/include/sys/ioctl.h:401:26: note: expanded from macro 'SIOCGIFADDR'
                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/sys/ioctl.h:174:23: note: expanded from macro '_IOWR'
                      ^~~~~~~~~
/usr/include/sys/ioctl.h:168:20: note: expanded from macro 'IOC_INOUT'
                   ^~~~~~
/usr/include/sys/ioctl.h:167:28: note: expanded from macro 'IOC_IN'
                ~~~~~~~~~~^ ~
```
Ref: https://curl.se/dev/log.cgi?id=20240808180420-3809007
@vszakats
Copy link
Member Author

vszakats commented Aug 8, 2024

The #undefs were done already for other redefinitions. I think it's safe to keep.

As for the other, what do you think of this?:

diff --git a/lib/memdebug.h b/lib/memdebug.h
index aba289f4e7..0e12cc27a0 100644
--- a/lib/memdebug.h
+++ b/lib/memdebug.h
@@ -33,7 +33,8 @@
 #include <curl/curl.h>
 #include "functypes.h"
 
-#if defined(__GNUC__) && __GNUC__ >= 3 && !defined(_AIX)
+#if defined(__GNUC__) && __GNUC__ >= 3 && \
+  !(defined(_AIX) && defined(_LINUX_SOURCE_COMPAT))
 /* AIX defines a macro named 'malloc', which breaks the line below */
 #  define ALLOC_FUNC __attribute__((malloc))
 #  define ALLOC_SIZE(s) __attribute__((alloc_size(s)))

@vszakats
Copy link
Member Author

vszakats commented Aug 8, 2024

Hm, this might be most future-proof, if it doesn't have a side-effect:

#if defined(__GNUC__) && __GNUC__ >= 3 && !(defined(_AIX) && defined(malloc))
/* ibm-clang defines _LINUX_SOURCE_COMPAT which in turn defines a macro named
  'malloc', which breaks the line below */

@dfandrich
Copy link
Contributor

Actually, how about #if defined(__GNUC__) && __GNUC__ >= 3 && !defined(malloc) That will fix things on any other strange platform or environment that defined a malloc macro, since it could easily happen outside AIX.

@vszakats
Copy link
Member Author

vszakats commented Aug 8, 2024

Actually, how about #if defined(__GNUC__) && __GNUC__ >= 3 && !defined(malloc) That will fix things on any other strange platform or environment that defined a malloc macro, since it could easily happen outside AIX.

I wasn't brave enough to go that far :) But yes, if there is no side-effect, it seems even better.

The ultimate solution would be if compilers supported the __malloc__ keyword alternative. I can see it used in a GitHub search, but couldn't find it earlier in the gcc manual.

@vszakats
Copy link
Member Author

vszakats commented Aug 8, 2024

Confirmed on godbolt that both __malloc__ and __alloc_size__ are accepted by gcc and clang. (The latter requires gcc 4.4 (possibly 4.3 or 4.2), just like its non-underscored counterpart.)

and also `__alloc_size__`. (requires gcc 4.4 (or 4.3/4.2) like
`alloc_size`.
@dfandrich
Copy link
Contributor

I suppose the simple case of a malloc macro simply redirecting to another function would be fine with this block, so an unconditional !defined(malloc) would be overkill. This PR at commit a1b7743, with the !(defined(_AIX) && defined(malloc)) clause instead, is working with ibm-clang.

FYI, I uploaded an AIX gcc snapshot autobuild log.

@dfandrich
Copy link
Contributor

The PR commit 0b3c791 works fine on ibm-clang 17.1 and gcc 10.3.

@vszakats
Copy link
Member Author

vszakats commented Aug 9, 2024

This still triggers, though a bit differently. Is it possible the cast is missing?:

/curl-8.10.0-20240808/lib/easy.c:607:17: warning: format '%d' expects argument of type 'int', but argument 3 has type 'long int' [-Wformat=]
607 | "call curl_multi_socket_action(socket "
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
608 | "%" CURL_FORMAT_SOCKET_T ")", fds[i].fd);
| ~~~~~~~~~

@dfandrich
Copy link
Contributor

dfandrich commented Aug 9, 2024 via email

@vszakats vszakats closed this in 9cb7f08 Aug 9, 2024
@vszakats vszakats deleted the aix branch August 9, 2024 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

3 participants