[PATCH] D63084: [llvm-readobj] Don't abort printing of dynamic table if string reference is invalid

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 11 02:33:13 PDT 2019


grimar added a comment.

Few comments are below. James, what do you think?



================
Comment at: llvm/test/tools/llvm-readobj/elf-dynamic-malformed.test:157
+# RUN: llvm-readelf --dynamic-table --needed-libs %t.bad-strtab | FileCheck %s --check-prefixes=BAD-STRTAB,BAD-STRTAB-GNU
+# BAD-STRTAB-LLVM: LoadName: <not found>
+# BAD-STRTAB-LLVM: 0x0000000000000001  NEEDED   Shared library: [dynamic strtab not found]
----------------
`<not found>` is inconsistent with `library name not found` below.
Should both use `<..>`?


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:1779
+    OS << "Invalid Offset<0x" << utohexstr(Value) << ">";
 }
 
----------------
It is a bit inconsistent.
I think you need to use upper/lower case for both messages, but do not mix.
I.e `dynamic strtab not found` -> `Dynamic strtab not found`.

I would also probably rewrite to "String table is empty or was not found" or something like that.
Because it is possible (in theory) that `DT_STRSZ` == 0, but `DT_STRTAB` points to somewhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63084





More information about the llvm-commits mailing list