Skip to content

[CustomDescriptors] Ensure a non-nullable type for ref.cast_desc_eq on null - #8975

Open
kripken wants to merge 2 commits into
WebAssembly:mainfrom
kripken:ref.br_on
Open

[CustomDescriptors] Ensure a non-nullable type for ref.cast_desc_eq on null#8975
kripken wants to merge 2 commits into
WebAssembly:mainfrom
kripken:ref.br_on

Conversation

@kripken

@kripken kripken commented Aug 6, 2026

Copy link
Copy Markdown
Member

If the desc is null, the cast is not even executed, making it unreachable.
We typed it as nullable if the ref was (in the code path type = ref->type;),
but emiting an uninhabitable type is more precise, and also avoids a fuzz
bug (see new test) where refinalize actually un-refined a type: it turned it
from non-nullable to nullable.

@kripken
kripken requested a review from tlively August 6, 2026 18:01
@kripken
kripken requested a review from a team as a code owner August 6, 2026 18:01

@tlively tlively left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should update the title to say br_on_cast_desc_eq rather than ref.cast_desc_eq.

Comment on lines +9 to +15
(type $0 (sub (descriptor $2) (struct)))
;; CHECK: (type $1 (sub $0 (descriptor $3) (struct)))
(type $1 (sub $0 (descriptor $3) (struct)))
;; CHECK: (type $2 (sub (describes $0) (struct)))
(type $2 (sub (describes $0) (struct)))
;; CHECK: (type $3 (sub $2 (describes $1) (struct (field i32))))
(type $3 (sub $2 (describes $1) (struct (field i32))))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we use any in the test instead of defining any types?

Comment on lines +64 to +69
(local.set $2
(br_on_cast_desc_eq $block (ref null $1) (ref null $1)
(ref.null $1)
(local.get $1)
)
)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The test says we can refine local $2 and therefore the br_on_cast_desc_eq must be refinalized to the correct type. But isn't that backward and self-justifying? Without the change to finalization, we should not be trying to refine local $2 in the first place. If this refinement is a new optimization, that's one thing, but I don't see how the change can be a correct bug fix.

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