[PATCH] [MC] Use the non-EH register mapping in the debug_frame section.

[Rafael Espíndola]

Anything blocking this?
On Feb 12, 2015 5:46 PM, "Jim Grosbach" <grosbach at apple.com> wrote:

> Fair enough. Works for me.
>
> -Jim
>
> > On Feb 12, 2015, at 1:52 PM, Frédéric Riss <friss at apple.com> wrote:
> >
> >
> >> On Feb 12, 2015, at 1:22 PM, Jim Grosbach <grosbach at apple.com> wrote:
> >>
> >> Oof. The patch itself seems reasonable. I tend to agree with your
> thought that it may not be a root cause. Do you have any ideas about what a
> deeper solution would look like?
> >
> > I guess there is no deeper solution. Storing the LLVM regnum in the CFI
> instruction would be marginally cleaner as it’s supposed to be more
> ‘canonical’, and it would need to be converted only once. You’d still have
> to make an arbitrary choice when emitting .cfi directives in AsmStreamer.
> They are used for both EH and standard frame info, but they take only one
> argument. This usecase really only works when both mapping are identical.
> >
> > As Rafael put it in another reply, we have to choose one number to
> represent the register anyway. The EH number is the most used one, thus
> it’s not a bad choice.
> >
> > Fred
> >
> >> -Jim
> >>
> >>> On Feb 12, 2015, at 8:32 AM, Frederic Riss <friss at apple.com> wrote:
> >>>
> >>> Hi grosbach,
> >>>
> >>> On 32bits x86 Darwin, the register mappings for the eh_frane and
> >>> debug_frame sections are different. Thus the same CFI instructions
> >>> should result in different registers in the object file. The
> >>> problem isn't target specific though, but it requires that the
> >>> mappings for EH register numbers be different from the standard
> >>> Dwarf one.
> >>>
> >>> The patch is really ugly. The CFI instructions are emitted with
> >>> register operands that are already Dwarf EH register numbers,
> >>> allthough they will be used to emit both EH and non-EH frame
> >>> descriptions. I first thought the right solution was to encode
> >>> LLVM register numbers in the CFI instructions and do the
> >>> conversion at object emission time. This however breaks a lot
> >>> of tests that where the .cfi_* asm directives are tested (which
> >>> means it would break targets not using the integrated assembler).
> >>>
> >>> What this patch does instead is to do a double conversion when
> >>> emitting the debug_frame section. Knowing the CFI instruction
> >>> references EH reg numbers, it converts them back to LLVM reg num
> >>> and again back to the correct Dwarf mapping.
> >>>
> >>> I'll add a test, but I first wanted to reach out for opinions
> >>> as this looks like a bandaid rahter than a fix.
> >>>
> >>> Fixes PR22363.
> >>>
> >>> http://reviews.llvm.org/D7593
> >>>
> >>> Files:
> >>> lib/MC/MCDwarf.cpp
> >>>
> >>> Index: lib/MC/MCDwarf.cpp
> >>> ===================================================================
> >>> --- lib/MC/MCDwarf.cpp
> >>> +++ lib/MC/MCDwarf.cpp
> >>> @@ -1045,11 +1045,16 @@
> >>> void FrameEmitterImpl::EmitCFIInstruction(MCObjectStreamer &Streamer,
> >>>                                         const MCCFIInstruction &Instr)
> {
> >>> int dataAlignmentFactor = getDataAlignmentFactor(Streamer);
> >>> +  auto *MRI = Streamer.getContext().getRegisterInfo();
> >>>
> >>> switch (Instr.getOperation()) {
> >>> case MCCFIInstruction::OpRegister: {
> >>>   unsigned Reg1 = Instr.getRegister();
> >>>   unsigned Reg2 = Instr.getRegister2();
> >>> +    if (!IsEH) {
> >>> +      Reg1 = MRI->getDwarfRegNum(MRI->getLLVMRegNum(Reg1, true),
> false);
> >>> +      Reg2 = MRI->getDwarfRegNum(MRI->getLLVMRegNum(Reg2, true),
> false);
> >>> +    }
> >>>   Streamer.EmitIntValue(dwarf::DW_CFA_register, 1);
> >>>   Streamer.EmitULEB128IntValue(Reg1);
> >>>   Streamer.EmitULEB128IntValue(Reg2);
> >>> @@ -1082,17 +1087,23 @@
> >>>   return;
> >>> }
> >>> case MCCFIInstruction::OpDefCfa: {
> >>> +    unsigned Reg = Instr.getRegister();
> >>> +    if (!IsEH)
> >>> +      Reg = MRI->getDwarfRegNum(MRI->getLLVMRegNum(Reg, true), false);
> >>>   Streamer.EmitIntValue(dwarf::DW_CFA_def_cfa, 1);
> >>> -    Streamer.EmitULEB128IntValue(Instr.getRegister());
> >>> +    Streamer.EmitULEB128IntValue(Reg);
> >>>   CFAOffset = -Instr.getOffset();
> >>>   Streamer.EmitULEB128IntValue(CFAOffset);
> >>>
> >>>   return;
> >>> }
> >>>
> >>> case MCCFIInstruction::OpDefCfaRegister: {
> >>> +    unsigned Reg = Instr.getRegister();
> >>> +    if (!IsEH)
> >>> +      Reg = MRI->getDwarfRegNum(MRI->getLLVMRegNum(Reg, true), false);
> >>>   Streamer.EmitIntValue(dwarf::DW_CFA_def_cfa_register, 1);
> >>> -    Streamer.EmitULEB128IntValue(Instr.getRegister());
> >>> +    Streamer.EmitULEB128IntValue(Reg);
> >>>
> >>>   return;
> >>> }
> >>> @@ -1103,6 +1114,9 @@
> >>>     Instr.getOperation() == MCCFIInstruction::OpRelOffset;
> >>>
> >>>   unsigned Reg = Instr.getRegister();
> >>> +    if (!IsEH)
> >>> +      Reg = MRI->getDwarfRegNum(MRI->getLLVMRegNum(Reg, true), false);
> >>> +
> >>>   int Offset = Instr.getOffset();
> >>>   if (IsRelative)
> >>>     Offset -= CFAOffset;
> >>> @@ -1136,6 +1150,8 @@
> >>> }
> >>> case MCCFIInstruction::OpRestore: {
> >>>   unsigned Reg = Instr.getRegister();
> >>> +    if (!IsEH)
> >>> +      Reg = MRI->getDwarfRegNum(MRI->getLLVMRegNum(Reg, true), false);
> >>>   Streamer.EmitIntValue(dwarf::DW_CFA_restore | Reg, 1);
> >>>   return;
> >>> }
> >>> @@ -1290,10 +1306,10 @@
> >>> if (CIEVersion == 1) {
> >>>   assert(MRI->getRARegister() <= 255 &&
> >>>          "DWARF 2 encodes return_address_register in one byte");
> >>> -    streamer.EmitIntValue(MRI->getDwarfRegNum(MRI->getRARegister(),
> true), 1);
> >>> +    streamer.EmitIntValue(MRI->getDwarfRegNum(MRI->getRARegister(),
> IsEH), 1);
> >>> } else {
> >>>   streamer.EmitULEB128IntValue(
> >>> -        MRI->getDwarfRegNum(MRI->getRARegister(), true));
> >>> +        MRI->getDwarfRegNum(MRI->getRARegister(), IsEH));
> >>> }
> >>>
> >>> // Augmentation Data Length (optional)
> >>>
> >>> EMAIL PREFERENCES
> >>> http://reviews.llvm.org/settings/panel/emailpreferences/
> >>> <D7593.19836.patch>
> >>
> >
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150224/ca82b299/attachment.html>