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

[friss]

Summary

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.

[friss]

Adding Rafael to the review as it seems he touched EmitCFIInstruction last/most

[• rafael]

It looks reasonable to me.

You could store the LLVM register numbers, but you would also need to convert in the asm printer and parser as the cfi instructions can unfortunately accept a numeric value.

[friss]

OK, I'll respin with a testcase.

The patch storing the LLVM register numbers is big, touches every target and would disrupt out of tree targets. Quite a big hammer for a very small gain so I'm glad you find this reasonable.

[grosbach]

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?

-Jim

[friss]

Now that I taugth llvm-dwarddump to dump debug_frame, add a test.
Code patch unchanged.

[• rafael]

LGTM

lib/MC/MCDwarf.cpp

 
 static void emitEncodingByte(MCObjectStreamer &Streamer, unsigned Encoding) {
   Streamer.EmitIntValue(Encoding, 1);
 }
 
 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);
     return;
   }
   case MCCFIInstruction::OpWindowSave: {
     Streamer.EmitIntValue(dwarf::DW_CFA_GNU_window_save, 1);
     return;
case MCCFIInstruction::OpDefCfaOffset: {
     else
       CFAOffset = -Instr.getOffset();
 
     Streamer.EmitULEB128IntValue(CFAOffset);
 
     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;
   }
 
   case MCCFIInstruction::OpOffset:
   case MCCFIInstruction::OpRelOffset: {
     const bool IsRelative =
       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;
     Offset = Offset / dataAlignmentFactor;
 
     if (Offset < 0) {
       Streamer.EmitIntValue(dwarf::DW_CFA_offset_extended_sf, 1);
       Streamer.EmitULEB128IntValue(Reg);
void FrameEmitterImpl::EmitCFIInstruction(MCObjectStreamer &Streamer,
   case MCCFIInstruction::OpSameValue: {
     unsigned Reg = Instr.getRegister();
     Streamer.EmitIntValue(dwarf::DW_CFA_same_value, 1);
     Streamer.EmitULEB128IntValue(Reg);
     return;
   }
   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;
   }
   case MCCFIInstruction::OpEscape:
     Streamer.EmitBytes(Instr.getValues());
     return;
   }
   llvm_unreachable("Unhandled case in switch");
const MCSymbol &FrameEmitterImpl::EmitCIE(MCObjectStreamer &streamer,
 
   // Data Alignment Factor
   streamer.EmitSLEB128IntValue(getDataAlignmentFactor(streamer));
 
   // Return Address Register
   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)
 
   unsigned augmentationLength = 0;
   if (IsEH) {
     if (personality) {
       // Personality Encoding

test/MC/X86/i386-darwin-frame-register.ll

+; RUN: llc -filetype=obj %s -o - | llvm-dwarfdump -debug-dump=frames - | FileCheck %s
+
+; IR reduced from a dummy:
+; void foo() {}
+
+; x86 Darwin uses different register mappings for eh_frame and debug_frame
+; sections. Check that the right mapping is used in debug_frame.
+; In the debug_frame mapping, regsiter 4 is ESP, thus the below tests that
+; the CFA is ESP+4 upon function entry.
+
+; CHECK: .debug_frame contents:
+; CHECK: ffffffff CIE
+; CHECK-NOT: {{CIE|FDE}}
+; CHECK:   DW_CFA_def_cfa: reg4 +4
+
+; ModuleID = 'foo.c'
+target datalayout = "e-m:o-p:32:32-f64:32:64-f80:128-n8:16:32-S128"
+target triple = "i386-apple-macosx10.10.0"
+
+; Function Attrs: nounwind ssp
+define void @foo() #0 {
+entry:
+  ret void
+}
+
+attributes #0 = { nounwind }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = !{!"0x11\0012\00clang version 3.7.0 (trunk 230514) (llvm/trunk 230518)\000\00\000\00\001", !1, !2, !2, !2, !2, !2} ; [ DW_TAG_compile_unit ] [/tmp/foo.c] [DW_LANG_C99]
+!1 = !{!"foo.c", !"/tmp"}
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 2}
+!4 = !{i32 2, !"Debug Info Version", i32 2}
+!5 = !{i32 1, !"PIC Level", i32 2}
+!6 = !{!"clang version 3.7.0 (trunk 230514) (llvm/trunk 230518)"}