Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src/leb128.cc: fix detection of too-big u64 LEB128s #2256

Merged
merged 3 commits into from
Jun 9, 2023
Merged

src/leb128.cc: fix detection of too-big u64 LEB128s #2256

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8ffd61f
src/leb128.cc: fix detection of too-big u64 LEB128s
keithw Jun 9, 2023
c131375
Add comment to regression test
keithw Jun 9, 2023
f93bdb8
Merge branch 'main' into fix-leb128-pr
keithw Jun 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/leb128.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,8 +239,8 @@ size_t ReadU64Leb128(const uint8_t* p,
*out_value = LEB128_9(uint64_t);
return 9;
} else if (p + 9 < end && (p[9] & 0x80) == 0) {
// The top bits set represent values > 32 bits.
if (p[9] & 0xf0) {
// The top bits set represent values > 64 bits.
if (p[9] & 0xfe) {
return 0;
}
*out_value = LEB128_10(uint64_t);
Expand Down
29 changes: 29 additions & 0 deletions test/regress/bad-u64-leb128.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
;;; TOOL: run-interp-spec
;;; ARGS: --enable-memory64
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we write this test without depending on --enable-memory64.

How about just a simple i64.const instruction?

Also, I guess this points to gap in the spec tests?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we write this test without depending on --enable-memory64

Unfortunately not, because the bug was in ReadU64Leb128. :-( Those are only used by memory64 (for memory min, memory max, and load/store offset -- but the first two are in units of pages, so limited to 2^48 anyway). The const literals are using signed Leb128s, e.g. ReadS64Leb128.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me try to submit this to the memory64 proposal repo...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK.. maybe add a comment to that effect.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Submitted PR here: WebAssembly/memory64#38

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK.. maybe add a comment to that effect.

Done.

;; u64 LEB128s are only used by memory64, for memory limits and
;; for the offset in a load/store. Limits are in units of pages, so
;; validation limits them to 2^48 for i64 memories. The offset can
;; reach UINT64_MAX but can't go over (tested here).
(assert_malformed
(module binary
"\00asm" "\01\00\00\00"
"\01\04\01\60\00\00" ;; Type section
"\03\02\01\00" ;; Function section
"\05\03\01\04\00" ;; Memory section (flags: i64)
"\0a\13\01" ;; Code section
;; function 0
"\11\00" ;; local type count
"\42\00" ;; i64.const 0
"\28" ;; i32.load
"\02" ;; alignment 2
"\ff\ff\ff\ff\ff\ff\ff\ff\ff\02" ;; offset 2^64 (one unused bit set)
"\1a" ;; drop
"\0b" ;; end
)
"malformed u64 LEB128"
)
(;; STDOUT ;;;
out/test/regress/bad-u64-leb128.txt:8: assert_malformed passed:
0000020: error: unable to read u64 leb128: load offset
1/1 tests passed.
;;; STDOUT ;;)