Fixed found while adding type hints #602

pmhahn · 2025-02-27T17:35:20Z

While working on #514 mypy found several issues, which I fixed myself.

You might want to handle this differently (in the future):

AssertionError: Unhandled augmentation string: b'armcc+'

arm-eabi-attr-names.o.elf is based on attr-names.s from binutils-gdb; does that need additional attribution?

Please check carefully as I might have introduced other issues.

Fixes: e972a57 ("Supplementary object files (eliben#426)") Signed-off-by: Philipp Hahn <[email protected]>

`get_section_by_name()` might return `None` in case architecture specific dumping is requested: ```console $ scripts/readelf.py --arch-specific test/testfiles_for_unittests/arm_with_form_indirect.elf … Traceback (most recent call last): File "scripts/readelf.py", line 1986, in <module> main() File "scripts/readelf.py", line 1956, in main readelf.display_arch_specific() File "scripts/readelf.py", line 800, in display_arch_specific self._display_arch_specific_arm() File "scripts/readelf.py", line 1832, in _display_arch_specific_arm self._display_attributes(attr_sec, describe_attr_tag_arm) File "scripts/readelf.py", line 1818, in _display_attributes for s in attr_sec.iter_subsections(): ^^^^^^^^^^^^^^^^^^^^^^^^^ AttributeError: 'NoneType' object has no attribute 'iter_subsections' ``` Fixes: 906f286 ("ARM build attributes section parsing (eliben#160)") Fixes: 43e7771 ("Add support for RISC-V attributes (eliben#459)") Signed-off-by: Philipp Hahn <[email protected]>

```console $ scripts/readelf.py --debug-dump frames test/testfiles_for_unittests/arm_with_form_indirect.elf … Traceback (most recent call last): File "scripts/readelf.py", line 1988, in <module> main() File "scripts/readelf.py", line 1964, in main readelf.display_debug_dump(args.debug_dump_what) File "scripts/readelf.py", line 913, in display_debug_dump self._dump_debug_frames() File "scripts/readelf.py", line 1381, in _dump_debug_frames self._dwarfinfo.CFI_entries()) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "elftools/dwarf/dwarfinfo.py", line 378, in CFI_entries return cfi.get_entries() ^^^^^^^^^^^^^^^^^ File "elftools/dwarf/callframe.py", line 79, in get_entries self.entries = self._parse_entries() ^^^^^^^^^^^^^^^^^^^^^ File "elftools/dwarf/callframe.py", line 88, in _parse_entries entries.append(self._parse_entry_at(offset)) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "elftools/dwarf/callframe.py", line 144, in _parse_entry_at aug_bytes, aug_dict = self._parse_cie_augmentation( ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "elftools/dwarf/callframe.py", line 284, in _parse_cie_augmentation assert augmentation.startswith(b'z'), ( ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ AssertionError: Unhandled augmentation string: b'armcc+' ``` See `decode_frame_entry_1()` from [binutils:gdb/dwarf2/frame.c:1790-1793](https://github.com/bminor/binutils-gdb/blob/master/gdb/dwarf2/frame.c#L1790) Signed-off-by: Philipp Hahn <[email protected]>

eliben · 2025-02-28T13:31:03Z

scripts/readelf.py

@@ -815,15 +815,14 @@ def display_hex_dump(self, section_spec):
            sys.stderr.write('readelf: Warning: Section \'%s\' was not dumped because it does not exist!\n' % (
                section_spec))
            return
-        if section['sh_type'] == 'SHT_NOBITS':
-            self._emitline("\nSection '%s' has no data to dump." % (
+        if section['sh_type'] == 'SHT_NOBITS' or not (data := section.data()):


Maybe I'm missing something, but what's the point of data := ... here? You're not using data inside the condition body

I moved the data = section.data() here from 3 lines below: Otherwise I would have to either repeat the output:

if section['sh_type'] == 'SHT_NOBITS': self._emitline("\nSection '%s' has no data to dump." % …) data = section.data() if not data: self._emitline("\nSection '%s' has no data to dump." % …)

or always call section.data() even in the case where sh_type = SHT_NOBITS:

data = section.data() if section['sh_type'] == 'SHT_NOBITS' or not data: self._emitline("\nSection '%s' has no data to dump." % …)

By using the walrus operator I get both: no repetition and the lazy calling. YMMV

eliben · 2025-02-28T13:37:14Z

elftools/elf/descriptions.py

@@ -237,30 +237,30 @@ def describe_note(x, machine):


 def describe_attr_tag_arm(tag, val, extra):
+    s = _DESCR_ATTR_TAG_ARM.get(tag, '"%s"' % tag)


Why do we have this? Unknown tags not defined in _DESCR_ATTR_TAG_ARM?

Also, what's the point of the string interpolation? Isn't tag already a string?

Yes, handling the unknown tags case. I copied the style from other locations where the value is put into quotes to show, that this is a raw value and not some looked up string from _DESCR_ATTR_TAG_ARM.
I can change this to s = _DESCR_ATTR_TAG_ARM.get(tag, tag) is you prefer.

eliben · 2025-02-28T13:38:38Z

If you add a new binary test file, try to add a source file alongside it (.s or .c etc.) with a comment on top explaining where it's from and how to convert it to the binary

```console $ elf='test/testfiles_for_readelf/arm-eabi-attr-names.o.elf' $ diff <(LC_ALL=C readelf -A "$elf") <(scripts/readelf.py -A "$elf") 10c10 < Tag_THUMB_ISA_use: Thumb-1 --- > Tag_Thumb_ISA_use: Thumb-1 16,17c16,17 < Tag_ABI_PCS_RW_data: PC-relative < Tag_ABI_PCS_RO_data: PC-relative --- > Tag_ABI_PCS_RW_use: PC-relative > Tag_ABI_PCS_RO_use: PC-relative 33c33 < Tag_compatibility: flag = 1, vendor = gnu --- > Tag_compatibility: Yes ``` ELF file was built using `arm-linux-gnueabi-gcc -c` using <https://sourceware.org/git?p=binutils-gdb.git;a=blob;f=gas/testsuite/gas/arm/attr-names.s;h=c43fb88f67b7be158cceb13cb84caddaa7cd47b8;hb=HEAD> Fixes: 906f286 ("ARM build attributes section parsing (eliben#160)") Signed-off-by: Philipp Hahn <[email protected]>

```console $ elf='test/testfiles_for_readelf/arm-eabi-attr-names.o.elf' $ diff <(LC_ALL=C readelf -x.text "$elf") <(scripts/readelf.py -x.text "$elf") 1c1,3 < Section '.text' has no data to dump. --- > > Hex dump of section '.text': > ``` Fixes: c50de91 ("Issue eliben#119: readelf - don't do hex/string dumps for SHT_NOBITS sections") Signed-off-by: Philipp Hahn <[email protected]>

`d_entry[…]` will fail when `d_entry is None`: ```python def describe_attr_tag_arm(…): d_entry = … if d_entry is None: … return … + d_entry[val] ``` > Value of type "None" is not indexable See <https://github.com/ARM-software/abi-aa/blob/main/addenda32/addenda32.rst#secondary-compatibility-tag>. Similar to <https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=binutils/readelf.c;h=70d0c533f91838351c1963d4464bca71738cb245;hb=HEAD#l18228> dump the architecture. Fixes: 906f286 ("ARM build attributes section parsing (eliben#160)") Signed-off-by: Philipp Hahn <[email protected]>

Refactor code and lookup `_DESCR_ATTR_TAG_ARM` only once for all cases. Also add a fallback in case the tag is not known. Signed-off-by: Philipp Hahn <[email protected]>

Iterator use `yield` and must not return anything other than `None`. Fixes: b0d7a76 ("Support getting RELR relocations from dynamic section (eliben#509)") Signed-off-by: Philipp Hahn <[email protected]>

pmhahn · 2025-03-02T08:10:56Z

If you add a new binary test file, try to add a source file alongside it (.s or .c etc.) with a comment on top explaining where it's from and how to convert it to the binary

I have added the source file with instructions on how to compile it.

eliben

Thanks for the clarifications
LGTM

This was referenced Feb 27, 2025

Add static typing #514

Open

Supplementary object files #426

Merged

pmhahn added 3 commits February 28, 2025 09:37

Fix supplementary_dwarfinfo.get_string_from{→ _}table

84ef966

Fixes: e972a57 ("Supplementary object files (eliben#426)") Signed-off-by: Philipp Hahn <[email protected]>

pmhahn force-pushed the fixes branch from 4e4629b to 64eafd5 Compare February 28, 2025 08:43

eliben reviewed Feb 28, 2025

View reviewed changes

pmhahn added 5 commits March 2, 2025 09:10

Refactor describe_attr_tag_arm

bc967b2

Refactor code and lookup `_DESCR_ATTR_TAG_ARM` only once for all cases. Also add a fallback in case the tag is not known. Signed-off-by: Philipp Hahn <[email protected]>

Fix iter_relocations

15b3fb7

Iterator use `yield` and must not return anything other than `None`. Fixes: b0d7a76 ("Support getting RELR relocations from dynamic section (eliben#509)") Signed-off-by: Philipp Hahn <[email protected]>

pmhahn force-pushed the fixes branch from 64eafd5 to 15b3fb7 Compare March 2, 2025 08:10

eliben approved these changes Mar 3, 2025

View reviewed changes

eliben merged commit 6c3623a into eliben:main Mar 3, 2025
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed found while adding type hints #602

Fixed found while adding type hints #602

pmhahn commented Feb 27, 2025

eliben Feb 28, 2025

pmhahn Mar 2, 2025

eliben Feb 28, 2025

pmhahn Mar 2, 2025

eliben commented Feb 28, 2025

pmhahn commented Mar 2, 2025

eliben left a comment

		@@ -237,30 +237,30 @@ def describe_note(x, machine):


		def describe_attr_tag_arm(tag, val, extra):
		s = _DESCR_ATTR_TAG_ARM.get(tag, '"%s"' % tag)

Fixed found while adding type hints #602

Fixed found while adding type hints #602

Conversation

pmhahn commented Feb 27, 2025

eliben Feb 28, 2025

Choose a reason for hiding this comment

pmhahn Mar 2, 2025

Choose a reason for hiding this comment

eliben Feb 28, 2025

Choose a reason for hiding this comment

pmhahn Mar 2, 2025

Choose a reason for hiding this comment

eliben commented Feb 28, 2025

pmhahn commented Mar 2, 2025

eliben left a comment

Choose a reason for hiding this comment