Skip to content

runtime, WebAssembly: add ImageInspectionWasm.cpp #39483

New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom
Merged
merged 2 commits into from
Oct 12, 2021
Merged

runtime, WebAssembly: add ImageInspectionWasm.cpp #39483

merged 2 commits into from
Oct 12, 2021

Conversation

MaxDesiatov
Copy link
Contributor

This change adds a stub for swift::lookupSymbol for the WebAssembly platform.

Related to SR-9307.

@MaxDesiatov
runtime, WebAssembly: add ImageInspectionWasm.cpp
a221ab1 
This change adds a stub for `swift::lookupSymbol` for the WebAssembly platform.

<!-- If this pull request resolves any bugs in the Swift bug tracker, provide a link: -->
Related to SR-9307.
@MaxDesiatov
Copy link
Contributor Author

@swift-ci please smoke test

@MaxDesiatov MaxDesiatov mentioned this pull request Sep 28, 2021
Closed
78 tasks
@MaxDesiatov
Copy link
Contributor Author

CC @kateinoigakukun

@MaxDesiatov MaxDesiatov force-pushed the maxd/wasm-image-inspection branch from 156a8bf to 9e6e39b Compare September 28, 2021 15:19
@MaxDesiatov
Copy link
Contributor Author

@swift-ci please smoke test

compnerd
compnerd reviewed Sep 28, 2021
View reviewed changes
Copy link
Member

@compnerd compnerd left a comment

Choose a reason for hiding this comment

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

Is there something that prevents symbol lookup on WASM? Why a stub rather than an implementation?

@kateinoigakukun
Copy link
Member

kateinoigakukun commented Sep 29, 2021 •
edited
Loading

@compnerd Currently, Wasm doesn't have a standard stable ABI for exporting address <-> symbol table for now.
It's currently working in progress. And also, there is no API to access such information from Wasm binary side. It's accessible only from host VM. https://github.com/WebAssembly/tool-conventions/blob/main/DynamicLinking.md
So it's reasonable for me to use a stub for now.

compnerd
compnerd requested changes Sep 29, 2021
View reviewed changes
Copy link
Member

@compnerd compnerd left a comment

Choose a reason for hiding this comment

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

Please add the context that @kateinoigakukun mentioned into the file as a comment.

@MaxDesiatov MaxDesiatov force-pushed the maxd/wasm-image-inspection branch from 296ec44 to 1ce6678 Compare September 29, 2021 15:35
@MaxDesiatov
Copy link
Contributor Author

@swift-ci please smoke test

kateinoigakukun
kateinoigakukun approved these changes Oct 8, 2021
View reviewed changes
Copy link
Member

@kateinoigakukun kateinoigakukun left a comment

Choose a reason for hiding this comment

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

There is no change request from my side.

@MaxDesiatov
Copy link
Contributor Author

@compnerd would you have a moment for re-review? Thanks

compnerd
compnerd approved these changes Oct 12, 2021
View reviewed changes
Copy link
Member

@compnerd compnerd left a comment

Choose a reason for hiding this comment

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

It's not really using a stub - its adding a placeholder for an implementation. This means that any fatalError is going to be useless under any configuration or build.

@MaxDesiatov MaxDesiatov merged commit a94e961 into main Oct 12, 2021
@MaxDesiatov MaxDesiatov deleted the maxd/wasm-image-inspection branch October 12, 2021 13:04
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@compnerd compnerd compnerd approved these changes

@kateinoigakukun kateinoigakukun kateinoigakukun approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@MaxDesiatov @kateinoigakukun @compnerd