[PATCH] D66734: [llvm-readobj/llvm-readelf] - Report a proper warning when dumping a broken dynamic relocation.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: include/llvm/Object/ELFTypes.h:253
+        object_error::parse_failed,
+        "st_name (0x" + utohexstr(Offset) +
+            ") is past the end of the string table of size 0x" +
----------------
jhenderson wrote:
> I think you want to replace `utohextr(Offset)` with `PRIx32` to take advantage of createStringError's printf formatting rules:
> 
> ```
> return createStringError(object_error::parse_failed,
>     "st_name (0x" PRIx32 ") is past the end of the string table"
>     " of size 0x%zx", Offset, StrTab.size());
> ```
Ok. I am not in favour of using format strings usually, because remembering non trivial format specifiers
is hard. And I also faced wierd issues before because of wrong specifer used by mistake (or because of a variable type change).

But since both you and Jordan think it is better.. Done.


================
Comment at: test/tools/llvm-readobj/elf-broken-dynamic-reloc-name.test:14
+
+# GNU:      'RELA' relocation section at offset 0x278 contains 24 bytes:
+# GNU-NEXT: Offset           Info             Type          Symbol's Value   Symbol's Name + Addend
----------------
jhenderson wrote:
> I think the offset here is irrelevant, right? Can it be replaced with `{{.+}}`?
Yes, done. BTW, I searched for "at offset" and noticed our tests are using `{{.*}}` for testing this line which is not entirely correct.


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

https://reviews.llvm.org/D66734