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

compiler: add compiler-rt and wasm symbols to table #3844

Merged
merged 1 commit into from
Jul 31, 2023

Conversation

charlie-haley
Copy link
Contributor

This PR adds compiler-rt and updates the makeArchive function to read symbols from wasm binaries and add them to the symbol table.

Fixes #3501

@deadprogram deadprogram requested a review from aykevl July 27, 2023 17:26
Copy link
Member

@aykevl aykevl left a comment

Choose a reason for hiding this comment

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

Looks good to me. Normally I'd like to see a test for changes like these, but I'm not quite sure how to test this reliably.

@@ -12,6 +12,7 @@ import (
"path/filepath"
"time"

wasm "github.com/aykevl/go-wasm"
Copy link
Member

Choose a reason for hiding this comment

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

Note that the package is already named "wasm", so this would be fine:

Suggested change
wasm "github.com/aykevl/go-wasm"
"github.com/aykevl/go-wasm"

(Many people think the directory name is used, but that's not true: the name from package wasm is used).

@aykevl aykevl merged commit 5b581d8 into tinygo-org:dev Jul 31, 2023
13 checks passed
@@ -7,6 +7,7 @@
"goarch": "arm",
"linker": "wasm-ld",
"libc": "wasi-libc",
"rtlib": "compiler-rt",
Copy link
Member

Choose a reason for hiding this comment

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

Actually now I realize this probably needs to be added to targets/wasm.json too. Feel free to make a new PR if you'd like.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

New PR for wasm.json #3848

@aykevl
Copy link
Member

aykevl commented Mar 12, 2024

@charlie-haley FYI: #4188

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