Use ARM-style representation for C++ method pointers under PNaCl/Emscripten

Before this change, Clang uses the x86 representation for C++ method
pointers when generating code for PNaCl.  However, the resulting code
will assume that function pointers are 0 mod 2.  This assumption is
not safe for PNaCl, where function pointers could have any value
(especially in future sandboxing models).

So, switch to using the ARM representation for PNaCl code, which makes
no assumptions about the alignment of function pointers.

Since we're changing the "le32" target, this change also applies to
Emscripten.  The change is beneficial for Emscripten too.  Emscripten
has a workaround to make function pointers 0 mod 2.  This change would
allow the workaround to be removed.

See: https://code.google.com/p/nativeclient/issues/detail?id=3450
llvm-svn: 187051

diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp b/clang/lib/CodeGen/ItaniumCXXABI.cpp
index 55814b4..a1e197c 100644 (file)
--- a/clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -35,11 +35,15 @@ using namespace CodeGen;
 namespace {
 class ItaniumCXXABI : public CodeGen::CGCXXABI {
 protected:
-  bool IsARM;
+  bool UseARMMethodPtrABI;
+  bool UseARMGuardVarABI;
 
 public:
-  ItaniumCXXABI(CodeGen::CodeGenModule &CGM, bool IsARM = false) :
-    CGCXXABI(CGM), IsARM(IsARM) { }
+  ItaniumCXXABI(CodeGen::CodeGenModule &CGM,
+                bool UseARMMethodPtrABI = false,
+                bool UseARMGuardVarABI = false) :
+    CGCXXABI(CGM), UseARMMethodPtrABI(UseARMMethodPtrABI),
+    UseARMGuardVarABI(UseARMGuardVarABI) { }
 
   bool isReturnTypeIndirect(const CXXRecordDecl *RD) const {
     // Structures with either a non-trivial destructor or a non-trivial
@@ -175,7 +179,9 @@ public:
 
 class ARMCXXABI : public ItaniumCXXABI {
 public:
-  ARMCXXABI(CodeGen::CodeGenModule &CGM) : ItaniumCXXABI(CGM, /*ARM*/ true) {}
+  ARMCXXABI(CodeGen::CodeGenModule &CGM) :
+    ItaniumCXXABI(CGM, /* UseARMMethodPtrABI = */ true,
+                  /* UseARMGuardVarABI = */ true) {}
 
   bool HasThisReturn(GlobalDecl GD) const {
     return (isa<CXXConstructorDecl>(GD.getDecl()) || (
@@ -208,9 +214,18 @@ CodeGen::CGCXXABI *CodeGen::CreateItaniumCXXABI(CodeGenModule &CGM) {
   // include the other 32-bit ARM oddities: constructor/destructor return values
   // and array cookies.
   case TargetCXXABI::GenericAArch64:
-    return  new ItaniumCXXABI(CGM, /*IsARM = */ true);
+    return new ItaniumCXXABI(CGM, /* UseARMMethodPtrABI = */ true,
+                             /* UseARMGuardVarABI = */ true);
 
   case TargetCXXABI::GenericItanium:
+    if (CGM.getContext().getTargetInfo().getTriple().getArch()
+        == llvm::Triple::le32) {
+      // For PNaCl, use ARM-style method pointers so that PNaCl code
+      // does not assume anything about the alignment of function
+      // pointers.
+      return new ItaniumCXXABI(CGM, /* UseARMMethodPtrABI = */ true,
+                               /* UseARMGuardVarABI = */ false);
+    }
     return new ItaniumCXXABI(CGM);
 
   case TargetCXXABI::Microsoft:
@@ -273,7 +288,7 @@ ItaniumCXXABI::EmitLoadOfMemberFunctionPointer(CodeGenFunction &CGF,
 
   // Compute the true adjustment.
   llvm::Value *Adj = RawAdj;
-  if (IsARM)
+  if (UseARMMethodPtrABI)
     Adj = Builder.CreateAShr(Adj, ptrdiff_1, "memptr.adj.shifted");
 
   // Apply the adjustment and cast back to the original struct type
@@ -288,7 +303,7 @@ ItaniumCXXABI::EmitLoadOfMemberFunctionPointer(CodeGenFunction &CGF,
   // If the LSB in the function pointer is 1, the function pointer points to
   // a virtual function.
   llvm::Value *IsVirtual;
-  if (IsARM)
+  if (UseARMMethodPtrABI)
     IsVirtual = Builder.CreateAnd(RawAdj, ptrdiff_1);
   else
     IsVirtual = Builder.CreateAnd(FnAsInt, ptrdiff_1);
@@ -307,7 +322,8 @@ ItaniumCXXABI::EmitLoadOfMemberFunctionPointer(CodeGenFunction &CGF,
 
   // Apply the offset.
   llvm::Value *VTableOffset = FnAsInt;
-  if (!IsARM) VTableOffset = Builder.CreateSub(VTableOffset, ptrdiff_1);
+  if (!UseARMMethodPtrABI)
+    VTableOffset = Builder.CreateSub(VTableOffset, ptrdiff_1);
   VTable = Builder.CreateGEP(VTable, VTableOffset);
 
   // Load the virtual function to call.
@@ -417,7 +433,7 @@ ItaniumCXXABI::EmitMemberPointerConversion(CodeGenFunction &CGF,
   }
 
   // The this-adjustment is left-shifted by 1 on ARM.
-  if (IsARM) {
+  if (UseARMMethodPtrABI) {
     uint64_t offset = cast<llvm::ConstantInt>(adj)->getZExtValue();
     offset <<= 1;
     adj = llvm::ConstantInt::get(adj->getType(), offset);
@@ -465,7 +481,7 @@ ItaniumCXXABI::EmitMemberPointerConversion(const CastExpr *E,
   }
 
   // The this-adjustment is left-shifted by 1 on ARM.
-  if (IsARM) {
+  if (UseARMMethodPtrABI) {
     uint64_t offset = cast<llvm::ConstantInt>(adj)->getZExtValue();
     offset <<= 1;
     adj = llvm::ConstantInt::get(adj->getType(), offset);
@@ -523,7 +539,7 @@ llvm::Constant *ItaniumCXXABI::BuildMemberPointer(const CXXMethodDecl *MD,
       Context.toCharUnitsFromBits(Context.getTargetInfo().getPointerWidth(0));
     uint64_t VTableOffset = (Index * PointerWidth.getQuantity());
 
-    if (IsARM) {
+    if (UseARMMethodPtrABI) {
       // ARM C++ ABI 3.2.1:
       //   This ABI specifies that adj contains twice the this
       //   adjustment, plus 1 if the member function is virtual. The
@@ -557,7 +573,8 @@ llvm::Constant *ItaniumCXXABI::BuildMemberPointer(const CXXMethodDecl *MD,
     llvm::Constant *addr = CGM.GetAddrOfFunction(MD, Ty);
 
     MemPtr[0] = llvm::ConstantExpr::getPtrToInt(addr, CGM.PtrDiffTy);
-    MemPtr[1] = llvm::ConstantInt::get(CGM.PtrDiffTy, (IsARM ? 2 : 1) *
+    MemPtr[1] = llvm::ConstantInt::get(CGM.PtrDiffTy,
+                                       (UseARMMethodPtrABI ? 2 : 1) *
                                        ThisAdjustment.getQuantity());
   }
   
@@ -641,7 +658,7 @@ ItaniumCXXABI::EmitMemberPointerComparison(CodeGenFunction &CGF,
 
   // Null member function pointers on ARM clear the low bit of Adj,
   // so the zero condition has to check that neither low bit is set.
-  if (IsARM) {
+  if (UseARMMethodPtrABI) {
     llvm::Value *One = llvm::ConstantInt::get(LPtr->getType(), 1);
 
     // Compute (l.adj | r.adj) & 1 and test it against zero.
@@ -681,7 +698,7 @@ ItaniumCXXABI::EmitMemberPointerIsNotNull(CodeGenFunction &CGF,
 
   // On ARM, a member function pointer is also non-null if the low bit of 'adj'
   // (the virtual bit) is set.
-  if (IsARM) {
+  if (UseARMMethodPtrABI) {
     llvm::Constant *One = llvm::ConstantInt::get(Ptr->getType(), 1);
     llvm::Value *Adj = Builder.CreateExtractValue(MemPtr, 1, "memptr.adj");
     llvm::Value *VirtualBit = Builder.CreateAnd(Adj, One, "memptr.virtualbit");
@@ -1078,7 +1095,7 @@ void ItaniumCXXABI::EmitGuardedInit(CodeGenFunction &CGF,
   } else {
     // Guard variables are 64 bits in the generic ABI and size width on ARM
     // (i.e. 32-bit on AArch32, 64-bit on AArch64).
-    guardTy = (IsARM ? CGF.SizeTy : CGF.Int64Ty);
+    guardTy = (UseARMGuardVarABI ? CGF.SizeTy : CGF.Int64Ty);
   }
   llvm::PointerType *guardPtrTy = guardTy->getPointerTo();
 
@@ -1121,7 +1138,7 @@ void ItaniumCXXABI::EmitGuardedInit(CodeGenFunction &CGF,
   //       if (__cxa_guard_acquire(&obj_guard))
   //         ...
   //     }
-  if (IsARM && !useInt8GuardVariable) {
+  if (UseARMGuardVarABI && !useInt8GuardVariable) {
     llvm::Value *V = Builder.CreateLoad(guard);
     llvm::Value *Test1 = llvm::ConstantInt::get(guardTy, 1);
     V = Builder.CreateAnd(V, Test1);

diff --git a/clang/test/CodeGenCXX/member-function-pointers.cpp b/clang/test/CodeGenCXX/member-function-pointers.cpp
index 84b54b6..23a7b00 100644 (file)
--- a/clang/test/CodeGenCXX/member-function-pointers.cpp
+++ b/clang/test/CodeGenCXX/member-function-pointers.cpp
@@ -4,6 +4,9 @@
 // RUN: %clang_cc1 %s -emit-llvm -o - -triple=i386-unknown-unknown | FileCheck -check-prefix GLOBAL-LP32 %s
 // RUN: %clang_cc1 %s -emit-llvm -o - -triple=armv7-unknown-unknown | FileCheck -check-prefix GLOBAL-ARM %s
 
+// PNaCl uses the same representation of method pointers as ARM.
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=le32-unknown-nacl | FileCheck -check-prefix GLOBAL-ARM %s
+
 struct A { int a; void f(); virtual void vf1(); virtual void vf2(); };
 struct B { int b; virtual void g(); };
 struct C : B, A { };

diff --git a/clang/test/CodeGenCXX/static-init-pnacl.cpp b/clang/test/CodeGenCXX/static-init-pnacl.cpp
new file mode 100644 (file)
index 0000000..65844e5
--- /dev/null
+++ b/clang/test/CodeGenCXX/static-init-pnacl.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -emit-llvm -triple=le32-unknown-nacl -o - %s | FileCheck %s
+
+int f();
+
+// Test that PNaCl uses the Itanium/x86 ABI in which the static
+// variable's guard variable is tested via "load i8 and compare with
+// zero" rather than the ARM ABI which uses "load i32 and test the
+// bottom bit".
+void g() {
+  static int a = f();
+}
+// CHECK: load atomic i8* bitcast (i64* @_ZGVZ1gvE1a to i8*) acquire
+// CHECK-NEXT: %guard.uninitialized = icmp eq i8 %0, 0
+// CHECK-NEXT: br i1 %guard.uninitialized, label %init.check, label %init.end