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.

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

c-writer.cc: Correctly handle label names when branching out of try block #2208

Merged
merged 1 commit into from
May 8, 2023

Conversation

keithw
Copy link
Member

@keithw keithw commented Apr 21, 2023

Sequenced behind #2207. Fixes #2205. wasm2c was making a LabelDecl with the raw Wasm name instead of the safe/unique C name. Because the label's C name is generated in BeginTry but the LabelDecl doesn't come until later, it's probably easiest just to re-retrieve the C name at that point. Or we could have BeginTry actually return the generated C name (along with the mark)...

The exception-handling proposal could benefit from better test coverage here (WebAssembly/exception-handling#210) but this is definitely our bug!

@keithw keithw requested a review from sbc100 April 21, 2023 02:32
Base automatically changed from w2c-cleanup-name-lookup to main April 27, 2023 18:01
@keithw keithw requested a review from shravanrn April 27, 2023 18:02
@keithw keithw force-pushed the w2c-exception-bug branch 3 times, most recently from ec58755 to 7c2a457 Compare May 5, 2023 00:04
@shravanrn
Copy link
Collaborator

Hmm... I'm afraid I don't understand this well enough to help review this, so I'll leave it up to @sbc100
Ping me, if you want me to dig in and figure it out

@keithw
Copy link
Member Author

keithw commented May 8, 2023

@sbc100 are you able to give this a review?

Copy link
Member

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

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

Rubberstamp lgtm. I'm afraid I don't fulling understand this change either, but if this test passed after and fails before this sgtm.

@keithw keithw enabled auto-merge (squash) May 8, 2023 20:28
@keithw
Copy link
Member Author

keithw commented May 8, 2023

Thank you!

@keithw keithw merged commit e04107f into main May 8, 2023
@keithw keithw deleted the w2c-exception-bug branch May 8, 2023 20:49
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.

wasm2c exception handling is more broken than we thought
3 participants