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

Mark Seaborn mseaborn at chromium.org
Wed Jul 24 09:25:13 PDT 2013


Author: mseaborn
Date: Wed Jul 24 11:25:13 2013
New Revision: 187051

URL: http://llvm.org/viewvc/llvm-project?rev=187051&view=rev
Log:
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

Added:
    cfe/trunk/test/CodeGenCXX/static-init-pnacl.cpp   (with props)
Modified:
    cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
    cfe/trunk/test/CodeGenCXX/member-function-pointers.cpp

Modified: cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp?rev=187051&r1=187050&r2=187051&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp (original)
+++ cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp Wed Jul 24 11:25:13 2013
@@ -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::CreateItaniu
   // 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::EmitLoadOfMemberFunctionP
 
   // 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::EmitLoadOfMemberFunctionP
   // 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::EmitLoadOfMemberFunctionP
 
   // 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::EmitMemberPointerConversi
   }
 
   // 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::EmitMemberPointerConversi
   }
 
   // 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::BuildMemb
       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::BuildMemb
     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::EmitMemberPointerComparis
 
   // 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::EmitMemberPointerIsNotNul
 
   // 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(Code
   } 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(Code
   //       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);

Modified: cfe/trunk/test/CodeGenCXX/member-function-pointers.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/member-function-pointers.cpp?rev=187051&r1=187050&r2=187051&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/member-function-pointers.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/member-function-pointers.cpp Wed Jul 24 11:25:13 2013
@@ -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 { };

Added: cfe/trunk/test/CodeGenCXX/static-init-pnacl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/static-init-pnacl.cpp?rev=187051&view=auto
==============================================================================
--- cfe/trunk/test/CodeGenCXX/static-init-pnacl.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/static-init-pnacl.cpp Wed Jul 24 11:25:13 2013
@@ -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

Propchange: cfe/trunk/test/CodeGenCXX/static-init-pnacl.cpp
------------------------------------------------------------------------------
    svn:eol-style = LF





More information about the cfe-commits mailing list