From 7998df6c5817071eb401d3d7d45983f4ea7d0151 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= Date: Tue, 7 Nov 2023 18:50:35 +0100 Subject: [PATCH 1/5] [JITLink][AArch32] Add dynamic lookup for relocation fixup infos --- .../llvm/ExecutionEngine/JITLink/aarch32.h | 77 ++++++-- .../ExecutionEngine/JITLink/ELF_aarch32.cpp | 2 + llvm/lib/ExecutionEngine/JITLink/aarch32.cpp | 165 +++++++++++------- 3 files changed, 169 insertions(+), 75 deletions(-) diff --git a/llvm/include/llvm/ExecutionEngine/JITLink/aarch32.h b/llvm/include/llvm/ExecutionEngine/JITLink/aarch32.h index 94669e0bceb83..04ed64a04a259 100644 --- a/llvm/include/llvm/ExecutionEngine/JITLink/aarch32.h +++ b/llvm/include/llvm/ExecutionEngine/JITLink/aarch32.h @@ -98,6 +98,7 @@ enum EdgeKind_aarch32 : Edge::Kind { Thumb_MovtPrel, LastThumbRelocation = Thumb_MovtPrel, + LastRelocation = LastThumbRelocation, }; /// Flags enum for AArch32-specific symbol properties @@ -163,49 +164,81 @@ struct HalfWords { const uint16_t Lo; // Second halfword }; -/// Collection of named constants per fixup kind. It may contain but is not -/// limited to the following entries: +/// Initialize the lookup table for dynamic FixupInfo checks, e.g. checkOpcode() +void populateFixupInfos(); + +/// FixupInfo base class is required for dynamic lookups. +struct FixupInfoBase { + virtual ~FixupInfoBase() {} +}; + +/// FixupInfo checks for Arm edge kinds work on 32-bit words +struct FixupInfoArm : public FixupInfoBase { + bool (*checkOpcode)(uint32_t Wd) = nullptr; +}; + +/// FixupInfo check for Thumb32 edge kinds work on a pair of 16-bit halfwords +struct FixupInfoThumb : public FixupInfoBase { + bool (*checkOpcode)(uint16_t Hi, uint16_t Lo) = nullptr; +}; + +/// Collection of named constants per fixup kind /// +/// Mandatory entries: /// Opcode - Values of the op-code bits in the instruction, with /// unaffected bits nulled /// OpcodeMask - Mask with all bits set that encode the op-code +/// +/// Other common entries: /// ImmMask - Mask with all bits set that encode the immediate value /// RegMask - Mask with all bits set that encode the register /// +/// Specializations can add further custom fields without restrictions. +/// template struct FixupInfo {}; -template <> struct FixupInfo { +namespace { +struct FixupInfoArmBranch : public FixupInfoArm { static constexpr uint32_t Opcode = 0x0a000000; - static constexpr uint32_t OpcodeMask = 0x0f000000; static constexpr uint32_t ImmMask = 0x00ffffff; - static constexpr uint32_t Unconditional = 0xe0000000; - static constexpr uint32_t CondMask = 0xe0000000; // excluding BLX bit }; +} // namespace -template <> struct FixupInfo : public FixupInfo { +template <> struct FixupInfo : public FixupInfoArmBranch { + static constexpr uint32_t OpcodeMask = 0x0f000000; +}; + +template <> struct FixupInfo : public FixupInfoArmBranch { static constexpr uint32_t OpcodeMask = 0x0e000000; + static constexpr uint32_t CondMask = 0xe0000000; // excluding BLX bit + static constexpr uint32_t Unconditional = 0xe0000000; static constexpr uint32_t BitH = 0x01000000; static constexpr uint32_t BitBlx = 0x10000000; }; -template <> struct FixupInfo { - static constexpr uint32_t Opcode = 0x03400000; +namespace { +struct FixupInfoArmMov : public FixupInfoArm { static constexpr uint32_t OpcodeMask = 0x0ff00000; static constexpr uint32_t ImmMask = 0x000f0fff; static constexpr uint32_t RegMask = 0x0000f000; }; +} // namespace + +template <> struct FixupInfo : public FixupInfoArmMov { + static constexpr uint32_t Opcode = 0x03400000; +}; -template <> struct FixupInfo : public FixupInfo { +template <> struct FixupInfo : public FixupInfoArmMov { static constexpr uint32_t Opcode = 0x03000000; }; -template <> struct FixupInfo { +template <> struct FixupInfo : public FixupInfoThumb { static constexpr HalfWords Opcode{0xf000, 0x9000}; static constexpr HalfWords OpcodeMask{0xf800, 0x9000}; static constexpr HalfWords ImmMask{0x07ff, 0x2fff}; }; -template <> struct FixupInfo { +template <> struct FixupInfo : public FixupInfoThumb { static constexpr HalfWords Opcode{0xf000, 0xc000}; static constexpr HalfWords OpcodeMask{0xf800, 0xc000}; static constexpr HalfWords ImmMask{0x07ff, 0x2fff}; @@ -213,15 +246,27 @@ template <> struct FixupInfo { static constexpr uint16_t LoBitNoBlx = 0x1000; }; -template <> struct FixupInfo { - static constexpr HalfWords Opcode{0xf2c0, 0x0000}; +namespace { +struct FixupInfoThumbMov : public FixupInfoThumb { static constexpr HalfWords OpcodeMask{0xfbf0, 0x8000}; static constexpr HalfWords ImmMask{0x040f, 0x70ff}; static constexpr HalfWords RegMask{0x0000, 0x0f00}; }; +} // namespace -template <> -struct FixupInfo : public FixupInfo { +template <> struct FixupInfo : public FixupInfoThumbMov { + static constexpr HalfWords Opcode{0xf2c0, 0x0000}; +}; + +template <> struct FixupInfo : public FixupInfoThumbMov { + static constexpr HalfWords Opcode{0xf2c0, 0x0000}; +}; + +template <> struct FixupInfo : public FixupInfoThumbMov { + static constexpr HalfWords Opcode{0xf240, 0x0000}; +}; + +template <> struct FixupInfo : public FixupInfoThumbMov { static constexpr HalfWords Opcode{0xf240, 0x0000}; }; diff --git a/llvm/lib/ExecutionEngine/JITLink/ELF_aarch32.cpp b/llvm/lib/ExecutionEngine/JITLink/ELF_aarch32.cpp index 132989fcbce02..b6c0fe6d2ef59 100644 --- a/llvm/lib/ExecutionEngine/JITLink/ELF_aarch32.cpp +++ b/llvm/lib/ExecutionEngine/JITLink/ELF_aarch32.cpp @@ -232,6 +232,8 @@ createLinkGraphFromELFObject_aarch32(MemoryBufferRef ObjectBuffer) { << ObjectBuffer.getBufferIdentifier() << "...\n"; }); + aarch32::populateFixupInfos(); + auto ELFObj = ObjectFile::createELFObjectFile(ObjectBuffer); if (!ELFObj) return ELFObj.takeError(); diff --git a/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp b/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp index b80b12ed24519..8d575a4471774 100644 --- a/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp +++ b/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp @@ -189,6 +189,8 @@ int64_t decodeRegMovtA1MovwA2(uint64_t Value) { return Rd4; } +namespace { + /// 32-bit Thumb instructions are stored as two little-endian halfwords. /// An instruction at address A encodes bytes A+1, A in the first halfword (Hi), /// followed by bytes A+3, A+2 in the second halfword (Lo). @@ -224,7 +226,6 @@ struct WritableArmRelocation { }; struct ArmRelocation { - ArmRelocation(const char *FixupPtr) : Wd{*reinterpret_cast(FixupPtr)} {} @@ -248,15 +249,98 @@ Error makeUnexpectedOpcodeError(const LinkGraph &G, const ArmRelocation &R, static_cast(R.Wd), G.getEdgeKindName(Kind))); } -template bool checkOpcode(const ThumbRelocation &R) { - uint16_t Hi = R.Hi & FixupInfo::OpcodeMask.Hi; - uint16_t Lo = R.Lo & FixupInfo::OpcodeMask.Lo; - return Hi == FixupInfo::Opcode.Hi && Lo == FixupInfo::Opcode.Lo; +static auto &getFixupInfoTable() { + static constexpr size_t Items = LastRelocation + 1; + static std::array, Items> FixupInfoTable; + return FixupInfoTable; +} + +template constexpr bool isArm() { + return FirstArmRelocation <= K && K <= LastArmRelocation; +} +template constexpr bool isThumb() { + return FirstThumbRelocation <= K && K <= LastThumbRelocation; +} + +template constexpr bool hasOpcode(...) { return false; } +template ::Opcode> +constexpr bool hasOpcode(int) { + return true; +} + +template static bool checkOpcodeArm(uint32_t Wd) { + return (Wd & FixupInfo::OpcodeMask) == FixupInfo::Opcode; +} + +template +static bool checkOpcodeThumb(uint16_t Hi, uint16_t Lo) { + return (Hi & FixupInfo::OpcodeMask.Hi) == FixupInfo::Opcode.Hi && + (Lo & FixupInfo::OpcodeMask.Lo) == FixupInfo::Opcode.Lo; +} + +template +static std::unique_ptr initFixupInfo() { + auto Entry = std::make_unique>(); + if constexpr (hasOpcode(0)) { + if constexpr (isArm()) + Entry->checkOpcode = checkOpcodeArm; + else if constexpr (isThumb()) + Entry->checkOpcode = checkOpcodeThumb; + else + llvm_unreachable("Visited edge kinds must either be Arm or Thumb"); + } + return Entry; +} + +template +void populateFixupInfos(TableT &Table) { + assert(K < Table.size() && "Index out of range"); + assert(Table.at(K) == nullptr && "Initialized entries are immutable"); + Table[K] = initFixupInfo(); + if constexpr (K < LastK) { + constexpr auto Next = static_cast(K + 1); + populateFixupInfos(Table); + } +} +} // namespace + +void populateFixupInfos() { + static std::once_flag Flag; + std::call_once(Flag, []() { + auto &Table = getFixupInfoTable(); + populateFixupInfos(Table); + populateFixupInfos(Table); + }); +} + +template +static const TargetModeSubclass *const getDynFixupInfo(Edge::Kind K) { + assert(K >= Edge::FirstRelocation && "Invalid value for edge kind"); + assert(K <= LastRelocation && "Invalid value for edge kind"); + assert(getFixupInfoTable().at(K) && "Please call populateFixupInfos() first"); + return static_cast(getFixupInfoTable().at(K).get()); +} + +static bool checkOpcode(const ArmRelocation &R, Edge::Kind Kind) { + assert(Kind >= FirstArmRelocation && Kind <= LastArmRelocation && + "Edge kind is no Arm relocation"); + const FixupInfoArm *const Info = getDynFixupInfo(Kind); + if (LLVM_LIKELY(Info) && LLVM_LIKELY(Info->checkOpcode)) + return Info->checkOpcode(R.Wd); + LLVM_DEBUG(dbgs() << "Can not perform Opcode check for aarch32 edge kind " + << getEdgeKindName(Kind)); + return true; } -template bool checkOpcode(const ArmRelocation &R) { - uint32_t Wd = R.Wd & FixupInfo::OpcodeMask; - return Wd == FixupInfo::Opcode; +static bool checkOpcode(const ThumbRelocation &R, Edge::Kind Kind) { + assert(Kind >= FirstThumbRelocation && Kind <= LastThumbRelocation && + "Edge kind is no Thumb relocation"); + const FixupInfoThumb *const Info = getDynFixupInfo(Kind); + if (LLVM_LIKELY(Info) && LLVM_LIKELY(Info->checkOpcode)) + return Info->checkOpcode(R.Hi, R.Lo); + LLVM_DEBUG(dbgs() << "Can not perform Opcode check for aarch32 edge kind " + << getEdgeKindName(Kind)); + return true; } template @@ -325,26 +409,16 @@ Expected readAddendData(LinkGraph &G, Block &B, Edge::OffsetT Offset, Expected readAddendArm(LinkGraph &G, Block &B, Edge::OffsetT Offset, Edge::Kind Kind) { ArmRelocation R(B.getContent().data() + Offset); + if (!checkOpcode(R, Kind)) + return makeUnexpectedOpcodeError(G, R, Kind); switch (Kind) { case Arm_Call: - if (!checkOpcode(R)) - return makeUnexpectedOpcodeError(G, R, Kind); - return decodeImmBA1BlA1BlxA2(R.Wd); - case Arm_Jump24: - if (!checkOpcode(R)) - return makeUnexpectedOpcodeError(G, R, Kind); return decodeImmBA1BlA1BlxA2(R.Wd); - case Arm_MovwAbsNC: - if (!checkOpcode(R)) - return makeUnexpectedOpcodeError(G, R, Kind); - return decodeImmMovtA1MovwA2(R.Wd); - case Arm_MovtAbs: - if (!checkOpcode(R)) - return makeUnexpectedOpcodeError(G, R, Kind); + case Arm_MovwAbsNC: return decodeImmMovtA1MovwA2(R.Wd); default: @@ -358,33 +432,23 @@ Expected readAddendArm(LinkGraph &G, Block &B, Edge::OffsetT Offset, Expected readAddendThumb(LinkGraph &G, Block &B, Edge::OffsetT Offset, Edge::Kind Kind, const ArmConfig &ArmCfg) { ThumbRelocation R(B.getContent().data() + Offset); + if (!checkOpcode(R, Kind)) + return makeUnexpectedOpcodeError(G, R, Kind); switch (Kind) { case Thumb_Call: - if (!checkOpcode(R)) - return makeUnexpectedOpcodeError(G, R, Kind); + case Thumb_Jump24: return LLVM_LIKELY(ArmCfg.J1J2BranchEncoding) ? decodeImmBT4BlT1BlxT2_J1J2(R.Hi, R.Lo) : decodeImmBT4BlT1BlxT2(R.Hi, R.Lo); - case Thumb_Jump24: - if (!checkOpcode(R)) - return makeUnexpectedOpcodeError(G, R, Kind); - return LLVM_LIKELY(ArmCfg.J1J2BranchEncoding) - ? decodeImmBT4BlT1BlxT2_J1J2(R.Hi, R.Lo) - : decodeImmBT4BlT1BlxT2(R.Hi, R.Lo); - case Thumb_MovwAbsNC: case Thumb_MovwPrelNC: - if (!checkOpcode(R)) - return makeUnexpectedOpcodeError(G, R, Kind); // Initial addend is interpreted as a signed value return SignExtend64<16>(decodeImmMovtT1MovwT3(R.Hi, R.Lo)); case Thumb_MovtAbs: case Thumb_MovtPrel: - if (!checkOpcode(R)) - return makeUnexpectedOpcodeError(G, R, Kind); // Initial addend is interpreted as a signed value return SignExtend64<16>(decodeImmMovtT1MovwT3(R.Hi, R.Lo)); @@ -446,6 +510,9 @@ Error applyFixupData(LinkGraph &G, Block &B, const Edge &E) { Error applyFixupArm(LinkGraph &G, Block &B, const Edge &E) { WritableArmRelocation R(B.getAlreadyMutableContent().data() + E.getOffset()); Edge::Kind Kind = E.getKind(); + if (!checkOpcode(R, Kind)) + return makeUnexpectedOpcodeError(G, R, Kind); + uint64_t FixupAddress = (B.getAddress() + E.getOffset()).getValue(); int64_t Addend = E.getAddend(); Symbol &TargetSymbol = E.getTarget(); @@ -453,8 +520,6 @@ Error applyFixupArm(LinkGraph &G, Block &B, const Edge &E) { switch (Kind) { case Arm_Jump24: { - if (!checkOpcode(R)) - return makeUnexpectedOpcodeError(G, R, Kind); if (hasTargetFlags(TargetSymbol, ThumbSymbol)) return make_error("Branch relocation needs interworking " "stub when bridging to Thumb: " + @@ -469,8 +534,6 @@ Error applyFixupArm(LinkGraph &G, Block &B, const Edge &E) { return Error::success(); } case Arm_Call: { - if (!checkOpcode(R)) - return makeUnexpectedOpcodeError(G, R, Kind); if ((R.Wd & FixupInfo::CondMask) != FixupInfo::Unconditional) return make_error("Relocation expects an unconditional " @@ -501,15 +564,11 @@ Error applyFixupArm(LinkGraph &G, Block &B, const Edge &E) { return Error::success(); } case Arm_MovwAbsNC: { - if (!checkOpcode(R)) - return makeUnexpectedOpcodeError(G, R, Kind); uint16_t Value = (TargetAddress + Addend) & 0xffff; writeImmediate(R, encodeImmMovtA1MovwA2(Value)); return Error::success(); } case Arm_MovtAbs: { - if (!checkOpcode(R)) - return makeUnexpectedOpcodeError(G, R, Kind); uint16_t Value = ((TargetAddress + Addend) >> 16) & 0xffff; writeImmediate(R, encodeImmMovtA1MovwA2(Value)); return Error::success(); @@ -526,8 +585,10 @@ Error applyFixupThumb(LinkGraph &G, Block &B, const Edge &E, const ArmConfig &ArmCfg) { WritableThumbRelocation R(B.getAlreadyMutableContent().data() + E.getOffset()); - Edge::Kind Kind = E.getKind(); + if (!checkOpcode(R, Kind)) + return makeUnexpectedOpcodeError(G, R, Kind); + uint64_t FixupAddress = (B.getAddress() + E.getOffset()).getValue(); int64_t Addend = E.getAddend(); Symbol &TargetSymbol = E.getTarget(); @@ -535,8 +596,6 @@ Error applyFixupThumb(LinkGraph &G, Block &B, const Edge &E, switch (Kind) { case Thumb_Jump24: { - if (!checkOpcode(R)) - return makeUnexpectedOpcodeError(G, R, Kind); if (!hasTargetFlags(TargetSymbol, ThumbSymbol)) return make_error("Branch relocation needs interworking " "stub when bridging to ARM: " + @@ -557,9 +616,6 @@ Error applyFixupThumb(LinkGraph &G, Block &B, const Edge &E, } case Thumb_Call: { - if (!checkOpcode(R)) - return makeUnexpectedOpcodeError(G, R, Kind); - int64_t Value = TargetAddress - FixupAddress + Addend; // The call instruction itself is Thumb. The call destination can either be @@ -596,32 +652,23 @@ Error applyFixupThumb(LinkGraph &G, Block &B, const Edge &E, } case Thumb_MovwAbsNC: { - if (!checkOpcode(R)) - return makeUnexpectedOpcodeError(G, R, Kind); uint16_t Value = (TargetAddress + Addend) & 0xffff; writeImmediate(R, encodeImmMovtT1MovwT3(Value)); return Error::success(); } - case Thumb_MovtAbs: { - if (!checkOpcode(R)) - return makeUnexpectedOpcodeError(G, R, Kind); uint16_t Value = ((TargetAddress + Addend) >> 16) & 0xffff; writeImmediate(R, encodeImmMovtT1MovwT3(Value)); return Error::success(); } case Thumb_MovwPrelNC: { - if (!checkOpcode(R)) - return makeUnexpectedOpcodeError(G, R, Kind); uint16_t Value = ((TargetAddress + Addend - FixupAddress) & 0xffff); - writeImmediate(R, encodeImmMovtT1MovwT3(Value)); + writeImmediate(R, encodeImmMovtT1MovwT3(Value)); return Error::success(); } case Thumb_MovtPrel: { - if (!checkOpcode(R)) - return makeUnexpectedOpcodeError(G, R, Kind); uint16_t Value = (((TargetAddress + Addend - FixupAddress) >> 16) & 0xffff); - writeImmediate(R, encodeImmMovtT1MovwT3(Value)); + writeImmediate(R, encodeImmMovtT1MovwT3(Value)); return Error::success(); } From a6512e2adf6666d4addf3e216518b65ee86186ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= Date: Sun, 12 Nov 2023 22:24:40 +0100 Subject: [PATCH 2/5] Fix deduction for definition of FixupInfo::Opcode --- llvm/lib/ExecutionEngine/JITLink/aarch32.cpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp b/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp index 8d575a4471774..a8c829246d66e 100644 --- a/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp +++ b/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp @@ -264,9 +264,9 @@ template constexpr bool isThumb() { template constexpr bool hasOpcode(...) { return false; } template ::Opcode> -constexpr bool hasOpcode(int) { - return true; -} +constexpr bool hasOpcode(int) { return true; } +template ::Opcode.Lo> +constexpr bool hasOpcode(unsigned) { return true; } template static bool checkOpcodeArm(uint32_t Wd) { return (Wd & FixupInfo::OpcodeMask) == FixupInfo::Opcode; @@ -282,12 +282,11 @@ template static std::unique_ptr initFixupInfo() { auto Entry = std::make_unique>(); if constexpr (hasOpcode(0)) { + static_assert(isArm() != isThumb(), "Classes are mutually exclusive"); if constexpr (isArm()) Entry->checkOpcode = checkOpcodeArm; - else if constexpr (isThumb()) + if constexpr (isThumb()) Entry->checkOpcode = checkOpcodeThumb; - else - llvm_unreachable("Visited edge kinds must either be Arm or Thumb"); } return Entry; } From c7ef2b3f8f596efc406f4a0e365dc7027ba46a65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= Date: Fri, 24 Nov 2023 15:59:33 +0100 Subject: [PATCH 3/5] Store DynFixupInfos as ManagedStatic to achieve lazy init --- .../llvm/ExecutionEngine/JITLink/aarch32.h | 4 +- .../ExecutionEngine/JITLink/ELF_aarch32.cpp | 2 - llvm/lib/ExecutionEngine/JITLink/aarch32.cpp | 140 +++++++++--------- .../ExecutionEngine/JITLink/AArch32Tests.cpp | 23 +++ 4 files changed, 98 insertions(+), 71 deletions(-) diff --git a/llvm/include/llvm/ExecutionEngine/JITLink/aarch32.h b/llvm/include/llvm/ExecutionEngine/JITLink/aarch32.h index 04ed64a04a259..6d48e7e4f5074 100644 --- a/llvm/include/llvm/ExecutionEngine/JITLink/aarch32.h +++ b/llvm/include/llvm/ExecutionEngine/JITLink/aarch32.h @@ -164,11 +164,9 @@ struct HalfWords { const uint16_t Lo; // Second halfword }; -/// Initialize the lookup table for dynamic FixupInfo checks, e.g. checkOpcode() -void populateFixupInfos(); - /// FixupInfo base class is required for dynamic lookups. struct FixupInfoBase { + static const FixupInfoBase *getDynFixupInfo(Edge::Kind K); virtual ~FixupInfoBase() {} }; diff --git a/llvm/lib/ExecutionEngine/JITLink/ELF_aarch32.cpp b/llvm/lib/ExecutionEngine/JITLink/ELF_aarch32.cpp index b6c0fe6d2ef59..132989fcbce02 100644 --- a/llvm/lib/ExecutionEngine/JITLink/ELF_aarch32.cpp +++ b/llvm/lib/ExecutionEngine/JITLink/ELF_aarch32.cpp @@ -232,8 +232,6 @@ createLinkGraphFromELFObject_aarch32(MemoryBufferRef ObjectBuffer) { << ObjectBuffer.getBufferIdentifier() << "...\n"; }); - aarch32::populateFixupInfos(); - auto ELFObj = ObjectFile::createELFObjectFile(ObjectBuffer); if (!ELFObj) return ELFObj.takeError(); diff --git a/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp b/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp index a8c829246d66e..f47e05d52604c 100644 --- a/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp +++ b/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp @@ -17,6 +17,7 @@ #include "llvm/ExecutionEngine/JITLink/JITLink.h" #include "llvm/Object/ELFObjectFile.h" #include "llvm/Support/Endian.h" +#include "llvm/Support/ManagedStatic.h" #include "llvm/Support/MathExtras.h" #define DEBUG_TYPE "jitlink" @@ -249,12 +250,6 @@ Error makeUnexpectedOpcodeError(const LinkGraph &G, const ArmRelocation &R, static_cast(R.Wd), G.getEdgeKindName(Kind))); } -static auto &getFixupInfoTable() { - static constexpr size_t Items = LastRelocation + 1; - static std::array, Items> FixupInfoTable; - return FixupInfoTable; -} - template constexpr bool isArm() { return FirstArmRelocation <= K && K <= LastArmRelocation; } @@ -278,68 +273,81 @@ static bool checkOpcodeThumb(uint16_t Hi, uint16_t Lo) { (Lo & FixupInfo::OpcodeMask.Lo) == FixupInfo::Opcode.Lo; } -template -static std::unique_ptr initFixupInfo() { - auto Entry = std::make_unique>(); - if constexpr (hasOpcode(0)) { - static_assert(isArm() != isThumb(), "Classes are mutually exclusive"); - if constexpr (isArm()) - Entry->checkOpcode = checkOpcodeArm; - if constexpr (isThumb()) - Entry->checkOpcode = checkOpcodeThumb; +class FixupInfoTable { + static constexpr size_t Items = LastRelocation + 1; + +public: + FixupInfoTable() { + populateEntries(); + populateEntries(); } - return Entry; -} -template -void populateFixupInfos(TableT &Table) { - assert(K < Table.size() && "Index out of range"); - assert(Table.at(K) == nullptr && "Initialized entries are immutable"); - Table[K] = initFixupInfo(); - if constexpr (K < LastK) { - constexpr auto Next = static_cast(K + 1); - populateFixupInfos(Table); + const FixupInfoBase *getEntry(Edge::Kind K) { + assert(K < Data.size() && "Index out of bounds"); + return Data.at(K).get(); } -} -} // namespace -void populateFixupInfos() { - static std::once_flag Flag; - std::call_once(Flag, []() { - auto &Table = getFixupInfoTable(); - populateFixupInfos(Table); - populateFixupInfos(Table); - }); -} +private: + template + void populateEntries() { + assert(K < Data.size() && "Index out of range"); + assert(Data.at(K) == nullptr && "Initialized entries are immutable"); + Data[K] = initEntry(); + if constexpr (K < LastK) { + constexpr auto Next = static_cast(K + 1); + populateEntries(); + } + } -template -static const TargetModeSubclass *const getDynFixupInfo(Edge::Kind K) { - assert(K >= Edge::FirstRelocation && "Invalid value for edge kind"); - assert(K <= LastRelocation && "Invalid value for edge kind"); - assert(getFixupInfoTable().at(K) && "Please call populateFixupInfos() first"); - return static_cast(getFixupInfoTable().at(K).get()); -} + template + static std::unique_ptr initEntry() { + auto Entry = std::make_unique>(); + if constexpr (hasOpcode(0)) { + static_assert(isArm() != isThumb(), "Classes are mutually exclusive"); + if constexpr (isArm()) + Entry->checkOpcode = checkOpcodeArm; + if constexpr (isThumb()) + Entry->checkOpcode = checkOpcodeThumb; + } + return Entry; + } -static bool checkOpcode(const ArmRelocation &R, Edge::Kind Kind) { +private: + std::array, Items> Data; +}; + +} // namespace + +ManagedStatic DynFixupInfos; + +static Error +checkOpcode(LinkGraph &G, const ArmRelocation &R, Edge::Kind Kind) { assert(Kind >= FirstArmRelocation && Kind <= LastArmRelocation && - "Edge kind is no Arm relocation"); - const FixupInfoArm *const Info = getDynFixupInfo(Kind); - if (LLVM_LIKELY(Info) && LLVM_LIKELY(Info->checkOpcode)) - return Info->checkOpcode(R.Wd); - LLVM_DEBUG(dbgs() << "Can not perform Opcode check for aarch32 edge kind " - << getEdgeKindName(Kind)); - return true; + "Edge kind must be Arm relocation"); + const FixupInfoBase *Entry = DynFixupInfos->getEntry(Kind); + const FixupInfoArm &Info = *static_cast(Entry); + assert(Info.checkOpcode && "Opcode check is mandatory for Arm edges"); + if (!Info.checkOpcode(R.Wd)) + return makeUnexpectedOpcodeError(G, R, Kind); + + return Error::success(); } -static bool checkOpcode(const ThumbRelocation &R, Edge::Kind Kind) { +static Error +checkOpcode(LinkGraph &G, const ThumbRelocation &R, Edge::Kind Kind) { assert(Kind >= FirstThumbRelocation && Kind <= LastThumbRelocation && - "Edge kind is no Thumb relocation"); - const FixupInfoThumb *const Info = getDynFixupInfo(Kind); - if (LLVM_LIKELY(Info) && LLVM_LIKELY(Info->checkOpcode)) - return Info->checkOpcode(R.Hi, R.Lo); - LLVM_DEBUG(dbgs() << "Can not perform Opcode check for aarch32 edge kind " - << getEdgeKindName(Kind)); - return true; + "Edge kind must be Thumb relocation"); + const FixupInfoBase *Entry = DynFixupInfos->getEntry(Kind); + const FixupInfoThumb &Info = *static_cast(Entry); + assert(Info.checkOpcode && "Opcode check is mandatory for Thumb edges"); + if (!Info.checkOpcode(R.Hi, R.Lo)) + return makeUnexpectedOpcodeError(G, R, Kind); + + return Error::success(); +} + +const FixupInfoBase *FixupInfoBase::getDynFixupInfo(Edge::Kind K) { + return DynFixupInfos->getEntry(K); } template @@ -408,8 +416,8 @@ Expected readAddendData(LinkGraph &G, Block &B, Edge::OffsetT Offset, Expected readAddendArm(LinkGraph &G, Block &B, Edge::OffsetT Offset, Edge::Kind Kind) { ArmRelocation R(B.getContent().data() + Offset); - if (!checkOpcode(R, Kind)) - return makeUnexpectedOpcodeError(G, R, Kind); + if (Error Err = checkOpcode(G, R, Kind)) + return std::move(Err); switch (Kind) { case Arm_Call: @@ -431,8 +439,8 @@ Expected readAddendArm(LinkGraph &G, Block &B, Edge::OffsetT Offset, Expected readAddendThumb(LinkGraph &G, Block &B, Edge::OffsetT Offset, Edge::Kind Kind, const ArmConfig &ArmCfg) { ThumbRelocation R(B.getContent().data() + Offset); - if (!checkOpcode(R, Kind)) - return makeUnexpectedOpcodeError(G, R, Kind); + if (Error Err = checkOpcode(G, R, Kind)) + return std::move(Err); switch (Kind) { case Thumb_Call: @@ -509,8 +517,8 @@ Error applyFixupData(LinkGraph &G, Block &B, const Edge &E) { Error applyFixupArm(LinkGraph &G, Block &B, const Edge &E) { WritableArmRelocation R(B.getAlreadyMutableContent().data() + E.getOffset()); Edge::Kind Kind = E.getKind(); - if (!checkOpcode(R, Kind)) - return makeUnexpectedOpcodeError(G, R, Kind); + if (Error Err = checkOpcode(G, R, Kind)) + return Err; uint64_t FixupAddress = (B.getAddress() + E.getOffset()).getValue(); int64_t Addend = E.getAddend(); @@ -585,8 +593,8 @@ Error applyFixupThumb(LinkGraph &G, Block &B, const Edge &E, WritableThumbRelocation R(B.getAlreadyMutableContent().data() + E.getOffset()); Edge::Kind Kind = E.getKind(); - if (!checkOpcode(R, Kind)) - return makeUnexpectedOpcodeError(G, R, Kind); + if (Error Err = checkOpcode(G, R, Kind)) + return Err; uint64_t FixupAddress = (B.getAddress() + E.getOffset()).getValue(); int64_t Addend = E.getAddend(); diff --git a/llvm/unittests/ExecutionEngine/JITLink/AArch32Tests.cpp b/llvm/unittests/ExecutionEngine/JITLink/AArch32Tests.cpp index dcc8d3b237ff3..5f44e4d930362 100644 --- a/llvm/unittests/ExecutionEngine/JITLink/AArch32Tests.cpp +++ b/llvm/unittests/ExecutionEngine/JITLink/AArch32Tests.cpp @@ -67,6 +67,29 @@ TEST(AArch32_ELF, EdgeKinds) { } } +TEST(AArch32_ELF, DynFixupInfos) { + // We can do an opcode check for all Arm edges + for (Edge::Kind K = FirstArmRelocation; K < LastArmRelocation; K += 1) { + const auto *Info = FixupInfoBase::getDynFixupInfo(K); + EXPECT_NE(Info, nullptr); + const auto *InfoArm = static_cast(Info); + EXPECT_NE(InfoArm->checkOpcode, nullptr); + EXPECT_FALSE(InfoArm->checkOpcode(0x00000000)); + } + // We can do an opcode check for all Thumb edges + for (Edge::Kind K = FirstThumbRelocation; K < LastThumbRelocation; K += 1) { + const auto *Info = FixupInfoBase::getDynFixupInfo(K); + EXPECT_NE(Info, nullptr); + const auto *InfoThumb = static_cast(Info); + EXPECT_NE(InfoThumb->checkOpcode, nullptr); + EXPECT_FALSE(InfoThumb->checkOpcode(0x0000, 0x0000)); + } + // We cannot do it for Data and generic edges + EXPECT_EQ(FixupInfoBase::getDynFixupInfo(FirstDataRelocation), nullptr); + EXPECT_EQ(FixupInfoBase::getDynFixupInfo(Edge::GenericEdgeKind::Invalid), + nullptr); +} + namespace llvm { namespace jitlink { namespace aarch32 { From 3e1d518c3a031fa3718d6db885747e097a61f881 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= Date: Fri, 24 Nov 2023 16:15:56 +0100 Subject: [PATCH 4/5] Drop hasOpcode() compile-time check and instead keep them mandatory for now --- llvm/lib/ExecutionEngine/JITLink/aarch32.cpp | 29 +++++++------------- 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp b/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp index f47e05d52604c..7ff26a5ff6043 100644 --- a/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp +++ b/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp @@ -257,12 +257,6 @@ template constexpr bool isThumb() { return FirstThumbRelocation <= K && K <= LastThumbRelocation; } -template constexpr bool hasOpcode(...) { return false; } -template ::Opcode> -constexpr bool hasOpcode(int) { return true; } -template ::Opcode.Lo> -constexpr bool hasOpcode(unsigned) { return true; } - template static bool checkOpcodeArm(uint32_t Wd) { return (Wd & FixupInfo::OpcodeMask) == FixupInfo::Opcode; } @@ -288,8 +282,7 @@ class FixupInfoTable { } private: - template - void populateEntries() { + template void populateEntries() { assert(K < Data.size() && "Index out of range"); assert(Data.at(K) == nullptr && "Initialized entries are immutable"); Data[K] = initEntry(); @@ -302,13 +295,11 @@ class FixupInfoTable { template static std::unique_ptr initEntry() { auto Entry = std::make_unique>(); - if constexpr (hasOpcode(0)) { - static_assert(isArm() != isThumb(), "Classes are mutually exclusive"); - if constexpr (isArm()) - Entry->checkOpcode = checkOpcodeArm; - if constexpr (isThumb()) - Entry->checkOpcode = checkOpcodeThumb; - } + static_assert(isArm() != isThumb(), "Classes are mutually exclusive"); + if constexpr (isArm()) + Entry->checkOpcode = checkOpcodeArm; + if constexpr (isThumb()) + Entry->checkOpcode = checkOpcodeThumb; return Entry; } @@ -320,8 +311,8 @@ class FixupInfoTable { ManagedStatic DynFixupInfos; -static Error -checkOpcode(LinkGraph &G, const ArmRelocation &R, Edge::Kind Kind) { +static Error checkOpcode(LinkGraph &G, const ArmRelocation &R, + Edge::Kind Kind) { assert(Kind >= FirstArmRelocation && Kind <= LastArmRelocation && "Edge kind must be Arm relocation"); const FixupInfoBase *Entry = DynFixupInfos->getEntry(Kind); @@ -333,8 +324,8 @@ checkOpcode(LinkGraph &G, const ArmRelocation &R, Edge::Kind Kind) { return Error::success(); } -static Error -checkOpcode(LinkGraph &G, const ThumbRelocation &R, Edge::Kind Kind) { +static Error checkOpcode(LinkGraph &G, const ThumbRelocation &R, + Edge::Kind Kind) { assert(Kind >= FirstThumbRelocation && Kind <= LastThumbRelocation && "Edge kind must be Thumb relocation"); const FixupInfoBase *Entry = DynFixupInfos->getEntry(Kind); From f9e510bd85134dfedfbd9ed3dc3f27c2e4e74717 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= Date: Fri, 24 Nov 2023 16:21:40 +0100 Subject: [PATCH 5/5] Polishing --- llvm/lib/ExecutionEngine/JITLink/aarch32.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp b/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp index 7ff26a5ff6043..b727ad3983135 100644 --- a/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp +++ b/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp @@ -307,10 +307,10 @@ class FixupInfoTable { std::array, Items> Data; }; -} // namespace - ManagedStatic DynFixupInfos; +} // namespace + static Error checkOpcode(LinkGraph &G, const ArmRelocation &R, Edge::Kind Kind) { assert(Kind >= FirstArmRelocation && Kind <= LastArmRelocation &&