Skip to content

Fix SafeHeap crash when start function calls an imported function#8306

Open
sumleo wants to merge 1 commit intoWebAssembly:mainfrom
sumleo:fix-safeheap-import-crash
Open

Fix SafeHeap crash when start function calls an imported function#8306
sumleo wants to merge 1 commit intoWebAssembly:mainfrom
sumleo:fix-safeheap-import-crash

Conversation

@sumleo
Copy link
Contributor

@sumleo sumleo commented Feb 12, 2026

Summary

findCalledFunctions in SafeHeap transitively walks all functions called from the start function to determine which functions should not be instrumented. When the start function calls an imported function, the import was added to the worklist and then FindAll<Call> was called on its null body, causing an assertion failure in the walker:

Assertion failed: (*currp), function pushTask, file wasm-traversal.h, line 286.

Reproducer:

(module
  (import "env" "some_import" (func $import))
  (import "env" "emscripten_get_sbrk_ptr" (func $emscripten_get_sbrk_ptr (result i32)))
  (memory 1 1 shared)
  (func $start (call $import))
  (start $start)
)
wasm-opt test.wat --safe-heap --enable-threads --enable-simd -S -o -

Fix

Skip imported functions in the traversal since they have no body to walk.

Test plan

  • Added test/lit/passes/safe-heap-start-import.wast with a start function that calls an import
  • Verified the crash reproduces without the fix and passes with it
  • All lit tests pass

findCalledFunctions transitively walks all functions called from the
start function to determine which functions should not be instrumented.
When the start function calls an imported function, the import was added
to the worklist and then FindAll<Call> was called on its null body,
causing an assertion failure in the walker (assert(*currp) in
wasm-traversal.h).

Skip imported functions in the traversal since they have no body to
walk.
Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

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

Thanks, looks good aside from some test comments.

@@ -0,0 +1,28 @@
;; NOTE: Assertions have been generated by update_lit_checks.py and should not be edited.
;; RUN: wasm-opt %s --safe-heap --enable-threads --enable-simd -S -o - | filecheck %s
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
;; RUN: wasm-opt %s --safe-heap --enable-threads --enable-simd -S -o - | filecheck %s
;; RUN: wasm-opt %s --safe-heap -S -o - | filecheck %s

These features are not used in the testcase.

(import "env" "some_import" (func $import))
;; CHECK: (import "env" "emscripten_get_sbrk_ptr" (func $emscripten_get_sbrk_ptr (result i32)))
(import "env" "emscripten_get_sbrk_ptr" (func $emscripten_get_sbrk_ptr (result i32)))
(memory 1 1 shared)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
(memory 1 1 shared)
(memory 1 1)

No need for this to be shared.

;; CHECK: (import "env" "some_import" (func $import))
(import "env" "some_import" (func $import))
;; CHECK: (import "env" "emscripten_get_sbrk_ptr" (func $emscripten_get_sbrk_ptr (result i32)))
(import "env" "emscripten_get_sbrk_ptr" (func $emscripten_get_sbrk_ptr (result i32)))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
(import "env" "emscripten_get_sbrk_ptr" (func $emscripten_get_sbrk_ptr (result i32)))

No need for this import.

(call $import)
)

(start $start)
Copy link
Member

Choose a reason for hiding this comment

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

By convention we put this before the first function (around line 16, here)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants