[mips] Fix TestDWARF32Version5Addr8AllForms test failure on MIPS hosts

[atanasyan]

Summary

The DIEExpr is used in debug information entries for either TLS variables or call sites. For now the last case is unsupported for targets with delay slots, for MIPS in particular.

The DIEExpr::EmitValue method calls a virtual EmitDebugValue routine which, in case of MIPS, always emits either .dtprelword or .dtpreldword directives. That is okay for "main" code, but in unit tests DIEExpr instances can be created not for TLS variables only even on MIPS hosts. That is a reason of the TestDWARF32Version5Addr8AllForms failure because handling of the R_MIPS_TLS_DTPREL relocation writes incorrect value into dwarf structures. And anyway unconditional emitting of .dtprelword directives will be incorrect when/if debug information entries for call sites become supported on MIPS.

The patch solves the problem by wrapping expression created in the MipsTargetObjectFile::getDebugThreadLocalSymbol method in to the MipsMCExpr expression with a new MEK_DTPREL tag. This tag is recognized in the MipsAsmPrinter::EmitDebugValue method and .dtprelword directives created in this case only. In other cases the expression saved as a regular data.

[dblaikie]

I'm not sure I follow this well enough (probably because I know nothing about MIPS relocation types) to be a primary reviewer - but could you explain a bit more how this came to be an issue? Which commit broke MIPS here? Is this something we need to support, or perhaps just an overly general unit test that doesn't quite need the coverage it has?

[atanasyan]

In D54937#1310677, @dblaikie wrote:

I'm not sure I follow this well enough (probably because I know nothing about MIPS relocation types) to be a primary reviewer - but could you explain a bit more how this came to be an issue? Which commit broke MIPS here? Is this something we need to support, or perhaps just an overly general unit test that doesn't quite need the coverage it has?

I try to fix failures on the MIPS buildbot - http://lab.llvm.org:8011/builders/llvm-mips-linux. This buildbot was switched off for a long time so it's difficult to say which commit makes it "red". Anyway the problem with the TestDWARF32Version5Addr8AllForms unit test looks like a bug in MIPS code.

Now MipsAsmPrinter::EmitDebugThreadLocal always emits a special MIPS specific TLS related relocations R_MIPS_TLS_DTPREL32 / R_MIPS_TLS_DTPREL64. Result of both these relocations is "a symbol value + 0x8000 offset". It's okay when DIEExpr makes deal with a TLS variable. In "production" code it's true because DIEExpr is used in two places: DwarfCompileUnit::addExpr and DwarfCompileUnit::addAddressExpr. The addExpr is TLS related. The addAddressExpr is unrelated to TLS, but for now is not called for MIPS (look at TODO in the DwarfDebug::constructCallSiteEntryDIEs). But in unit tests DIEExpr is created in one more place - dwarfgen::DIE::addAttribute. When this code is compiled and executed on MIPS we get incorrect DWARF because 0x8000 offset is applied to regular (non-TLS) data.

[atanasyan]

• Rebased against the trunk

[dblaikie]

Fair enough - thanks!

[atanasyan]

Thanks for review.

llvm/trunk/lib/Target/Mips/MCTargetDesc/MipsMCCodeEmitter.cpp

if (Kind == MCExpr::Target) {
     const MipsMCExpr *MipsExpr = cast<MipsMCExpr>(Expr);
 
     Mips::Fixups FixupKind = Mips::Fixups(0);
     switch (MipsExpr->getKind()) {
     case MipsMCExpr::MEK_None:
     case MipsMCExpr::MEK_Special:
       llvm_unreachable("Unhandled fixup kind!");
       break;
+    case MipsMCExpr::MEK_DTPREL:
+      llvm_unreachable("MEK_DTPREL is used for TLS DIEExpr only");
+      break;
     case MipsMCExpr::MEK_CALL_HI16:
       FixupKind = Mips::fixup_Mips_CALL_HI16;
       break;
     case MipsMCExpr::MEK_CALL_LO16:
       FixupKind = Mips::fixup_Mips_CALL_LO16;
       break;
     case MipsMCExpr::MEK_DTPREL_HI:
       FixupKind = isMicroMips(STI) ? Mips::fixup_MICROMIPS_TLS_DTPREL_HI16

llvm/trunk/lib/Target/Mips/MCTargetDesc/MipsMCExpr.h

 namespace llvm {
 
 class MipsMCExpr : public MCTargetExpr {
 public:
   enum MipsExprKind {
     MEK_None,
     MEK_CALL_HI16,
     MEK_CALL_LO16,
+    MEK_DTPREL,
     MEK_DTPREL_HI,
     MEK_DTPREL_LO,
     MEK_GOT,
     MEK_GOTTPREL,
     MEK_GOT_CALL,
     MEK_GOT_DISP,
     MEK_GOT_HI16,
     MEK_GOT_LO16,

llvm/trunk/lib/Target/Mips/MCTargetDesc/MipsMCExpr.cpp

 void MipsMCExpr::printImpl(raw_ostream &OS, const MCAsmInfo *MAI) const {
   int64_t AbsVal;
 
   switch (Kind) {
   case MEK_None:
   case MEK_Special:
     llvm_unreachable("MEK_None and MEK_Special are invalid");
     break;
+  case MEK_DTPREL:
+    llvm_unreachable("MEK_DTPREL is used for TLS DIEExpr only");
+    break;
   case MEK_CALL_HI16:
     OS << "%call_hi";
     break;
   case MEK_CALL_LO16:
     OS << "%call_lo";
     break;
   case MEK_DTPREL_HI:
     OS << "%dtprel_hi";
MipsMCExpr::evaluateAsRelocatableImpl(MCValue &Res,
   // %hi/%lo/etc. here. Fixup is a null pointer when either of these is the
   // caller.
   if (Res.isAbsolute() && Fixup == nullptr) {
     int64_t AbsVal = Res.getConstant();
     switch (Kind) {
     case MEK_None:
     case MEK_Special:
       llvm_unreachable("MEK_None and MEK_Special are invalid");
+    case MEK_DTPREL:
+      llvm_unreachable("MEK_DTPREL is used for TLS DIEExpr only");
     case MEK_DTPREL_HI:
     case MEK_DTPREL_LO:
     case MEK_GOT:
     case MEK_GOTTPREL:
     case MEK_GOT_CALL:
     case MEK_GOT_DISP:
     case MEK_GOT_HI16:
     case MEK_GOT_LO16:

 }
 
 void MipsMCExpr::fixELFSymbolsInTLSFixups(MCAssembler &Asm) const {
   switch (getKind()) {
   case MEK_None:
   case MEK_Special:
     llvm_unreachable("MEK_None and MEK_Special are invalid");
     break;
+  case MEK_DTPREL:
+    llvm_unreachable("MEK_DTPREL is used for TLS DIEExpr only");
+    break;
   case MEK_CALL_HI16:
   case MEK_CALL_LO16:
   case MEK_GOT:
   case MEK_GOT_CALL:
   case MEK_GOT_DISP:
   case MEK_GOT_HI16:
   case MEK_GOT_LO16:
   case MEK_GOT_OFST:

llvm/trunk/lib/Target/Mips/MipsAsmPrinter.cpp

 void MipsAsmPrinter::PrintDebugValueComment(const MachineInstr *MI,
                                            raw_ostream &OS) {
   // TODO: implement
 }
 
 // Emit .dtprelword or .dtpreldword directive
 // and value for debug thread local expression.
 void MipsAsmPrinter::EmitDebugValue(const MCExpr *Value, unsigned Size) const {
+  if (auto *MipsExpr = dyn_cast<MipsMCExpr>(Value)) {
+    if (MipsExpr && MipsExpr->getKind() == MipsMCExpr::MEK_DTPREL) {
       switch (Size) {
       case 4:
-    OutStreamer->EmitDTPRel32Value(Value);
+        OutStreamer->EmitDTPRel32Value(MipsExpr->getSubExpr());
         break;
       case 8:
-    OutStreamer->EmitDTPRel64Value(Value);
+        OutStreamer->EmitDTPRel64Value(MipsExpr->getSubExpr());
         break;
       default:
         llvm_unreachable("Unexpected size of expression value.");
       }
+      return;
+    }
+  }
+  AsmPrinter::EmitDebugValue(Value, Size);
 }
 
 // Align all targets of indirect branches on bundle size.  Used only if target
 // is NaCl.
 void MipsAsmPrinter::NaClAlignIndirectJumpTargets(MachineFunction &MF) {
   // Align all blocks that are jumped to through jump table.
   if (MachineJumpTableInfo *JtInfo = MF.getJumpTableInfo()) {
     const std::vector<MachineJumpTableEntry> &JT = JtInfo->getJumpTables();

llvm/trunk/lib/Target/Mips/MipsTargetObjectFile.cpp

 //===-- MipsTargetObjectFile.cpp - Mips Object Files ----------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "MipsTargetObjectFile.h"
 #include "MipsSubtarget.h"
 #include "MipsTargetMachine.h"
+#include "MCTargetDesc/MipsMCExpr.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCSectionELF.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Target/TargetMachine.h"
MCSection *MipsTargetObjectFile::getSectionForConstant(const DataLayout &DL,
   // Otherwise, we work the same as ELF.
   return TargetLoweringObjectFileELF::getSectionForConstant(DL, Kind, C, Align);
 }
 
 const MCExpr *
 MipsTargetObjectFile::getDebugThreadLocalSymbol(const MCSymbol *Sym) const {
   const MCExpr *Expr =
       MCSymbolRefExpr::create(Sym, MCSymbolRefExpr::VK_None, getContext());
-  return MCBinaryExpr::createAdd(
+  Expr = MCBinaryExpr::createAdd(
       Expr, MCConstantExpr::create(0x8000, getContext()), getContext());
+  return MipsMCExpr::create(MipsMCExpr::MEK_DTPREL, Expr, getContext());
 }