r329007 - Fix some DenseMap use-after-rehash bugs and hoist MethodVFTableLocation
Reid Kleckner via cfe-commits
cfe-commits at lists.llvm.org
Mon Apr 2 13:00:39 PDT 2018
Author: rnk
Date: Mon Apr 2 13:00:39 2018
New Revision: 329007
URL: http://llvm.org/viewvc/llvm-project?rev=329007&view=rev
Log:
Fix some DenseMap use-after-rehash bugs and hoist MethodVFTableLocation
This re-lands r328845 with fixes for crbug.com/827810.
The initial motiviation was to hoist MethodVFTableLocation to global
scope so it could be forward declared.
In this patch, I noticed that MicrosoftVTableContext uses some risky
patterns. It has methods that return references to data stored in
DenseMaps. I've made some of them return by value for trivial structs
and I've moved some things into separate allocations.
Modified:
cfe/trunk/include/clang/AST/Mangle.h
cfe/trunk/include/clang/AST/VTableBuilder.h
cfe/trunk/lib/AST/MicrosoftMangle.cpp
cfe/trunk/lib/AST/VTableBuilder.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
Modified: cfe/trunk/include/clang/AST/Mangle.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Mangle.h?rev=329007&r1=329006&r2=329007&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/Mangle.h (original)
+++ cfe/trunk/include/clang/AST/Mangle.h Mon Apr 2 13:00:39 2018
@@ -30,6 +30,7 @@ namespace clang {
class CXXDestructorDecl;
class CXXMethodDecl;
class FunctionDecl;
+ struct MethodVFTableLocation;
class NamedDecl;
class ObjCMethodDecl;
class StringLiteral;
@@ -201,7 +202,8 @@ public:
raw_ostream &Out) = 0;
virtual void mangleVirtualMemPtrThunk(const CXXMethodDecl *MD,
- raw_ostream &) = 0;
+ const MethodVFTableLocation &ML,
+ raw_ostream &Out) = 0;
virtual void mangleCXXVirtualDisplacementMap(const CXXRecordDecl *SrcRD,
const CXXRecordDecl *DstRD,
Modified: cfe/trunk/include/clang/AST/VTableBuilder.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/VTableBuilder.h?rev=329007&r1=329006&r2=329007&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/VTableBuilder.h (original)
+++ cfe/trunk/include/clang/AST/VTableBuilder.h Mon Apr 2 13:00:39 2018
@@ -479,41 +479,42 @@ struct VirtualBaseInfo {
VPtrInfoVector VBPtrPaths;
};
+struct MethodVFTableLocation {
+ /// If nonzero, holds the vbtable index of the virtual base with the vfptr.
+ uint64_t VBTableIndex;
+
+ /// If nonnull, holds the last vbase which contains the vfptr that the
+ /// method definition is adjusted to.
+ const CXXRecordDecl *VBase;
+
+ /// This is the offset of the vfptr from the start of the last vbase, or the
+ /// complete type if there are no virtual bases.
+ CharUnits VFPtrOffset;
+
+ /// Method's index in the vftable.
+ uint64_t Index;
+
+ MethodVFTableLocation()
+ : VBTableIndex(0), VBase(nullptr), VFPtrOffset(CharUnits::Zero()),
+ Index(0) {}
+
+ MethodVFTableLocation(uint64_t VBTableIndex, const CXXRecordDecl *VBase,
+ CharUnits VFPtrOffset, uint64_t Index)
+ : VBTableIndex(VBTableIndex), VBase(VBase), VFPtrOffset(VFPtrOffset),
+ Index(Index) {}
+
+ bool operator<(const MethodVFTableLocation &other) const {
+ if (VBTableIndex != other.VBTableIndex) {
+ assert(VBase != other.VBase);
+ return VBTableIndex < other.VBTableIndex;
+ }
+ return std::tie(VFPtrOffset, Index) <
+ std::tie(other.VFPtrOffset, other.Index);
+ }
+};
+
class MicrosoftVTableContext : public VTableContextBase {
public:
- struct MethodVFTableLocation {
- /// If nonzero, holds the vbtable index of the virtual base with the vfptr.
- uint64_t VBTableIndex;
-
- /// If nonnull, holds the last vbase which contains the vfptr that the
- /// method definition is adjusted to.
- const CXXRecordDecl *VBase;
-
- /// This is the offset of the vfptr from the start of the last vbase, or the
- /// complete type if there are no virtual bases.
- CharUnits VFPtrOffset;
-
- /// Method's index in the vftable.
- uint64_t Index;
-
- MethodVFTableLocation()
- : VBTableIndex(0), VBase(nullptr), VFPtrOffset(CharUnits::Zero()),
- Index(0) {}
-
- MethodVFTableLocation(uint64_t VBTableIndex, const CXXRecordDecl *VBase,
- CharUnits VFPtrOffset, uint64_t Index)
- : VBTableIndex(VBTableIndex), VBase(VBase),
- VFPtrOffset(VFPtrOffset), Index(Index) {}
-
- bool operator<(const MethodVFTableLocation &other) const {
- if (VBTableIndex != other.VBTableIndex) {
- assert(VBase != other.VBase);
- return VBTableIndex < other.VBTableIndex;
- }
- return std::tie(VFPtrOffset, Index) <
- std::tie(other.VFPtrOffset, other.Index);
- }
- };
private:
ASTContext &Context;
@@ -522,7 +523,7 @@ private:
MethodVFTableLocationsTy;
MethodVFTableLocationsTy MethodVFTableLocations;
- typedef llvm::DenseMap<const CXXRecordDecl *, VPtrInfoVector>
+ typedef llvm::DenseMap<const CXXRecordDecl *, std::unique_ptr<VPtrInfoVector>>
VFPtrLocationsMapTy;
VFPtrLocationsMapTy VFPtrLocations;
@@ -559,7 +560,7 @@ public:
const VTableLayout &getVFTableLayout(const CXXRecordDecl *RD,
CharUnits VFPtrOffset);
- const MethodVFTableLocation &getMethodVFTableLocation(GlobalDecl GD);
+ MethodVFTableLocation getMethodVFTableLocation(GlobalDecl GD);
const ThunkInfoVectorTy *getThunkInfo(GlobalDecl GD) override {
// Complete destructors don't have a slot in a vftable, so no thunks needed.
Modified: cfe/trunk/lib/AST/MicrosoftMangle.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/MicrosoftMangle.cpp?rev=329007&r1=329006&r2=329007&view=diff
==============================================================================
--- cfe/trunk/lib/AST/MicrosoftMangle.cpp (original)
+++ cfe/trunk/lib/AST/MicrosoftMangle.cpp Mon Apr 2 13:00:39 2018
@@ -135,7 +135,8 @@ public:
bool shouldMangleStringLiteral(const StringLiteral *SL) override;
void mangleCXXName(const NamedDecl *D, raw_ostream &Out) override;
void mangleVirtualMemPtrThunk(const CXXMethodDecl *MD,
- raw_ostream &) override;
+ const MethodVFTableLocation &ML,
+ raw_ostream &Out) override;
void mangleThunk(const CXXMethodDecl *MD, const ThunkInfo &Thunk,
raw_ostream &) override;
void mangleCXXDtorThunk(const CXXDestructorDecl *DD, CXXDtorType Type,
@@ -297,9 +298,8 @@ public:
void mangleMemberDataPointer(const CXXRecordDecl *RD, const ValueDecl *VD);
void mangleMemberFunctionPointer(const CXXRecordDecl *RD,
const CXXMethodDecl *MD);
- void mangleVirtualMemPtrThunk(
- const CXXMethodDecl *MD,
- const MicrosoftVTableContext::MethodVFTableLocation &ML);
+ void mangleVirtualMemPtrThunk(const CXXMethodDecl *MD,
+ const MethodVFTableLocation &ML);
void mangleNumber(int64_t Number);
void mangleTagTypeKind(TagTypeKind TK);
void mangleArtificalTagType(TagTypeKind TK, StringRef UnqualifiedName,
@@ -607,7 +607,7 @@ MicrosoftCXXNameMangler::mangleMemberFun
if (MD->isVirtual()) {
MicrosoftVTableContext *VTContext =
cast<MicrosoftVTableContext>(getASTContext().getVTableContext());
- const MicrosoftVTableContext::MethodVFTableLocation &ML =
+ MethodVFTableLocation ML =
VTContext->getMethodVFTableLocation(GlobalDecl(MD));
mangleVirtualMemPtrThunk(MD, ML);
NVOffset = ML.VFPtrOffset.getQuantity();
@@ -644,8 +644,7 @@ MicrosoftCXXNameMangler::mangleMemberFun
}
void MicrosoftCXXNameMangler::mangleVirtualMemPtrThunk(
- const CXXMethodDecl *MD,
- const MicrosoftVTableContext::MethodVFTableLocation &ML) {
+ const CXXMethodDecl *MD, const MethodVFTableLocation &ML) {
// Get the vftable offset.
CharUnits PointerWidth = getASTContext().toCharUnitsFromBits(
getASTContext().getTargetInfo().getPointerWidth(0));
@@ -2775,14 +2774,9 @@ static void mangleThunkThisAdjustment(co
}
}
-void
-MicrosoftMangleContextImpl::mangleVirtualMemPtrThunk(const CXXMethodDecl *MD,
- raw_ostream &Out) {
- MicrosoftVTableContext *VTContext =
- cast<MicrosoftVTableContext>(getASTContext().getVTableContext());
- const MicrosoftVTableContext::MethodVFTableLocation &ML =
- VTContext->getMethodVFTableLocation(GlobalDecl(MD));
-
+void MicrosoftMangleContextImpl::mangleVirtualMemPtrThunk(
+ const CXXMethodDecl *MD, const MethodVFTableLocation &ML,
+ raw_ostream &Out) {
msvc_hashing_ostream MHO(Out);
MicrosoftCXXNameMangler Mangler(*this, MHO);
Mangler.getStream() << '?';
Modified: cfe/trunk/lib/AST/VTableBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/VTableBuilder.cpp?rev=329007&r1=329006&r2=329007&view=diff
==============================================================================
--- cfe/trunk/lib/AST/VTableBuilder.cpp (original)
+++ cfe/trunk/lib/AST/VTableBuilder.cpp Mon Apr 2 13:00:39 2018
@@ -2367,8 +2367,6 @@ namespace {
class VFTableBuilder {
public:
- typedef MicrosoftVTableContext::MethodVFTableLocation MethodVFTableLocation;
-
typedef llvm::DenseMap<GlobalDecl, MethodVFTableLocation>
MethodVFTableLocationsTy;
@@ -3544,10 +3542,9 @@ static void computeFullPathsForVFTables(
}
}
-static bool
-vfptrIsEarlierInMDC(const ASTRecordLayout &Layout,
- const MicrosoftVTableContext::MethodVFTableLocation &LHS,
- const MicrosoftVTableContext::MethodVFTableLocation &RHS) {
+static bool vfptrIsEarlierInMDC(const ASTRecordLayout &Layout,
+ const MethodVFTableLocation &LHS,
+ const MethodVFTableLocation &RHS) {
CharUnits L = LHS.VFPtrOffset;
CharUnits R = RHS.VFPtrOffset;
if (LHS.VBase)
@@ -3568,14 +3565,14 @@ void MicrosoftVTableContext::computeVTab
const VTableLayout::AddressPointsMapTy EmptyAddressPointsMap;
{
- VPtrInfoVector VFPtrs;
- computeVTablePaths(/*ForVBTables=*/false, RD, VFPtrs);
- computeFullPathsForVFTables(Context, RD, VFPtrs);
+ auto VFPtrs = llvm::make_unique<VPtrInfoVector>();
+ computeVTablePaths(/*ForVBTables=*/false, RD, *VFPtrs);
+ computeFullPathsForVFTables(Context, RD, *VFPtrs);
VFPtrLocations[RD] = std::move(VFPtrs);
}
MethodVFTableLocationsTy NewMethodLocations;
- for (const std::unique_ptr<VPtrInfo> &VFPtr : VFPtrLocations[RD]) {
+ for (const std::unique_ptr<VPtrInfo> &VFPtr : *VFPtrLocations[RD]) {
VFTableBuilder Builder(*this, RD, *VFPtr);
VFTableIdTy id(RD, VFPtr->FullOffsetInMDC);
@@ -3720,7 +3717,7 @@ MicrosoftVTableContext::getVFPtrOffsets(
computeVTableRelatedInformation(RD);
assert(VFPtrLocations.count(RD) && "Couldn't find vfptr locations");
- return VFPtrLocations[RD];
+ return *VFPtrLocations[RD];
}
const VTableLayout &
@@ -3733,7 +3730,7 @@ MicrosoftVTableContext::getVFTableLayout
return *VFTableLayouts[id];
}
-const MicrosoftVTableContext::MethodVFTableLocation &
+MethodVFTableLocation
MicrosoftVTableContext::getMethodVFTableLocation(GlobalDecl GD) {
assert(cast<CXXMethodDecl>(GD.getDecl())->isVirtual() &&
"Only use this method for virtual methods or dtors");
Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=329007&r1=329006&r2=329007&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Mon Apr 2 13:00:39 2018
@@ -1416,7 +1416,7 @@ llvm::DISubprogram *CGDebugInfo::CreateC
// deleting dtor.
const auto *DD = dyn_cast<CXXDestructorDecl>(Method);
GlobalDecl GD = DD ? GlobalDecl(DD, Dtor_Deleting) : GlobalDecl(Method);
- MicrosoftVTableContext::MethodVFTableLocation ML =
+ MethodVFTableLocation ML =
CGM.getMicrosoftVTableContext().getMethodVFTableLocation(GD);
VIndex = ML.Index;
Modified: cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp?rev=329007&r1=329006&r2=329007&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp (original)
+++ cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp Mon Apr 2 13:00:39 2018
@@ -230,7 +230,7 @@ public:
getThisArgumentTypeForMethod(const CXXMethodDecl *MD) override {
MD = MD->getCanonicalDecl();
if (MD->isVirtual() && !isa<CXXDestructorDecl>(MD)) {
- MicrosoftVTableContext::MethodVFTableLocation ML =
+ MethodVFTableLocation ML =
CGM.getMicrosoftVTableContext().getMethodVFTableLocation(MD);
// The vbases might be ordered differently in the final overrider object
// and the complete object, so the "this" argument may sometimes point to
@@ -616,9 +616,8 @@ private:
const VBTableGlobals &enumerateVBTables(const CXXRecordDecl *RD);
/// \brief Generate a thunk for calling a virtual member function MD.
- llvm::Function *EmitVirtualMemPtrThunk(
- const CXXMethodDecl *MD,
- const MicrosoftVTableContext::MethodVFTableLocation &ML);
+ llvm::Function *EmitVirtualMemPtrThunk(const CXXMethodDecl *MD,
+ const MethodVFTableLocation &ML);
public:
llvm::Type *ConvertMemberPointerType(const MemberPointerType *MPT) override;
@@ -1336,7 +1335,7 @@ MicrosoftCXXABI::getVirtualFunctionProlo
LookupGD = GlobalDecl(DD, Dtor_Deleting);
}
- MicrosoftVTableContext::MethodVFTableLocation ML =
+ MethodVFTableLocation ML =
CGM.getMicrosoftVTableContext().getMethodVFTableLocation(LookupGD);
CharUnits Adjustment = ML.VFPtrOffset;
@@ -1385,7 +1384,7 @@ Address MicrosoftCXXABI::adjustThisArgum
// with the base one, so look up the deleting one instead.
LookupGD = GlobalDecl(DD, Dtor_Deleting);
}
- MicrosoftVTableContext::MethodVFTableLocation ML =
+ MethodVFTableLocation ML =
CGM.getMicrosoftVTableContext().getMethodVFTableLocation(LookupGD);
CharUnits StaticOffset = ML.VFPtrOffset;
@@ -1851,8 +1850,7 @@ CGCallee MicrosoftCXXABI::getVirtualFunc
llvm::Value *VTable = CGF.GetVTablePtr(VPtr, Ty, MethodDecl->getParent());
MicrosoftVTableContext &VFTContext = CGM.getMicrosoftVTableContext();
- MicrosoftVTableContext::MethodVFTableLocation ML =
- VFTContext.getMethodVFTableLocation(GD);
+ MethodVFTableLocation ML = VFTContext.getMethodVFTableLocation(GD);
// Compute the identity of the most derived class whose virtual table is
// located at the MethodVFTableLocation ML.
@@ -1937,16 +1935,16 @@ MicrosoftCXXABI::enumerateVBTables(const
return VBGlobals;
}
-llvm::Function *MicrosoftCXXABI::EmitVirtualMemPtrThunk(
- const CXXMethodDecl *MD,
- const MicrosoftVTableContext::MethodVFTableLocation &ML) {
+llvm::Function *
+MicrosoftCXXABI::EmitVirtualMemPtrThunk(const CXXMethodDecl *MD,
+ const MethodVFTableLocation &ML) {
assert(!isa<CXXConstructorDecl>(MD) && !isa<CXXDestructorDecl>(MD) &&
"can't form pointers to ctors or virtual dtors");
// Calculate the mangled name.
SmallString<256> ThunkName;
llvm::raw_svector_ostream Out(ThunkName);
- getMangleContext().mangleVirtualMemPtrThunk(MD, Out);
+ getMangleContext().mangleVirtualMemPtrThunk(MD, ML, Out);
// If the thunk has been generated previously, just return it.
if (llvm::GlobalValue *GV = CGM.getModule().getNamedValue(ThunkName))
@@ -2760,8 +2758,7 @@ MicrosoftCXXABI::EmitMemberFunctionPoint
FirstField = CGM.GetAddrOfFunction(MD, Ty);
} else {
auto &VTableContext = CGM.getMicrosoftVTableContext();
- MicrosoftVTableContext::MethodVFTableLocation ML =
- VTableContext.getMethodVFTableLocation(MD);
+ MethodVFTableLocation ML = VTableContext.getMethodVFTableLocation(MD);
FirstField = EmitVirtualMemPtrThunk(MD, ML);
// Include the vfptr adjustment if the method is in a non-primary vftable.
NonVirtualBaseAdjustment += ML.VFPtrOffset;
More information about the cfe-commits
mailing list