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

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

In D56554#1354885 <https://reviews.llvm.org/D56554#1354885>, @ruiu wrote:

> In D56554#1353572 <https://reviews.llvm.org/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 -> detect `findSection(".note.GNU-stack")` (I might miss offhand some details here to be sure if it is reliable) 3.1. detected -> emit GNU STACK segment 3.2. not detected -> do not emit GNU STACK segment
> >
> >   Additionally we will specify explicitly in the clang driver for Linux and FreeBSD `-z noexecstack`.
>
>
> I think I don't like the very idea of using "marker sections" in input object files to change the linker behavior. That's too subtle and seems error-prone to me.
>
> After thinking for a while, I started thinking that the first version of this patch is probably exactly what we want. I had been thinking that `-z {no,}execstack` and `-z {no,}gnustack` are four different options, but what we actually want to get is tri-state:
>
> - emit PT_GNU_STACK with RWX permission
> - emit PT_GNU_STACK with RW permission
> - do not emit PT_GNU_STACK
>
>   So we could map them to `-z {execstack,noexecstack,nognustack}`, respectively, with default set to `-z noexecstack`. You guys can pass `-z nognustack` to the linker to tell the linker to not emit it at all. That's exactly this patch does.


Actually after a longer thought, I recommend to hardcode inside lld a check for `TargetTriple.isOSNetBSD()`. Using `-z nognustack` isn't portable to other linkers.



================
Comment at: ELF/Writer.cpp:1979
 
-  // 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
-  // stack, you can pass -z execstack, but that's not recommended for
-  // security reasons.
-  unsigned Perm = PF_R | PF_W;
-  if (Config->ZExecstack)
-    Perm |= PF_X;
-  AddHdr(PT_GNU_STACK, Perm)->p_memsz = Config->ZStackSize;
+  if (!Config->ZNognustack) {
+    // PT_GNU_STACK is a special section to tell the loader to make the
----------------
Please go for `if (!Config->TargetTriple.isOSNetBSD()) {`


Repository:
  rLLD LLVM Linker

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56554/new/

https://reviews.llvm.org/D56554