[llvm] r249522 - [X86] Emit .cfi_escape GNU_ARGS_SIZE when adjusting the stack before calls

Adrian Prantl via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 8 08:49:41 PDT 2015


> On Oct 8, 2015, at 12:51 AM, Kuperstein, Michael M <michael.m.kuperstein at intel.com> wrote:
> 
> Thinking about it a bit more, I don’t see why it ought to be illegal for the first non-frame-setup instruction to be a CFI directive.
> I’m not sure there are any examples except GNU_ARGS_SIZE – but I don’t think it makes sense to special-case it.
>  
> I’ve removed the assert in r249668.
> Adrian, are you ok with that?

I went back and read the commit message for the assertion. Some backends used to forget to mark CFI instructions that were in the middle of the function prologue with the MachineInstr::FrameSetup. DwarfDebug scans for the first instruction without the FrameSetup flag and attaches the prologue_end marker to the address of the instruction in the DWARF line table. If the function prologue is interrupted by a CFI instruction that is not marked with FrameSetup the prologue_end marker would be on the CFI instruction and the function would start at line 0 because CFI instructions usually don’t have a debug location.

If it is legal for a CFI instruction to follow the prologue there needs to be an assertion that it has a valid debug location. Alternatively findPrologueEndLoc() could skip ahead to the first non-CFI, non-FrameSetup instruction.

-- adrian

>  
> Michael
>  
> From: friss at apple.com [mailto:friss at apple.com] 
> Sent: Thursday, October 08, 2015 06:14
> To: Douglas Katzman
> Cc: Kuperstein, Michael M; llvm-commits at lists.llvm.org; Adrian Prantl; David Blaikie; Eric Christopher
> Subject: Re: [llvm] r249522 - [X86] Emit .cfi_escape GNU_ARGS_SIZE when adjusting the stack before calls
>  
>  
> On Oct 7, 2015, at 6:59 PM, Douglas Katzman <dougk at google.com> wrote:
>  
> Do you happen to have a testcase handy?
>  
> The sanitizers on Linux comprise a test case it seems - they started failing with this commit.
> http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/20647 
>  
> Without my understanding this code in the least, if you just do what the assertion says to do (as here), it fixes the sanitizers:
>  
> diff --git a/lib/Target/X86/X86FrameLowering.cpp b/lib/Target/X86/X86FrameLowering.cpp
> index 7f05e5b..d85b6a8 100644
> --- a/lib/Target/X86/X86FrameLowering.cpp
> +++ b/lib/Target/X86/X86FrameLowering.cpp
> @@ -384,8 +384,9 @@ void X86FrameLowering::BuildCFI(MachineBasicBlock &MBB,
>                                  MCCFIInstruction CFIInst) const {
>    MachineFunction &MF = *MBB.getParent();
>    unsigned CFIIndex = MF.getMMI().addFrameInst(CFIInst);
> -  BuildMI(MBB, MBBI, DL, TII.get(TargetOpcode::CFI_INSTRUCTION))
> -      .addCFIIndex(CFIIndex);
> +  MachineInstrBuilder MIB = BuildMI(MBB, MBBI, DL, TII.get(TargetOpcode::CFI_INSTRUCTION));
> +  MIB.addCFIIndex(CFIIndex);
> +  MIB->setFlag(MachineInstr::FrameSetup);
>  } 
>  
> This will fix the assert, but I think it is conceptually wrong. FrameSetup is supposed to describe the prologue. This will insert this flag on instructions all over the function. I can’t think of any bad consequences though. CC:ing some other people with debug info knowledge.
>  
> Fred
>  
> ---------------------------------------------------------------------
> Intel Israel (74) Limited
> 
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.
> 



More information about the llvm-commits mailing list