Fix Android MIPS exception personality encoding.

[logan]

Summary

Android MIPS executable loader prohibits the relocation
in the read-only section. Thus, we have to use
DW_EH_PE_indirect instead.

[dsanders]

My background is in C and Embedded-C compilers so my knowledge of things like CFI is rather limited at the moment and I don't think I'm suitable to review this patch. Hopefully someone who knows this area better will also take a look.

Bearing that in mind, a couple things look a bit odd to me. I've put the comments inline.

lib/MC/MCObjectFileInfo.cpp

328

I'm fairly sure that the if-statement should be testing for RelocM == Reloc::PIC_ rather than Android.

I'm not so sure what the correct PersonalityEncoding is but DW_EH_PE_indirect by itself seems odd. I see that all the other targets are combining DW_EH_PE_indirect with DW_EH_PE_pcrel and one of the data size values. If this is the right thing to do for MIPS then it removes the need for the change to getCFIPersonalitySymbol(). However, it's possible that it should be combining DW_EH_PE_indirect with DW_EH_PE_absptr.

[joerg]

The problem here is that MIPS doesn't allow PC-relative relocations against a symbol in a different section. That said, I believe indirect encoding should be used for MIPS in the PIC case in general. I'm not sure about the correctness of the rest of the patch yet.

[logan]

The indirect pointer change should not limit to Android.

After checking the output of GCC on Debian mipsel port,
it seems that this change should be applied regardless the -fPIC
flags.

Besides, Joerg's comment is correct. It seems that
MIPS doesn't allow PC-relative relocations between sections, thus
we should use DW_EH_PE_absptr instead of PW_EH_PE_pcrel.

p.s. DW_EH_PE_absptr = 0

[joerg]

lib/CodeGen/AsmPrinter/DwarfCFIException.cpp

63

Don't we just want to check DW_EH_PE_indirect here? Non-indirect references would have already been in place.

lib/CodeGen/TargetLoweringObjectFileImpl.cpp

52

Same here.

lib/MC/MCObjectFileInfo.cpp

327

Just plain DW_EH_PE_indirect, I think. The absptr is implicit.

[logan]

Address Joerg's comments.

[logan]

Committed as rL209907.

lib/CodeGen/AsmPrinter/DwarfCFIException.cpp

void DwarfCFIException::endModule() {
 
   if (!Asm->MAI->isExceptionHandlingDwarf())
     return;
 
   const TargetLoweringObjectFile &TLOF = Asm->getObjFileLowering();
 
   unsigned PerEncoding = TLOF.getPersonalityEncoding();
 
-  if ((PerEncoding & 0x70) != dwarf::DW_EH_PE_pcrel)
+  if ((PerEncoding & 0x70) != dwarf::DW_EH_PE_pcrel &&
+      (PerEncoding & 0x80) != dwarf::DW_EH_PE_indirect)
     return;
 
   // Emit references to all used personality functions
   bool AtLeastOne = false;
   const std::vector<const Function*> &Personalities = MMI->getPersonalities();
   for (size_t i = 0, e = Personalities.size(); i != e; ++i) {
     if (!Personalities[i])
       continue;

lib/CodeGen/TargetLoweringObjectFileImpl.cpp

 //===----------------------------------------------------------------------===//
 //                                  ELF
 //===----------------------------------------------------------------------===//
 
 MCSymbol *TargetLoweringObjectFileELF::getCFIPersonalitySymbol(
     const GlobalValue *GV, Mangler &Mang, const TargetMachine &TM,
     MachineModuleInfo *MMI) const {
   unsigned Encoding = getPersonalityEncoding();
-  switch (Encoding & 0x70) {
-  default:
-    report_fatal_error("We do not support this DWARF encoding yet!");
-  case dwarf::DW_EH_PE_absptr:
-    return TM.getSymbol(GV, Mang);
-  case dwarf::DW_EH_PE_pcrel: {
+  if ((Encoding & 0x70) == dwarf::DW_EH_PE_pcrel ||
+      (Encoding & 0x80) == dwarf::DW_EH_PE_indirect)
     return getContext().GetOrCreateSymbol(StringRef("DW.ref.") +
                                           TM.getSymbol(GV, Mang)->getName());
-  }
-  }
+  if ((Encoding & 0x70) == dwarf::DW_EH_PE_absptr)
+    return TM.getSymbol(GV, Mang);
+  report_fatal_error("We do not support this DWARF encoding yet!");
 }
 
 void TargetLoweringObjectFileELF::emitPersonalityValue(MCStreamer &Streamer,
                                                        const TargetMachine &TM,
                                                        const MCSymbol *Sym) const {
   SmallString<64> NameData("DW.ref.");
   NameData += Sym->getName();
   MCSymbol *Label = getContext().GetOrCreateSymbol(NameData);

lib/MC/MCObjectFileInfo.cpp

if (RelocM == Reloc::PIC_) {
       TTypeEncoding = dwarf::DW_EH_PE_indirect | dwarf::DW_EH_PE_pcrel |
         dwarf::DW_EH_PE_sdata8;
     } else {
       PersonalityEncoding = dwarf::DW_EH_PE_absptr;
       LSDAEncoding = dwarf::DW_EH_PE_absptr;
       TTypeEncoding = dwarf::DW_EH_PE_absptr;
     }
     break;
+  case Triple::mipsel:
+    if (T.getEnvironment() == Triple::Android) {
+      // For Android, we have to use indirect reference to personality
+      // function; otherwise, some warning will be raised during link time.
+      // This will even cause some SEGV in the loader on some early Android
+      // MIPS devices.
+      PersonalityEncoding = dwarf::DW_EH_PE_indirect;
+    }
+    break;
   case Triple::ppc64:
   case Triple::ppc64le:
     PersonalityEncoding = dwarf::DW_EH_PE_indirect | dwarf::DW_EH_PE_pcrel |
       dwarf::DW_EH_PE_udata8;
     LSDAEncoding = dwarf::DW_EH_PE_pcrel | dwarf::DW_EH_PE_udata8;
     TTypeEncoding = dwarf::DW_EH_PE_indirect | dwarf::DW_EH_PE_pcrel |
       dwarf::DW_EH_PE_udata8;
     break;

test/CodeGen/Mips/ehframe-indirect.ll

+; RUN: llc -mtriple=mipsel-linux-android < %s | FileCheck %s
+
+define i32 @main() {
+; CHECK: .cfi_startproc
+; CHECK: .cfi_personality 128, DW.ref.__gxx_personality_v0
+
+entry:
+  invoke void @foo() to label %cont unwind label %lpad
+; CHECK: foo
+; CHECK: jalr
+
+lpad:
+  %0 = landingpad { i8*, i32 } personality i8*
+    bitcast (i32 (...)* @__gxx_personality_v0 to i8*) catch i8* null
+  ret i32 0
+
+cont:
+  ret i32 0
+}
+; CHECK: .cfi_endproc
+
+declare i32 @__gxx_personality_v0(...)
+
+declare void @foo()
+
+; CHECK: .hidden DW.ref.__gxx_personality_v0
+; CHECK: .weak DW.ref.__gxx_personality_v0
+; CHECK: .section .data.DW.ref.__gxx_personality_v0,"aGw",@progbits,DW.ref.__gxx_personality_v0,comdat
+; CHECK: .align 2
+; CHECK: .type DW.ref.__gxx_personality_v0,@object
+; CHECK: .size DW.ref.__gxx_personality_v0, 4
+; CHECK: DW.ref.__gxx_personality_v0:
+; CHECK: .4byte __gxx_personality_v0