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

preload: fix build issue with clang 19 #1572

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

clan
Copy link

@clan clan commented Mar 3, 2025

librdmacm/preload.c:796:9: error: at most one overload for a given name may lack the 'overloadable' attribute

@jgunthorpe
Copy link
Member

That does not seem correct since it broke everything, I don't think attribute overload is the right answer here

@clan
Copy link
Author

clan commented Mar 4, 2025

updated.
remaining error of ci are reported by cgcc, I don't have the environment to test.

BTW: it seems CI check the first commit only? I mean, if commit B is pushed during checking commit A, CI will continue check commit A, and commit B is ignored?

@jgunthorpe
Copy link
Member

That seems like a more believable change, but it probably needs a cmake fragment to detect what the libc is using for it's function signature.

@clan
Copy link
Author

clan commented Mar 4, 2025

That seems like a more believable change, but it probably needs a cmake fragment to detect what the libc is using for it's function signature.

do you mean to detect glibc & musl? or earlier glibc version which don't have __SOCKADDR_ARG?

clan added a commit to clan/gentoo that referenced this pull request Mar 5, 2025
@clan clan force-pushed the clang branch 3 times, most recently from 2e9521b to 3d46188 Compare March 5, 2025 09:02
clan added a commit to clan/gentoo that referenced this pull request Mar 5, 2025
@jgunthorpe
Copy link
Member

I don't know where this is coming from, but the usual way to handle it is to have cmake detect what is correct and then have the C code match it.

@jgunthorpe
Copy link
Member

I suppose the sparse failure means the cmake test is not quite right

@clan
Copy link
Author

clan commented Mar 6, 2025

2025-03-05T09:06:33.0044262Z -- Performing Test HAVE_SOCKADDR_ARG
2025-03-05T09:06:33.0869996Z -- Performing Test HAVE_SOCKADDR_ARG - Success
...
2025-03-05T09:06:43.8696098Z /usr/bin/cgcc -DVERBS_DEBUG -D__CHECK_ENDIAN__ -Drspreload_EXPORTS -I/__w/1/s/build-sparse/include -I/usr/include/libnl3 -I/usr/include/drm -std=gnu99 -Wall -Wextra -Wno-sign-compare -Wno-unused-parameter -Wmissing-prototypes -Wmissing-declarations -Wwrite-strings -Wformat=2 -Wcast-function-type -Wformat-nonliteral -Wdate-time -Wnested-externs -Wshadow -Wstrict-prototypes -Wold-style-definition -Werror -Wredundant-decls -O2 -g -DNDEBUG -fPIC -MD -MT librdmacm/CMakeFiles/rspreload.dir/preload.c.o -MF librdmacm/CMakeFiles/rspreload.dir/preload.c.o.d -o librdmacm/CMakeFiles/rspreload.dir/preload.c.o -c /__w/1/s/librdmacm/preload.c
2025-03-05T09:06:43.8697310Z /__w/1/s/librdmacm/preload.c:807:60: error: expected structure or union
2025-03-05T09:06:43.8708740Z /__w/1/s/librdmacm/preload.c: In function 'recvfrom':
2025-03-05T09:06:43.8709127Z /__w/1/s/librdmacm/preload.c:806:56: error: 'src_addr' is a pointer; did you mean to use '->'?
2025-03-05T09:06:43.8709414Z   806 |                 rrecvfrom(fd, buf, len, flags, src_addr.__sockaddr__, addrlen) :
2025-03-05T09:06:43.8709642Z       |                                                        ^
2025-03-05T09:06:43.8709831Z       |                                                        ->
2025-03-05T09:06:43.8710012Z /__w/1/s/librdmacm/preload.c:807:60: error: 'src_addr' is a pointer; did you mean to use '->'?
2025-03-05T09:06:43.8710237Z   807 |                 real.recvfrom(fd, buf, len, flags, src_addr.__sockaddr__, addrlen);
2025-03-05T09:06:43.8710558Z       |                                                            ^
2025-03-05T09:06:43.8710731Z       |                                                            ->
2025-03-05T09:06:43.8710962Z /__w/1/s/librdmacm/preload.c:798:33: error: parameter 'src_addr' set but not used [-Werror=unused-but-set-parameter]
2025-03-05T09:06:43.8711243Z   798 |                  __SOCKADDR_ARG src_addr, socklen_t *addrlen)
2025-03-05T09:06:43.8716772Z /__w/1/s/librdmacm/preload.c:812:1: error: control reaches end of non-void function [-Werror=return-type]
2025-03-05T09:06:43.8717181Z   812 | }
2025-03-05T09:06:43.8717434Z       | ^
2025-03-05T09:06:43.8717707Z cc1: all warnings being treated as errors

Where can I have the cgcc installed and test? It seems the problem is caused by incorrect preprocess result.

clan added 3 commits March 6, 2025 11:15
librdmacm/preload.c:796:9: error: at most one overload for a given name may lack the 'overloadable' attribute
librdmacm/preload.c:796:9: warning: no previous prototype for function 'recvfrom' [-Wmissing-prototypes]

Signed-off-by: Z. Liu <[email protected]>
Signed-off-by: Z. Liu <[email protected]>
@clan
Copy link
Author

clan commented Mar 6, 2025

    uses with any of the listed types to be allowed without complaint.
    G++ 2.7 does not support transparent unions so there we want the
    old-style declaration, too.  */
-#if defined __cplusplus || !__GNUC_PREREQ (2, 7) || !defined __USE_GNU
+#if 1
 # define __SOCKADDR_ARG                struct sockaddr *__restrict
 # define __CONST_SOCKADDR_ARG  const struct sockaddr *
 #else

find the reason finally, buildlib/sparse-include/*/sys-socket.h.diff, why?

@jgunthorpe
Copy link
Member

I can't remember what the sparse issue is that required that change, but shouldn't cmake should still be able to detect the difference? Hmm. maybe the order of the header patching prevents that from working.
I suggest adjusting your CMake logic to also check HAVE_SPARSE and make the right choice.

@clan
Copy link
Author

clan commented Mar 6, 2025

I can't remember what the sparse issue is that required that change, but shouldn't cmake should still be able to detect the difference? Hmm. maybe the order of the header patching prevents that from working. I suggest adjusting your CMake logic to also check HAVE_SPARSE and make the right choice.

My first intution is that we should do the patch first before run cmake which made the check reasonable rather than depend on some hard coded value.

update: add ${BUILD_INCLUDE} for __SOCKADDR_ARG test so test and spare run based on same source

because the sparse check has sys/socket.h patched

Signed-off-by: Z. Liu <[email protected]>
clan added a commit to clan/gentoo that referenced this pull request Mar 7, 2025
@jgunthorpe
Copy link
Member

that makes sense and looks right

gentoo-bot pushed a commit to gentoo/gentoo that referenced this pull request Mar 10, 2025
patch submitted to linux-rdma/rdma-core#1572

Signed-off-by: Z. Liu <[email protected]>
Closes: #40867
Signed-off-by: Sam James <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants