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

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 30 05:49:02 PDT 2020


grimar added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/gnu-phdrs.test:326
+
+# ERROR-INTERP:      Type           Offset
+# ERROR-INTERP-NEXT: INTERP         0x000230
----------------
jhenderson wrote:
> 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.
> You could even avoid the file concatenation etc by just capturing the variable based on the first INTERP's offset, if you want.

This looks good I think. Thanks for suggestion!
I've updated the test.


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

https://reviews.llvm.org/D79013





More information about the llvm-commits mailing list