[1.0.3] fix building with LLVM 7, 8, and 9 #903
Conversation
| #else | ||
| using de_offset_t = uint64_t; | ||
| #endif | ||
| llvm::DataExtractor ds(llvm::StringRef(reinterpret_cast<const char*>(stacksizes.data()), stacksizes.size()), true, 8); |
There was a problem hiding this comment.
okay okay, I know this looks all sorts of ick, but take a look at,
https://github.com/llvm/llvm-project/blob/release/11.x/llvm/include/llvm/Support/DataExtractor.h#L85
The ctor that takes an ArrayRef does the exact same cast + StringRef as I'm doing here. So this is not really even a change for existing working versions.
| de_offset_t offset = 0; | ||
|
|
||
| while(ds.isValidOffsetForAddress(offset)) { | ||
| ds.getAddress(&offset); |
There was a problem hiding this comment.
isValidOffsetForAddress() guarantees there is enough space in the buffer for getAddress() to succeed.
| ds.getAddress(&offset); | ||
| const de_offset_t offset_before_read = offset; | ||
| const uint64_t stack_size = ds.getULEB128(&offset); | ||
| WAVM_ASSERT_THROW(offset_before_read != offset); |
There was a problem hiding this comment.
This older interface of getULEB128() documents that if it fails to read the variable sized int, offset is not incremented. The Cursor/Error interface is a lot better, but our use case here is simple enough this still seems clear enough.
| using de_offset_t = uint32_t; | ||
| #else | ||
| using de_offset_t = uint64_t; | ||
| #endif |
There was a problem hiding this comment.
I don't see any sort of exposed typedef/using to make this cleaner.
|
Note:start |
Spring claims that it builds with LLVM versions 7 through 11. Code says it,
spring/CMakeLists.txt
Lines 68 to 70 in 1606ade
README says it,
spring/README.md
Line 55 in 1606ade
Unfortunately building with versions 7-9 was broken in leap 5.0. This change restores building with LLVM 7 through 11 by using an older interface that's common across all supported LLVM versions.
An example exhaustive workflow run with all versions 7-11 enabled:
https://github.com/AntelopeIO/spring/actions/runs/11223407160
(this is off a demonstration only branch that cherry-picks the workflow back to 1.0, as otherwise it only exists on main. plus also then adding #902 on top)