[PATCH] D77289: [Object] Fix crash caused by unhandled error.

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

Thinking about it, I'm reluctant to approve this as is, without a further change to actually report the error (possibly in a seperate patch). Prior to this patch, we have a failure that at least can be identified in certain build versions, whereas now the malformed-ness is not detectable at all. Perhaps this could temporarily use `report_fatal_error` with a follow-up patch bubbling the error further up the stack?



================
Comment at: llvm/include/llvm/Object/ELFObjectFile.h:628
 
-  auto DotSymtabSecSyms = EF.symbols(DotSymtabSec);
-  if (DotSymtabSecSyms && ESym == (*DotSymtabSecSyms).begin())
-    Result |= SymbolRef::SF_FormatSpecific;
-  auto DotDynSymSecSyms = EF.symbols(DotDynSymSec);
-  if (DotDynSymSecSyms && ESym == (*DotDynSymSecSyms).begin())
+  auto IsNULLSymbol = [this](const Elf_Sym *Sym, const Elf_Shdr *SymSec) {
+    Expected<typename ELFT::SymRange> SymbolsOrErr = EF.symbols(SymSec);
----------------
I think `IsNullSymbol` would be fine, and more in keeping with LLVM naming standards.


================
Comment at: llvm/test/Object/invalid-symtab-size.test:13
+
+## TODO: Currently, llvm-nm will print 0-index NULL symbol from .dynsym. We should correct this behavior in future.
+# RUN: llvm-nm --dynamic %t.32-bit.o 2>&1 | FileCheck --implicit-check-not=warning --check-prefix=DYN %s
----------------
FIXME rather than TODO here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77289