on 2024/1/12 19:03, Alexandre Oliva wrote: > On Jan 12, 2024, "Kewen.Lin" <[email protected]> wrote: > >>>> By checking PR112917, IMHO we should keep this unbiasing >>>> guarded under SPARC_STACK_BOUNDARY_HACK (TARGET_ARCH64 && >>>> TARGET_STACK_BIAS), similar to some existing code special >>>> treating SPARC stack bias. >>> >>> I'm afraid this change will most certainly regress 32-bit sparc, because >>> of the large register save area. > >> Oh, I read the comments and commit logs in PR112917, mainly >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112917#{c4,c5,c6}, >> and the "sparc64" in subject of commit r14-6737 also implies >> that this unbiasing is only required for sparc64, so I thought >> it should be safe to guard with SPARC_STACK_BOUNDARY_HACK. > > It is safe, in a way, because that protects potentially active stack > areas, but it's unsafe in that it may leak data that stack scrubbing was > supposed to scrub. There's no conservative solution here, alas; we have > to get it just right. > > Specifically on sparc32, if __builtin_scrub_leave allocated its own > frame (it doesn't) with the large register-save area for its potential > (but inexistent) callees to use, it could overlap with a large chunk of > the very stack frame that it's supposed to clear.
Thanks for the further explanation! > > Unfortunately, this is slowly drifting away from the notion of stack > address. I mean, all of the following could conceivably be returned by > __builtin_stack_address: > > - the (biased) stack pointer > > - the address of the red zone > > - the unbiased stack pointer > > - the address of the save area reserved by callees for potential callees > > - the boundary between caller- and callee-used stack space > > The last one is what we need for stack scrubbing, so that's what I'm > planning to implement, but I'm pondering whether to change > __builtin_stack_address() to take an extra argument to select among the > different possibilities, or of other means to query these various > offsets. It feels like overthinking, so I'm trying to push these > thoughts aside, but... Does anyone think that would be a desirable > feature? We can always add it later. One immature idea: maybe we can introduce a hook with clear meaning for the last one and its default implementation still adopts the function __builtin_stack_address directly, if this default implementation for some port is imperfect, someone who is familiar with its own ABIs can further enhance it with its own hook implementation. BR, Kewen
