[PATCH] D46653: Start support for linking object files with split stacks

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: ELF/InputSection.cpp:223
+  if (getFile<ELFT>() == nullptr)
+    return nullptr;
+
----------------
When can this happen? That code is perhaps dead.


================
Comment at: ELF/InputSection.cpp:863
+    if (F->Value <= Offset && Offset < F->Value + F->Size)
+      return true;
+  return false;
----------------
This line never executes when running the test cases.


================
Comment at: ELF/InputSection.cpp:882
+    if (Rel.Sym->isLocal())
+      continue;
+    Defined *D = dyn_cast_or_null<Defined>(Rel.Sym);
----------------
This 'continue' is untested by a test case. The code is "dead" because of that.


================
Comment at: ELF/InputSection.cpp:887
+    if (!D)
+      continue;
+
----------------
This 'continue' is untested by a test case it seems.


================
Comment at: ELF/InputSection.cpp:900
+    if (D->Type != STT_FUNC)
+      continue;
+    InputSection *IS = dyn_cast_or_null<InputSection>(D->Section);
----------------
The same - untested.


================
Comment at: ELF/InputSection.cpp:907
+    if (IS && IS->getFile<ELFT>()->SplitStack)
+      continue;
+    if (enclosingPrologueAdjusted(Rel.Offset, AdjustedPrologues))
----------------
Untested.


================
Comment at: ELF/InputSection.cpp:909
+    if (enclosingPrologueAdjusted(Rel.Offset, AdjustedPrologues))
+      continue;
+    if (Defined *F = getEnclosingFunction<ELFT>(Rel.Offset)) {
----------------
Untested. enclosingPrologueAdjusted always returns false when running tests.


================
Comment at: ELF/InputSection.cpp:917
+    if (!getFile<ELFT>()->SomeNoSplitStack)
+      error("Function call at " + getErrorLocation(Buf + Rel.Offset) +
+            " crosses a split-stack boundary, but unable " +
----------------
This error is untested (because of the broken tests first of all I believe).


================
Comment at: test/ELF/Inputs/x86-64-split-stack-main.s:1
+
+	.text
----------------
Excessive empty line.


================
Comment at: test/ELF/Inputs/x86-64-split-stack-main.s:12
+	.section	.note.GNU-stack,"", at progbits
+	
----------------
Excessive empty line.


================
Comment at: test/ELF/x86-64-split-stack-prologue-adjust-fail.s:5
+
+# RUN: not /usr/local/google/home/saugustine/binutils/build/gold/ld-new --defsym __morestack=0x100 %t1.o %t2.o -o %t |& FileCheck %s
+
----------------
/usr/local/google/home/saugustine/binutils/build/gold/ld-new -> ld.lld here and below


================
Comment at: test/ELF/x86-64-split-stack-prologue-adjust-fail.s:10
+
+# RUN: not /usr/local/google/home/saugustine/binutils/build/gold/ld-new -r --defsym __morestack=0x100 %t1.o %t2.o -o %t |& FileCheck %s -check-prefix=RELOCATABLE
+# RELOCATABLE: cannot mix split-stack
----------------
Looks like a mistype, you need to change "|&" to "|" as currently, it triggers "shell parser error" when trying to run tests under windows.


================
Comment at: test/ELF/x86-64-split-stack-prologue-adjust-fail.s:13
+# RELOCATABLE: and non-split-stack
+# RELOCATABLE: when using -r
+
----------------
I do not know where these strings are coming from, I think you need to test the "Function call at ..." error message here
and these strings are coming from gold probably? (because of /usr/local/google/home/saugustine/binutils/build/gold/ld-new).


================
Comment at: test/ELF/x86-64-split-stack-prologue-adjust-silent.s:6
+# RUN: /usr/local/google/home/saugustine/binutils/build/gold/ld-new --defsym __morestack=0x100 %t1.o %t2.o -o %t
+# RUN: objdump -d %t | FileCheck %s
+
----------------
You need to use llvm-objdump and not GNU objdump.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D46653