Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 18 Mar 2014 19:23:41 +0000 (UTC)
From:      Dimitry Andric <dim@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r263312 - in head/contrib/llvm: include/llvm/CodeGen lib/CodeGen lib/CodeGen/SelectionDAG lib/MC/MCParser lib/Target/X86
Message-ID:  <201403181923.s2IJNfZv006943@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: dim
Date: Tue Mar 18 19:23:41 2014
New Revision: 263312
URL: http://svnweb.freebsd.org/changeset/base/263312

Log:
  Pull in r196939 from upstream llvm trunk (by Reid Kleckner):
  
    Reland "Fix miscompile of MS inline assembly with stack realignment"
  
    This re-lands commit r196876, which was reverted in r196879.
  
    The tests have been fixed to pass on platforms with a stack alignment
    larger than 4.
  
    Update to clang side tests will land shortly.
  
  Pull in r196986 from upstream llvm trunk (by Reid Kleckner):
  
    Revert the backend fatal error from r196939
  
    The combination of inline asm, stack realignment, and dynamic allocas
    turns out to be too common to reject out of hand.
  
    ASan inserts empy inline asm fragments and uses aligned allocas.
    Compiling any trivial function containing a dynamic alloca with ASan is
    enough to trigger the check.
  
    XFAIL the test cases that would be miscompiled and add one that uses the
    relevant functionality.
  
  Pull in r202930 from upstream llvm trunk (by Hans Wennborg):
  
    Check for dynamic allocas and inline asm that clobbers sp before building
    selection dag (PR19012)
  
    In X86SelectionDagInfo::EmitTargetCodeForMemcpy we check with MachineFrameInfo
    to make sure that ESI isn't used as a base pointer register before we choose to
    emit rep movs (which clobbers esi).
  
    The problem is that MachineFrameInfo wouldn't know about dynamic allocas or
    inline asm that clobbers the stack pointer until SelectionDAGBuilder has
    encountered them.
  
    This patch fixes the problem by checking for such things when building the
    FunctionLoweringInfo.
  
    Differential Revision: http://llvm-reviews.chandlerc.com/D2954
  
  Together, these commits fix the problem encountered in the devel/emacs
  port on the i386 architecture, where a combination of stack realignment,
  alloca() and memcpy() could incidentally clobber the %esi register,
  leading to segfaults in the temacs build-time utility.
  
  See also: http://llvm.org/PR18171 and http://llvm.org/PR19012
  
  Reported by:	ashish
  PR:		ports/183064
  MFC after:	1 week

Modified:
  head/contrib/llvm/include/llvm/CodeGen/FunctionLoweringInfo.h
  head/contrib/llvm/include/llvm/CodeGen/MachineFrameInfo.h
  head/contrib/llvm/include/llvm/CodeGen/MachineFunction.h
  head/contrib/llvm/lib/CodeGen/MachineFunction.cpp
  head/contrib/llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
  head/contrib/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  head/contrib/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
  head/contrib/llvm/lib/MC/MCParser/AsmParser.cpp
  head/contrib/llvm/lib/Target/X86/X86FrameLowering.cpp
  head/contrib/llvm/lib/Target/X86/X86RegisterInfo.cpp

Modified: head/contrib/llvm/include/llvm/CodeGen/FunctionLoweringInfo.h
==============================================================================
--- head/contrib/llvm/include/llvm/CodeGen/FunctionLoweringInfo.h	Tue Mar 18 18:10:35 2014	(r263311)
+++ head/contrib/llvm/include/llvm/CodeGen/FunctionLoweringInfo.h	Tue Mar 18 19:23:41 2014	(r263312)
@@ -41,6 +41,7 @@ class MachineBasicBlock;
 class MachineFunction;
 class MachineModuleInfo;
 class MachineRegisterInfo;
+class SelectionDAG;
 class TargetLowering;
 class Value;
 
@@ -125,7 +126,7 @@ public:
   /// set - Initialize this FunctionLoweringInfo with the given Function
   /// and its associated MachineFunction.
   ///
-  void set(const Function &Fn, MachineFunction &MF);
+  void set(const Function &Fn, MachineFunction &MF, SelectionDAG *DAG);
 
   /// clear - Clear out all the function-specific state. This returns this
   /// FunctionLoweringInfo to an empty state, ready to be used for a

Modified: head/contrib/llvm/include/llvm/CodeGen/MachineFrameInfo.h
==============================================================================
--- head/contrib/llvm/include/llvm/CodeGen/MachineFrameInfo.h	Tue Mar 18 18:10:35 2014	(r263311)
+++ head/contrib/llvm/include/llvm/CodeGen/MachineFrameInfo.h	Tue Mar 18 19:23:41 2014	(r263312)
@@ -223,6 +223,10 @@ class MachineFrameInfo {
   /// Whether the "realign-stack" option is on.
   bool RealignOption;
 
+  /// True if the function includes inline assembly that adjusts the stack
+  /// pointer.
+  bool HasInlineAsmWithSPAdjust;
+
   const TargetFrameLowering *getFrameLowering() const;
 public:
     explicit MachineFrameInfo(const TargetMachine &TM, bool RealignOpt)
@@ -240,6 +244,7 @@ public:
     LocalFrameSize = 0;
     LocalFrameMaxAlign = 0;
     UseLocalStackAllocationBlock = false;
+    HasInlineAsmWithSPAdjust = false;
   }
 
   /// hasStackObjects - Return true if there are any stack objects in this
@@ -451,6 +456,10 @@ public:
   bool hasCalls() const { return HasCalls; }
   void setHasCalls(bool V) { HasCalls = V; }
 
+  /// Returns true if the function contains any stack-adjusting inline assembly.
+  bool hasInlineAsmWithSPAdjust() const { return HasInlineAsmWithSPAdjust; }
+  void setHasInlineAsmWithSPAdjust(bool B) { HasInlineAsmWithSPAdjust = B; }
+
   /// getMaxCallFrameSize - Return the maximum size of a call frame that must be
   /// allocated for an outgoing function call.  This is only available if
   /// CallFrameSetup/Destroy pseudo instructions are used by the target, and
@@ -521,7 +530,7 @@ public:
   /// variable sized object is created, whether or not the index returned is
   /// actually used.
   ///
-  int CreateVariableSizedObject(unsigned Alignment);
+  int CreateVariableSizedObject(unsigned Alignment, const AllocaInst *Alloca);
 
   /// getCalleeSavedInfo - Returns a reference to call saved info vector for the
   /// current function.

Modified: head/contrib/llvm/include/llvm/CodeGen/MachineFunction.h
==============================================================================
--- head/contrib/llvm/include/llvm/CodeGen/MachineFunction.h	Tue Mar 18 18:10:35 2014	(r263311)
+++ head/contrib/llvm/include/llvm/CodeGen/MachineFunction.h	Tue Mar 18 19:23:41 2014	(r263312)
@@ -131,8 +131,8 @@ class MachineFunction {
   /// about the control flow of such functions.
   bool ExposesReturnsTwice;
 
-  /// True if the function includes MS-style inline assembly.
-  bool HasMSInlineAsm;
+  /// True if the function includes any inline assembly.
+  bool HasInlineAsm;
 
   MachineFunction(const MachineFunction &) LLVM_DELETED_FUNCTION;
   void operator=(const MachineFunction&) LLVM_DELETED_FUNCTION;
@@ -218,15 +218,14 @@ public:
     ExposesReturnsTwice = B;
   }
 
-  /// Returns true if the function contains any MS-style inline assembly.
-  bool hasMSInlineAsm() const {
-    return HasMSInlineAsm;
+  /// Returns true if the function contains any inline assembly.
+  bool hasInlineAsm() const {
+    return HasInlineAsm;
   }
 
-  /// Set a flag that indicates that the function contains MS-style inline
-  /// assembly.
-  void setHasMSInlineAsm(bool B) {
-    HasMSInlineAsm = B;
+  /// Set a flag that indicates that the function contains inline assembly.
+  void setHasInlineAsm(bool B) {
+    HasInlineAsm = B;
   }
   
   /// getInfo - Keep track of various per-function pieces of information for

Modified: head/contrib/llvm/lib/CodeGen/MachineFunction.cpp
==============================================================================
--- head/contrib/llvm/lib/CodeGen/MachineFunction.cpp	Tue Mar 18 18:10:35 2014	(r263311)
+++ head/contrib/llvm/lib/CodeGen/MachineFunction.cpp	Tue Mar 18 19:23:41 2014	(r263312)
@@ -525,13 +525,14 @@ int MachineFrameInfo::CreateSpillStackOb
 /// variable sized object is created, whether or not the index returned is
 /// actually used.
 ///
-int MachineFrameInfo::CreateVariableSizedObject(unsigned Alignment) {
+int MachineFrameInfo::CreateVariableSizedObject(unsigned Alignment,
+                                                const AllocaInst *Alloca) {
   HasVarSizedObjects = true;
   Alignment =
     clampStackAlignment(!getFrameLowering()->isStackRealignable() ||
                           !RealignOption,
                         Alignment, getFrameLowering()->getStackAlignment()); 
-  Objects.push_back(StackObject(0, Alignment, 0, false, false, true, 0));
+  Objects.push_back(StackObject(0, Alignment, 0, false, false, true, Alloca));
   ensureMaxAlignment(Alignment);
   return (int)Objects.size()-NumFixedObjects-1;
 }

Modified: head/contrib/llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
==============================================================================
--- head/contrib/llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp	Tue Mar 18 18:10:35 2014	(r263311)
+++ head/contrib/llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp	Tue Mar 18 19:23:41 2014	(r263312)
@@ -33,6 +33,7 @@
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Target/TargetInstrInfo.h"
+#include "llvm/Target/TargetFrameLowering.h"
 #include "llvm/Target/TargetLowering.h"
 #include "llvm/Target/TargetOptions.h"
 #include "llvm/Target/TargetRegisterInfo.h"
@@ -55,7 +56,8 @@ static bool isUsedOutsideOfDefiningBlock
   return false;
 }
 
-void FunctionLoweringInfo::set(const Function &fn, MachineFunction &mf) {
+void FunctionLoweringInfo::set(const Function &fn, MachineFunction &mf,
+                               SelectionDAG *DAG) {
   const TargetLowering *TLI = TM.getTargetLowering();
 
   Fn = &fn;
@@ -100,6 +102,43 @@ void FunctionLoweringInfo::set(const Fun
   for (; BB != EB; ++BB)
     for (BasicBlock::const_iterator I = BB->begin(), E = BB->end();
          I != E; ++I) {
+      // Look for dynamic allocas.
+      if (const AllocaInst *AI = dyn_cast<AllocaInst>(I)) {
+        if (!AI->isStaticAlloca()) {
+          unsigned Align = std::max(
+              (unsigned)TLI->getDataLayout()->getPrefTypeAlignment(
+                AI->getAllocatedType()),
+              AI->getAlignment());
+          unsigned StackAlign = TM.getFrameLowering()->getStackAlignment();
+          if (Align <= StackAlign)
+            Align = 0;
+          // Inform the Frame Information that we have variable-sized objects.
+          MF->getFrameInfo()->CreateVariableSizedObject(Align ? Align : 1, AI);
+        }
+      }
+
+      // Look for inline asm that clobbers the SP register.
+      if (isa<CallInst>(I) || isa<InvokeInst>(I)) {
+        ImmutableCallSite CS(I);
+        if (const InlineAsm *IA = dyn_cast<InlineAsm>(CS.getCalledValue())) {
+          unsigned SP = TLI->getStackPointerRegisterToSaveRestore();
+          std::vector<TargetLowering::AsmOperandInfo> Ops =
+            TLI->ParseConstraints(CS);
+          for (size_t I = 0, E = Ops.size(); I != E; ++I) {
+            TargetLowering::AsmOperandInfo &Op = Ops[I];
+            if (Op.Type == InlineAsm::isClobber) {
+              // Clobbers don't have SDValue operands, hence SDValue().
+              TLI->ComputeConstraintToUse(Op, SDValue(), DAG);
+              std::pair<unsigned, const TargetRegisterClass*> PhysReg =
+                TLI->getRegForInlineAsmConstraint(Op.ConstraintCode,
+                                                  Op.ConstraintVT);
+              if (PhysReg.first == SP)
+                MF->getFrameInfo()->setHasInlineAsmWithSPAdjust(true);
+            }
+          }
+        }
+      }
+
       // Mark values used outside their block as exported, by allocating
       // a virtual register for them.
       if (isUsedOutsideOfDefiningBlock(I))

Modified: head/contrib/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
==============================================================================
--- head/contrib/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp	Tue Mar 18 18:10:35 2014	(r263311)
+++ head/contrib/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp	Tue Mar 18 19:23:41 2014	(r263312)
@@ -851,12 +851,20 @@ void RegsForValue::AddInlineAsmOperands(
   SDValue Res = DAG.getTargetConstant(Flag, MVT::i32);
   Ops.push_back(Res);
 
+  unsigned SP = TLI.getStackPointerRegisterToSaveRestore();
   for (unsigned Value = 0, Reg = 0, e = ValueVTs.size(); Value != e; ++Value) {
     unsigned NumRegs = TLI.getNumRegisters(*DAG.getContext(), ValueVTs[Value]);
     MVT RegisterVT = RegVTs[Value];
     for (unsigned i = 0; i != NumRegs; ++i) {
       assert(Reg < Regs.size() && "Mismatch in # registers expected");
-      Ops.push_back(DAG.getRegister(Regs[Reg++], RegisterVT));
+      unsigned TheReg = Regs[Reg++];
+      Ops.push_back(DAG.getRegister(TheReg, RegisterVT));
+
+      if (TheReg == SP && Code == InlineAsm::Kind_Clobber) {
+        // If we clobbered the stack pointer, MFI should know about it.
+        assert(DAG.getMachineFunction().getFrameInfo()->
+            hasInlineAsmWithSPAdjust());
+      }
     }
   }
 }
@@ -3370,9 +3378,7 @@ void SelectionDAGBuilder::visitAlloca(co
   setValue(&I, DSA);
   DAG.setRoot(DSA.getValue(1));
 
-  // Inform the Frame Information that we have just allocated a variable-sized
-  // object.
-  FuncInfo.MF->getFrameInfo()->CreateVariableSizedObject(Align ? Align : 1);
+  assert(FuncInfo.MF->getFrameInfo()->hasVarSizedObjects());
 }
 
 void SelectionDAGBuilder::visitLoad(const LoadInst &I) {

Modified: head/contrib/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
==============================================================================
--- head/contrib/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp	Tue Mar 18 18:10:35 2014	(r263311)
+++ head/contrib/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp	Tue Mar 18 19:23:41 2014	(r263312)
@@ -419,7 +419,7 @@ bool SelectionDAGISel::runOnMachineFunct
   SplitCriticalSideEffectEdges(const_cast<Function&>(Fn), this);
 
   CurDAG->init(*MF, TTI, TLI);
-  FuncInfo->set(Fn, *MF);
+  FuncInfo->set(Fn, *MF, CurDAG);
 
   if (UseMBPI && OptLevel != CodeGenOpt::None)
     FuncInfo->BPI = &getAnalysis<BranchProbabilityInfo>();
@@ -428,7 +428,8 @@ bool SelectionDAGISel::runOnMachineFunct
 
   SDB->init(GFI, *AA, LibInfo);
 
-  MF->setHasMSInlineAsm(false);
+  MF->setHasInlineAsm(false);
+
   SelectAllBasicBlocks(Fn);
 
   // If the first basic block in the function has live ins that need to be
@@ -511,7 +512,7 @@ bool SelectionDAGISel::runOnMachineFunct
   for (MachineFunction::const_iterator I = MF->begin(), E = MF->end(); I != E;
        ++I) {
 
-    if (MFI->hasCalls() && MF->hasMSInlineAsm())
+    if (MFI->hasCalls() && MF->hasInlineAsm())
       break;
 
     const MachineBasicBlock *MBB = I;
@@ -522,8 +523,8 @@ bool SelectionDAGISel::runOnMachineFunct
           II->isStackAligningInlineAsm()) {
         MFI->setHasCalls(true);
       }
-      if (II->isMSInlineAsm()) {
-        MF->setHasMSInlineAsm(true);
+      if (II->isInlineAsm()) {
+        MF->setHasInlineAsm(true);
       }
     }
   }

Modified: head/contrib/llvm/lib/MC/MCParser/AsmParser.cpp
==============================================================================
--- head/contrib/llvm/lib/MC/MCParser/AsmParser.cpp	Tue Mar 18 18:10:35 2014	(r263311)
+++ head/contrib/llvm/lib/MC/MCParser/AsmParser.cpp	Tue Mar 18 19:23:41 2014	(r263312)
@@ -4192,6 +4192,11 @@ bool AsmParser::parseMSInlineAsm(
         AsmStrRewrites.push_back(AsmRewrite(AOK_Input, Start, SymName.size()));
       }
     }
+
+    // Consider implicit defs to be clobbers.  Think of cpuid and push.
+    const uint16_t *ImpDefs = Desc.getImplicitDefs();
+    for (unsigned I = 0, E = Desc.getNumImplicitDefs(); I != E; ++I)
+      ClobberRegs.push_back(ImpDefs[I]);
   }
 
   // Set the number of Outputs and Inputs.

Modified: head/contrib/llvm/lib/Target/X86/X86FrameLowering.cpp
==============================================================================
--- head/contrib/llvm/lib/Target/X86/X86FrameLowering.cpp	Tue Mar 18 18:10:35 2014	(r263311)
+++ head/contrib/llvm/lib/Target/X86/X86FrameLowering.cpp	Tue Mar 18 19:23:41 2014	(r263312)
@@ -50,7 +50,7 @@ bool X86FrameLowering::hasFP(const Machi
   return (MF.getTarget().Options.DisableFramePointerElim(MF) ||
           RegInfo->needsStackRealignment(MF) ||
           MFI->hasVarSizedObjects() ||
-          MFI->isFrameAddressTaken() || MF.hasMSInlineAsm() ||
+          MFI->isFrameAddressTaken() || MFI->hasInlineAsmWithSPAdjust() ||
           MF.getInfo<X86MachineFunctionInfo>()->getForceFramePointer() ||
           MMI.callsUnwindInit() || MMI.callsEHReturn());
 }

Modified: head/contrib/llvm/lib/Target/X86/X86RegisterInfo.cpp
==============================================================================
--- head/contrib/llvm/lib/Target/X86/X86RegisterInfo.cpp	Tue Mar 18 18:10:35 2014	(r263311)
+++ head/contrib/llvm/lib/Target/X86/X86RegisterInfo.cpp	Tue Mar 18 19:23:41 2014	(r263312)
@@ -403,18 +403,15 @@ bool X86RegisterInfo::hasBasePointer(con
    if (!EnableBasePointer)
      return false;
 
-   // When we need stack realignment and there are dynamic allocas, we can't
-   // reference off of the stack pointer, so we reserve a base pointer.
-   //
-   // This is also true if the function contain MS-style inline assembly.  We
-   // do this because if any stack changes occur in the inline assembly, e.g.,
-   // "pusha", then any C local variable or C argument references in the
-   // inline assembly will be wrong because the SP is not properly tracked.
-   if ((needsStackRealignment(MF) && MFI->hasVarSizedObjects()) ||
-       MF.hasMSInlineAsm())
-     return true;
-
-   return false;
+   // When we need stack realignment, we can't address the stack from the frame
+   // pointer.  When we have dynamic allocas or stack-adjusting inline asm, we
+   // can't address variables from the stack pointer.  MS inline asm can
+   // reference locals while also adjusting the stack pointer.  When we can't
+   // use both the SP and the FP, we need a separate base pointer register.
+   bool CantUseFP = needsStackRealignment(MF);
+   bool CantUseSP =
+       MFI->hasVarSizedObjects() || MFI->hasInlineAsmWithSPAdjust();
+   return CantUseFP && CantUseSP;
 }
 
 bool X86RegisterInfo::canRealignStack(const MachineFunction &MF) const {



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201403181923.s2IJNfZv006943>