From: Richard Smith Date: Tue, 1 Dec 2015 01:10:48 +0000 (+0000) Subject: Fix use-after-free when a C++ thread_local variable gets replaced (because its X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=8c57950603ca9fdb7b0dfd285900f71d7a888e09;p=clang Fix use-after-free when a C++ thread_local variable gets replaced (because its type changes when the initializer is attached). Don't hold onto the GlobalVariable*; recompute it from the VarDecl* instead. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@254359 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/CodeGen/CGCXXABI.h b/lib/CodeGen/CGCXXABI.h index 7667361f7b..3f240b1802 100644 --- a/lib/CodeGen/CGCXXABI.h +++ b/lib/CodeGen/CGCXXABI.h @@ -538,11 +538,9 @@ public: /// thread_local variables, a list of functions to perform the /// initialization. virtual void EmitThreadLocalInitFuncs( - CodeGenModule &CGM, - ArrayRef> - CXXThreadLocals, + CodeGenModule &CGM, ArrayRef CXXThreadLocals, ArrayRef CXXThreadLocalInits, - ArrayRef CXXThreadLocalInitVars) = 0; + ArrayRef CXXThreadLocalInitVars) = 0; // Determine if references to thread_local global variables can be made // directly or require access through a thread wrapper function. diff --git a/lib/CodeGen/CGDeclCXX.cpp b/lib/CodeGen/CGDeclCXX.cpp index df75045c78..adba731687 100644 --- a/lib/CodeGen/CGDeclCXX.cpp +++ b/lib/CodeGen/CGDeclCXX.cpp @@ -337,7 +337,7 @@ CodeGenModule::EmitCXXGlobalVarDeclInitFunc(const VarDecl *D, // FIXME: We only need to register one __cxa_thread_atexit function for the // entire TU. CXXThreadLocalInits.push_back(Fn); - CXXThreadLocalInitVars.push_back(Addr); + CXXThreadLocalInitVars.push_back(D); } else if (PerformInit && ISA) { EmitPointerToInitFunc(D, Addr, Fn, ISA); } else if (auto *IPA = D->getAttr()) { diff --git a/lib/CodeGen/CodeGenModule.cpp b/lib/CodeGen/CodeGenModule.cpp index 042984a43d..a15c43c313 100644 --- a/lib/CodeGen/CodeGenModule.cpp +++ b/lib/CodeGen/CodeGenModule.cpp @@ -1986,7 +1986,7 @@ CodeGenModule::GetOrCreateLLVMGlobal(StringRef MangledName, if (D->getTLSKind()) { if (D->getTLSKind() == VarDecl::TLS_Dynamic) - CXXThreadLocals.push_back(std::make_pair(D, GV)); + CXXThreadLocals.push_back(D); setTLSMode(GV, *D); } @@ -2379,7 +2379,7 @@ void CodeGenModule::EmitGlobalVarDefinition(const VarDecl *D, if (D->getTLSKind() && !GV->isThreadLocal()) { if (D->getTLSKind() == VarDecl::TLS_Dynamic) - CXXThreadLocals.push_back(std::make_pair(D, GV)); + CXXThreadLocals.push_back(D); setTLSMode(GV, *D); } diff --git a/lib/CodeGen/CodeGenModule.h b/lib/CodeGen/CodeGenModule.h index 9133ecfb02..d1a3efbeae 100644 --- a/lib/CodeGen/CodeGenModule.h +++ b/lib/CodeGen/CodeGenModule.h @@ -378,13 +378,12 @@ private: StaticExternCMap StaticExternCValues; /// \brief thread_local variables defined or used in this TU. - std::vector > - CXXThreadLocals; + std::vector CXXThreadLocals; /// \brief thread_local variables with initializers that need to run /// before any thread_local variable in this TU is odr-used. std::vector CXXThreadLocalInits; - std::vector CXXThreadLocalInitVars; + std::vector CXXThreadLocalInitVars; /// Global variables with initializers that need to run before main. std::vector CXXGlobalInits; diff --git a/lib/CodeGen/ItaniumCXXABI.cpp b/lib/CodeGen/ItaniumCXXABI.cpp index 3b501c30e7..f5b2d90d50 100644 --- a/lib/CodeGen/ItaniumCXXABI.cpp +++ b/lib/CodeGen/ItaniumCXXABI.cpp @@ -327,10 +327,9 @@ public: llvm::Value *Val); void EmitThreadLocalInitFuncs( CodeGenModule &CGM, - ArrayRef> - CXXThreadLocals, + ArrayRef CXXThreadLocals, ArrayRef CXXThreadLocalInits, - ArrayRef CXXThreadLocalInitVars) override; + ArrayRef CXXThreadLocalInitVars) override; bool usesThreadWrapperFunction() const override { return true; } LValue EmitThreadLocalVarDeclLValue(CodeGenFunction &CGF, const VarDecl *VD, @@ -2200,10 +2199,9 @@ ItaniumCXXABI::getOrCreateThreadLocalWrapper(const VarDecl *VD, } void ItaniumCXXABI::EmitThreadLocalInitFuncs( - CodeGenModule &CGM, - ArrayRef> - CXXThreadLocals, ArrayRef CXXThreadLocalInits, - ArrayRef CXXThreadLocalInitVars) { + CodeGenModule &CGM, ArrayRef CXXThreadLocals, + ArrayRef CXXThreadLocalInits, + ArrayRef CXXThreadLocalInitVars) { llvm::Function *InitFunc = nullptr; if (!CXXThreadLocalInits.empty()) { // Generate a guarded initialization function. @@ -2226,9 +2224,9 @@ void ItaniumCXXABI::EmitThreadLocalInitFuncs( .GenerateCXXGlobalInitFunc(InitFunc, CXXThreadLocalInits, Address(Guard, GuardAlign)); } - for (auto &I : CXXThreadLocals) { - const VarDecl *VD = I.first; - llvm::GlobalVariable *Var = I.second; + for (const VarDecl *VD : CXXThreadLocals) { + llvm::GlobalVariable *Var = + cast(CGM.GetGlobalValue(CGM.getMangledName(VD))); // Some targets require that all access to thread local variables go through // the thread wrapper. This means that we cannot attempt to create a thread @@ -2305,9 +2303,7 @@ void ItaniumCXXABI::EmitThreadLocalInitFuncs( LValue ItaniumCXXABI::EmitThreadLocalVarDeclLValue(CodeGenFunction &CGF, const VarDecl *VD, QualType LValType) { - QualType T = VD->getType(); - llvm::Type *Ty = CGF.getTypes().ConvertTypeForMem(T); - llvm::Value *Val = CGF.CGM.GetAddrOfGlobalVar(VD, Ty); + llvm::Value *Val = CGF.CGM.GetAddrOfGlobalVar(VD); llvm::Function *Wrapper = getOrCreateThreadLocalWrapper(VD, Val); Val = CGF.Builder.CreateCall(Wrapper); diff --git a/lib/CodeGen/MicrosoftCXXABI.cpp b/lib/CodeGen/MicrosoftCXXABI.cpp index 8ef5331a1a..bb34a4b4a3 100644 --- a/lib/CodeGen/MicrosoftCXXABI.cpp +++ b/lib/CodeGen/MicrosoftCXXABI.cpp @@ -377,11 +377,9 @@ public: const ReturnAdjustment &RA) override; void EmitThreadLocalInitFuncs( - CodeGenModule &CGM, - ArrayRef> - CXXThreadLocals, + CodeGenModule &CGM, ArrayRef CXXThreadLocals, ArrayRef CXXThreadLocalInits, - ArrayRef CXXThreadLocalInitVars) override; + ArrayRef CXXThreadLocalInitVars) override; bool usesThreadWrapperFunction() const override { return false; } LValue EmitThreadLocalVarDeclLValue(CodeGenFunction &CGF, const VarDecl *VD, @@ -2197,11 +2195,9 @@ void MicrosoftCXXABI::registerGlobalDtor(CodeGenFunction &CGF, const VarDecl &D, } void MicrosoftCXXABI::EmitThreadLocalInitFuncs( - CodeGenModule &CGM, - ArrayRef> - CXXThreadLocals, + CodeGenModule &CGM, ArrayRef CXXThreadLocals, ArrayRef CXXThreadLocalInits, - ArrayRef CXXThreadLocalInitVars) { + ArrayRef CXXThreadLocalInitVars) { // This will create a GV in the .CRT$XDU section. It will point to our // initialization function. The CRT will call all of these function // pointers at start-up time and, eventually, at thread-creation time. @@ -2219,7 +2215,8 @@ void MicrosoftCXXABI::EmitThreadLocalInitFuncs( std::vector NonComdatInits; for (size_t I = 0, E = CXXThreadLocalInitVars.size(); I != E; ++I) { - llvm::GlobalVariable *GV = CXXThreadLocalInitVars[I]; + llvm::GlobalVariable *GV = cast( + CGM.GetGlobalValue(CGM.getMangledName(CXXThreadLocalInitVars[I]))); llvm::Function *F = CXXThreadLocalInits[I]; // If the GV is already in a comdat group, then we have to join it. diff --git a/test/CodeGenCXX/cxx11-thread-local.cpp b/test/CodeGenCXX/cxx11-thread-local.cpp index b96b027bb8..e00a881a66 100644 --- a/test/CodeGenCXX/cxx11-thread-local.cpp +++ b/test/CodeGenCXX/cxx11-thread-local.cpp @@ -20,6 +20,18 @@ struct U { static thread_local int m; }; // DARWIN: @_ZN1U1mE = internal thread_local global i32 0 thread_local int U::m = f(); +namespace MismatchedInitType { + // Check that we don't crash here when we're forced to create a new global + // variable (with a different type) when we add the initializer. + union U { + int a; + float f; + constexpr U() : f(0.0) {} + }; + static thread_local U u; + void *p = &u; +} + template struct V { static thread_local int m; }; template thread_local int V::m = g();