llvm::byteswap ignores alignment requirements

[rorth]

The MachOTest.UnalignedLC test (LLVM-Unit :: BinaryFormat/./BinaryFormatTests/11/193) has been failing on and off in the past on Solaris/sparcv9. However, since LLVM 18 the failure has become consistent, as I've just re-discovered:

• In a 1-stage build with clang-17 as build compiler, the test PASSes.
• However, when switching the build compiler to clang-18, the test consistently FAILs as I found when trying that clang-17 to clang-18 switch on the Solaris/sparcv9 buildbot.
• In a 2-stage build, the failure is consistent irrespective of build compiler.

The failure is like this:

GTEST_OUTPUT=json:/var/llvm/local-sparcv9-release-stage2-A-flang-clang18/tools/clang/stage2-bins/unittests/BinaryFormat/./BinaryFormatTests-LLVM-Unit-25980-11-193.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=193 GTEST_SHARD_INDEX=11 /var/llvm/local-sparcv9-release-stage2-A-flang-clang18/tools/clang/stage2-bins/unittests/BinaryFormat/./BinaryFormatTests
--

Note: This is test shard 12 of 193.
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from MachOTest
[ RUN      ] MachOTest.UnalignedLC
Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it):
0  BinaryFormatTests 0x0000000100180b4c llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) + 36
1  BinaryFormatTests 0x00000001001814f4 SignalHandler(int) + 896
2  libc.so.1         0x00007fffff0c6410 __sighndlr + 12
3  libc.so.1         0x00007fffff0b8cb8 call_user_handler + 1024
4  libc.so.1         0x00007fffff0b9078 sigacthandler + 160
5  BinaryFormatTests 0x00000001000d1e00 MachOTest_UnalignedLC_Test::TestBody() + 48
6  BinaryFormatTests 0x00000001002383f8 testing::Test::Run() + 432
7  BinaryFormatTests 0x0000000100239ba4 testing::TestInfo::Run() + 484
8  BinaryFormatTests 0x000000010023abe4 testing::TestSuite::Run() + 1244
9  BinaryFormatTests 0x000000010024c778 testing::internal::UnitTestImpl::RunAllTests() + 2788
10 BinaryFormatTests 0x000000010024bb0c testing::UnitTest::Run() + 128
11 BinaryFormatTests 0x00000001002196dc main + 168
12 BinaryFormatTests 0x00000001000c82a4 _start + 100

--
exit: -10

With more debug info from a past build, there's

FAIL: LLVM-Unit :: BinaryFormat/./BinaryFormatTests/MachOTest.UnalignedLC (1755 of 68187)
******************** TEST 'LLVM-Unit :: BinaryFormat/./BinaryFormatTests/MachOTest.UnalignedLC' FAILED ********************
Note: Google Test filter = MachOTest.UnalignedLC
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from MachOTest
[ RUN      ] MachOTest.UnalignedLC
0  BinaryFormatTests 0x0000000100269930 llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 44
1  BinaryFormatTests 0x0000000100269d14 PrintStackTraceSignalHandler(void*) + 12
2  BinaryFormatTests 0x0000000100268bb4 llvm::sys::RunSignalHandlers() + 132
3  BinaryFormatTests 0x000000010026aa80 SignalHandler(int) + 216
4  libc.so.1         0xffffffff7eedc840 __sighndlr + 12
5  libc.so.1         0xffffffff7eecf27c call_user_handler + 852
6  libc.so.1         0xffffffff7eecf5d4 sigacthandler + 84
7  BinaryFormatTests 0x00000001001e228c void llvm::sys::swapByteOrder<unsigned int>(unsigned int&) + 8
8  BinaryFormatTests 0x00000001001dfdc4 llvm::MachO::swapStruct(llvm::MachO::mach_header&) + 4
9  BinaryFormatTests 0x00000001001dfc04 MachOTest_UnalignedLC_Test::TestBody() + 48
10 BinaryFormatTests 0x00000001002eeeb8 void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) + 40
11 BinaryFormatTests 0x00000001002d8974 void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) + 44
12 BinaryFormatTests 0x00000001002d8904 testing::Test::Run() + 164
13 BinaryFormatTests 0x00000001002d9204 testing::TestInfo::Run() + 212
14 BinaryFormatTests 0x00000001002d9a00 testing::TestCase::Run() + 216
15 BinaryFormatTests 0x00000001002de948 testing::internal::UnitTestImpl::RunAllTests() + 684
16 BinaryFormatTests 0x00000001002f0c64 bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) + 40
17 BinaryFormatTests 0x00000001002de654 bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) + 44
18 BinaryFormatTests 0x00000001002de58c testing::UnitTest::Run() + 148
19 BinaryFormatTests 0x00000001002d0ac4 RUN_ALL_TESTS() + 12
20 BinaryFormatTests 0x00000001002d0aa8 main + 136
21 BinaryFormatTests 0x00000001001d5fa4 _start + 100

The program dies with SIGBUS like so:

    Incurred fault #5, FLTACCESS  %pc = 0x1001E228C
      siginfo: SIGBUS BUS_ADRALN addr=0xFFFFFFFF7FFFE263
    Received signal #10, SIGBUS [default]
      siginfo: SIGBUS BUS_ADRALN addr=0xFFFFFFFF7FFFE263

i.e. the input operand isn't properly aligned:

Thread 2 received signal SIGBUS, Bus error.
[Switching to Thread 1 (LWP 1)]
0x00000001001e228c in void llvm::sys::swapByteOrder<unsigned int>(unsigned int&) ()
1: x/i $pc
=> 0x1001e228c <_ZN4llvm3sys13swapByteOrderIjEEvRT_+8>:	ld  [ %i0 ], %o0
(gdb) p/x $i0
$1 = 0xffffffff7fffe123
(gdb) where
#0  0x00000001001e228c in void llvm::sys::swapByteOrder<unsigned int>(unsigned int&) ()
#1  0x00000001001dfdcc in llvm::MachO::swapStruct(llvm::MachO::mach_header&) ()
#2  0x00000001001dfc0c in MachOTest_UnalignedLC_Test::TestBody() ()

SPARC being a strict-alignment target, natural alignment is required for all operands, so e.g. an unsigned int must be at least 2-byte aligned, which isn't the case here.

I think I ultimately traced this to llvm::byteswap being ignorant of this requirement. Considering the call chain

  llvm/unittests/BinaryFormat/MachOTest.cpp (TEST(MachOTest, UnalignedLC))
  -> llvm/include/llvm/BinaryFormat/MachO.h (swapStruct)
  -> llvm/include/llvm/Support/SwapByteOrder.h (sys::swapByteOrder)
  -> llvm/include/llvm/Support/SwapByteOrder.h (sys::getSwappedBytes)
  -> llvm/include/llvm/ADT/bit.h (llvm::byteswap)

llvm::byteswap does nothing to deal with unaligned input.

While one could force the alignment in the testcase, its name (UnalignedLC) strongly suggests that it's exactly about the unaligned case, so that seems wrong. Besides, llvm::byteswap needs to be robust irrespective of input IMO.

[rorth]

FWIW, this is obviously not a Solaris issue: I see exactly the same failure in the LLVM 18 release builds on Debian/sparc64.

[rorth]

The analysis was only partially correct: llvm::byteswap isn't involved (or even reached) since it gets its args by value.

However, MachOTest.UnalignedLC (or rather swapStruct) passes intentionally misaligned pointers/references, thus lying to sys::swapByteOrder about it's args. E.g. when sys::swapByteOrder is called with an unsigned int& like for mh.magic, it assumes (and IMO can do so) that the pointer is properly aigned for the type before dereferencing it. On strict-alignement targets like SPARC, this cannot work and leads to the observed SIGBUS.

I think the test is at fault and needs to account for that possibility.

Unfortunately, the test author doesn't seem to have a github account at all, and there's no tag for Mach-O issues or the llvm testsuite.

[efriedma-quic]

The original test author is no longer working on LLVM.

I think the test might just be bogus: I don't think the actual MachO parser/writer code ever tries to use swapStruct like this. As you note, even if swapStruct itself somehow worked, constructing the reference would be undefined behavior.

The ubsan bot would probably pick this up... except I think it runs on a little-endian host, and this particular test only does the swapping on big-endian hosts.

[rorth]

The ubsan bot would probably pick this up... except I think it runs on a little-endian host, and this particular test only does the swapping on big-endian hosts.

FWIW, I've compiled MachOTest.cpp with -fsanitize=undefined on a big-endian host, but that came up blank.