Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 34 additions & 31 deletions elftools/elf/relocation.py
Original file line number Diff line number Diff line change
Expand Up @@ -240,9 +240,10 @@ def _do_apply_relocation(self, stream, reloc, symtab):
'Unexpected REL relocation for x64: %s' % reloc)
recipe = self._RELOCATION_RECIPES_X64.get(reloc_type, None)
elif self.elffile.get_machine_arch() == 'MIPS':
if reloc.is_RELA():
raise ELFRelocationError(
'Unexpected RELA relocation for MIPS: %s' % reloc)
if reloc_type == ENUM_RELOC_TYPE_MIPS['R_MIPS_64']:
if reloc['r_type2'] != 0 or reloc['r_type3'] != 0 or reloc['r_ssym'] != 0:
raise ELFRelocationError(
'Multiple relocations in R_MIPS_64 are not implemented: %s' % reloc)
recipe = self._RELOCATION_RECIPES_MIPS.get(reloc_type, None)
elif self.elffile.get_machine_arch() == 'ARM':
if reloc.is_RELA():
Expand Down Expand Up @@ -283,7 +284,7 @@ def _do_apply_relocation(self, stream, reloc, symtab):
value=original_value,
sym_value=sym_value,
offset=reloc['r_offset'],
addend=reloc['r_addend'] if recipe.has_addend else 0)
addend=reloc.entry.get('r_addend', 0))
# 3. Write the relocated value back into the stream
stream.seek(reloc['r_offset'])

Expand All @@ -296,18 +297,17 @@ def _do_apply_relocation(self, stream, reloc, symtab):
# Relocations are represented by "recipes". Each recipe specifies:
# bytesize: The number of bytes to read (and write back) to the section.
# This is the unit of data on which relocation is performed.
# has_addend: Does this relocation have an extra addend?
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. Here it is: #495

# calc_func: A function that performs the relocation on an extracted
# value, and returns the updated value.
#
_RELOCATION_RECIPE_TYPE = namedtuple('_RELOCATION_RECIPE_TYPE',
'bytesize has_addend calc_func')
'bytesize calc_func')

def _reloc_calc_identity(value, sym_value, offset, addend=0):
return value

def _reloc_calc_sym_plus_value(value, sym_value, offset, addend=0):
return sym_value + value
return sym_value + value + addend

def _reloc_calc_sym_plus_value_pcrel(value, sym_value, offset, addend=0):
return sym_value + value - offset
Expand All @@ -326,80 +326,83 @@ def _bpf_64_32_reloc_calc_sym_plus_addend(value, sym_value, offset, addend=0):

_RELOCATION_RECIPES_ARM = {
ENUM_RELOC_TYPE_ARM['R_ARM_ABS32']: _RELOCATION_RECIPE_TYPE(
bytesize=4, has_addend=False,
bytesize=4,
calc_func=_reloc_calc_sym_plus_value),
ENUM_RELOC_TYPE_ARM['R_ARM_CALL']: _RELOCATION_RECIPE_TYPE(
bytesize=4, has_addend=False,
bytesize=4,
calc_func=_arm_reloc_calc_sym_plus_value_pcrel),
}

_RELOCATION_RECIPES_AARCH64 = {
ENUM_RELOC_TYPE_AARCH64['R_AARCH64_ABS64']: _RELOCATION_RECIPE_TYPE(
bytesize=8, has_addend=True, calc_func=_reloc_calc_sym_plus_addend),
bytesize=8, calc_func=_reloc_calc_sym_plus_addend),
ENUM_RELOC_TYPE_AARCH64['R_AARCH64_ABS32']: _RELOCATION_RECIPE_TYPE(
bytesize=4, has_addend=True, calc_func=_reloc_calc_sym_plus_addend),
bytesize=4, calc_func=_reloc_calc_sym_plus_addend),
ENUM_RELOC_TYPE_AARCH64['R_AARCH64_PREL32']: _RELOCATION_RECIPE_TYPE(
bytesize=4, has_addend=True,
bytesize=4,
calc_func=_reloc_calc_sym_plus_addend_pcrel),
}

# https://dmz-portal.mips.com/wiki/MIPS_relocation_types
_RELOCATION_RECIPES_MIPS = {
ENUM_RELOC_TYPE_MIPS['R_MIPS_NONE']: _RELOCATION_RECIPE_TYPE(
bytesize=4, has_addend=False, calc_func=_reloc_calc_identity),
bytesize=4, calc_func=_reloc_calc_identity),
ENUM_RELOC_TYPE_MIPS['R_MIPS_32']: _RELOCATION_RECIPE_TYPE(
bytesize=4, has_addend=False,
bytesize=4,
calc_func=_reloc_calc_sym_plus_value),
ENUM_RELOC_TYPE_MIPS['R_MIPS_64']: _RELOCATION_RECIPE_TYPE(
bytesize=8,
calc_func=_reloc_calc_sym_plus_value),
}

_RELOCATION_RECIPES_PPC64 = {
ENUM_RELOC_TYPE_PPC64['R_PPC64_ADDR32']: _RELOCATION_RECIPE_TYPE(
bytesize=4, has_addend=True, calc_func=_reloc_calc_sym_plus_addend),
bytesize=4, calc_func=_reloc_calc_sym_plus_addend),
ENUM_RELOC_TYPE_PPC64['R_PPC64_REL32']: _RELOCATION_RECIPE_TYPE(
bytesize=4, has_addend=True, calc_func=_reloc_calc_sym_plus_addend_pcrel),
bytesize=4, calc_func=_reloc_calc_sym_plus_addend_pcrel),
ENUM_RELOC_TYPE_PPC64['R_PPC64_ADDR64']: _RELOCATION_RECIPE_TYPE(
bytesize=8, has_addend=True, calc_func=_reloc_calc_sym_plus_addend),
bytesize=8, calc_func=_reloc_calc_sym_plus_addend),
}

_RELOCATION_RECIPES_X86 = {
ENUM_RELOC_TYPE_i386['R_386_NONE']: _RELOCATION_RECIPE_TYPE(
bytesize=4, has_addend=False, calc_func=_reloc_calc_identity),
bytesize=4, calc_func=_reloc_calc_identity),
ENUM_RELOC_TYPE_i386['R_386_32']: _RELOCATION_RECIPE_TYPE(
bytesize=4, has_addend=False,
bytesize=4,
calc_func=_reloc_calc_sym_plus_value),
ENUM_RELOC_TYPE_i386['R_386_PC32']: _RELOCATION_RECIPE_TYPE(
bytesize=4, has_addend=False,
bytesize=4,
calc_func=_reloc_calc_sym_plus_value_pcrel),
}

_RELOCATION_RECIPES_X64 = {
ENUM_RELOC_TYPE_x64['R_X86_64_NONE']: _RELOCATION_RECIPE_TYPE(
bytesize=8, has_addend=True, calc_func=_reloc_calc_identity),
bytesize=8, calc_func=_reloc_calc_identity),
ENUM_RELOC_TYPE_x64['R_X86_64_64']: _RELOCATION_RECIPE_TYPE(
bytesize=8, has_addend=True, calc_func=_reloc_calc_sym_plus_addend),
bytesize=8, calc_func=_reloc_calc_sym_plus_addend),
ENUM_RELOC_TYPE_x64['R_X86_64_PC32']: _RELOCATION_RECIPE_TYPE(
bytesize=4, has_addend=True,
bytesize=4,
calc_func=_reloc_calc_sym_plus_addend_pcrel),
ENUM_RELOC_TYPE_x64['R_X86_64_32']: _RELOCATION_RECIPE_TYPE(
bytesize=4, has_addend=True, calc_func=_reloc_calc_sym_plus_addend),
bytesize=4, calc_func=_reloc_calc_sym_plus_addend),
ENUM_RELOC_TYPE_x64['R_X86_64_32S']: _RELOCATION_RECIPE_TYPE(
bytesize=4, has_addend=True, calc_func=_reloc_calc_sym_plus_addend),
bytesize=4, calc_func=_reloc_calc_sym_plus_addend),
}

# https://www.kernel.org/doc/html/latest/bpf/llvm_reloc.html#different-relocation-types
_RELOCATION_RECIPES_EBPF = {
ENUM_RELOC_TYPE_BPF['R_BPF_NONE']: _RELOCATION_RECIPE_TYPE(
bytesize=8, has_addend=False, calc_func=_reloc_calc_identity),
bytesize=8, calc_func=_reloc_calc_identity),
ENUM_RELOC_TYPE_BPF['R_BPF_64_64']: _RELOCATION_RECIPE_TYPE(
bytesize=8, has_addend=False, calc_func=_reloc_calc_identity),
bytesize=8, calc_func=_reloc_calc_identity),
ENUM_RELOC_TYPE_BPF['R_BPF_64_32']: _RELOCATION_RECIPE_TYPE(
bytesize=8, has_addend=False, calc_func=_bpf_64_32_reloc_calc_sym_plus_addend),
bytesize=8, calc_func=_bpf_64_32_reloc_calc_sym_plus_addend),
ENUM_RELOC_TYPE_BPF['R_BPF_64_NODYLD32']: _RELOCATION_RECIPE_TYPE(
bytesize=4, has_addend=False, calc_func=_reloc_calc_identity),
bytesize=4, calc_func=_reloc_calc_identity),
ENUM_RELOC_TYPE_BPF['R_BPF_64_ABS64']: _RELOCATION_RECIPE_TYPE(
bytesize=8, has_addend=False, calc_func=_reloc_calc_identity),
bytesize=8, calc_func=_reloc_calc_identity),
ENUM_RELOC_TYPE_BPF['R_BPF_64_ABS32']: _RELOCATION_RECIPE_TYPE(
bytesize=4, has_addend=False, calc_func=_reloc_calc_identity),
bytesize=4, calc_func=_reloc_calc_identity),
}


2 changes: 1 addition & 1 deletion scripts/dwarfdump.py
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ def __init__(self, filename, file, output):
self.elffile = ELFFile(file)
self.output = output
self._dwarfinfo = self.elffile.get_dwarf_info()
arches = {"EM_386": "i386", "EM_X86_64": "x86-64", "EM_ARM": "littlearm", "EM_AARCH64": "littleaarch64", "EM_LOONGARCH64": "loongarch64", "EM_RISCV": "littleriscv"}
arches = {"EM_386": "i386", "EM_X86_64": "x86-64", "EM_ARM": "littlearm", "EM_AARCH64": "littleaarch64", "EM_LOONGARCH64": "loongarch64", "EM_RISCV": "littleriscv", "EM_MIPS": "mips"}
arch = arches[self.elffile['e_machine']]
bits = self.elffile.elfclass
self._emitline("%s: file format elf%d-%s" % (filename, bits, arch))
Expand Down
7 changes: 5 additions & 2 deletions test/run_dwarfdump_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,11 +102,14 @@ def compare_output(s1, s2):
and errmsg is empty. Otherwise success is False and errmsg holds a
description of the mismatch.
"""
def remove_zero_address_symbol(s):
return s.replace('(0x0000000000000000 ".text")', '(0x0000000000000000)')

def prepare_lines(s):
return [line for line in s.lower().splitlines() if line.strip() != '']

lines1 = prepare_lines(s1)
lines2 = prepare_lines(s2)
lines1 = prepare_lines(remove_zero_address_symbol(s1))
lines2 = prepare_lines(remove_zero_address_symbol(s2))

if len(lines1) != len(lines2):
return False, 'Number of lines different: %s vs %s' % (
Expand Down
Binary file added test/testfiles_for_dwarfdump/dwarf_mips64el.o.elf
Binary file not shown.
1 change: 1 addition & 0 deletions test/testfiles_for_dwarfdump/dwarf_mips64el/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
/result
5 changes: 5 additions & 0 deletions test/testfiles_for_dwarfdump/dwarf_mips64el/dwarf_mips64el.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
int f() {
int var1 = 3;
int var2 = 5;
return var1 + var2;
}
27 changes: 27 additions & 0 deletions test/testfiles_for_dwarfdump/dwarf_mips64el/flake.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 18 additions & 0 deletions test/testfiles_for_dwarfdump/dwarf_mips64el/flake.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
description = "A flake for building a mips64el .o file for testing pyelftools";

inputs.nixpkgs.url = github:NixOS/nixpkgs/nixos-23.05;

outputs = { self, nixpkgs }: {

defaultPackage.x86_64-linux =
with (import nixpkgs { system = "x86_64-linux"; }).pkgsCross.mips64el-linux-gnuabi64;
stdenv.mkDerivation {
name = "dwarf_mips64el";
src = self;
buildPhase = "$CC -g -c ./dwarf_mips64el.c";
installPhase = "mkdir -p $out; cp dwarf_mips64el.o $out/";
};

};
}