Skip to content

Conversation

@shravanrn
Copy link
Collaborator

No description provided.

@shravanrn shravanrn requested review from keithw and sbc100 October 16, 2024 02:35
@shravanrn shravanrn changed the title wasm2c: Reset the segment register after call_indirect as other code may change the value of the segment wasm2c: Reset the segment register after call_indirect as the register may have changed Oct 16, 2024
@shravanrn shravanrn force-pushed the segue-call-indirect branch from ab93288 to 4fa4821 Compare October 16, 2024 02:39
@shravanrn
Copy link
Collaborator Author

@sbc100 Please have a look when you have a chance

@shravanrn shravanrn enabled auto-merge (rebase) October 18, 2024 18:59
@shravanrn shravanrn force-pushed the segue-call-indirect branch from 4fa4821 to 9fca2fe Compare October 18, 2024 18:59
@keithw
Copy link
Member

keithw commented Oct 18, 2024

Would it be possible to share more context on when/why this is necessary? E.g., why reset it after a call_indirect but not an ordinary call?

@shravanrn
Copy link
Collaborator Author

Would it be possible to share more context on when/why this is necessary? E.g., why reset it after a call_indirect but not an ordinary call?

Sure thing. The call indirect has the potential to invoke arbitrary host functions (because function table could be imported and thus have entries set by the host), while a direct call is mostly reserved for other wasm2c generated code. For the call indirect, the code we have today works fine if the invoked host functions either (1) don't modify %gs or (2) modify and restore %gs. But if the invoked host function clobbers %gs, this poses an issue. There are potential edge cases where this could happen --- for instance in some debug/sanitizer flags are used. Resetting the values after a call indirect is the straightforward fix.

@shravanrn shravanrn merged commit d6e97af into WebAssembly:main Oct 18, 2024
@shravanrn shravanrn deleted the segue-call-indirect branch October 18, 2024 22:19
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.

3 participants