[ELF] Try to not emit weird diagnostics on undefined symbols

[davide]

Summary

Fixes https://bugs.llvm.org/show_bug.cgi?id=34872

[ruiu]

ELF/Relocations.cpp

865–866 ↗

(On Diff #118305)

Can you just continue without checking for ErrorCount?

[davide]

ELF/Relocations.cpp

865–866 ↗

(On Diff #118305)

Not quite :(

---

Testing Time: 2.76s

---

Failing Tests (36):

[ruiu]

ELF/Relocations.cpp

865–866 ↗

(On Diff #118305)

Why are they failing?

[davide]

ELF/Relocations.cpp

865–866 ↗

(On Diff #118305)

Because recordUndefined is not a function returning a boolean which communicates to the caller whether the reported symbol was actually undefined, it just does side effect calling error (or warn).

[grimar]

test/ELF/pr34872.s

8 ↗

(On Diff #118305)

Testing full error message with NOT is probably brittle and
can make check useless if we change message.
I would just check we have no other errors about symbol:
#CHECK-NOT: fmod

[davide]

test/ELF/pr34872.s

8 ↗

(On Diff #118305)

it's unlikely this message will change imho.

[jhenderson]

test/ELF/pr34872.s

8 ↗

(On Diff #118305)

+1 for @grimar's suggestion. We cannot predict how the error messages might change in the future: even if it's unlikely, it's still possible that they might for some reason (e.g. we might decide to do a wholesale change in their structure, like the move to change to multi-line errors for duplicate symbols).

[davide]

Fine, I'll change the test. Any other comments or this can go in once the test is fixed? @ruiu ?

[ruiu]

LGTM

lld/trunk/ELF/Relocations.cpp

for (auto I = Rels.begin(), End = Rels.end(); I != End; ++I) {
     uint64_t Offset = GetOffset.get(Rel.r_offset);
     if (Offset == uint64_t(-1))
       continue;
 
     // Report undefined symbols. The fact that we report undefined
     // symbols here means that we report undefined symbols only when
     // they have relocations pointing to them. We don't care about
     // undefined symbols that are in dead-stripped sections.
-    if (!Body.isLocal() && Body.isUndefined() && !Body.symbol()->isWeak())
+    if (!Body.isLocal() && Body.isUndefined() && !Body.symbol()->isWeak()) {
       reportUndefined<ELFT>(Body, Sec, Rel.r_offset);
 
+      // If we report an undefined, and we have an error, go on.
+      if (ErrorCount)
+        continue;
+    }
+
     RelExpr Expr = Target->getRelExpr(Type, Body, *Sec.File,
                                       Sec.Data.begin() + Rel.r_offset);
 
     // Ignore "hint" relocations because they are only markers for relaxation.
     if (isRelExprOneOf<R_HINT, R_NONE>(Expr))
       continue;
 
     bool Preemptible = isPreemptible(Body, Type);

lld/trunk/test/ELF/Inputs/undefined-error.s

+callq fmod@PLT

lld/trunk/test/ELF/pr34872.s

+# RUN: llvm-mc %s -filetype=obj -triple=x86_64-pc-linux -o %t.o
+# RUN: llvm-mc %p/Inputs/undefined-error.s -filetype=obj \
+# RUN:    -triple=x86_64-pc-linux -o %t2.o
+# RUN: ld.lld -shared %t2.o -o %t2.so
+# RUN: not ld.lld %t2.so %t.o 2>&1 | FileCheck %s
+
+# CHECK: undefined symbol: fmod
+# Check we're not emitting other diagnostics for this symbol.
+# CHECK-NOT: fmod
+
+.global main
+
+main:
+  callq fmod