search

eliben / pyelftools

Parsing ELF and DWARF in Python
Other
2.03k stars 512 forks source link

24 bit integer support, forms strx3, addrx3 #553

Closed sevaa closed 7 months ago

sevaa commented 7 months ago

Forms strx3, addrx3 were both TODOs.

I don't have a binary with either of those forms, but I have a crash report along the lines of KeyError: DW_FORM_strx3, so it exists in the wild.

There are currently no forms that correspond to signed 24 bit ints.

This functionality, I believe, was the original reason for the attempted migration to construct 2.10.

While at it, I've noticed some gaps in the the enums. I thought we concluded that we don't need a sample binary for every single constant. The LLVM vendor extensions for LNCT come from https://llvm.org/docs/AMDGPUDwarfExtensionsForHeterogeneousDebugging.html .

sevaa commented 7 months ago

Removed the global declarations. There might be a teeny tiny hit on performance.

sevaa commented 7 months ago

Speaking of possible performance enhancements. We have a bunch of statements here and there along the lines of:

x = struct_parse(structs.Dwarf_uleb128(''), stream)

where there is an unnecessary overhead of constructing a new instance of the stateless ULEB128 parser. We could wring some performance out of constructing one early on (maybe even globally :) ) and reusing it throughout. What do you think, and if so, what should we call it?

eliben commented 7 months ago

Speaking of possible performance enhancements. We have a bunch of statements here and there along the lines of:

x = struct_parse(structs.Dwarf_uleb128(''), stream)

where there is an unnecessary overhead of constructing a new instance of the stateless ULEB128 parser. We could wring some performance out of constructing one early on (maybe even globally :) ) and reusing it throughout. What do you think, and if so, what should we call it?

We should prove it actually matters for performance first :)

sevaa commented 7 months ago

We should prove it actually matters for performance first

UPD: it does. Replacing construction with instance reuse in just one place - the line where we read the DIE abbrev code - reduced the firehose test time from 4 sec to 3.5. It's on the hottest path, that's why it was in my sights.