Hi Alexandre, on 2024/1/11 17:05, Alexandre Oliva wrote: > On Jan 7, 2024, "Kewen.Lin" <li...@linux.ibm.com> wrote: > >> As PR113100 shows, the unbiasing introduced by r14-6737 can >> cause the scrubbing to overrun and screw some critical data >> on stack like saved toc base consequently cause segfault on >> Power. > > Ugh. Sorry about the breakage, and thanks for addressing it during my > absence. Happy GNU Year! :-) >
No problem! Happy New Year! :) >> 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. CC Rainer (reporter of PR112917), maybe he already noticed if it's regressed or not. > > I had been hesitant to introduce yet another target configuration knob, > but it looks like this is what we're going to have to do to accommodate > all targets. > >> I also expect the culprit commit can >> affect those ports with nonzero STACK_POINTER_OFFSET. > > IMHO it really shouldn't. STACK_POINTER_OFFSET should be the "Offset > from the stack pointer register to the first location at which outgoing > arguments are placed", which suggests to me that no data that the callee > couldn't change should go in the area below (or above) %sp+S_P_O. > > ISTM that PPC sets up a save area between the outgoing args and the Yes, taking 64-bit PowerPC ELF abi 1.9 as example: | Parameter save area (SP + 48) | TOC save area (SP + 40) | link editor doubleword (SP + 32) | compiler doubleword (SP + 24) | LR save area (SP + 16) | CR save area (SP + 8) SP ---> +-- Back chain (SP + 0) https://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi.html#STACK 64-bit PowerPC ELF abi v2 drops "link editor doubleword" and "compiler doubleword". PR113100 failures mainly suffer from the TOC saved value changed when it's used for TOC BASE restoring. > stack pointer; I don't think that's very common, but I suppose other > targets that do so would also define STACK_POINTER_OFFSET to nonzero so > as to reserve those bits. But whether they should be cleared by stack > scrubbing, as on sparc, or preserved, as on ppc, depends on the ABI > conventions, so we probably can't help yet another knob :-/ I agree, it really depends on ABI conventions, taking 64-bit PowerPC ELF abi as example, some of them is safe to be scrubbed like "LR save area", some of them depends on if they are used like "TOC save area". It reminds me that even if on somewhere there is no failures with scrubbing them, it doesn't really mean it's always safe to scrub since it's possible that there are no enough testing coverage. For example, back chain gets cleared but no issues would be exposed if there is no test case happening to do some jobs with back chain. From this perspective, excepting for the special need of sparc unbiasing, without examining the specific ABIs, IMHO it's more conservative (not risky) not to scrub this area than scrubbing it? > > I'll take care of that, and update the corresponding documentation. Nice, thanks! Welcome back. :-) BR, Kewen
