[PATCH v3 0/5] fcntl fortification
Carlos O'Donell
carlos@redhat.com
Mon Jun 19 12:58:55 GMT 2023
On 6/17/23 18:22, Sergey Bugaev via Libc-alpha wrote:
> Hello,
>
> this is v3 of the fcntl fortification work. v1 was at [0], v2 at [1].
>
> [0]: https://sourceware.org/pipermail/libc-alpha/2023-May/148332.html
> [1]: https://sourceware.org/pipermail/libc-alpha/2023-May/148569.html
Pre-commit CI regression caused by this series on both aarch64 and aarch32.
https://patchwork.sourceware.org/project/glibc/patch/20230617222218.642125-6-bugaevc@gmail.com/
# Running glibc:io ...
# FAIL: io/check-installed-headers-c
# FAIL: io/check-installed-headers-cxx
#
# Running glibc:misc ...
# FAIL: misc/check-installed-headers-c
# FAIL: misc/check-installed-headers-cxx
#
# Running glibc:rt ...
# FAIL: rt/check-installed-headers-c
# FAIL: rt/check-installed-headers-cxx
> Changes since v2:
>
> - This is now in its own separate header, bits/fcntl3.h
> (bits/fcntl2.h is used by the open fortification)
>
> - Clang is now supported in addition to GCC!
> - Clang does not support nor need the "-Wsystem-headers" pragma
> - Clang does support error/warning attributes since recently
> - There seems to be a bug in Clang which prevents the type mismatch
> warning from actually firing. Specifically, it appears that Clang
> gets confused about C functions names vs symbol names when it comes
> to attribute ((warning)), and does not emit the warning if the
> function declared with __warnattr has a symbol name matching that of
> another function that has not been declared with __warnattr.
>
> While this could be worked around in glibc (such as by adding
> __fcntl_warn as a real wrapper function when built with Clang), I
> think this just needs to be fixed in Clang. Any LLVM developers
> here? :D
>
> - Changed hide_constant utility to use an empty inline asm statement
> instead of volatile and noinline, as per the discussion. I did not
> make this into a general-purpose glibc-wide utility because I don't
> know what a fitting name and place (header) for it would be. If you'd
> like to see it glibc-wide, please suggest me where to put it and how
> to name it!
>
> - Fixed the C++ template linkage thing
>
> - Addressed misc review comments
>
> - Looked into applying __builtin_constant_p to the result of the cmd
> check and not the cmd value itself, as suggested by Florian.
>
> Unfortunately this does not work at all :( __builtin_constant_p starts
> returning 0 given anything remotely complex like even a trivial inline
> function call (so technically hide_constant would still work if it was
> just 'return value;'), even if the function is (later?) fully inlined
> and const-folded.
>
> *Maybe* this could be made to work if I used an obscene amount of
> macros instead of inline functions (to handle all the various commands
> being conditionally defined), but I don't want to go there.
>
> So since this didn't work out, I left the runtime __fcntl_2 function,
> but split it into a separate patch, so you can apply it or drop it
> depending on what you prefer in the end.
>
> Clang / C++ demo:
>
> ------------------------------------------------------------------
> $ clang++ test-fcntl.cpp -D _FORTIFY_SOURCE=2 -O2
> In file included from test-fcntl.cpp:1:
> In file included from /usr/include/fcntl.h:348:
> /usr/include/bits/fcntl3.h:394:5: error: call to '__fcntl_missing_arg' declared with 'error' attribute: fcntl with with this command needs 3 arguments
> __fcntl_missing_arg ();
> ^
> 1 error generated.
> ------------------------------------------------------------------
>
> Sergey
>
--
Cheers,
Carlos.
More information about the Libc-alpha
mailing list