From 21fe45076c371e9a9f27e15c4e068e77a185fe62 Mon Sep 17 00:00:00 2001 From: Mark Seaborn Date: Wed, 24 Jul 2013 16:25:13 +0000 Subject: [PATCH] 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 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@187051 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/ItaniumCXXABI.cpp | 49 +++++++++++++------- test/CodeGenCXX/member-function-pointers.cpp | 3 ++ test/CodeGenCXX/static-init-pnacl.cpp | 14 ++++++ 3 files changed, 50 insertions(+), 16 deletions(-) create mode 100644 test/CodeGenCXX/static-init-pnacl.cpp diff --git a/lib/CodeGen/ItaniumCXXABI.cpp b/lib/CodeGen/ItaniumCXXABI.cpp index 55814b455b..a1e197c6cb 100644 --- a/lib/CodeGen/ItaniumCXXABI.cpp +++ b/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(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(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(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/test/CodeGenCXX/member-function-pointers.cpp b/test/CodeGenCXX/member-function-pointers.cpp index 84b54b67d3..23a7b0009f 100644 --- a/test/CodeGenCXX/member-function-pointers.cpp +++ b/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/test/CodeGenCXX/static-init-pnacl.cpp b/test/CodeGenCXX/static-init-pnacl.cpp new file mode 100644 index 0000000000..65844e5d34 --- /dev/null +++ b/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 -- 2.40.0