Page MenuHomePhabricator

[JITLink] Initial AArch32 backend
ClosedPublic

Authored by sgraenitz on Feb 15 2023, 2:02 AM.

Details

Summary

This first version lays the foundations for AArch32 support in JITLink. ELFLinkGraphBuilder_aarch32 processes REL-type relocations and populates LinkGraphs from ELF object files for both big- and little-endian systems. The ArmCfg member controls subarchitecture-specific details throughout the linking process (i.e. it's passed to ELFJITLinker_aarch32).

Relocation types follow the ABI documentation's division into classes: Data (endian-sensitive), Arm (32-bit little-endian) and Thumb (2x 16-bit little-endian, "Thumb32" in the docs). The implementation of instruction encoding/decoding for relocation resolution is implemented symmetrically and is testable in isolation (see AArch32 category in JITLinkTests).

Callable Thumb functions are marked with a ThumbSymbol target-flag and stored in the LinkGraph with their real addresses. The thumb-bit is added back in when the owning JITDylib requests the address for such a symbol.

The StubsManager can generate (absolute) Thumb-state stubs for branch range extensions on v7+ targets. Proper GOT/PLT handling is not yet implemented.

This patch is based on the backend implementation in ez-clang and has just enough functionality to model the infrastructure and link a Thumb function main() that calls printf() to dump "Hello Arm!" on Armv7a. It was tested on Raspberry Pi with 32-bit Raspbian OS.

Diff Detail

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
peter.smith added inline comments.Feb 28 2023, 1:39 AM
llvm/lib/ExecutionEngine/JITLink/aarch32.cpp
245

Yes, the code in visitEdge, quoted here. If there is a BLX instruction with a Thumb_Call relocation where the target isn't defined, it looks like a stub will be created.

bool visitEdge(LinkGraph &G, Block *B, Edge &E) {
    if (E.getTarget().isDefined())
      return false;

    switch (E.getKind()) {
    case Thumb_Call:
    case Thumb_Jump24: {
      DEBUG_WITH_TYPE("jitlink", {
        dbgs() << "  Fixing " << G.getEdgeKindName(E.getKind()) << " edge at "
               << B->getFixupAddress(E) << " (" << B->getAddress() << " + "
               << formatv("{0:x}", E.getOffset()) << ")\n";
      });
      E.setTarget(this->getEntryForTarget(G, E.getTarget()));
      return true;
    }
    }
    return false;
  }

I would not expect a code-generator to generate a BLX in this situation as it has no idea whether the target is Arm or Thumb so I would expect a BL. It is possible to write in assembly language but I don't know how well that needs to be supported in a JIT.

In static linkers an R_ARM_CALL (Arm state) or R_ARM_THM_CALL (Thumb state) relocation can be associated with a BL or BLX. As the static linker always knows the (Arm/Thumb) state of the destination it can write a BL instruction if both source and target are the same state or a BLX if they are different. This property permits code-generators to only use BL with R_ARM_CALL/R_ARM_THM_CALL, with BLX reserved for assembler. It also permits Thumb code to reuse Arm stubs.

I don't know how often this would happen in a JIT though.

262

R_ARM_THM_JUMP24 is used for the wide unconditional branch regardless of whether J1J2 encoding is available. It was introduced when the BLX instruction was made available. BL <immediate> can be converted into BLX <immediate> but B.w <immediate> cannot be converted in BX <immediate> as BX <immediate> doesn't exist (only BX <register>).

In summary R_ARM_THM_CALL is for BL and BLX instructions, R_ARM_THM_JUMP24 is for B.w instructions. The J1J2 encoding applies to both BL, BLX and B.w.

sgraenitz updated this revision to Diff 502411.Mar 5 2023, 2:45 AM
sgraenitz marked an inline comment as done.

Ignore .ARM.exidx sections by checking for type ELF::SHT_ARM_EXIDX

sgraenitz updated this revision to Diff 502412.Mar 5 2023, 2:47 AM
sgraenitz marked an inline comment as done.

Drop unused flags in ArmConfig

sgraenitz updated this revision to Diff 502413.Mar 5 2023, 2:51 AM
sgraenitz marked 3 inline comments as done.

Drop LoBitH check when reading initial addend for Thumb_Call

sgraenitz updated this revision to Diff 502414.Mar 5 2023, 2:53 AM
sgraenitz marked 2 inline comments as done.

Support J1J2BranchEncoding for Thumb_Jump24

sgraenitz updated this revision to Diff 502415.Mar 5 2023, 2:55 AM
sgraenitz marked an inline comment as done.

Add comment on instruction endianness for makeHalfWords() in AArch32Tests

sgraenitz updated this revision to Diff 502416.Mar 5 2023, 2:58 AM

Move range checks behind BL-BLX conversion for Thumb_Call

sgraenitz updated this revision to Diff 502417.Mar 5 2023, 3:03 AM

Support J1J2BranchEncoding for Thumb_Jump24 -- also in readAddend()

sgraenitz updated this revision to Diff 502418.Mar 5 2023, 3:06 AM

Raise runtime errors for unsupported edge kinds in subroutines of readAddend() and applyFixup() (instead of asserting)

Just one small comment about when traversing relocations from .ARM.exidx sections get implemented.

I'm out of comments from the Arm side. Good luck with the JITLink specific changes. I'm happy to look at the Arm side of any future updates or new changes.

llvm/lib/ExecutionEngine/JITLink/ELF_aarch32.cpp
120

I know this is a FIXME at the moment so nothing needs to be changed. For when it does get implemented, only relocations with an offset of 4 (modulo 8) should be traversed. An .ARM.exidx table entry is 8-bytes in size, made up of 2 4-byte entries:

R_ARM_PREL31 to function this is a table foreither inline unwind instructions or a R_ARM_PREL31 to a .ARM.extab section containing unwind instructions

The first word at offset 0 (modulo 8) is used to construct a table of offsets to all the functions with table entries. I expect that we would want to avoid following relocations in this context.

sgraenitz updated this revision to Diff 503310.Mar 8 2023, 4:04 AM
sgraenitz marked an inline comment as done.

Add more comprehensive TODO comment for relocations in .ARM.exidx section

Thanks @peter.smith for your help, it's very valuable! I appreciate your offer to help with future reviews as well.

I am planning to land this before end of March. Right now, it's waiting for D144714. In the meantime, if anyone has more feedback, please go ahead.

lhames added inline comments.Mar 11 2023, 3:38 PM
llvm/include/llvm/ExecutionEngine/JITLink/aarch32.h
232

We use $__STUBS elsewhere, rather than S__STUBS.

As a side note: I threw a $ in here to make a clash less likely, but I don't think it's actually an illegal symbol character in ELF or COFF so we should probably pick some universal prefix going forward. Maybe just __llvm_jitlink?

llvm/lib/ExecutionEngine/JITLink/ELF_aarch32.cpp
79–90

This should return the name of the JITLink edge enum, rather than the ELF relocation. That's not a blocker for landing though.

llvm/lib/ExecutionEngine/JITLink/aarch32.cpp
134

Could this be two types -- ThumbRelocation and MutableThumbRelocation, with ThumbRelocations constructible from MutableThumbRelocations?

245

We definitely want to support this eventually, and the JIT linker should have all the information that it needs to do so (though it's possible that we'll need to fix some plumbing in the symbol tables to transfer the thumb bit through).

I don't think we need this in the initial commit though.

sgraenitz updated this revision to Diff 504554.Mar 13 2023, 2:21 AM
sgraenitz marked an inline comment as done.

Make two distinct types for ThumbRelocation and WritableThumbRelocation

llvm/lib/ExecutionEngine/JITLink/ELF_aarch32.cpp
79–90

Do we need the name of the internal edge-kind? I moved the function here from aarch32.h to allow format-specific output. (I still have to see how it works out with intermediate edge kinds.)

llvm/lib/ExecutionEngine/JITLink/aarch32.cpp
134

Yes, it's cleaner like that and it could be structs not classes

dkurt removed a subscriber: dkurt.Mar 13 2023, 12:47 PM
sgraenitz updated this revision to Diff 507405.Mar 22 2023, 9:45 AM
sgraenitz marked an inline comment as done.

Rename stubs section SSTUBS -> llvm_jitlink_STUBS

sgraenitz updated this revision to Diff 507407.Mar 22 2023, 9:48 AM

Extend test to check symbol address type arm/thumb

sgraenitz updated this revision to Diff 507411.Mar 22 2023, 9:54 AM

Remove thumb bit from LinkGraph addresses and store it as target flag in jitlink::Symbol

sgraenitz updated this revision to Diff 507481.Mar 22 2023, 1:16 PM

Rebase and actually run the test in a 32-bit address range

sgraenitz updated this revision to Diff 507484.Mar 22 2023, 1:21 PM

Fix test warning: missing parameter -slab-page-size

sgraenitz updated this revision to Diff 507505.Mar 22 2023, 2:15 PM

In the test, inject an absolute symbol address for printf

I successfully rebased the patch to current mainline and check-llvm is happy. From my side this is ready to land.

lhames accepted this revision.Mar 22 2023, 2:55 PM

Relocation names should be generic (at least for getEdgeKindName), but otherwise LGTM!

Thank you very much for this Stefan: it's a huge contribution, with very neat solutions to hard problems, and a big step forward for LLVM JIT support!

llvm/lib/ExecutionEngine/JITLink/ELF_aarch32.cpp
79–90

The purpose of getEdgeKindName is definitely to name the Edge::Kind enum values (to make it easy to look them up in the JITLink docs and applyRelocation), but I think it makes sense to add support for a reverse mapping to the original relocation names too. We should introduce a new function for this, but that can be done post-commit.

This revision is now accepted and ready to land.Mar 22 2023, 2:55 PM
sgraenitz updated this revision to Diff 507524.Mar 22 2023, 3:27 PM

Move getEdgeKindName() back to aarch32.h and let it return the JITLink-internal name for consistency.
Instead, feed getELFAArch32EdgeKindName() into the LinkGraph. For the moment the reverse-mapping getELFRelocationType() is only used in tests.

Thanks to everyone for their valuable input. I think this has turned out really well. It's in a good shape now to start iterating.

sgraenitz marked 2 inline comments as done.Mar 22 2023, 3:36 PM
sgraenitz edited the summary of this revision. (Show Details)Mar 23 2023, 3:09 AM

Proper GOT/PLT handling is not yet implemented.

Thank you! Just in time to avoid some confusion for future readers! :)

sgraenitz edited the summary of this revision. (Show Details)Mar 23 2023, 3:24 AM
This revision was landed with ongoing or failed builds.Mar 23 2023, 3:26 AM
This revision was automatically updated to reflect the committed changes.
gulfem added a subscriber: gulfem.Mar 23 2023, 12:05 PM

We started seeing a segmentation fault while running ExecutionEngine tests on Mac.

******************** TEST 'LLVM :: ExecutionEngine/MCJIT/2003-01-04-ArgumentBug.ll' FAILED ********************
Script:
--
: 'RUN: at line 1';   /opt/s/w/ir/x/w/staging/llvm_build/bin/lli -jit-kind=mcjit /opt/s/w/ir/x/w/llvm-llvm-project/llvm/test/ExecutionEngine/MCJIT/2003-01-04-ArgumentBug.ll > /dev/null
: 'RUN: at line 2';   /opt/s/w/ir/x/w/staging/llvm_build/bin/lli /opt/s/w/ir/x/w/llvm-llvm-project/llvm/test/ExecutionEngine/MCJIT/2003-01-04-ArgumentBug.ll > /dev/null
--
Exit Code: 139

Command Output (stderr):
--
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.	Program arguments: /opt/s/w/ir/x/w/staging/llvm_build/bin/lli /opt/s/w/ir/x/w/llvm-llvm-project/llvm/test/ExecutionEngine/MCJIT/2003-01-04-ArgumentBug.ll
 #0 0x000000010d026c4b llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/opt/s/w/ir/x/w/staging/llvm_build/bin/lli+0x100ebfc4b)
 #1 0x000000010d024ac9 llvm::sys::RunSignalHandlers() (/opt/s/w/ir/x/w/staging/llvm_build/bin/lli+0x100ebdac9)
 #2 0x000000010d0272de SignalHandler(int) (/opt/s/w/ir/x/w/staging/llvm_build/bin/lli+0x100ec02de)
 #3 0x00007fff20919d7d (/usr/lib/system/libsystem_platform.dylib+0x7fff20353d7d)
 #4 0x0000000000000000 
 #5 0x000000010c1a3344 llvm::orc::LLJIT::deinitialize(llvm::orc::JITDylib&) (/opt/s/w/ir/x/w/staging/llvm_build/bin/lli+0x10003c344)
 #6 0x000000010c1a0beb runOrcJIT(char const*) (/opt/s/w/ir/x/w/staging/llvm_build/bin/lli+0x100039beb)
 #7 0x000000010c19bb5e main (/opt/s/w/ir/x/w/staging/llvm_build/bin/lli+0x100034b5e)
 #8 0x00007fff208eff3d (/usr/lib/system/libdyld.dylib+0x7fff20329f3d)
 #9 0x0000000000000002 
/opt/s/w/ir/x/w/staging/llvm_build/test/ExecutionEngine/MCJIT/Output/2003-01-04-ArgumentBug.ll.script: line 2: 11096 Segmentation fault: 11  /opt/s/w/ir/x/w/staging/llvm_build/bin/lli /opt/s/w/ir/x/w/llvm-llvm-project/llvm/test/ExecutionEngine/MCJIT/2003-01-04-ArgumentBug.ll > /dev/null

https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-mac-x64/b8785839382041226465/overview

Interesting, thanks for the report. I will have a look tomorrow.

This was due to my change to use a target flag to represent the thumb bit. Obviously, these are target-specific and ObjectLinkingLayer has to account for that. Relanding now with the fix.

We are seeing a build error on Windows building with MSVC. Attempt to access a private typedef.

[3510/4703] Building CXX object lib\ExecutionEngine\JITLink\CMakeFiles\LLVMJITLink.dir\ELF_aarch32.cpp.obj
FAILED: lib/ExecutionEngine/JITLink/CMakeFiles/LLVMJITLink.dir/ELF_aarch32.cpp.obj
C:\PROGRA~2\MICROS~1\2019\BUILDT~1\VC\Tools\MSVC\1428~1.299\bin\HostX64\x64\cl.exe  /nologo /TP -DGTEST_HAS_RTTI=0 -DSCE_TARGET_EAP=1 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_GLIBCXX_ASSERTIONS -D_HAS_EXCEPTIONS=0 -D_LIBCPP_ENABLE_ASSERTIONS -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__SCE__ -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Ilib\ExecutionEngine\JITLink -IC:\j\w\779ddbee\p\llvm\lib\ExecutionEngine\JITLink -Iinclude -IC:\j\w\779ddbee\p\llvm\include -IC:\j\w\779ddbee\p\SIE\Telemetry\telemetry_utils\include -IC:\j\w\779ddbee\p\SIE\Telemetry\telemetry_api\include /DWIN32 /D_WINDOWS   /Zc:inline /Zc:preprocessor /Zc:__cplusplus /Oi /bigobj /W4 -wd4141 -wd4146 -wd4244 -wd4267 -wd4291 -wd4351 -wd4456 -wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4100 -wd4127 -wd4512 -wd4505 -wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4709 -wd5105 -wd4324 -w14062 -we4238 /Gw /MT /O2 /Ob2 /DCHECKING  /EHs-c- /GR- -UNDEBUG /Zc:__cplusplus -std:c++17 /showIncludes /Folib\ExecutionEngine\JITLink\CMakeFiles\LLVMJITLink.dir\ELF_aarch32.cpp.obj /Fdlib\ExecutionEngine\JITLink\CMakeFiles\LLVMJITLink.dir\LLVMJITLink.pdb /FS -c C:\j\w\779ddbee\p\llvm\lib\ExecutionEngine\JITLink\ELF_aarch32.cpp
C:\j\w\779ddbee\p\llvm\lib\ExecutionEngine\JITLink\ELF_aarch32.cpp(188): error C2248: 'llvm::jitlink::ELFLinkGraphBuilder<llvm::object::ELFType<llvm::support::little,false>>::ELFFile': cannot access private typedef declared in class 'llvm::jitlink::ELFLinkGraphBuilder<llvm::object::ELFType<llvm::support::little,false>>'
C:\j\w\779ddbee\p\llvm\lib\ExecutionEngine\JITLink\ELFLinkGraphBuilder.h(58): note: see declaration of 'llvm::jitlink::ELFLinkGraphBuilder<llvm::object::ELFType<llvm::support::little,false>>::ELFFile'
C:\j\w\779ddbee\p\llvm\lib\ExecutionEngine\JITLink\ELF_aarch32.cpp(103): note: see declaration of 'llvm::jitlink::ELFLinkGraphBuilder<llvm::object::ELFType<llvm::support::little,false>>'
C:\j\w\779ddbee\p\llvm\lib\ExecutionEngine\JITLink\ELF_aarch32.cpp(247): note: see reference to class template instantiation 'llvm::jitlink::ELFLinkGraphBuilder_aarch32<llvm::support::little>' being compiled

Thanks for your note. Apparently MSVC refers to use the typedef in the base class https://github.com/llvm/llvm-project/blob/release/16.x/llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h#L58 while it should use llvm::object::ELFFile. Interesting behavior to bail out in this case. Let me fix that quickly.

Thank you for the swift action. Unfortunately another MSVC-ism has reared its head.

FAILED: lib/ExecutionEngine/JITLink/CMakeFiles/LLVMJITLink.dir/aarch32.cpp.obj
C:\PROGRA~2\MICROS~1\2019\BUILDT~1\VC\Tools\MSVC\1428~1.299\bin\HostX64\x64\cl.exe  /nologo /TP -DGTEST_HAS_RTTI=0 -DSCE_TARGET_EAP=1 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_GLIBCXX_ASSERTIONS -D_HAS_EXCEPTIONS=0 -D_LIBCPP_ENABLE_ASSERTIONS -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__SCE__ -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Ilib\ExecutionEngine\JITLink -IC:\j\w\779ddbee\p\llvm\lib\ExecutionEngine\JITLink -Iinclude -IC:\j\w\779ddbee\p\llvm\include -IC:\j\w\779ddbee\p\SIE\Telemetry\telemetry_utils\include -IC:\j\w\779ddbee\p\SIE\Telemetry\telemetry_api\include /DWIN32 /D_WINDOWS   /Zc:inline /Zc:preprocessor /Zc:__cplusplus /Oi /bigobj /W4 -wd4141 -wd4146 -wd4244 -wd4267 -wd4291 -wd4351 -wd4456 -wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4100 -wd4127 -wd4512 -wd4505 -wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4709 -wd5105 -wd4324 -w14062 -we4238 /Gw /MT /O2 /Ob2 /DCHECKING  /EHs-c- /GR- -UNDEBUG /Zc:__cplusplus -std:c++17 /showIncludes /Folib\ExecutionEngine\JITLink\CMakeFiles\LLVMJITLink.dir\aarch32.cpp.obj /Fdlib\ExecutionEngine\JITLink\CMakeFiles\LLVMJITLink.dir\LLVMJITLink.pdb /FS -c C:\j\w\779ddbee\p\llvm\lib\ExecutionEngine\JITLink\aarch32.cpp
C:\j\w\779ddbee\p\llvm\lib\ExecutionEngine\JITLink\aarch32.cpp(160): error C2672: 'formatv': no matching overloaded function found
C:\j\w\779ddbee\p\llvm\lib\ExecutionEngine\JITLink\aarch32.cpp(161): error C2893: Failed to specialize function template 'llvm::formatv_object<unknown-type> llvm::formatv(const char *,Ts &&...)'
C:\j\w\779ddbee\p\llvm\include\llvm/Support/FormatVariadic.h(251): note: see declaration of 'llvm::formatv'
C:\j\w\779ddbee\p\llvm\lib\ExecutionEngine\JITLink\aarch32.cpp(161): note: With the following template arguments:
C:\j\w\779ddbee\p\llvm\lib\ExecutionEngine\JITLink\aarch32.cpp(161): note: 'Ts={const llvm::support::ulittle16_t &, const llvm::support::ulittle16_t &, const char *}'
C:\j\w\779ddbee\p\llvm\lib\ExecutionEngine\JITLink\aarch32.cpp(159): error C2672: 'llvm::make_error': no matching overloaded function found

This was due to my change to use a target flag to represent the thumb bit. Obviously, these are target-specific and ObjectLinkingLayer has to account for that. Relanding now with the fix.

Thanks, and the issue is fixed in your reland.

Thank you for the swift action. Unfortunately another MSVC-ism has reared its head.

Should be fixed with 4cb0b7ce3b4987446264312d582dac9c9a98a488

The latest commit 4cb0b7ce fixed the 'formatv' error, but now there is a different error:

With VS2019, Microsoft (R) C/C++ Optimizing Compiler Version 19.29.30148 for x64

FAILED: lib/ExecutionEngine/JITLink/CMakeFiles/LLVMJITLink.dir/aarch32.cpp.obj
C:\PROGRA~2\MICROS~1\2019\BUILDT~1\VC\Tools\MSVC\1428~1.299\bin\HostX64\x64\cl.exe /nologo /TP -DGTEST_HAS_RTTI=0 -DSCE_TARGET_EAP=1 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_GLIBCXX_ASSERTIONS -D_HAS_EXCEPTIONS=0 -D_LIBCPP_ENABLE_ASSERTIONS -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -DSCE -DSTDC_CONSTANT_MACROS -DSTDC_FORMAT_MACROS -DSTDC_LIMIT_MACROS -Ilib\ExecutionEngine\JITLink -IC:\j\w\779ddbee\p\llvm\lib\ExecutionEngine\JITLink -Iinclude -IC:\j\w\779ddbee\p\llvm\include ............ -wd4324 -w14062 -we4238 /Gw /MT /O2 /Ob2 /DCHECKING /EHs-c- /GR- -UNDEBUG /Zc:cplusplus -std:c++17 /showIncludes /Folib\ExecutionEngine\JITLink\CMakeFiles\LLVMJITLink.dir\aarch32.cpp.obj /Fdlib\ExecutionEngine\JITLink\CMakeFiles\LLVMJITLink.dir\LLVMJITLink.pdb /FS -c C:\j\w\779ddbee\p\llvm\lib\ExecutionEngine\JITLink\aarch32.cpp
C:\j\w\779ddbee\p\llvm\include\llvm/Support/FormatProviders.h(128): error C2872: 'detail': ambiguous symbol
C:\j\w\779ddbee\p\llvm\include\llvm/Support/Chrono.h(76): note: could be 'llvm::detail'
C:\j\w\779ddbee\p\llvm\include\llvm/Support/Endian.h(202): note: or 'llvm::support::detail'

This error is the key to these problem. There are two namespace named 'details', llvm::details and llvm::support::details. Adding 'using namespace support' introduces both at the same level, so details:: becomes ambiguous.

Perhaps moving 'using namespace support' to inside function readAddendData can do the trick. That is the only function that needs it.

The latest commit 4cb0b7ce fixed the 'formatv' error, but now there is a different error:

Is this a public facing build bot? If not I am afraid it will have to wait until next week. The public facing MSVC bots seem ok, e.g.: https://lab.llvm.org/buildbot/#/builders/123

Sunil_Srivastava added a comment.EditedMar 26 2023, 10:29 AM

Is this a public facing build bot?

No, but multiple machines in our organization, using both VS2019 and VS2022 are showing these errors. I am myself puzzled as to why no public bots are failing.

Moreover, after fixing this error (by moving 'using namespace support' inside readAddendData), we ran into another error in another file (same compiler, same options):

C:\j\w\p\cpu-toolchain-ppr\llvm\lib\ExecutionEngine\JITLink\ELF_aarch32.cpp(188): error C2248: 'llvm::jitlink::ELFLinkGraphBuilder<llvm::object::ELFType<llvm::support::little,false>>::ELFFile': cannot access private typedef declared in class 'llvm::jitlink::ELFLinkGraphBuilder<llvm::object::ELFType<llvm::support::little,false>>'
C:\j\w\p\cpu-toolchain-ppr\llvm\lib\ExecutionEngine\JITLink\ELFLinkGraphBuilder.h(58): note: see declaration of 'llvm::jitlink::ELFLinkGraphBuilder<llvm::object::ELFType<llvm::support::little,false>>::ELFFile'
C:\j\w\p\cpu-toolchain-ppr\llvm\lib\ExecutionEngine\JITLink\ELF_aarch32.cpp(103): note: see declaration of 'llvm::jitlink::ELFLinkGraphBuilder<llvm::object::ELFType<llvm::support::little,false>>'
C:\j\w\p\cpu-toolchain-ppr\llvm\lib\ExecutionEngine\JITLink\ELF_aarch32.cpp(247): note: see reference to class template instantiation 'llvm::jitlink::ELFLinkGraphBuilder_aarch32<llvm::support::little>' being compiled
C:\j\w\p\cpu-toolchain-ppr\llvm\lib\ExecutionEngine\JITLink\ELF_aarch32.cpp(188): error C2143: syntax error: missing ')' before '<'
C:\j\w\p\cpu-toolchain-ppr\llvm\lib\ExecutionEngine\JITLink\ELF_aarch32.cpp(188): error C2143: syntax error: missing ';' before '<'
C:\j\w\p\cpu-toolchain-ppr\llvm\lib\ExecutionEngine\JITLink\ELF_aarch32.cpp(188): error C2059: syntax error: '<'
C:\j\w\p\cpu-toolchain-ppr\llvm\lib\ExecutionEngine\JITLink\ELF_aarch32.cpp(188): error C2059: syntax error: ')'
C:\j\w\p\cpu-toolchain-ppr\llvm\lib\ExecutionEngine\JITLink\ELF_aarch32.cpp(188): error C2334: unexpected token(s) preceding ':'; skipping apparent function body
C:\j\w\p\cpu-toolchain-ppr\llvm\lib\ExecutionEngine\JITLink\ELF_aarch32.cpp(244): fatal error C1075: '{': no matching token found

I think the key here is the line (in ELFLinkGraphBuilder.h)

using ELFFile = object::ELFFile<ELFT>;

and then using ELFFIle as a template in a scope where this using statement is visible.

As before, I cannot explain why public bots are not showing this, but I can see it on more than one machines.

Is this a public facing build bot?

No, but multiple machines in our organization, using both VS2019 and VS2022 are showing these errors. I am myself puzzled as to why no public bots are failing.

Hi Sunil, I just ran a build with VS2022 (MSVC 19.33 on Windows 10) myself and it works without any issue. The clang-x64-windows-msvc bot I linked runs VS2019. It works as well and it did build the respective translation unit: https://lab.llvm.org/buildbot/api/v2/logs/26643253/raw

[969/2521] Building CXX object lib\Analysis\CMakeFiles\LLVMAnalysis.dir\ModuleSummaryAnalysis.cpp.obj
[970/2521] Building CXX object lib\ExecutionEngine\Interpreter\CMakeFiles\LLVMInterpreter.dir\Interpreter.cpp.obj
[971/2521] Building CXX object lib\ExecutionEngine\JITLink\CMakeFiles\LLVMJITLink.dir\aarch32.cpp.obj
[972/2521] Building CXX object lib\ExecutionEngine\CMakeFiles\LLVMExecutionEngine.dir\ExecutionEngine.cpp.obj

I think the key here is the line (in ELFLinkGraphBuilder.h)

using ELFFile = object::ELFFile<ELFT>;

and then using ELFFIle as a template in a scope where this using statement is visible.

As before, I cannot explain why public bots are not showing this, but I can see it on more than one machines.

Can you please make the necessary changes and set up a review for it? Without a public facing build bot or a way to reproduce your issue, all my attempts to fix it will be in the dark. Thanks!