From cddcabadf265eec52080a46cf951063e7cb3bf4a Mon Sep 17 00:00:00 2001 From: Rafael Espindola Date: Thu, 4 Jun 2015 05:59:23 +0000 Subject: [PATCH] Bring back r239006 with a fix. The fix is just that getOther had not been updated for packing the st_other values in fewer bits and could return spurious values: - unsigned Other = (getFlags() & (0x3f << ELF_STO_Shift)) >> ELF_STO_Shift; + unsigned Other = (getFlags() & (0x7 << ELF_STO_Shift)) >> ELF_STO_Shift; Original message: Pack the MCSymbolELF bit fields into MCSymbol's Flags. This reduces MCSymolfELF from 64 bytes to 56 bytes on x86_64. While at it, also make getOther/setOther easier to use by accepting unshifted STO_* values. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@239012 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/MC/MCSymbolELF.h | 13 +- lib/MC/ELFObjectWriter.cpp | 3 +- lib/MC/MCSymbolELF.cpp | 175 ++++++++++++++---- .../Mips/MCTargetDesc/MipsELFObjectWriter.cpp | 2 +- .../Mips/MCTargetDesc/MipsELFStreamer.cpp | 5 +- .../Mips/MCTargetDesc/MipsTargetStreamer.cpp | 12 +- .../PowerPC/MCTargetDesc/PPCMCTargetDesc.cpp | 16 +- 7 files changed, 152 insertions(+), 74 deletions(-) diff --git a/include/llvm/MC/MCSymbolELF.h b/include/llvm/MC/MCSymbolELF.h index 374e6b87f71..c943054a4fd 100644 --- a/include/llvm/MC/MCSymbolELF.h +++ b/include/llvm/MC/MCSymbolELF.h @@ -17,15 +17,9 @@ class MCSymbolELF : public MCSymbol { /// symbol has no size this field will be NULL. const MCExpr *SymbolSize = nullptr; - mutable unsigned BindingSet : 1; - mutable unsigned UsedInReloc : 1; - mutable unsigned WeakrefUsedInReloc : 1; - mutable unsigned IsSignature : 1; - public: MCSymbolELF(const StringMapEntry *Name, bool isTemporary) - : MCSymbol(true, Name, isTemporary), BindingSet(false), - UsedInReloc(false), WeakrefUsedInReloc(false), IsSignature(false) {} + : MCSymbol(true, Name, isTemporary) {} void setSize(const MCExpr *SS) { SymbolSize = SS; } const MCExpr *getSize() const { return SymbolSize; } @@ -42,7 +36,7 @@ public: void setBinding(unsigned Binding) const; unsigned getBinding() const; - bool isBindingSet() const { return BindingSet; } + bool isBindingSet() const; void setUsedInReloc() const; bool isUsedInReloc() const; @@ -54,6 +48,9 @@ public: bool isSignature() const; static bool classof(const MCSymbol *S) { return S->isELF(); } + +private: + void setIsBindingSet() const; }; } diff --git a/lib/MC/ELFObjectWriter.cpp b/lib/MC/ELFObjectWriter.cpp index d811cfe0ba4..f8cf7d2ea5d 100644 --- a/lib/MC/ELFObjectWriter.cpp +++ b/lib/MC/ELFObjectWriter.cpp @@ -463,8 +463,7 @@ void ELFObjectWriter::writeSymbol(SymbolTableWriter &Writer, // Other and Visibility share the same byte with Visibility using the lower // 2 bits uint8_t Visibility = Symbol.getVisibility(); - uint8_t Other = Symbol.getOther() << 2; - Other |= Visibility; + uint8_t Other = Symbol.getOther() | Visibility; uint64_t Value = SymbolValue(*MSD.Symbol, Layout); uint64_t Size = 0; diff --git a/lib/MC/MCSymbolELF.cpp b/lib/MC/MCSymbolELF.cpp index f4d3d176c3a..c3620651f88 100644 --- a/lib/MC/MCSymbolELF.cpp +++ b/lib/MC/MCSymbolELF.cpp @@ -16,27 +16,71 @@ namespace llvm { namespace { enum { - ELF_STT_Shift = 0, // Shift value for STT_* flags - ELF_STB_Shift = 4, // Shift value for STB_* flags - ELF_STV_Shift = 8, // Shift value for STV_* flags - ELF_STO_Shift = 10 // Shift value for STO_* flags + // Shift value for STT_* flags. 7 possible values. 3 bits. + ELF_STT_Shift = 0, + + // Shift value for STB_* flags. 4 possible values, 2 bits. + ELF_STB_Shift = 3, + + // Shift value for STV_* flags. 4 possible values, 2 bits. + ELF_STV_Shift = 5, + + // Shift value for STO_* flags. 3 bits. All the values are between 0x20 and + // 0xe0, so we shift right by 5 before storing. + ELF_STO_Shift = 7, + + // One bit. + ELF_IsSignature_Shift = 10, + + // One bit. + ELF_WeakrefUsedInReloc_Shift = 11, + + // One bit. + ELF_UsedInReloc_Shift = 12, + + // One bit. + ELF_BindingSet_Shift = 13 }; } void MCSymbolELF::setBinding(unsigned Binding) const { - BindingSet = true; - assert(Binding == ELF::STB_LOCAL || Binding == ELF::STB_GLOBAL || - Binding == ELF::STB_WEAK || Binding == ELF::STB_GNU_UNIQUE); - uint32_t OtherFlags = getFlags() & ~(0xf << ELF_STB_Shift); - setFlags(OtherFlags | (Binding << ELF_STB_Shift)); + setIsBindingSet(); + unsigned Val; + switch (Binding) { + default: + llvm_unreachable("Unsupported Binding"); + case ELF::STB_LOCAL: + Val = 0; + break; + case ELF::STB_GLOBAL: + Val = 1; + break; + case ELF::STB_WEAK: + Val = 2; + break; + case ELF::STB_GNU_UNIQUE: + Val = 3; + break; + } + uint32_t OtherFlags = getFlags() & ~(0x3 << ELF_STB_Shift); + setFlags(OtherFlags | (Val << ELF_STB_Shift)); } unsigned MCSymbolELF::getBinding() const { if (isBindingSet()) { - uint32_t Binding = (getFlags() & (0xf << ELF_STB_Shift)) >> ELF_STB_Shift; - assert(Binding == ELF::STB_LOCAL || Binding == ELF::STB_GLOBAL || - Binding == ELF::STB_WEAK || Binding == ELF::STB_GNU_UNIQUE); - return Binding; + uint32_t Val = (getFlags() & (0x3 << ELF_STB_Shift)) >> ELF_STB_Shift; + switch (Val) { + default: + llvm_unreachable("Invalid value"); + case 0: + return ELF::STB_LOCAL; + case 1: + return ELF::STB_GLOBAL; + case 2: + return ELF::STB_WEAK; + case 3: + return ELF::STB_GNU_UNIQUE; + } } if (isDefined()) @@ -51,26 +95,58 @@ unsigned MCSymbolELF::getBinding() const { } void MCSymbolELF::setType(unsigned Type) const { - assert(Type == ELF::STT_NOTYPE || Type == ELF::STT_OBJECT || - Type == ELF::STT_FUNC || Type == ELF::STT_SECTION || - Type == ELF::STT_COMMON || Type == ELF::STT_TLS || - Type == ELF::STT_GNU_IFUNC); - - uint32_t OtherFlags = getFlags() & ~(0xf << ELF_STT_Shift); - setFlags(OtherFlags | (Type << ELF_STT_Shift)); + unsigned Val; + switch (Type) { + default: + llvm_unreachable("Unsupported Binding"); + case ELF::STT_NOTYPE: + Val = 0; + break; + case ELF::STT_OBJECT: + Val = 1; + break; + case ELF::STT_FUNC: + Val = 2; + break; + case ELF::STT_SECTION: + Val = 3; + break; + case ELF::STT_COMMON: + Val = 4; + break; + case ELF::STT_TLS: + Val = 5; + break; + case ELF::STT_GNU_IFUNC: + Val = 6; + break; + } + uint32_t OtherFlags = getFlags() & ~(0x7 << ELF_STT_Shift); + setFlags(OtherFlags | (Val << ELF_STT_Shift)); } unsigned MCSymbolELF::getType() const { - uint32_t Type = (getFlags() & (0xf << ELF_STT_Shift)) >> ELF_STT_Shift; - assert(Type == ELF::STT_NOTYPE || Type == ELF::STT_OBJECT || - Type == ELF::STT_FUNC || Type == ELF::STT_SECTION || - Type == ELF::STT_COMMON || Type == ELF::STT_TLS || - Type == ELF::STT_GNU_IFUNC); - return Type; + uint32_t Val = (getFlags() & (0x7 << ELF_STT_Shift)) >> ELF_STT_Shift; + switch (Val) { + default: + llvm_unreachable("Invalid value"); + case 0: + return ELF::STT_NOTYPE; + case 1: + return ELF::STT_OBJECT; + case 2: + return ELF::STT_FUNC; + case 3: + return ELF::STT_SECTION; + case 4: + return ELF::STT_COMMON; + case 5: + return ELF::STT_TLS; + case 6: + return ELF::STT_GNU_IFUNC; + } } -// Visibility is stored in the first two bits of st_other -// st_other values are stored in the second byte of get/setFlags void MCSymbolELF::setVisibility(unsigned Visibility) { assert(Visibility == ELF::STV_DEFAULT || Visibility == ELF::STV_INTERNAL || Visibility == ELF::STV_HIDDEN || Visibility == ELF::STV_PROTECTED); @@ -86,31 +162,52 @@ unsigned MCSymbolELF::getVisibility() const { return Visibility; } -// Other is stored in the last six bits of st_other -// st_other values are stored in the second byte of get/setFlags void MCSymbolELF::setOther(unsigned Other) { - uint32_t OtherFlags = getFlags() & ~(0x3f << ELF_STO_Shift); + assert((Other & 0x1f) == 0); + Other >>= 5; + assert(Other <= 0x7); + uint32_t OtherFlags = getFlags() & ~(0x7 << ELF_STO_Shift); setFlags(OtherFlags | (Other << ELF_STO_Shift)); } unsigned MCSymbolELF::getOther() const { - unsigned Other = (getFlags() & (0x3f << ELF_STO_Shift)) >> ELF_STO_Shift; - return Other; + unsigned Other = (getFlags() & (0x7 << ELF_STO_Shift)) >> ELF_STO_Shift; + return Other << 5; } void MCSymbolELF::setUsedInReloc() const { - UsedInReloc = true; + uint32_t OtherFlags = getFlags() & ~(0x1 << ELF_UsedInReloc_Shift); + setFlags(OtherFlags | (1 << ELF_UsedInReloc_Shift)); } bool MCSymbolELF::isUsedInReloc() const { - return UsedInReloc; + return getFlags() & (0x1 << ELF_UsedInReloc_Shift); } -void MCSymbolELF::setIsWeakrefUsedInReloc() const { WeakrefUsedInReloc = true; } +void MCSymbolELF::setIsWeakrefUsedInReloc() const { + uint32_t OtherFlags = getFlags() & ~(0x1 << ELF_WeakrefUsedInReloc_Shift); + setFlags(OtherFlags | (1 << ELF_WeakrefUsedInReloc_Shift)); +} -bool MCSymbolELF::isWeakrefUsedInReloc() const { return WeakrefUsedInReloc; } +bool MCSymbolELF::isWeakrefUsedInReloc() const { + return getFlags() & (0x1 << ELF_WeakrefUsedInReloc_Shift); +} -void MCSymbolELF::setIsSignature() const { IsSignature = true; } +void MCSymbolELF::setIsSignature() const { + uint32_t OtherFlags = getFlags() & ~(0x1 << ELF_IsSignature_Shift); + setFlags(OtherFlags | (1 << ELF_IsSignature_Shift)); +} -bool MCSymbolELF::isSignature() const { return IsSignature; } +bool MCSymbolELF::isSignature() const { + return getFlags() & (0x1 << ELF_IsSignature_Shift); +} + +void MCSymbolELF::setIsBindingSet() const { + uint32_t OtherFlags = getFlags() & ~(0x1 << ELF_BindingSet_Shift); + setFlags(OtherFlags | (1 << ELF_BindingSet_Shift)); +} + +bool MCSymbolELF::isBindingSet() const { + return getFlags() & (0x1 << ELF_BindingSet_Shift); +} } diff --git a/lib/Target/Mips/MCTargetDesc/MipsELFObjectWriter.cpp b/lib/Target/Mips/MCTargetDesc/MipsELFObjectWriter.cpp index 82ae41330bc..982a7f54e82 100644 --- a/lib/Target/Mips/MCTargetDesc/MipsELFObjectWriter.cpp +++ b/lib/Target/Mips/MCTargetDesc/MipsELFObjectWriter.cpp @@ -384,7 +384,7 @@ bool MipsELFObjectWriter::needsRelocateWithSymbol(const MCSymbol &Sym, return true; case ELF::R_MIPS_32: - if (cast(Sym).getOther() & (ELF::STO_MIPS_MICROMIPS >> 2)) + if (cast(Sym).getOther() & ELF::STO_MIPS_MICROMIPS) return true; // falltrough case ELF::R_MIPS_26: diff --git a/lib/Target/Mips/MCTargetDesc/MipsELFStreamer.cpp b/lib/Target/Mips/MCTargetDesc/MipsELFStreamer.cpp index d1e3a47f94b..b45d9cf621d 100644 --- a/lib/Target/Mips/MCTargetDesc/MipsELFStreamer.cpp +++ b/lib/Target/Mips/MCTargetDesc/MipsELFStreamer.cpp @@ -44,10 +44,7 @@ void MipsELFStreamer::createPendingLabelRelocs() { for (auto *L : Labels) { auto *Label = cast(L); getAssembler().registerSymbol(*Label); - // The "other" values are stored in the last 6 bits of the second byte. - // The traditional defines for STO values assume the full byte and thus - // the shift to pack it. - Label->setOther(ELF::STO_MIPS_MICROMIPS >> 2); + Label->setOther(ELF::STO_MIPS_MICROMIPS); } } diff --git a/lib/Target/Mips/MCTargetDesc/MipsTargetStreamer.cpp b/lib/Target/Mips/MCTargetDesc/MipsTargetStreamer.cpp index 787d9a7b450..a051f4c123f 100644 --- a/lib/Target/Mips/MCTargetDesc/MipsTargetStreamer.cpp +++ b/lib/Target/Mips/MCTargetDesc/MipsTargetStreamer.cpp @@ -462,10 +462,7 @@ void MipsTargetELFStreamer::emitLabel(MCSymbol *S) { if (Type != ELF::STT_FUNC) return; - // The "other" values are stored in the last 6 bits of the second byte - // The traditional defines for STO values assume the full byte and thus - // the shift to pack it. - Symbol->setOther(ELF::STO_MIPS_MICROMIPS >> 2); + Symbol->setOther(ELF::STO_MIPS_MICROMIPS); } void MipsTargetELFStreamer::finish() { @@ -527,13 +524,10 @@ void MipsTargetELFStreamer::emitAssignment(MCSymbol *S, const MCExpr *Value) { const auto &RhsSym = cast( static_cast(Value)->getSymbol()); - if (!(RhsSym.getOther() & (ELF::STO_MIPS_MICROMIPS >> 2))) + if (!(RhsSym.getOther() & ELF::STO_MIPS_MICROMIPS)) return; - // The "other" values are stored in the last 6 bits of the second byte. - // The traditional defines for STO values assume the full byte and thus - // the shift to pack it. - Symbol->setOther(ELF::STO_MIPS_MICROMIPS >> 2); + Symbol->setOther(ELF::STO_MIPS_MICROMIPS); } MCELFStreamer &MipsTargetELFStreamer::getStreamer() { diff --git a/lib/Target/PowerPC/MCTargetDesc/PPCMCTargetDesc.cpp b/lib/Target/PowerPC/MCTargetDesc/PPCMCTargetDesc.cpp index 0f9697bb141..aba262c3b1f 100644 --- a/lib/Target/PowerPC/MCTargetDesc/PPCMCTargetDesc.cpp +++ b/lib/Target/PowerPC/MCTargetDesc/PPCMCTargetDesc.cpp @@ -169,13 +169,10 @@ public: if (Res != ELF::decodePPC64LocalEntryOffset(Encoded)) report_fatal_error(".localentry expression cannot be encoded."); - // The "other" values are stored in the last 6 bits of the second byte. - // The traditional defines for STO values assume the full byte and thus - // the shift to pack it. - unsigned Other = S->getOther() << 2; + unsigned Other = S->getOther(); Other &= ~ELF::STO_PPC64_LOCAL_MASK; Other |= Encoded; - S->setOther(Other >> 2); + S->setOther(Other); // For GAS compatibility, unless we already saw a .abiversion directive, // set e_flags to indicate ELFv2 ABI. @@ -191,13 +188,10 @@ public: return; const auto &RhsSym = cast( static_cast(Value)->getSymbol()); - // The "other" values are stored in the last 6 bits of the second byte. - // The traditional defines for STO values assume the full byte and thus - // the shift to pack it. - unsigned Other = Symbol->getOther() << 2; + unsigned Other = Symbol->getOther(); Other &= ~ELF::STO_PPC64_LOCAL_MASK; - Other |= (RhsSym.getOther() << 2) & ELF::STO_PPC64_LOCAL_MASK; - Symbol->setOther(Other >> 2); + Other |= RhsSym.getOther() & ELF::STO_PPC64_LOCAL_MASK; + Symbol->setOther(Other); } }; -- 2.34.1