[PATCH] D80496: [ELF] Add -z rel and -z rela

[George Rimar via Phabricator via llvm-commits]

grimar added a comment.

Generally looks fine I think. Few comments from me.



================
Comment at: lld/ELF/Driver.cpp:852
+      return true;
+  }
+
----------------
It would be more consistent with the rest code to use `StringRef` comparsion instead of `strcmp`.
E.g:

```
static GnuStackKind getZGnuStack(opt::InputArgList &args) {
  for (auto *arg : args.filtered_reverse(OPT_z)) {
    if (StringRef("execstack") == arg->getValue())
      return GnuStackKind::Exec;
...
```


================
Comment at: lld/test/ELF/i386-zrela.s:1
+# REQUIRES: x86
+## The i386 psABI uses Elf64_Rela relocation entries. We produce
----------------
Perhaps, the name of the test should be `i386-zrel-zrela.s` since you test both?
(this and all other comments also applies for `x86-64-zrel.s`)


================
Comment at: lld/test/ELF/i386-zrela.s:30
+
+# RUN: ld.lld -shared -z rel -z rela %t.o -o %t2.so
+# RUN: llvm-readobj -d -r %t2.so | FileCheck --check-prefix=RELA %s
----------------
I think you need to have just `-z rela` too.


================
Comment at: lld/test/ELF/i386-zrela.s:43
+# RELA-NEXT:   R_386_TLS_TPOFF tls 0x2A
+# RELA-NEXT:   R_386_32 _start 0x2A
+# RELA-NEXT: }
----------------
Does it worth to show what happens for `R_386_RELATIVE`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80496