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

fix Issue 20421 - Exceptions don't work when linking through lld-link #433

Merged
merged 1 commit into from
Feb 10, 2020

Conversation

rainers
Copy link
Member

@rainers rainers commented Feb 9, 2020

patch LLD to not set IMAGE_DLL_CHARACTERISTICS_NO_SEH, the MS linker also doesn't do this.

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @rainers!

Bugzilla references

Auto-close Bugzilla Severity Description
20421 major Exceptions don't work when linking through lld-link

patch LLD to not set IMAGE_DLL_CHARACTERISTICS_NO_SEH, the MS linker also doesn't do this.
@thewilsonator
Copy link
Collaborator

Should this target stable?

@wilzbach wilzbach merged commit bc53396 into dlang:master Feb 10, 2020
@wilzbach
Copy link
Member

The beta for 2.091 should be tagged in five days...

@rainers
Copy link
Member Author

rainers commented Feb 10, 2020

Should this target stable?

This doesn't automatically go into the releaase, needs another change to the create_release script. @wilzbach Can you upload the lld-artifacts to downloads.dlang.org? Thanks.

@rainers
Copy link
Member Author

rainers commented Feb 12, 2020

@wilzbach Can you upload the lld-artifacts to downloads.dlang.org? Thanks.

Gentle Ping :) I hope to get the follow-up into the next release. BTW: it might also need a change to dmd (pass /SAFESEH:NO to lld).

@wilzbach
Copy link
Member

@wilzbach Can you upload the lld-artifacts to downloads.dlang.org? Thanks.

Gentle Ping :) I hope to get the follow-up into the next release. BTW: it might also need a change to dmd (pass /SAFESEH:NO to lld).

Sorry. They both should be up now:

downloads.dlang.org/other/lld-link-9.0.0-seh-x64.zip
downloads.dlang.org/other/lld-link-9.0.0-seh.zip

@wilzbach
Copy link
Member

needs another change to the create_release script.

#434

@wilzbach
Copy link
Member

@wilzbach Can you upload the lld-artifacts to downloads.dlang.org? Thanks.

BTW I don't think that we have to upload the artifacts to this bucket. We could also just use GitHub's releases here. Even manually dropping them in via e.g. https://github.com/dlang/installer/releases/new this could be faster (and if we have to do this more often, we could automate further).

That being said I don't mind your pings, just thinking about how to decrease the overall workload.

@rainers
Copy link
Member Author

rainers commented Feb 12, 2020

Sorry. They both should be up now:

Thanks. For some reason I don't see it on the download page (probably needs to be regenerated, too), but it's used by your PR.

Even manually dropping them in via e.g. https://github.com/dlang/installer/releases/new this could be faster

Sounds good to me. I'll try to remember next time.

@wilzbach
Copy link
Member

Sounds good to me.

We would have to change the URL in the build script, but this might be easier as anyone with write permissions to this repo can "upload" (aka tag a new release) and bump the URL path.

@rainers
Copy link
Member Author

rainers commented Feb 19, 2020

downloads.dlang.org/other/lld-link-9.0.0-seh.zip

@wilzbach it seems the signature file is broken as the buildkite release build fails to verify it.

@wilzbach
Copy link
Member

downloads.dlang.org/other/lld-link-9.0.0-seh.zip

@wilzbach it seems the signature file is broken as the buildkite release build fails to verify it.

Ah I didn't realize that we were actually checking for those and thus I didn't upload them for this once. I uploaded them now and restarted your PR.
BTW as I think one needs to be of the D keyring (https://dlang.org/gpg_keys.html) in order to sign things, what do you think about switching to sha256 hashes for all binaries uploaded "newly" via GitHub releases?

@rainers
Copy link
Member Author

rainers commented Feb 19, 2020

Ah I didn't realize that we were actually checking for those and thus I didn't upload them for this once. I uploaded them now and restarted your PR.

Thanks. Unfortunately the signature that I created will probably also not be accepted, because I got it from gpgsm, not gpg (the latter wouldn't let me import the certificate).

what do you think about switching to sha256 hashes for all binaries uploaded "newly" via GitHub releases?

Probably simpler to handle, but depends on what the actual goal of these checks are. If we want to avoid that external sources change without notice, the sha256 hashes should be good enough. If we want the sources to come from trusted parties, the signature is safer. Both ways we assume that the git repository isn't tampered with. Considering github releases are as safe as the repository itself, the checks seem rather gratuitous to begin with.

@rainers
Copy link
Member Author

rainers commented Feb 19, 2020

I uploaded them now and restarted your PR.

They are available now, but gpg doesn't like them:

c:\>gpg --verify C:\Users\Rainer\Downloads\lld-link-9.0.0-seh.zip.sig C:\Users\Rainer\Downloads\lld-link-9.0.0-seh.zip
gpg: Signature made 02/19/20 09:51:54 W. Europe Standard Time
gpg:                using RSA key 8FDB8D357AF468A9428ACE3C2055F76601A36FB0
gpg: BAD signature from "Sebastian Wilzbach <seb@wilzba.ch>" [unknown]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants