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

arm: Explicitly disable unwind tables #2482

Merged
merged 1 commit into from
Jan 14, 2022

Conversation

QuLogic
Copy link
Contributor

@QuLogic QuLogic commented Jan 4, 2022

Some clang builds (e.g., Fedora's) enable unwind tables by default. As tinygo does not need nor support them, that leads to undefined symbols when linking.

Fixes #2477

@deadprogram
Copy link
Member

@QuLogic just wanted to point out #2331 which I think will need unwind tables if using C++ exceptions if I am not mistaken.

@QuLogic
Copy link
Contributor Author

QuLogic commented Jan 11, 2022

That's probably okay. This PR currently only minimally affects arm builds where that option would have been disabled by default on Debian, and that PR would have had to explicitly enable it to use (but it doesn't, so I'll assume that arm is not yet a supported platform or it doesn't need unwind tables.)

@QuLogic
Copy link
Contributor Author

QuLogic commented Jan 11, 2022

I don't know why LLVM needs to rebuild, and times out, for this PR but not others.

@deadprogram
Copy link
Member

@QuLogic you should rebase against dev as the macOS build is no longer being performed by CircleCI.

Some clang builds (e.g., Fedora's) enable unwind tables by default. As
tinygo does not need nor support them, that leads to undefined symbols
when linking.
@QuLogic
Copy link
Contributor Author

QuLogic commented Jan 13, 2022

Oops, thought I did, but it must have been before pulling.

@deadprogram
Copy link
Member

Thanks @QuLogic now merging.

@deadprogram deadprogram merged commit b2ef729 into tinygo-org:dev Jan 14, 2022
@QuLogic QuLogic deleted the arm-no-unwind branch January 14, 2022 06:40
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.

It's already merged, but just wanted to say this looks good to me.

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