| Message ID | 20240728-nolibc-llvm-v1-4-bc384269bc35@weissschuh.net (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | tools/nolibc: improve LLVM/clang support | expand |
On Sun, Jul 28, 2024 at 12:09:58PM +0200, Thomas Weißschuh wrote: > The current entrypoint attributes optimize("Os", "omit-frame-pointer") > are intended to avoid all compiler generated code, like function > porologue and epilogue. > This is the exact usecase implemented by the attribute "naked". > > Unfortunately this is not implemented by GCC for all targets, > so only use it where available. > This also provides compatibility with clang, which recognizes the > "naked" attribute but not the previously used attribute "optimized". > > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> > --- > tools/include/nolibc/compiler.h | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/tools/include/nolibc/compiler.h b/tools/include/nolibc/compiler.h > index fe3701863634..f77bb7d3e1a8 100644 > --- a/tools/include/nolibc/compiler.h > +++ b/tools/include/nolibc/compiler.h > @@ -9,6 +9,15 @@ > #define __entrypoint __attribute__((optimize("Os", "omit-frame-pointer"))) > #define __entrypoint_epilogue() __builtin_unreachable() > > +#if defined(__has_attribute) > +# if __has_attribute(naked) > +# undef __entrypoint > +# define __entrypoint __attribute__((naked)) > +# undef __entrypoint_epilogue > +# define __entrypoint_epilogue() > +# endif > +#endif /* defined(__has_attribute) */ I would find it cleaner to enclose the previous declaration with the #if rather than #undef everything just after it has been defined. Also it's not very common to undo declarations just after they've been done, and it makes quick code analysis harder. I think that it can resolve to roughly this: #if defined(__has_attribute) && __has_attribute(naked) # define __entrypoint __attribute__((naked)) # define __entrypoint_epilogue() #else # define __entrypoint __attribute__((optimize("Os", "omit-frame-pointer"))) # define __entrypoint_epilogue() __builtin_unreachable() #endif What do you think ? Willy
Aug 3, 2024 11:26:07 Willy Tarreau <w@1wt.eu>: > On Sun, Jul 28, 2024 at 12:09:58PM +0200, Thomas Weißschuh wrote: >> The current entrypoint attributes optimize("Os", "omit-frame-pointer") >> are intended to avoid all compiler generated code, like function >> porologue and epilogue. >> This is the exact usecase implemented by the attribute "naked". >> >> Unfortunately this is not implemented by GCC for all targets, >> so only use it where available. >> This also provides compatibility with clang, which recognizes the >> "naked" attribute but not the previously used attribute "optimized". >> >> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> >> --- >> tools/include/nolibc/compiler.h | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> >> diff --git a/tools/include/nolibc/compiler.h b/tools/include/nolibc/compiler.h >> index fe3701863634..f77bb7d3e1a8 100644 >> --- a/tools/include/nolibc/compiler.h >> +++ b/tools/include/nolibc/compiler.h >> @@ -9,6 +9,15 @@ >> #define __entrypoint __attribute__((optimize("Os", "omit-frame-pointer"))) >> #define __entrypoint_epilogue() __builtin_unreachable() >> >> +#if defined(__has_attribute) >> +# if __has_attribute(naked) >> +# undef __entrypoint >> +# define __entrypoint __attribute__((naked)) >> +# undef __entrypoint_epilogue >> +# define __entrypoint_epilogue() >> +# endif >> +#endif /* defined(__has_attribute) */ > > I would find it cleaner to enclose the previous declaration with the #if > rather than #undef everything just after it has been defined. Also it's > not very common to undo declarations just after they've been done, and > it makes quick code analysis harder. > > I think that it can resolve to roughly this: > > #if defined(__has_attribute) && __has_attribute(naked) > # define __entrypoint __attribute__((naked)) > # define __entrypoint_epilogue() > #else > # define __entrypoint __attribute__((optimize("Os", "omit-frame-pointer"))) > # define __entrypoint_epilogue() __builtin_unreachable() > #endif We would need to duplicate the define for the !defined(__has_attribute) case. I wanted to avoid that duplication. > What do you think ? With the reasoning above I'll let you choose.
On Sat, Aug 03, 2024 at 08:28:08PM +0200, Thomas Weißschuh wrote: > > I think that it can resolve to roughly this: > > > > #if defined(__has_attribute) && __has_attribute(naked) > > # define __entrypoint __attribute__((naked)) > > # define __entrypoint_epilogue() > > #else > > # define __entrypoint __attribute__((optimize("Os", "omit-frame-pointer"))) > > # define __entrypoint_epilogue() __builtin_unreachable() > > #endif > > We would need to duplicate the define for the > !defined(__has_attribute) case. I don't understand why. Above both are tested on the first line. Am I missing something ? > I wanted to avoid that duplication. > > What do you think ? > > With the reasoning above I'll let you choose. I'm fine with avoiding duplication, I just don't understand why there should be. Willy
Aug 3, 2024 20:33:11 Willy Tarreau <w@1wt.eu>: > On Sat, Aug 03, 2024 at 08:28:08PM +0200, Thomas Weißschuh wrote: >>> I think that it can resolve to roughly this: >>> >>> #if defined(__has_attribute) && __has_attribute(naked) >>> # define __entrypoint __attribute__((naked)) >>> # define __entrypoint_epilogue() >>> #else >>> # define __entrypoint __attribute__((optimize("Os", "omit-frame-pointer"))) >>> # define __entrypoint_epilogue() __builtin_unreachable() >>> #endif >> >> We would need to duplicate the define for the >> !defined(__has_attribute) case. > > I don't understand why. Above both are tested on the first line. > Am I missing something ? This specifically does not work [0]: a result, combining the two tests into a single expression as shown below would only be valid with a compiler that supports the operator but not with others that don’t. > >> I wanted to avoid that duplication. >>> What do you think ? >> >> With the reasoning above I'll let you choose. > > I'm fine with avoiding duplication, I just don't understand why there > should be. > > Willy [0] https://gcc.gnu.org/onlinedocs/cpp/_005f_005fhas_005fattribute.html
On Sat, Aug 03, 2024 at 10:55:07PM +0200, Thomas Weißschuh wrote: > Aug 3, 2024 20:33:11 Willy Tarreau <w@1wt.eu>: > > > On Sat, Aug 03, 2024 at 08:28:08PM +0200, Thomas Weißschuh wrote: > >>> I think that it can resolve to roughly this: > >>> > >>> #if defined(__has_attribute) && __has_attribute(naked) > >>> # define __entrypoint __attribute__((naked)) > >>> # define __entrypoint_epilogue() > >>> #else > >>> # define __entrypoint __attribute__((optimize("Os", "omit-frame-pointer"))) > >>> # define __entrypoint_epilogue() __builtin_unreachable() > >>> #endif > >> > >> We would need to duplicate the define for the > >> !defined(__has_attribute) case. > > > > I don't understand why. Above both are tested on the first line. > > Am I missing something ? > > This specifically does not work [0]: > > a result, combining the two tests into a single expression as shown > below would only be valid with a compiler that supports the operator but not > with others that don't. Ah I didn't remember about that one, thanks for the reference. Indeed it's annoying then. We have a similar construct in compiler.h: #if defined(__has_attribute) # if __has_attribute(no_stack_protector) # define __no_stack_protector __attribute__((no_stack_protector)) # else # define __no_stack_protector __attribute__((__optimize__("-fno-stack-protector"))) # endif #else # define __no_stack_protector __attribute__((__optimize__("-fno-stack-protector"))) #endif /* defined(__has_attribute) */ Maybe it would be a good opportunity to have our own macro so as to simplify such tests: #if defined(__has_attribute) # define nolibc_has_attribute(x) __has_attribute(x) #else # define nolibc_has_attribute(x) 0 #endif #if nolibc_has_attribute(no_stack_protector) # define __no_stack_protector __attribute__((no_stack_protector)) #else # define __no_stack_protector __attribute__((__optimize__("-fno-stack-protector" #endif Then: #if nolibc_has_attribute(naked) # define __entrypoint __attribute__((naked)) # define __entrypoint_epilogue() #else # define __entrypoint __attribute__((optimize("Os", "omit-frame-pointer"))) # define __entrypoint_epilogue() __builtin_unreachable() #endif It's as you want. Either we take your #undef-based solution or we take this opportunity to clean up as above. I'm fine with both. Thanks! Willy
diff --git a/tools/include/nolibc/compiler.h b/tools/include/nolibc/compiler.h index fe3701863634..f77bb7d3e1a8 100644 --- a/tools/include/nolibc/compiler.h +++ b/tools/include/nolibc/compiler.h @@ -9,6 +9,15 @@ #define __entrypoint __attribute__((optimize("Os", "omit-frame-pointer"))) #define __entrypoint_epilogue() __builtin_unreachable() +#if defined(__has_attribute) +# if __has_attribute(naked) +# undef __entrypoint +# define __entrypoint __attribute__((naked)) +# undef __entrypoint_epilogue +# define __entrypoint_epilogue() +# endif +#endif /* defined(__has_attribute) */ + #if defined(__SSP__) || defined(__SSP_STRONG__) || defined(__SSP_ALL__) || defined(__SSP_EXPLICIT__) #define _NOLIBC_STACKPROTECTOR
The current entrypoint attributes optimize("Os", "omit-frame-pointer") are intended to avoid all compiler generated code, like function porologue and epilogue. This is the exact usecase implemented by the attribute "naked". Unfortunately this is not implemented by GCC for all targets, so only use it where available. This also provides compatibility with clang, which recognizes the "naked" attribute but not the previously used attribute "optimized". Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> --- tools/include/nolibc/compiler.h | 9 +++++++++ 1 file changed, 9 insertions(+)