From d5eb0a9484013197bd04ca40ead93fd5532a0c45 Mon Sep 17 00:00:00 2001 From: Dimitry Andric Date: Wed, 23 Apr 2025 23:18:17 +0200 Subject: [PATCH] [JITLink][i386] Avoid 'i386' name clashing with built-in macro When compiling llvm on an actual i386 platform, both clang and gcc define a built-in macro `i386` to the value 1. This clashes with the `llvm::jitlink::i386` namespace: ``` In file included from llvm/lib/ExecutionEngine/JITLink/ELF_i386.cpp:18: llvm/include/llvm/ExecutionEngine/JITLink/i386.h:19:24: error: expected '{' 19 | namespace llvm::jitlink::i386 { | ^ llvm/include/llvm/ExecutionEngine/JITLink/i386.h:19:26: error: expected unqualified-id 19 | namespace llvm::jitlink::i386 { | ^ ``` The macro name 'i386' is obviously a historical bad choice, but since it existed long before llvm, llvm should either rename its namespace, or actively undefine the macro in `i386.h` (similar to e.g. https://github.com/google/swiftshader/blob/master/third_party/llvm-16.0/Android.bp#L1510). This particular pull request implements the rename, but is meant to gauge opinions on how to solve this issue. --- .../llvm/ExecutionEngine/JITLink/i386.h | 36 +++++++-------- llvm/lib/ExecutionEngine/JITLink/ELF_i386.cpp | 44 +++++++++---------- llvm/lib/ExecutionEngine/JITLink/JITLink.cpp | 4 +- llvm/lib/ExecutionEngine/JITLink/i386.cpp | 8 ++-- .../ExecutionEngine/JITLink/StubsTests.cpp | 4 +- 5 files changed, 48 insertions(+), 48 deletions(-) diff --git a/llvm/include/llvm/ExecutionEngine/JITLink/i386.h b/llvm/include/llvm/ExecutionEngine/JITLink/i386.h index efe8182934dd7..f687d01c28de8 100644 --- a/llvm/include/llvm/ExecutionEngine/JITLink/i386.h +++ b/llvm/include/llvm/ExecutionEngine/JITLink/i386.h @@ -16,7 +16,7 @@ #include "llvm/ExecutionEngine/JITLink/JITLink.h" #include "llvm/ExecutionEngine/JITLink/TableManager.h" -namespace llvm::jitlink::i386 { +namespace llvm::jitlink::i386_ { /// Represets i386 fixups enum EdgeKind_i386 : Edge::Kind { @@ -184,7 +184,7 @@ const char *getEdgeKindName(Edge::Kind K); /// Apply fixup expression for edge to block content. inline Error applyFixup(LinkGraph &G, Block &B, const Edge &E, const Symbol *GOTSymbol) { - using namespace i386; + using namespace i386_; using namespace llvm::support; char *BlockWorkingMem = B.getAlreadyMutableContent().data(); @@ -192,23 +192,23 @@ inline Error applyFixup(LinkGraph &G, Block &B, const Edge &E, auto FixupAddress = B.getAddress() + E.getOffset(); switch (E.getKind()) { - case i386::None: { + case i386_::None: { break; } - case i386::Pointer32: { + case i386_::Pointer32: { uint32_t Value = E.getTarget().getAddress().getValue() + E.getAddend(); *(ulittle32_t *)FixupPtr = Value; break; } - case i386::PCRel32: { + case i386_::PCRel32: { int32_t Value = E.getTarget().getAddress() - FixupAddress + E.getAddend(); *(little32_t *)FixupPtr = Value; break; } - case i386::Pointer16: { + case i386_::Pointer16: { uint32_t Value = E.getTarget().getAddress().getValue() + E.getAddend(); if (LLVM_LIKELY(isUInt<16>(Value))) *(ulittle16_t *)FixupPtr = Value; @@ -217,7 +217,7 @@ inline Error applyFixup(LinkGraph &G, Block &B, const Edge &E, break; } - case i386::PCRel16: { + case i386_::PCRel16: { int32_t Value = E.getTarget().getAddress() - FixupAddress + E.getAddend(); if (LLVM_LIKELY(isInt<16>(Value))) *(little16_t *)FixupPtr = Value; @@ -226,13 +226,13 @@ inline Error applyFixup(LinkGraph &G, Block &B, const Edge &E, break; } - case i386::Delta32: { + case i386_::Delta32: { int32_t Value = E.getTarget().getAddress() - FixupAddress + E.getAddend(); *(little32_t *)FixupPtr = Value; break; } - case i386::Delta32FromGOT: { + case i386_::Delta32FromGOT: { assert(GOTSymbol && "No GOT section symbol"); int32_t Value = E.getTarget().getAddress() - GOTSymbol->getAddress() + E.getAddend(); @@ -240,9 +240,9 @@ inline Error applyFixup(LinkGraph &G, Block &B, const Edge &E, break; } - case i386::BranchPCRel32: - case i386::BranchPCRel32ToPtrJumpStub: - case i386::BranchPCRel32ToPtrJumpStubBypassable: { + case i386_::BranchPCRel32: + case i386_::BranchPCRel32ToPtrJumpStub: + case i386_::BranchPCRel32ToPtrJumpStubBypassable: { int32_t Value = E.getTarget().getAddress() - FixupAddress + E.getAddend(); *(little32_t *)FixupPtr = Value; break; @@ -328,14 +328,14 @@ class GOTTableManager : public TableManager { bool visitEdge(LinkGraph &G, Block *B, Edge &E) { Edge::Kind KindToSet = Edge::Invalid; switch (E.getKind()) { - case i386::Delta32FromGOT: { + case i386_::Delta32FromGOT: { // we need to make sure that the GOT section exists, but don't otherwise // need to fix up this edge getGOTSection(G); return false; } - case i386::RequestGOTAndTransformToDelta32FromGOT: - KindToSet = i386::Delta32FromGOT; + case i386_::RequestGOTAndTransformToDelta32FromGOT: + KindToSet = i386_::Delta32FromGOT; break; default: return false; @@ -374,7 +374,7 @@ class PLTTableManager : public TableManager { static StringRef getSectionName() { return "$__STUBS"; } bool visitEdge(LinkGraph &G, Block *B, Edge &E) { - if (E.getKind() == i386::BranchPCRel32 && !E.getTarget().isDefined()) { + if (E.getKind() == i386_::BranchPCRel32 && !E.getTarget().isDefined()) { DEBUG_WITH_TYPE("jitlink", { dbgs() << " Fixing " << G.getEdgeKindName(E.getKind()) << " edge at " << B->getFixupAddress(E) << " (" << B->getAddress() << " + " @@ -382,7 +382,7 @@ class PLTTableManager : public TableManager { }); // Set the edge kind to Branch32ToPtrJumpStubBypassable to enable it to // be optimized when the target is in-range. - E.setKind(i386::BranchPCRel32ToPtrJumpStubBypassable); + E.setKind(i386_::BranchPCRel32ToPtrJumpStubBypassable); E.setTarget(getEntryForTarget(G, E.getTarget())); return true; } @@ -414,6 +414,6 @@ class PLTTableManager : public TableManager { /// target Error optimizeGOTAndStubAccesses(LinkGraph &G); -} // namespace llvm::jitlink::i386 +} // namespace llvm::jitlink::i386_ #endif // LLVM_EXECUTIONENGINE_JITLINK_I386_H diff --git a/llvm/lib/ExecutionEngine/JITLink/ELF_i386.cpp b/llvm/lib/ExecutionEngine/JITLink/ELF_i386.cpp index 4ce43c1962c84..600f44e56b6d4 100644 --- a/llvm/lib/ExecutionEngine/JITLink/ELF_i386.cpp +++ b/llvm/lib/ExecutionEngine/JITLink/ELF_i386.cpp @@ -29,8 +29,8 @@ constexpr StringRef ELFGOTSymbolName = "_GLOBAL_OFFSET_TABLE_"; Error buildTables_ELF_i386(LinkGraph &G) { LLVM_DEBUG(dbgs() << "Visiting edges in graph:\n"); - i386::GOTTableManager GOT; - i386::PLTTableManager PLT(GOT); + i386_::GOTTableManager GOT; + i386_::PLTTableManager PLT(GOT); visitExistingEdges(G, GOT, PLT); return Error::success(); } @@ -59,7 +59,7 @@ class ELFJITLinker_i386 : public JITLinker { if (Sym.getName() != nullptr && *Sym.getName() == ELFGOTSymbolName) if (auto *GOTSection = G.findSectionByName( - i386::GOTTableManager::getSectionName())) { + i386_::GOTTableManager::getSectionName())) { GOTSymbol = &Sym; return {*GOTSection, true}; } @@ -79,7 +79,7 @@ class ELFJITLinker_i386 : public JITLinker { // record it, otherwise we'll create our own. // If there's a GOT section but we didn't find an external GOT symbol... if (auto *GOTSection = - G.findSectionByName(i386::GOTTableManager::getSectionName())) { + G.findSectionByName(i386_::GOTTableManager::getSectionName())) { // Check for an existing defined symbol. for (auto *Sym : GOTSection->symbols()) @@ -106,15 +106,15 @@ class ELFJITLinker_i386 : public JITLinker { } Error applyFixup(LinkGraph &G, Block &B, const Edge &E) const { - return i386::applyFixup(G, B, E, GOTSymbol); + return i386_::applyFixup(G, B, E, GOTSymbol); } }; template class ELFLinkGraphBuilder_i386 : public ELFLinkGraphBuilder { private: - static Expected getRelocationKind(const uint32_t Type) { - using namespace i386; + static Expected getRelocationKind(const uint32_t Type) { + using namespace i386_; switch (Type) { case ELF::R_386_NONE: return EdgeKind_i386::None; @@ -179,7 +179,7 @@ class ELFLinkGraphBuilder_i386 : public ELFLinkGraphBuilder { Base::GraphSymbols.size()), inconvertibleErrorCode()); - Expected Kind = getRelocationKind(Rel.getType(false)); + Expected Kind = getRelocationKind(Rel.getType(false)); if (!Kind) return Kind.takeError(); @@ -187,23 +187,23 @@ class ELFLinkGraphBuilder_i386 : public ELFLinkGraphBuilder { int64_t Addend = 0; switch (*Kind) { - case i386::EdgeKind_i386::None: + case i386_::EdgeKind_i386::None: break; - case i386::EdgeKind_i386::Pointer32: - case i386::EdgeKind_i386::PCRel32: - case i386::EdgeKind_i386::RequestGOTAndTransformToDelta32FromGOT: - case i386::EdgeKind_i386::Delta32: - case i386::EdgeKind_i386::Delta32FromGOT: - case i386::EdgeKind_i386::BranchPCRel32: - case i386::EdgeKind_i386::BranchPCRel32ToPtrJumpStub: - case i386::EdgeKind_i386::BranchPCRel32ToPtrJumpStubBypassable: { + case i386_::EdgeKind_i386::Pointer32: + case i386_::EdgeKind_i386::PCRel32: + case i386_::EdgeKind_i386::RequestGOTAndTransformToDelta32FromGOT: + case i386_::EdgeKind_i386::Delta32: + case i386_::EdgeKind_i386::Delta32FromGOT: + case i386_::EdgeKind_i386::BranchPCRel32: + case i386_::EdgeKind_i386::BranchPCRel32ToPtrJumpStub: + case i386_::EdgeKind_i386::BranchPCRel32ToPtrJumpStubBypassable: { const char *FixupContent = BlockToFix.getContent().data() + (FixupAddress - BlockToFix.getAddress()); Addend = *(const support::little32_t *)FixupContent; break; } - case i386::EdgeKind_i386::Pointer16: - case i386::EdgeKind_i386::PCRel16: { + case i386_::EdgeKind_i386::Pointer16: + case i386_::EdgeKind_i386::PCRel16: { const char *FixupContent = BlockToFix.getContent().data() + (FixupAddress - BlockToFix.getAddress()); Addend = *(const support::little16_t *)FixupContent; @@ -215,7 +215,7 @@ class ELFLinkGraphBuilder_i386 : public ELFLinkGraphBuilder { Edge GE(*Kind, Offset, *GraphSymbol, Addend); LLVM_DEBUG({ dbgs() << " "; - printEdge(dbgs(), BlockToFix, GE, i386::getEdgeKindName(*Kind)); + printEdge(dbgs(), BlockToFix, GE, i386_::getEdgeKindName(*Kind)); dbgs() << "\n"; }); @@ -229,7 +229,7 @@ class ELFLinkGraphBuilder_i386 : public ELFLinkGraphBuilder { Triple TT, SubtargetFeatures Features) : ELFLinkGraphBuilder(Obj, std::move(SSP), std::move(TT), std::move(Features), FileName, - i386::getEdgeKindName) {} + i386_::getEdgeKindName) {} }; Expected> @@ -273,7 +273,7 @@ void link_ELF_i386(std::unique_ptr G, Config.PostPrunePasses.push_back(buildTables_ELF_i386); // Add GOT/Stubs optimizer pass. - Config.PreFixupPasses.push_back(i386::optimizeGOTAndStubAccesses); + Config.PreFixupPasses.push_back(i386_::optimizeGOTAndStubAccesses); } if (auto Err = Ctx->modifyPassConfig(*G, Config)) return Ctx->notifyFailed(std::move(Err)); diff --git a/llvm/lib/ExecutionEngine/JITLink/JITLink.cpp b/llvm/lib/ExecutionEngine/JITLink/JITLink.cpp index e1209e1e95496..e682ee0ca1983 100644 --- a/llvm/lib/ExecutionEngine/JITLink/JITLink.cpp +++ b/llvm/lib/ExecutionEngine/JITLink/JITLink.cpp @@ -466,7 +466,7 @@ AnonymousPointerCreator getAnonymousPointerCreator(const Triple &TT) { case Triple::x86_64: return x86_64::createAnonymousPointer; case Triple::x86: - return i386::createAnonymousPointer; + return i386_::createAnonymousPointer; case Triple::loongarch32: case Triple::loongarch64: return loongarch::createAnonymousPointer; @@ -482,7 +482,7 @@ PointerJumpStubCreator getPointerJumpStubCreator(const Triple &TT) { case Triple::x86_64: return x86_64::createAnonymousPointerJumpStub; case Triple::x86: - return i386::createAnonymousPointerJumpStub; + return i386_::createAnonymousPointerJumpStub; case Triple::loongarch32: case Triple::loongarch64: return loongarch::createAnonymousPointerJumpStub; diff --git a/llvm/lib/ExecutionEngine/JITLink/i386.cpp b/llvm/lib/ExecutionEngine/JITLink/i386.cpp index e984bb10983d0..90d5c610dc412 100644 --- a/llvm/lib/ExecutionEngine/JITLink/i386.cpp +++ b/llvm/lib/ExecutionEngine/JITLink/i386.cpp @@ -14,7 +14,7 @@ #define DEBUG_TYPE "jitlink" -namespace llvm::jitlink::i386 { +namespace llvm::jitlink::i386_ { const char *getEdgeKindName(Edge::Kind K) { switch (K) { @@ -55,7 +55,7 @@ Error optimizeGOTAndStubAccesses(LinkGraph &G) { for (auto *B : G.blocks()) for (auto &E : B->edges()) { - if (E.getKind() == i386::BranchPCRel32ToPtrJumpStubBypassable) { + if (E.getKind() == i386_::BranchPCRel32ToPtrJumpStubBypassable) { auto &StubBlock = E.getTarget().getBlock(); assert(StubBlock.getSize() == sizeof(PointerJumpStubContent) && "Stub block should be stub sized"); @@ -74,7 +74,7 @@ Error optimizeGOTAndStubAccesses(LinkGraph &G) { int64_t Displacement = TargetAddr - EdgeAddr + 4; if (isInt<32>(Displacement)) { - E.setKind(i386::BranchPCRel32); + E.setKind(i386_::BranchPCRel32); E.setTarget(GOTTarget); LLVM_DEBUG({ dbgs() << " Replaced stub branch with direct branch:\n "; @@ -88,4 +88,4 @@ Error optimizeGOTAndStubAccesses(LinkGraph &G) { return Error::success(); } -} // namespace llvm::jitlink::i386 +} // namespace llvm::jitlink::i386_ diff --git a/llvm/unittests/ExecutionEngine/JITLink/StubsTests.cpp b/llvm/unittests/ExecutionEngine/JITLink/StubsTests.cpp index 8a53d0a560ba3..95d9e219f3899 100644 --- a/llvm/unittests/ExecutionEngine/JITLink/StubsTests.cpp +++ b/llvm/unittests/ExecutionEngine/JITLink/StubsTests.cpp @@ -102,13 +102,13 @@ TEST(StubsTest, StubsGeneration_i386) { LinkGraph G("foo", std::make_shared(), Triple("i386-unknown-linux-gnu"), SubtargetFeatures(), getGenericEdgeKindName); - auto [PointerSym, StubSym] = GenerateStub(G, 4U, i386::Pointer32); + auto [PointerSym, StubSym] = GenerateStub(G, 4U, i386_::Pointer32); EXPECT_EQ(std::distance(StubSym.getBlock().edges().begin(), StubSym.getBlock().edges().end()), 1U); auto &JumpEdge = *StubSym.getBlock().edges().begin(); - EXPECT_EQ(JumpEdge.getKind(), i386::Pointer32); + EXPECT_EQ(JumpEdge.getKind(), i386_::Pointer32); EXPECT_EQ(&JumpEdge.getTarget(), &PointerSym); EXPECT_EQ(StubSym.getBlock().getContent(), ArrayRef(PointerJumpStubContent));