Page MenuHomePhabricator

[llvm-readobj]Add additional testing for various ELF features
ClosedPublic

Authored by jhenderson on Feb 26 2019, 7:52 AM.

Details

Summary

This patch adds testing of areas of the code that are not fully tested, in particular dynamic table printing, ELF type printing, handling of edge cases where things are missing/empty (relocations/program header tables/section header table), and the --string-dump switch.

Diff Detail

Repository
rL LLVM

Event Timeline

jhenderson created this revision.Feb 26 2019, 7:52 AM
Herald added a project: Restricted Project. · View Herald Transcript
Herald added a subscriber: srhines. · View Herald Transcript
Higuoxing added inline comments.Feb 26 2019, 8:25 PM
test/tools/llvm-readobj/elf-malformed-PT_DYNAMIC.test
16–17 ↗(On Diff #188378)

Here should be

# RUN: %python -c "with open(r'%t.truncated1', 'r+') as f: f.truncate(0x1001)"
                                           ^
# RUN: not llvm-readobj %t.truncated1 --dynamic-table 2>&1 | FileCheck %s
                                    ^

right?

22 ↗(On Diff #188378)

Here should be

# RUN: %python -c "with open(r'%t.truncated2', 'r+') as f: f.truncate(0x1001)"
                                           ^
# RUN: not llvm-readobj %t.truncated2 --dynamic-table 2>&1 | FileCheck %s
                                    ^
rupprecht accepted this revision.Feb 26 2019, 9:40 PM

I always like having more tests :D

test/tools/llvm-readobj/elf-dynamic-no-PT_DYNAMIC.test
1 ↗(On Diff #188378)

nit: it seems like uppercase filenames are rarely used, "elf-dynamic-no-pt-dynamic.test" might be better
Same with the other pt_dynamic test

This revision is now accepted and ready to land.Feb 26 2019, 9:40 PM
MaskRay added inline comments.
test/tools/llvm-readobj/elf-dynamic-tags.test
213 ↗(On Diff #188378)

0x1f to enumerate all DF_* flags?

251 ↗(On Diff #188378)

Use 0x03ffffff to enumerate all DF_1_* flags?

grimar added inline comments.Feb 27 2019, 2:05 AM
test/tools/llvm-readobj/elf-dynamic-malformed.test
92 ↗(On Diff #188378)

Do I correctly understand that issue here is that DT_NEEDED has a value 0x1, which
is an offset in a .dynstr, and the issue is that DT_STRSZ says that .dynstr has a total size of 1
(and so offset 0x1 is past the end of the section)?

If so, it is not really obvious perhaps, I would suggest extending the comment for this sub-test.

test/tools/llvm-readobj/elf-dynamic-tags.test
275 ↗(On Diff #188378)

Could you explain the logic of numeric tags for me?

I see you're testing LOPROC and (HIPROC-1) it seems:

DYNAMIC_TAG_MARKER(LOOS, 0x60000000) Start of environment specific tags.
DYNAMIC_TAG_MARKER(HIOS, 0x6FFFFFFF)
End of environment specific tags.
DYNAMIC_TAG_MARKER(LOPROC, 0x70000000) Start of processor specific tags.
DYNAMIC_TAG_MARKER(HIPROC, 0x7FFFFFFF)
End of processor specific tags.

But what is 0x6000000D and 0x6ffff000 then?

Higuoxing added inline comments.Feb 27 2019, 3:45 AM
test/tools/llvm-readobj/elf-dynamic-tags.test
150 ↗(On Diff #188378)

I think here should be linked with .dynstr

- Name: .dynamic
  Link: 1

Or, objdump cannot dump this. But, never mind here :)

jhenderson marked 5 inline comments as done.Feb 27 2019, 4:04 AM
jhenderson added inline comments.
test/tools/llvm-readobj/elf-dynamic-malformed.test
92 ↗(On Diff #188378)

Yes, that's correct. I'll improve it.

test/tools/llvm-readobj/elf-dynamic-tags.test
213 ↗(On Diff #188378)

I'll go the whole hog and do 0xffffffffffffffff, to enumerate all flags current and future.

251 ↗(On Diff #188378)

Ditto.

275 ↗(On Diff #188378)

I can't remember my own logic now!

I'm interested in other reserved values effectively, that don't have a known meaning in LLVM, but looking at the known values, I don't see any reason to not use 0x60000000. Note that 0x6FFFFFFF is DT_VERNEEDNUM, so I can't use that. The highest I can use is 0x6FFFFFF8, I believe. I could also test a mid-range value too, if that's desirable?

By the way, I'm using 0x7ffffffe, because 0x7fffffff is DT_FILTER.

test/tools/llvm-readobj/elf-malformed-PT_DYNAMIC.test
16–17 ↗(On Diff #188378)

Oops, thanks. I'll fix that. I renamed the file, so there must be one still in my test output directory of the old name.

grimar added inline comments.Feb 27 2019, 4:22 AM
test/tools/llvm-readobj/elf-dynamic-tags.test
275 ↗(On Diff #188378)

Not sure about 0x7ffffffe, because it is surrounded by DT_AUXILIARY(0x7FFFFFFD) and DT_FILTER(0x7FFFFFFF),
I guess someone might want to use it one day.

Seems to test that we can work with unknown tags you can just test a single value and perhaps any
obviously unused/weird value like 0x6ABCDEF0 would work here?

Higuoxing added inline comments.Feb 27 2019, 4:39 AM
test/tools/llvm-readobj/elf-dynamic-tags.test
275 ↗(On Diff #188378)

0x7ffffffe is DT_USED
I find some code example, but cannot find docs on it.

#define	DT_USED		0x7ffffffe	/* ignored - same as needed */

https://github.com/switchbrew/switch-tools/blob/master/src/elf_common.h

and

	case DT_USED:
	case DT_INIT_ARRAY:
	case DT_FINI_ARRAY:
	  if (do_dynamic)
	    {
	      if (entry->d_tag == DT_USED
		  && VALID_DYNAMIC_NAME (entry->d_un.d_val))
		{
		  char *name = GET_DYNAMIC_NAME (entry->d_un.d_val);

		  if (*name)
		    {
		      printf (_("Not needed object: [%s]\n"), name);
		      break;
		    }
		}

	      print_vma (entry->d_un.d_val, PREFIX_HEX);
	      putchar ('\n');
	    }
	  break;

http://web.mit.edu/freebsd/head/contrib/binutils/binutils/readelf.c

jhenderson marked 10 inline comments as done.

Address review comments:

  1. Rename tests to lower case.
  2. Fix test file names.
  3. Change tags in reserved range to be more sensible.
  4. Use all values in DT_FLAGS and DT_FLAGS_1 tags.
  5. Add a couple of comments for clarity.
grimar accepted this revision.Feb 27 2019, 8:02 AM

LGTM.

Higuoxing accepted this revision.Feb 27 2019, 8:18 AM
jhenderson added inline comments.Feb 27 2019, 8:33 AM
test/tools/llvm-readobj/elf-dynamic-tags.test
150 ↗(On Diff #188378)

I'm trying to keep these tests as minimal as possible, because there's already a lot of stuff in it. llvm-readobj does not require the link field, because it uses the dynamic tags.

This revision was automatically updated to reflect the committed changes.