[PATCH] D79013: [llvm-readelf] - Do not crash when the PT_INTERP has a broken offset.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/gnu-phdrs.test:316
+
+## Show the size of the output produced. It is used below.
+# RUN: wc -c < %t.err | FileCheck %s --check-prefix=SIZE
----------------
used in the YAML below.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/gnu-phdrs.test:326
+
+# ERROR-INTERP:      Type           Offset
+# ERROR-INTERP-NEXT: INTERP         0x000230
----------------
MaskRay wrote:
> The main problem with the test is that if yaml2obj layout is updated, the p_offset fields will need an update which may be a bit tricky. Since the file size is displayed, this is probably fine.
> 
> 560 = size(Ehdr) + size(Phdrs) + section contents with padding + size(Shdrs) = 64 + 56*5 + alignTo(1+0x13, 8) + 64*3 = 560
You could improve things slightly with a bit of cleverness in FileCheck, although I don't know whether it's a good idea. I think it should work as described, possibly with a few tweaks:

Firstly, emit the `wc` output to a file, and concatenate the llvm-readelf program header dump to the same file. Then, use FileCheck's numeric variables to capture the size and re-use it:

```
# RUN: wc -c < %t.err > %t.err.out
# RUN: FileCheck %s --check-prefix=SIZE --input-file=%t.err.out
# SIZE: 560
## Optionally use echo to add some extra data here that you can check to avoid matching something else.

## Write the additional 'C', '\0, 'C' bytes to the end.
# RUN: echo -n -e "C\x00C" >> %t.err

# RUN: llvm-readelf --program-headers %t.err 2>&1 >> %t.err.out
# RUN: FileCheck %s --input-file=%t.err.out -DFILE=%t.err --check-prefix=ERROR-INTERP

# ERROR-INTERP: [[#SIZE:]]
## If you add the extra data, check it here.
# ERROR-INTERP:      Type           Offset
# ERROR-INTERP-NEXT: INTERP         [[#SIZE]]
# ERROR-INTERP-NEXT:     [Requesting program interpreter: C]
# ERROR-INTERP-NEXT: INTERP         [[#SIZE+1]]
# ERROR-INTERP-NEXT:     [Requesting program interpreter: ]
```
and so on.

This has the benefit of only needing to change the YAML and one single check, should the layout change. You could even avoid the file concatenation etc by just capturing the variable based on the first INTERP's offset, if you want.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/gnu-phdrs.test:349
+## Case 1: the offset points to the second additional byte,
+##         which is null byte.
+  - Type:   PT_INTERP
----------------
is a null


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:4132
+      OS << "\n";
+      auto ReportError = [&](const Twine &Msg) {
+        reportWarning(
----------------
Maybe rename to `ReportBadInterp` or something. `ReportError` is a bit confusing when it's not an error!


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

https://reviews.llvm.org/D79013