From 5c8a22f11bd4784cd6b9c375289dadcdc521d49e Mon Sep 17 00:00:00 2001 From: Pete Cooper Date: Tue, 9 Jun 2015 19:56:05 +0000 Subject: [PATCH] Make MCSymbol::Name be a union of uint64_t and a pointer. This should hopefully fix the 32-bit bots which were allocating space for a pointer but needed to be aligned to 64-bits. Now we allocate enough space for a uint64_t and a pointer and cast to the appropriate storage git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@239428 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/MC/MCSymbol.h | 27 ++++++++++++++++++--------- lib/MC/MCSymbol.cpp | 15 +++++++++------ 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/include/llvm/MC/MCSymbol.h b/include/llvm/MC/MCSymbol.h index 36ff8ba7270..544cf8abe60 100644 --- a/include/llvm/MC/MCSymbol.h +++ b/include/llvm/MC/MCSymbol.h @@ -120,20 +120,29 @@ protected: // MCContext creates and uniques these. friend class MCExpr; friend class MCContext; - typedef const StringMapEntry NameEntryTy; - MCSymbol(SymbolKind Kind, NameEntryTy *Name, bool isTemporary) + /// \brief The name for a symbol. + /// MCSymbol contains a uint64_t so is probably aligned to 8. On a 32-bit + /// system, the name is a pointer so isn't going to satisfy the 8 byte + /// alignment of uint64_t. Account for that here. + typedef union { + const StringMapEntry *NameEntry; + uint64_t AlignmentPadding; + } NameEntryStorageTy; + + MCSymbol(SymbolKind Kind, const StringMapEntry *Name, bool isTemporary) : Value(nullptr), IsTemporary(isTemporary), IsRedefinable(false), IsUsed(false), IsRegistered(false), IsExternal(false), IsPrivateExtern(false), HasName(!!Name), Kind(Kind) { Offset = 0; if (Name) - getNameEntryPtr() = Name; + getNameEntryPtr().NameEntry = Name; } // Provide custom new/delete as we will only allocate space for a name // if we need one. - void *operator new(size_t s, NameEntryTy *Name, MCContext &Ctx); + void *operator new(size_t s, const StringMapEntry *Name, + MCContext &Ctx); private: @@ -160,14 +169,14 @@ private: } /// \brief Get a reference to the name field. Requires that we have a name - NameEntryTy *&getNameEntryPtr() { + NameEntryStorageTy &getNameEntryPtr() { assert(HasName && "Name is required"); - NameEntryTy **Name = reinterpret_cast(this); + NameEntryStorageTy *Name = reinterpret_cast(this); return *(Name - 1); } - NameEntryTy *const &getNameEntryPtr() const { + const NameEntryStorageTy &getNameEntryPtr() const { assert(HasName && "Name is required"); - NameEntryTy *const *Name = reinterpret_cast(this); + const auto *Name = reinterpret_cast(this); return *(Name - 1); } @@ -177,7 +186,7 @@ public: if (!HasName) return StringRef(); - return getNameEntryPtr()->first(); + return getNameEntryPtr().NameEntry->first(); } bool isRegistered() const { return IsRegistered; } diff --git a/lib/MC/MCSymbol.cpp b/lib/MC/MCSymbol.cpp index e82630640ba..a5097bc90f6 100644 --- a/lib/MC/MCSymbol.cpp +++ b/lib/MC/MCSymbol.cpp @@ -19,17 +19,20 @@ using namespace llvm; // Sentinel value for the absolute pseudo section. MCSection *MCSymbol::AbsolutePseudoSection = reinterpret_cast(1); -void *MCSymbol::operator new(size_t s, NameEntryTy *Name, MCContext &Ctx) { - size_t Size = s + (Name ? sizeof(Name) : 0); +void *MCSymbol::operator new(size_t s, const StringMapEntry *Name, + MCContext &Ctx) { + // We may need more space for a Name to account for alignment. So allocate + // space for the storage type and not the name pointer. + size_t Size = s + (Name ? sizeof(NameEntryStorageTy) : 0); // For safety, ensure that the alignment of a pointer is enough for an // MCSymbol. This also ensures we don't need padding between the name and // symbol. - assert(alignOf() <= alignOf() && + assert(alignOf() <= alignOf() && "Bad alignment of MCSymbol"); - void *Storage = Ctx.allocate(Size, alignOf()); - NameEntryTy **Start = static_cast(Storage); - NameEntryTy **End = Start + (Name ? 1 : 0); + void *Storage = Ctx.allocate(Size, alignOf()); + NameEntryStorageTy *Start = static_cast(Storage); + NameEntryStorageTy *End = Start + (Name ? 1 : 0); return End; } -- 2.34.1