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

Problems caused by __SOCKADDR_ARG transparent union in sys/socket.h when _GNU_SOURCE is on #441

Open
mattmccutchen-cci opened this issue Apr 2, 2021 · 0 comments
Labels
priority:3 This labels bugs that are not very critical but still need to be addressed.

Comments

@mattmccutchen-cci
Copy link
Contributor

We started seeing an error in the 3C benchmark tests when we turned on implicit checked header inclusion (#440 and microsoft/checkedc-clang#998). Our icecast benchmark contains the equivalent of the following:

#define _GNU_SOURCE 1
#include <sys/socket.h>

With implicit checked header inclusion, the second line effectively becomes #include <sys/socket_checked.h>, and compiling the above gives:

In file included from socklen-conftest2-work.c:2:
In file included from /home/matt/3c-3.wt/build/lib/clang/11.0.0/include/sys/socket.h:29:
/home/matt/3c-3.wt/build/lib/clang/11.0.0/include/sys/socket_checked.h:41:35: error: interface type only allowed for a declaration with pointer or array type
    __CONST_SOCKADDR_ARG __addr : itype(_Ptr<const struct sockaddr>), 
                                  ^
/home/matt/3c-3.wt/build/lib/clang/11.0.0/include/sys/socket_checked.h:47:29: error: interface type only allowed for a declaration with pointer or array type
    __SOCKADDR_ARG __addr : itype(_Ptr<struct sockaddr> __restrict), 
                            ^

And so forth for the rest of the declarations in sys/socket_checked.h that use __SOCKADDR_ARG or __CONST_SOCKADDR_ARG. (Without implicit checked header inclusion, we didn't see the problem because our #include updating script had a bug and didn't process checked headers in subdirectories such as sys/socket_checked.h: oops.)

When _GNU_SOURCE is off, __CONST_SOCKADDR_ARG is a typedef for const struct sockaddr * (and similarly for __SOCKADDR_ARG), which exactly corresponds to the itype, and everything works as expected. But when _GNU_SOURCE is on, __CONST_SOCKADDR_ARG is a typedef for a magical "transparent union" of several pointer types, including const struct sockaddr *. This is a compiler extension (implemented by at least GCC and Clang) that allows the function to be redeclared later in the translation unit with the parameter type being any one of the members of the transparent union and the compiler will accept it, as an exception to the normal restrictions on redeclaring a function with different parameter types. In this case, I think the goal was to be compatible with user source code that redeclared socket-related functions using certain types similar but not identical to const struct sockaddr *. However, this seems to break the Checked C compiler because the transparent union is not itself a pointer type, so it cannot have a pointer type as its itype.

One way to fix the immediate problem is for sys/socket_checked.h to use const struct sockaddr *, etc. directly instead of __CONST_SOCKADDR_ARG. This theoretically would break existing plain C code that turned on _GNU_SOURCE and relied on the ability to use the other types, though such code would eventually need changes to be ported to Checked C anyway. I tried that approach, but then 3C raised a fatal error, the root cause of which was the two declarations of bind with different unchecked parameter types (the transparent union in the system header and const struct sockaddr * in the Checked C header). 3C's error message in this situation currently isn't great; obviously I knew the error was caused by the sys/socket_checked.h change that I was testing, but that probably wouldn't be clear to an average 3C user who hit the error in a different but analogous situation. So apart from any fix to sys/socket_checked.h, it might be worth considering having the Checked C compiler place additional restrictions on transparent unions or at least provide an API for program analysis tools such as 3C that cannot handle them to disable support for them in the compiler so they will be rejected up front.

For now, our workaround will probably be to patch icecast to turn off _GNU_SOURCE.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority:3 This labels bugs that are not very critical but still need to be addressed.
Projects
None yet
Development

No branches or pull requests

2 participants