Ulrich Weigand has confirmed 3 of my 4 llvm bug submittals for clang 3.8.0 targeting powerpc/powerpc64. . .

[Mark Millard]

[Some more details confirmed.]

On 2016-Mar-9, at 12:24 PM, Mark Millard <markmi at dsl-only.net> wrote:
> 
> On 2016-Mar-9, at 11:16 AM, Mark Millard <markmi at dsl-only.net> wrote:
>> 
>> [He also includes a note about ELFV2 ABI for powerpc64le.]
>> 
>> Quoting llvm 26856 Comment 6 (he put the text in the 26856 submittal but the content also covers 26519 and most of 26844):
>> 
>>> Ulrich Weigand 2016-03-09 11:53:17 CST
>>> 
>>> Yes, there's indeed a couple of problems here, which affect different areas.
>>> 
>>> 1) On 32-bit ppc, LLVM violates the ABI by storing below the stack pointer even though the ABI does not provide a "red zone".  This affects every function with a stack frame, and could in theory lead to spurious crashes when an asynchronous signal overwrites this area.  This seems to be a known issue; the source code contains FIXME lines:
>>>   // FIXME: On PPC32 SVR4, we must not spill before claiming the stackframe.
>>> 
>>> 2) In some scenarios, registers may be spilled/restored twice to the stack.  This happens because while most of the spilling happens in PPCFrameLowering::spillCalleeSavedRegisters, a few selected registers are also spilled in PPCFrameLowering::emitPrologue.  Those registers are the frame pointer, base pointer, PIC base pointer, link register, and condition code register.  For the latter two, code ensures that they can never be spilled in both places (for CR, there is extra code in spillCalleeSavedRegisters; for LR, the register is removed from SavedRegs in determineCalleeSaves).
>>> 
>>> However, for FP, BP, and PBP, nothing ensures the registers are not spilled twice.  It is probably *rare* for this to happen, because the register allocator will not use those registers within the function if they're needed for their special purpose, but it can happen in rare cases.  This includes the case of a system unwinder routine that uses __builtin_unwind_init, but could also include other routines that clobber one of those registers, e.g. the following case:
>>> 
>>> void func (void);
>>> 
>>> void test (void)
>>> {
>>> func ();
>>> asm ("nop" : : : "31");
>>> }
>>> 
>>> When it happens that a register is spilled twice, the code as such still works correctly, but the DWARF CFI unwind info associated with the routine will be broken, which can mess up both C++ exception handling and debugging.
>>> 
>>> 3) For the specific case of system unwinder routines that use __builtin_unwind_init and/or __builtin_eh_return, special things need to happen in the prolog and epilog that are not required for any other routine.  This in particular includes setting up save areas and CFI records for the EH data registers (r3 ... r6).  [See bug #26844. ]  For the ELFv2 ABI (powerpc64le), it also include using three separate save areas for the three caller-saved condition register fields, so that the EH logic can overwrite their values independently.
>>> 
>>> None of this is currently implemented in LLVM, since on Linux generally GCC is used to build the system unwind libraries, and no other code in the system ever needs those special constructs.
>> 
>> 
>> ===
>> Mark Millard
>> markmi at dsl-only.net
> 
> One point of his note is wrong: when the 2nd "spill" of a register is after it had been changed it makes a bigger difference. I commented back:
> 
> 
>> However, for FP, BP, and PBP, nothing ensures the registers are not spilled
>> twice.. . .
>> 
>> When it happens that a register is spilled twice, the code as such still
>> works correctly, but the DWARF CFI unwind info associated with the routine
>> will be broken, which can mess up both C++ exception handling and debugging.
> 
> 
> I will note that the Frame Pointer Register (r31) being saved again to the same location but after it was adjusted to match the adjusted stack pointer in the callee does not work correctly in that the restore of the Frame Pointer for the return to the caller will restore the wrong pointer value.
> 
> If the caller then uses r31 without separately also restoring r31 first then it will be addressing the wrong memory on the stack.
> 
> The observed/reported code sequence had the 2nd r31 store in the callee after r31 had been adjusted to match the adjusted stack pointer in the callee.
> 
> So more than C++ exception handling and debugging is broken for the reported code sequence.

The new comments are. . .

> Comment # 9 on bug 26856 from Ulrich Weigand
> (In reply to comment #8)
> 
> > (In reply to comment #6
> )
> > 
> > > However, for FP, BP, and PBP, nothing ensures the registers are not spilled
> > > twice.. . .
> > > 
> > > When it happens that a register is spilled twice, the code as such still
> > > works correctly, but the DWARF CFI unwind info associated with the routine
> > > will be broken, which can mess up both C++ exception handling and debugging.
> > 
> > I will note that the Frame Pointer Register (r31) being saved again to the
> > same location but after it was adjusted to match the adjusted stack pointer
> > in the callee does not work correctly in that the restore of the Frame
> > Pointer for the return to the caller will restore the wrong pointer value.
> > 
> > If the caller then uses r31 without separately also restoring r31 first then
> > it will be addressing the wrong memory on the stack.
> > 
> > The observed/reported code sequence had the 2nd r31 store in the callee
> > after r31 had been adjusted to match the adjusted stack pointer in the
> > callee.
> > 
> > So more than C++ exception handling and debugging is broken for the reported
> > code sequence.
> 
> 
> Ah, right.  I had been under the impression that the back-end would use two
> different save areas, but indeed it does use the same area, just addressed
> slightly differently.  The resulting code is then just simply incorrect.

and. . .

> Comment # 10 on bug 26856 from Ulrich Weigand
> (In reply to comment #7)
> 
> 
> > The observed behavior for the powerpc (what should be) SVR4 context is that
> > the save/restore logic for CR fields 2, 3, 4 is present but no .eh_frame
> > information is written out for the EH logic to use for CR.
> 
> 
> Huh, indeed.  This is yet another bug, which also affects functions for the
> powerpc (SVR4) ABI in general, not just unwind system routines.
> 
> This seems to be a logic issue here:
> 
>       // For SVR4, don't emit a move for the CR spill slot if we haven't
>       // spilled CRs.
>       if (isSVR4ABI && (PPC::CR2 <= Reg && Reg <= PPC::CR4)
>           && !MustSaveCR)
>         continue;
> 
> MustSaveCR is always false for 32-bit, it is only used on 64-bit.  This has the
> effect that on 32-bit, we never get any CFI for the CRs.



===
Mark Millard
markmi at dsl-only.net