[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

mgorny marked an inline comment as done. mgorny added inline comments. Comment at: lld/ELF/Driver.cpp:357 + + // default + return GnuStackKind::NoExec; MaskRay wrote: > mgorny wrote: > > MaskRay wrote: > > > MaskRay wrote: > > > > This is obvious. The comment c

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

MaskRay added inline comments. Comment at: lld/ELF/Driver.cpp:357 + + // default + return GnuStackKind::NoExec; mgorny wrote: > MaskRay wrote: > > MaskRay wrote: > > > This is obvious. The comment can be removed. > > Not done. `// default` can be deleted I thin

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

This revision was automatically updated to reflect the committed changes. mgorny marked an inline comment as done. Closed by commit rG2a0fcae3d4d1: [lld] [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK (authored by mgorny). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

krytarowski added a comment. This looks good as an intermediate step to make lld saner. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56554/new/ https://reviews.llvm.org/D56554 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https:/

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

mgorny updated this revision to Diff 225915. mgorny added a comment. Rebased and updated coding style. @ruiu, ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56554/new/ https://reviews.llvm.org/D56554 Files: lld/ELF/Config.h lld/ELF/Driver.cpp lld/ELF/Writer.cpp lld/docs/l

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

mgorny marked 2 inline comments as done. mgorny added inline comments. Comment at: lld/ELF/Driver.cpp:357 + + // default + return GnuStackKind::NoExec; MaskRay wrote: > MaskRay wrote: > > This is obvious. The comment can be removed. > Not done. `// default` can

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: lld/ELF/Driver.cpp:357 + + // default + return GnuStackKind::NoExec; MaskRay wrote: > This is obvious. The comment can be removed. Not do

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

MaskRay added a comment. This patch makes sense to me. It can benefit other OS/environment as well. Comment at: lld/ELF/Driver.cpp:357 + + // default + return GnuStackKind::NoExec; This is obvious. The comment can be removed. CHANGES SINCE LAST ACTION htt

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

vit9696 added a comment. @ruiu, we have some hardware, which bootloader does not support PT_GNU_STACK segments, and would therefore benefit quite a bit from this option. As this does not really depend on NetBSD support, could you please merge this patch in time for 9.0? Thanks! CHANGES SINCE

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

mgorny updated this revision to Diff 190681. mgorny marked 2 inline comments as done. mgorny added a comment. Herald added a project: LLVM. Implemented @ruiu's suggestions. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56554/new/ https://reviews.llvm.org/D56554 Files: lld/ELF/Config.

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

mgorny marked 5 inline comments as done. mgorny added inline comments. Comment at: ELF/Config.h:191 bool ZGlobal; + GnuStackKind ZGnustack; bool ZHazardplt; ruiu wrote: > Members are (roughly) sorted alphabetically, so move this below the last > boolean m

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

ruiu added a comment. Overall looks good. > Do you mean that of those three options, the last one specified should take > precedence? Yes. Comment at: ELF/Config.h:191 bool ZGlobal; + GnuStackKind ZGnustack; bool ZHazardplt; Members are (roughly) sort

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

mgorny added a comment. Besides our verbose purists? Not that I'm aware of. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56554/new/ https://reviews.llvm.org/D56554 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

MaskRay added a comment. > This segment is not supported at all on NetBSD (stack is always > non-executable), and the option is meant to be used to disable emitting it. The string `.note.GNU-stack` takes only a few bytes in `.shstrtab` and a few for an `Elf64_Shdr` instance. Are there any tools

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

mgorny added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56554/new/ https://reviews.llvm.org/D56554 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

mgorny updated this revision to Diff 183729. mgorny added a comment. Ok, here's my proposition of using trinary enum. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56554/new/ https://reviews.llvm.org/D56554 Files: ELF/Config.h ELF/Driver.cpp ELF/Writer.cpp docs/ld.lld.1 test/

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

mgorny added a comment. In D56554#1369606 , @mgorny wrote: > In D56554#1369592 , @ruiu wrote: > > > No, I'm suggesting you add execstack, noexecstack and nognustack as a > > tri-state -z flag. Does this sound good?

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

mgorny added a comment. In D56554#1369592 , @ruiu wrote: > No, I'm suggesting you add execstack, noexecstack and nognustack as a > tri-state -z flag. Does this sound good? Do you mean that of those three options, the last one specified should take prec

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

ruiu added a comment. No, I'm suggesting you add execstack, noexecstack and nognustack as a tri-state -z flag. Does this sound good? Repository: rLLD LLVM Linker CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56554/new/ https://reviews.llvm.org/D56554 _

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

mgorny added a comment. Just to be clear, are you suggesting that I add `-z gnustack` as well? Repository: rLLD LLVM Linker CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56554/new/ https://reviews.llvm.org/D56554 ___ cfe-commits mailing

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

ruiu added a comment. In D56554#1357385 , @mgorny wrote: > @ruiu, what do you think? The current logic forces some precedence, i.e. if > you pass `-z nognustack`, further `-z {no,}execstack` are ignored. I suppose > we could just force passing `-z nognus

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

mgorny marked 2 inline comments as done. mgorny added a comment. @ruiu, what do you think? The current logic forces some precedence, i.e. if you pass `-z nognustack`, further `-z {no,}execstack` are ignored. I suppose we could just force passing `-z nognustack` as last option clang-wise. Repos

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

MaskRay added a comment. Should `ZNognustack` and `ZExecstack` be unified to a tri-state enum variable? Repository: rLLD LLVM Linker CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56554/new/ https://reviews.llvm.org/D56554 ___ cfe-commits

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

krytarowski added a comment. In D56554#1354885 , @ruiu wrote: > In D56554#1353572 , @krytarowski > wrote: > > > What do you think about this new logic: > > > > 1. specified `-z execstack` -> do not emit GNU STACK s

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

ruiu added a comment. In D56554#1353572 , @krytarowski wrote: > What do you think about this new logic: > > 1. specified `-z execstack` -> do not emit GNU STACK segment > 2. specified `-z noexecstack` -> emit GNU STACK segment > 3. no option specified -> d

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

krytarowski added a comment. @ruiu actually if we can get D56215 merged we will be able to tune it specifically for NetBSD (with `if (Config->TargetTriple.isOSNetBSD()) {`) and retain intact the current Linux-biased logic for everybody who deserves to use it.

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

mgorny added a comment. Actually, I've just researched a bit and default stack permissions depend on arch in glibc, so assuming it's RWX by default is wrong. Repository: rLLD LLVM Linker CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56554/new/ https://reviews.llvm.org/D56554

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

mgorny added a comment. @ruiu, what if one of the systems changes defaults (e.g. due to Hardening) and starts defaulting to noexecstack? In that case we'd want `-z execstack' to actually emit PT_GNU_STACK, and I don't think we really are able to 100% detect the default in clang. I'd really see

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

krytarowski added a comment. What do you think about this new logic: 1. specified `-z execstack` -> do not emit GNU STACK segment 2. specified `-z noexecstack` -> emit GNU STACK segment 3. no option specified -> detect `findSection(".note.GNU-stack")` (I might miss offhand some details here to b

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

ruiu added a comment. GNU linkers assume that input object files that work with non-executable stack has a .note.GNU-stack section, and they emit a PT_GNU_STACK segment to mark the stack area non-executable if all input files have that marker section. If restoring default means we implement tha

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

krytarowski added a comment. I understand the concerns, but lld is biased with being a Linux linker. We need to customize it (restore defaults?) for NetBSD. Repository: rLLD LLVM Linker CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56554/new/ https://reviews.llvm.org/D56554 _

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

krytarowski added a comment. Shouldn't the logic of emitting/not-emitting be based on `findSection(".note.GNU-stack")`? @mgorny can you please investigate it? Repository: rLLD LLVM Linker CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56554/new/ https://reviews.llvm.org/D56554 ___

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

ruiu added a comment. In D56554#1353527 , @krytarowski wrote: > In D56554#1353513 , @ruiu wrote: > > > In D56554#1353487 , @krytarowski > > wrote: > > > > > In D56554#135336

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

krytarowski added a comment. Actually looking at GNU ld(1) source code, the logic is a little bit more complex and it depends on more aspects like relocations. Repository: rLLD LLVM Linker CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56554/new/ https://reviews.llvm.org/D56554 __

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

krytarowski added a comment. In D56554#1353513 , @ruiu wrote: > In D56554#1353487 , @krytarowski > wrote: > > > In D56554#1353368 , @ruiu wrote: > > > > > The absence of PT_

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

ruiu added a comment. In D56554#1353487 , @krytarowski wrote: > In D56554#1353368 , @ruiu wrote: > > > The absence of PT_GNU_STACK segment makes stack area executable on systems > > that recognizes PT_GNU_STACK seg

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

krytarowski added a comment. In D56554#1353368 , @ruiu wrote: > The absence of PT_GNU_STACK segment makes stack area executable on systems > that recognizes PT_GNU_STACK segment. So, I think if `-z execstack` is > specified, we should omit PT_GNU_STACK s

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

ruiu added a comment. The absence of PT_GNU_STACK segment makes stack area executable on systems that recognizes PT_GNU_STACK segment. So, I think if `-z execstack` is specified, we should omit PT_GNU_STACK segment rather than adding it, which I think you guys want. If we do that, it seems `-z

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

krytarowski added inline comments. Comment at: ELF/Driver.cpp:874 Config->ZGlobal = hasZOption(Args, "global"); + Config->ZNognustack = hasZOption(Args, "nognustack"); Config->ZHazardplt = hasZOption(Args, "hazardplt"); I would add `gnustack` vs `nognustac

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

krytarowski added inline comments. Comment at: ELF/Writer.cpp:1980 + if (!Config->ZNognustack) { +// PT_GNU_STACK is a special section to tell the loader to make the +// pages for the stack non-executable. If you really want an executable section -> segm

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

mgorny marked an inline comment as done. mgorny added inline comments. Comment at: docs/ld.lld.1:515 +.Dv PT_GNU_STACK +segment. .It Cm norelro krytarowski wrote: > section? I think 'segment' is actually the correct term here. Repository: rLLD LLVM Linker C

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

krytarowski added a comment. Looks good, we don't GNU STACK on NetBSD. Repository: rLLD LLVM Linker CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56554/new/ https://reviews.llvm.org/D56554 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

krytarowski added inline comments. Comment at: docs/ld.lld.1:515 +.Dv PT_GNU_STACK +segment. .It Cm norelro section? Repository: rLLD LLVM Linker CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56554/new/ https://reviews.llvm.org/D56554 __

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

mgorny created this revision. mgorny added reviewers: ruiu, krytarowski. Herald added subscribers: llvm-commits, arichardson, emaste. Herald added a reviewer: espindola. Add a new '-z nognustack' option that suppresses emitting PT_GNU_STACK segment. This segment is not supported at all on NetBSD