Support MIPS64 .o files

[noamraph]

I added support for parsing mips64 .o files.

Testing

I'll start from the testing. I added a file test/testfiles_for_dwarfdump/dwarf_mips64el.o.elf.

I created this file using a "nix flake". To generate this file, install nix, enable flakes by running echo "experimental-features = nix-command flakes" > ~/.config/nix/nix.conf, go to the directory test/testfiles_for_dwarfdump/dwarf_mips64el and run nix build. Copy the file result/dwarf_mips64el.o into ../dwarf_mips64el.o.elf.

Annoyingly, I had to mess with the test. llvm-dwarfdump doesn't perform a relocation, and reports a location experssion to be DW_OP_addr 0x0, while pyelftools reports DW_OP_addr 0x4. I think that pyelftools is right, since it agrees with gcc's objdump. To verify, go to test/testfiles_for_dwarfdump/dwarf_mips64el and run:

nix develop -c bash -c 'mips64el-unknown-linux-gnuabi64-objdump --dwarf=info ../dwarf_mips64el.o.elf'

You get:

../dwarf_mips64el.o.elf:     file format elf64-tradlittlemips

Contents of the .debug_info section:
...
 <1><1e>: Abbrev Number: 1 (DW_TAG_variable)
...
    <28>   DW_AT_location    : 9 byte block: 3 4 0 0 0 0 0 0 0  (DW_OP_addr: 4)
...

To workaround this, I modified run_dwarfdump_tests.py to change the output of llvm-dwarfdump for this specific input.

Code

I added support for the R_MIPS_64 relocation type, in its simple form, where it defines just one relocation. I raised ELFRelocationError if it defines more than one relocation (as it can).

I changed _reloc_calc_sym_plus_value to add addend to its result. The test fails without it, and I think that if an addend is defined, it should always be added.

I removed the _RELOCATION_RECIPE_TYPE.has_addend field. The reason is that the R_MIPS_32 relocation sometimes has an addend and sometimes doesn't. This simplifies the code, and all tests pass, so I hope it's OK.

What do you think?

Thanks a lot for pyelftools! It helps me a lot!
Noam

[sevaa]

Create an issue in the LLVM bug tracker. Won't be the first time we point fingers at the reference tools.

[noamraph]

@sevaa after thinking about it, I think that there is no clear answer on whether relocations on globals should be performed when analyzing the .o file. The reason is that the relocated address (0x4 in this case) is still not the real address that the global would be in.

So I updated the example .o file to only include locals.

I now had to make a smaller change to run_dwarfdump_test.py: in one place, llvm-dwarfdump showed (0x0000000000000000 ".text") and pyelftools showed just (0x0000000000000000) (which I think makes sense). So I added a replacement from the first string to the other.

What do you think?

[eliben]

I'm not sure I understand why you removed has_addend, can you elaborate more (beyond what the PR description says)? Is this a change that can be discussed separately from your implementation to support MIPS64?

[noamraph]

Sure. I removed has_addend because it was the easiest way I found to make what I needed work.

See commit ec3d215 which is before removing has_addend. It has this test failing:

❯ venv/bin/python scripts/readelf.py --debug-dump=info test/testfiles_for_readelf/simple_mips_gcc.o.elf
Traceback (most recent call last):
  File "scripts/readelf.py", line 1852, in <module>
    main()
  File "scripts/readelf.py", line 1828, in main
    readelf.display_debug_dump(args.debug_dump_what)
  File "scripts/readelf.py", line 899, in display_debug_dump
    self._init_dwarfinfo()
  File "scripts/readelf.py", line 1104, in _init_dwarfinfo
    self._dwarfinfo = self.elffile.get_dwarf_info()
  File "./elftools/elf/elffile.py", line 277, in get_dwarf_info
    relocate_dwarf_sections)
  File "./elftools/elf/elffile.py", line 795, in _read_dwarf_section
    section_stream, reloc_section)
  File "./elftools/elf/relocation.py", line 216, in apply_section_relocations
    self._do_apply_relocation(stream, reloc, symtab)
  File "./elftools/elf/relocation.py", line 287, in _do_apply_relocation
    addend=reloc['r_addend'] if recipe.has_addend else 0)
  File "./elftools/elf/relocation.py", line 39, in __getitem__
    return self.entry[name]
  File "./elftools/construct/lib/container.py", line 35, in __getitem__
    return self.__dict__[name]
KeyError: 'r_addend'

This is because I changed ENUM_RELOC_TYPE_MIPS['R_MIPS_32'] to have has_addend=True. If I change it back to False, this fails:

❯ venv/bin/python test/run_dwarfdump_tests.py test/testfiles_for_dwarfdump/dwarf_mips64el.o.elf
Test file 'test/testfiles_for_dwarfdump/dwarf_mips64el.o.elf'
.......................FAIL
....for file test/testfiles_for_dwarfdump/dwarf_mips64el.o.elf
....for option "--debug-info"
....Output #1 is llvm-dwarfdump, Output #2 is pyelftools
@@ Mismatch on line #6:
>>              dw_at_name [dw_form_strp]	( .debug_str[0x000000dc] = "./dwarf_mips64el.c")<<
>>              dw_at_name [dw_form_strp]	( .debug_str[0x00000000] = "gnu c17 12.2.0 -mel -march=mips64r2 -mabi=64 -mllsc -mips64r2 -g -o2 -fpic -fstack-protector-strong -fno-strict-overflow -frandom-seed=kdyy43gwzl --param=ssp-buffer-size=4")<<
 ([('equal', 0, 61, 0, 61), ('replace', 61, 63, 61, 63), ('equal', 63, 68, 63, 68), ('insert', 68, 68, 68, 78), ('equal', 68, 69, 78, 79), ('replace', 69, 76, 79, 95), ('equal', 76, 82, 95, 101), ('replace', 82, 86, 101, 239), ('equal', 86, 88, 239, 241)])
@@ Output #1 dumped to file: /tmp/out-1-dwarf_mips64el.o.elf---debug-info-l4hdubyk.stdout
@@ Output #2 dumped to file: /tmp/out-2-dwarf_mips64el.o.elf---debug-info-ng5agd0v.stdout

Conclusion: FAIL

I have figured it was OK to remove this field from the recipe, since you can know if there is an r_addend field from the relocation object.

Anyway, I now created another branch without removing the r_addend field, which instead uses two recipe dicts instead of one: _RELOCATION_RECIPES_MIPS_RELA and _RELOCATION_RECIPES_MIPS_REL. It's here: https://github.com/noamraph/pyelftools/tree/mips64-o-with-has-addend

What do you think? Do you prefer the latter approach?

Thanks!
Noam

[eliben]

Could you please send your other branch as a separate PR? It should be easy to create a new PR from a different branch. A PR lets me see the delta/patch of the change much more conveniently than looking at a whole branch

[noamraph]

Sure. Here it is: #495

[eliben]

Closing, since #495 was merged