Add import / export sections test#6497
Add import / export sections test#6497oleks-rip merged 1 commit intoXRPLF:ripple/wasmi-host-functionsfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## ripple/wasmi-host-functions #6497 +/- ##
===========================================================
Coverage 80.3% 80.3%
===========================================================
Files 876 876
Lines 69908 69908
Branches 7590 7589 -1
===========================================================
+ Hits 56167 56168 +1
+ Misses 13741 13740 -1 see 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
oleks-rip
force-pushed
the
imp_exp_tst
branch
2 times, most recently
from
facafb8 to
44129ac
Compare
|
|
||
| do | ||
| { | ||
| byte = (*it++) & 0x7F; |
There was a problem hiding this comment.
Claude flagged a bug, seems reasonable to me. Please take a look. @oleks-rip
Decodes a ULEB128 value from an iterator. This has a bug:
byte = (*it++) & 0x7F; // masks off the high bit → byte is now 0–127
byte <<= shift;
val |= byte;
shift += 7;
++count;
} while (byte >= 0x80); // ← always false!
After & 0x7F, byte is at most 127. Then byte <<= shift operates on a uint64_t so it can grow, but on the first iteration shift is 0, so byte remains 0–127, which is
always < 0x80. The loop always exits after one iteration, meaning it only ever reads a single byte.
This is masked in the test because the import/export sections are < 128 bytes (53 and 51), so their sizes happen to fit in a single ULEB128 byte. But for any section >=
128 bytes, this would return the wrong size.
Fix: save the raw byte before masking to check the continuation bit:
std::uint8_t raw;
do {
raw = *it++;
val |= (std::uint64_t(raw & 0x7F)) << shift;
shift += 7;
++count;
} while (raw & 0x80);
mvadari
left a comment
mvadari
left a comment
There was a problem hiding this comment.
Why are so many lines deleted in fixtures.cpp when there are no removals in fixtures.h?
They are not deleted, they are formatted with new clang-format settings |
Can that be done separately, given that this PR's parent branch isn't failing the clang-format check? |
removed |
pwang200
left a comment
pwang200
left a comment
There was a problem hiding this comment.
I only check the bug in my previous comment, and assumed other code is not changed except formatting. Force push make it hard to see what else is changed. Please let me know if you changed anything other than formatting.
removed formatting and rebases with latest changes from wasmi-host-functions |
oleks-rip
merged commit 27468dd
into
XRPLF:ripple/wasmi-host-functions
3 participants
High Level Overview of Change
Change in cycle every byte of import / export section and try to load module test
Type of Change
.gitignore, formatting, dropping support for older tooling)