Stream: git-wasmtime

Topic: wasmtime / PR #1455 cranelift-object: mark output as usin...


view this post on Zulip Wasmtime GitHub notifications bot (Apr 01 2020 at 15:16):

froydnj opened PR #1455 from object-noexecstack to master:

The faerie crate does this by default. object itself does not, but
it seems reasonable to make this change for parity between
cranelift-faerie and cranelift-object.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 01 2020 at 15:53):

bjorn3 submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 01 2020 at 15:53):

bjorn3 created PR Review Comment:

Should this only be added for GNU systems or is it fine to just keep it on other systems without purpose?

view this post on Zulip Wasmtime GitHub notifications bot (Apr 01 2020 at 15:53):

bjorn3 submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 01 2020 at 16:08):

froydnj submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 01 2020 at 16:08):

froydnj created PR Review Comment:

I read this as "should we only add this for *gnu targets (e.g. not FreeBSD)" or "should we unconditionally add it everywhere (e.g. Darwin, Windows)", is that correct? I don't think we should add it everywhere, so that leaves just the first question.

Browsing through assembly files in Firefox (Firefox code itself and various third-party libraries), it looks like people are inconsistent in their choices: some restrict this note to Linux-only and some restrict it to ELF-only. I think making the condition target_lexicon::OperatingSystem::Linux would be fine-ish (I don't know what the *BSDs do with respect to this section; I see that lld ignores this section entirely and controls executable stack-ness with a command-line switch). I don't think I'd want to try to condition on *gnu-ness; matching on target_lexicon::Environment to catch all of the cases would be annoying, I think -- and maybe you also have to catch musl-ness etc. as well?

Making it ELF-only seems like the safest option, and the linker should just drop these sections anyway.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 01 2020 at 16:19):

bjorn3 submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 01 2020 at 16:19):

bjorn3 created PR Review Comment:

I read this as "should we only add this for *gnu targets (e.g. not FreeBSD)" or "should we unconditionally add it everywhere (e.g. Darwin, Windows)", is that correct?

Almost. I meant should this be added for all ELF systems, or only GNU based ones.

I think making the condition target_lexicon::OperatingSystem::Linux would be fine-ish

There are several non-Linux systems which use a GNU userspace, like Hurd and (as option?) Plan 9.

I see that lld ignores this section entirely and controls executable stack-ness with a command-line switch

:tada: .note.GNU-stack makes it way too easy to accidentally end up with an executable stack.

Making it ELF-only seems like the safest option, and the linker should just drop these sections anyway.

:+1:

view this post on Zulip Wasmtime GitHub notifications bot (Apr 01 2020 at 16:20):

froydnj merged PR #1455.


Last updated: Dec 23 2024 at 12:05 UTC