Introduce new attribute norelax
#94
Conversation
This commit introduces the `norelax` attribute for RISC-V, designed to prevent linker relaxation for functions that may be called at very early stages of program execution. The attribute supports multiple syntax styles: - GNU: `__attribute__((norelax))` - C++11: `[[riscv::norelax]]` - C23: `[[riscv::norelax]]` Functions declared with this attribute will not be relaxed by the linker, ensuring stability in cases where the global pointer (GP) may not yet be initialized. This is particularly relevant for cases like `ifunc` resolvers, which are invoked before GP initialization. Implementation Note: The attribute functionality can be achieved with `.option push; .option norelax; <function-body>; .option pop`. This attribute is particularly useful in scenarios where functions are invoked before the GP is properly set up, preventing potential early-stage execution errors.
|
It looks good to me. This is essential to build the IFUNC resolver discussed here. |
|
Do we need to apply this automatically to ifunc resolvers? If it's opt-in, people might forget? |
I think so. I also suggest adding a compiler option like '-mrelax-ifunc' to allow user to switch. |
…ute to ifunc resolver
|
Changes:
|
|
It doesn't seem clean or even possible for compilers to automatically apply this attribute to ifunc resolvers, since they are specified via the symbol name as a string, rather than passed in as a constant expression or something. The text should probably say "Functions used as ifunc resolvers should use this attribute. The compiler should apply this to automatically generated ifunc resolvers.", possibly with a note with elaboration |
|
Changes:
|
|
@dramforever thanks for the suggestion, but I keep use |
There was a problem hiding this comment.
LGTM.
I am glad we have some guidance. I see how hard it is to find the right function to apply the attribute to automatically, so I see we can really only do this in an implementation on a best-effort basis (or where the implementation, and not user code, is solely responsible for generating the resolver)
I don't think a flag changes the "users might forget the attribute" concern I have, as users might also forget the flag, so I'm happy to leave the wording as it is now, and implementations can try their best to let users know if stuff might go wrong.
Are we going to look at some of the other advice in the email thread, such as adding a dynamic tag so the dynamic linker can initialize gp even earlier, avoiding some of the GP relaxation problems we've seen? Maybe that's a psABI question and not a C api question.
|
Hm. For .init/.fini and .init_array/.fini_array you already need to set GP. I feel like this isn't a problem for the toolchain to solve, and rtld should just be setting GP prior to calling any user-provided code in general, which includes IFUNC resolvers, no? |
IFUNC resolver is invoked earlier than the .init/.fini and .init_array/.fini_array...we also tried to initialized GP before invoke IFUNC resolver, but conclusion is suppress GP relaxation within IFUNC resolver would be safer. [1] https://patchwork.sourceware.org/project/glibc/patch/[email protected]/ |
This patch adds norelax function attribute that be discussed in riscv-c-api-doc PR#94. URL:riscv-non-isa/riscv-c-api-doc#94 gcc/ChangeLog: * config/riscv/riscv.cc (riscv_declare_function_name): Add new attribute.
It's making a problem where there isn't one. It's not hard to just make the run-time linker just do the right thing, and then you don't need to care what the code is when compiling, GP will always be defined. |
And it's not just something you need to set on the resolver. You need to set it on every function it transitively calls. It's going make a mess. |
|
If the issue is GP why do we need to disable all relaxation and not just the relaxation for the relocations that can relax to GP? |
We have no specific option like "norelaxgp" for now in binutils. To achieve it without binutils and glibc change, I think reusing it might be OK. |
| NOTE: The use case for this attribute is to apply it to functions that may be invoked | ||
| at a very early stage of the process, when the GP may not be initialized properly at that | ||
| moment. | ||
| NOTE: Functions used as ifunc resolvers should use this attribute. The compiler |
There was a problem hiding this comment.
NOTE: Functions used as ifunc resolvers should use this attribute.
Should be: Functions used as ifunc resolvers and all possible ifunc resolver callees should use this attribute.
I think so. However, my opinion is that having this option is a safe way to make sure the compiler-generated IFUNC resolver works correctly. This "norelax" attribute can be used in For the user-provided IFUNC resolver function, let's let the user know that their GP might not be set correctly and add the "norelax" attribute if they want to provide the best compatibility in all versions of GLIBC as it's broken now since GLIBC 2.40. We should care about the user's code that might be run on some machines with GLIBC that do not set up GP correctly before calling the IFUNC resolver. Then, we can have time to discuss this ABI grey area and what the dynamic loader should do before calling the IFUNC resolver and then fixing GLIBC. If we only modify the ABI and resolve the issues with GLIBC, some users might encounter problems. These users might be using newer versions of GCC or LLVM but still have old versions of GLIBC installed. |
The norelax option is an extremely roundabout way to make ifunc resolver safe. So roundabout that it isn't even inherently safer. For example, in the ELF spec it is specified (basically) that a GOT So "safe way" is a horrible argument to use here |
Nice catch. I will rethink about what to do next. |
|
I'm having deja vu from when we dealt with this for init_array. Isn't @jrtc27 right that the transitive closure of the ifunc resolver might be the whole binary? Isn't the simplest solution just to make sure that GP is initialized early enough, even if that involves some hackery? |
I changed my mind to support you and @jrtc27's opinion. So the problem remains here is how to make this behavior safe: https://inbox.sourceware.org/libc-alpha/mhng-b09f06ce-b46f-41d2-8b1a-d2765799ed4a@palmer-ri-x1c9/ |
There was a problem hiding this comment.
I am convinced that this is not the right way to solve the global pointer initialization issue for ifuncs.
Part of my approval was because I expected this might also be useful to embedded/firmware developers who want to write more code in C than they do right now, but I think the usual places that this is needed in assembly are still very connected to writing that code directly in assembly - for instance compilers are not responsible for laying out the instructions used by vectored interrupts, this has to be done by an assembler.
I did a quick code search on github and (apart from uses for gp initialization) there are a few instances of programs using .option norelax to prevent R_RISCV_ALIGN relaxations (presumably when they were unsupported by lld? I'm not sure why else you would want to do this), a use in a (userspace) static key mechanism, and some slightly weird uses from inline assembly.
I'm not sure that any of this code could be rewritten directly in C (rather than in assembly as it is now), so I'm not sure this is worth supporting.
(Read this as: I retract my approval, but I won't block this if we find other compelling usecases)
This commit introduces the
norelaxattribute for RISC-V, designed to prevent linker relaxation for functions that may be called at very early stages of program execution. The attribute supports multiple syntax styles:__attribute__((norelax))[[riscv::norelax]][[riscv::norelax]]Functions declared with this attribute will not be relaxed by the linker, ensuring stability in cases where the global pointer (GP) may not yet be initialized.
This is particularly relevant for cases like
ifuncresolvers, which are invoked before GP initialization.Implementation Note:
The attribute functionality can be achieved with
.option push; .option norelax; <function-body>; .option pop.This attribute is particularly useful in scenarios where functions are invoked before the GP is properly set up, preventing potential early-stage execution errors.