[PATCH] D81988: [ELF] Fix a dyn_cast<Defined>(nullptr) crash if a local symbol appears in InputFile::symbols

[George Rimar via Phabricator via llvm-commits]

grimar added a comment.

In D81988#2097485 <https://reviews.llvm.org/D81988#2097485>, @jhenderson wrote:

> would we be better off emitting some kind of warning or error about the out-of-order list earlier on?


+1 to this question.



================
Comment at: lld/test/ELF/invalid/broken-symtab-duplicate-symbol.test:2
+# REQUIRES: x86
+## Test that we check nullptr entries in InputFile::symbols (which is supposed to be a
+## non-local symbol list). `local` is a nullptr entry.
----------------
I do not think you should mention the particular internal API names.
This comment should be more general probably. Like:

```
In this test we have a local symbol that follows a global one. This is not correct .... Check that ...
```


================
Comment at: lld/test/ELF/invalid/broken-symtab-duplicate-symbol.test:15
+  Data:    ELFDATA2LSB
+  OSABI:   ELFOSABI_FREEBSD
+  Type:    ET_REL
----------------
`OSABI` is not needed.


================
Comment at: lld/test/ELF/invalid/broken-symtab-duplicate-symbol.test:28
+    Section:  .text
+    Binding:  STB_LOCAL
----------------
Seems there is a double spacing here and above (`Sections`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81988