Skip to content

Commit f8ca9e5

Browse files
[llvm][llvm-objdump] Fix fatbin handling on 32-bit systems (#141620)
Which fixes a test failure seen on the bots, introduced by #140286. ``` [ RUN ] OffloadingBundleTest.checkExtractOffloadBundleFatBinary ObjectTests: ../llvm/llvm/include/llvm/ADT/StringRef.h:618: StringRef llvm::StringRef::drop_front(size_t) const: Assertion `size() >= N && "Dropping more elements than exist"' failed. 0 0x0a24a990 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/home/tcwg-buildbot/worker/clang-armv8-quick/stage1/unittests/Object/./ObjectTests+0x31a990) 1 0x0a248364 llvm::sys::RunSignalHandlers() (/home/tcwg-buildbot/worker/clang-armv8-quick/stage1/unittests/Object/./ObjectTests+0x318364) 2 0x0a24b410 SignalHandler(int, siginfo_t*, void*) Signals.cpp:0:0 3 0xf46ed6f0 __default_rt_sa_restorer ./signal/../sysdeps/unix/sysv/linux/arm/sigrestorer.S:80:0 4 0xf46ddb06 ./csu/../sysdeps/unix/sysv/linux/arm/libc-do-syscall.S:47:0 5 0xf471d292 __pthread_kill_implementation ./nptl/pthread_kill.c:44:76 6 0xf46ec840 gsignal ./signal/../sysdeps/posix/raise.c:27:6 ``` Also reported on 32-bit x86. I think the cause is the code was casting the result of StringRef.find into an int64_t. The failure value of find is StringRef::npos, which is defined as: static constexpr size_t npos = ~size_t(0); * size_t(0) is 32 bits of 0s * the inverse of that is 32 bits of 1s * Cast to int64_t needs to widen this, and it will preserve the original value in doing so, which is 0xffffffff. * The result is 0x00000000ffffffff, which is >= 0, so we keep searching and try to go off the end of the file. Or put another way, this equivalent function returns true when compiled for a 32-bit system: ``` bool fn() { size_t foo = ~size_t(0); int64_t foo64 = (int64_t)foo; return foo64 >= 0; } ``` Using size_t throughout fixes the problem. Also I don't see a reason it needs to be a signed number, given that it always searches forward from the current offset.
1 parent a69487d commit f8ca9e5

File tree

1 file changed

+5
-7
lines changed

1 file changed

+5
-7
lines changed

llvm/lib/Object/OffloadBundle.cpp

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,11 @@ Error extractOffloadBundle(MemoryBufferRef Contents, uint64_t SectionOffset,
3838
StringRef FileName,
3939
SmallVectorImpl<OffloadBundleFatBin> &Bundles) {
4040

41-
uint64_t Offset = 0;
42-
int64_t NextbundleStart = 0;
41+
size_t Offset = 0;
42+
size_t NextbundleStart = 0;
4343

4444
// There could be multiple offloading bundles stored at this section.
45-
while (NextbundleStart >= 0) {
46-
45+
while (NextbundleStart != StringRef::npos) {
4746
std::unique_ptr<MemoryBuffer> Buffer =
4847
MemoryBuffer::getMemBuffer(Contents.getBuffer().drop_front(Offset), "",
4948
/*RequiresNullTerminator=*/false);
@@ -60,10 +59,9 @@ Error extractOffloadBundle(MemoryBufferRef Contents, uint64_t SectionOffset,
6059

6160
// Find the next bundle by searching for the magic string
6261
StringRef Str = Buffer->getBuffer();
63-
NextbundleStart =
64-
(int64_t)Str.find(StringRef("__CLANG_OFFLOAD_BUNDLE__"), 24);
62+
NextbundleStart = Str.find(StringRef("__CLANG_OFFLOAD_BUNDLE__"), 24);
6563

66-
if (NextbundleStart >= 0)
64+
if (NextbundleStart != StringRef::npos)
6765
Offset += NextbundleStart;
6866
}
6967

0 commit comments

Comments
 (0)