[PowerPC] add secure plt support for TLS symbols

[spetrovic]

Summary

This patch supports secure plt mode for TLS symbols, it's extension for https://reviews.llvm.org/D42112.

[nemanjai]

I think overall, it would be good to explain the semantics of this patch. Maybe even provide a reference to a document that explains it if it isn't something that can be summarized into a comment in the review (or a comment in the code).

Also, I imagine you'll want to add some code to getGlobalBaseReg() to disable shrink-wrapping since you're adding a clobber of the LR without introducing any frame references, so shrink-wrapping may move the prologue out of the entry.

lib/Target/PowerPC/InstPrinter/PPCInstPrinter.cpp

445 ↗

(On Diff #142010)

Nit: naming convention - variables start with an upper case letter. I realize it was that way, but since you're changing its type, might as well change its name.

lib/Target/PowerPC/PPCISelDAGToDAG.cpp

4011 ↗

(On Diff #142010)

A comment explaining why this is needed would be great.

4019 ↗

(On Diff #142010)

I think it's pertinent to add a comment to calls like this one outlining why the virtual SDNode returned by this function does not need to have a use and it still won't end up being DCE'd.

test/CodeGen/PowerPC/ppc32-secure-plt-tls.ll

13 ↗

(On Diff #142010)

I imagine there's supposed to be code up here that sets up the base pointer. Do we not want to check for that code?

[spetrovic]

Regarding secure plt in general I was using this document: http://devpit.org/wiki/Debugging_PowerPC_ELF_Binaries. This patch just expands secure-plt support for TLS symbols, basically the same thing as for functions symbols or global symbols in the last patch.
Also as a reference I'm using gcc.
I haven't run into example that fails because of shrink-wrapping. If this is a potential issue would not it have appeared so far, since
getGlobalBaseReg() is existing function and it was used before secure-plt support? Do you have some test case that potentially can reproduce issue related with shrink-wrapping?

test/CodeGen/PowerPC/ppc32-secure-plt-tls.ll

13 ↗

(On Diff #142010)

These 3 instructions (mflr,addis,addi) are setting up the base pointer. There is one more instruction in this sequence that I forgot to check (bl). I have added it in updated patch.

[spetrovic]

ping

[spetrovic]

ping

[hfinkel]

lib/Target/PowerPC/PPCAsmPrinter.cpp

494 ↗

(On Diff #143295)

Please add a comment here explaining why we're adding 32768.

lib/Target/PowerPC/PPCISelDAGToDAG.cpp

4021 ↗

(On Diff #143295)

Period at the end of the comment.

[spetrovic]

ping

[hfinkel]

LGTM

lib/Target/PowerPC/PPCAsmPrinter.cpp

493 ↗

(On Diff #148797)

Can you please reference a document and a section number, or similar?

[spetrovic]

lib/Target/PowerPC/PPCAsmPrinter.cpp

493 ↗

(On Diff #148797)

I was referencing on source from binutils tools and gcc. Is that OK with you ?

llvm/lib/Target/PowerPC/InstPrinter/PPCInstPrinter.cpp

void PPCInstPrinter::printMemRegReg(const MCInst *MI, unsigned OpNo,
   printOperand(MI, OpNo+1, O);
 }
 
 void PPCInstPrinter::printTLSCall(const MCInst *MI, unsigned OpNo,
                                   raw_ostream &O) {
   // On PPC64, VariantKind is VK_None, but on PPC32, it's VK_PLT, and it must
   // come at the _end_ of the expression.
   const MCOperand &Op = MI->getOperand(OpNo);
-  const MCSymbolRefExpr &refExp = cast<MCSymbolRefExpr>(*Op.getExpr());
-  O << refExp.getSymbol().getName();
+  const MCSymbolRefExpr *RefExp = nullptr;
+  const MCConstantExpr *ConstExp = nullptr;
+  if (const MCBinaryExpr *BinExpr = dyn_cast<MCBinaryExpr>(Op.getExpr())) {
+    RefExp = cast<MCSymbolRefExpr>(BinExpr->getLHS());
+    ConstExp = cast<MCConstantExpr>(BinExpr->getRHS());
+  } else
+    RefExp = cast<MCSymbolRefExpr>(Op.getExpr());
+
+  O << RefExp->getSymbol().getName();
   O << '(';
   printOperand(MI, OpNo+1, O);
   O << ')';
-  if (refExp.getKind() != MCSymbolRefExpr::VK_None)
-    O << '@' << MCSymbolRefExpr::getVariantKindName(refExp.getKind());
+  if (RefExp->getKind() != MCSymbolRefExpr::VK_None)
+    O << '@' << MCSymbolRefExpr::getVariantKindName(RefExp->getKind());
+  if (ConstExp != nullptr)
+    O << '+' << ConstExp->getValue();
 }
 
 /// showRegistersWithPercentPrefix - Check if this register name should be
 /// printed with a percentage symbol as prefix.
 bool PPCInstPrinter::showRegistersWithPercentPrefix(const char *RegName) const {
   if (!FullRegNamesWithPercent || TT.isOSDarwin() || TT.getOS() == Triple::AIX)
     return false;
 

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

void PPCAsmPrinter::EmitTlsCall(const MachineInstr *MI,
   assert(MI->getOperand(1).isReg() &&
          ((Subtarget->isPPC64() && MI->getOperand(1).getReg() == PPC::X3) ||
           (!Subtarget->isPPC64() && MI->getOperand(1).getReg() == PPC::R3)) &&
          "GETtls[ld]ADDR[32] must read GPR3");
 
   if (!Subtarget->isPPC64() && !Subtarget->isDarwin() &&
       isPositionIndependent())
     Kind = MCSymbolRefExpr::VK_PLT;
-  const MCSymbolRefExpr *TlsRef =
+  const MCExpr *TlsRef =
     MCSymbolRefExpr::create(TlsGetAddr, Kind, OutContext);
+
+  // Add 32768 offset to the symbol so we follow up the latest GOT/PLT ABI.
+  if (Kind == MCSymbolRefExpr::VK_PLT && Subtarget->isSecurePlt())
+    TlsRef = MCBinaryExpr::createAdd(TlsRef,
+                                     MCConstantExpr::create(32768, OutContext),
+                                     OutContext);
   const MachineOperand &MO = MI->getOperand(2);
   const GlobalValue *GValue = MO.getGlobal();
   MCSymbol *MOSymbol = getSymbol(GValue);
   const MCExpr *SymVar = MCSymbolRefExpr::create(MOSymbol, VK, OutContext);
   EmitToStreamer(*OutStreamer,
                  MCInstBuilder(Subtarget->isPPC64() ?
                                PPC::BL8_NOP_TLS : PPC::BL_TLS)
                  .addExpr(TlsRef)

llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp

if (N->getValueType(0) == MVT::i64) {
       return;
     }
     break;
 
   case ISD::SETCC:
     if (trySETCC(N))
       return;
     break;
-
+  // These nodes will be transformed into GETtlsADDR32 node, which
+  // later becomes BL_TLS __tls_get_addr(sym at tlsgd)@PLT
+  case PPCISD::ADDI_TLSLD_L_ADDR:
+  case PPCISD::ADDI_TLSGD_L_ADDR: {
+    const Module *Mod = MF->getFunction().getParent();
+    if (PPCLowering->getPointerTy(CurDAG->getDataLayout()) != MVT::i32 ||
+        !PPCSubTarget->isSecurePlt() || !PPCSubTarget->isTargetELF() ||
+        Mod->getPICLevel() == PICLevel::SmallPIC)
+      break;
+      // Attach global base pointer on GETtlsADDR32 node in order to
+      // generate secure plt code for TLS symbols.
+      getGlobalBaseReg();
+  }
+    break;
   case PPCISD::CALL: {
     const Module *M = MF->getFunction().getParent();
 
     if (PPCLowering->getPointerTy(CurDAG->getDataLayout()) != MVT::i32 ||
         (!TM.isPositionIndependent() || !PPCSubTarget->isSecurePlt()) ||
         !PPCSubTarget->isTargetELF() || M->getPICLevel() == PICLevel::SmallPIC)
       break;
 

llvm/test/CodeGen/PowerPC/ppc32-secure-plt-tls.ll

+; RUN: llc < %s -mtriple=powerpc-unknown-linux-gnu -mattr=+secure-plt -relocation-model=pic | FileCheck -check-prefix=SECURE-PLT-TLS %s
+
+@a = thread_local local_unnamed_addr global i32 6, align 4
+define i32 @main() local_unnamed_addr #0 {
+entry:
+  %0 = load i32, i32* @a, align 4
+  ret i32 %0
+}
+
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 7, !"PIC Level", i32 2}
+
+; SECURE-PLT-TLS:       mflr 30
+; SECURE-PLT-TLS-NEXT:  addis 30, 30, .LTOC-.L0$pb@ha
+; SECURE-PLT-TLS-NEXT:  addi 30, 30, .LTOC-.L0$pb@l
+; SECURE-PLT-TLS-NEXT:  bl .L{{.*}}
+; SECURE-PLT-TLS:       bl __tls_get_addr(a@tlsgd)@PLT+32768
 No newline at end of file