[PATCH] D49456: [AArch64] Support execute-only LOAD segments.

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: ELF/Options.td:135-136
+defm execute_only: B<"execute-only",
+    "Do not mark executable sections readable.",
+    "Mark executable sections readable.">;
+
----------------
Remove period from the end of the messages.

Please add "(default)" like other messages.


================
Comment at: ELF/ScriptParser.cpp:491
   Config->SingleRoRx = true;
+  Config->ExecuteOnly = false;
 
----------------
I don't think silently disabling a security feature is a good idea. I'd just mark executable sections execute-only even if linker script is in use, because that's what user told to the linker to do. Other option would be to report an error when -execute-only and a linker script is used together, but I think that confuses users as to why that combination is not allowed.


================
Comment at: test/ELF/execute-only.s:7
+
+// CHECK:      LOAD           {{.*}} 0x00000000cafe0000 0x00000000cafe0000 0x000004 0x000004   E 0x{{.*}}
+// CHECK-NOT:  LOAD           {{.*}} 0x00000000cafe0000 0x00000000cafe0000 0x000004 0x000004 R E 0x{{.*}}
----------------
Remove excess space characters after "LOAD" so that it fits within 80 columns.


https://reviews.llvm.org/D49456