converted elf_analysis plugin to new base class #1266
Conversation
jstucke
force-pushed
the
elf-analysis-plugin-v1
branch
from
c14b112 to
b307c7e
Compare
jstucke
force-pushed
the
elf-analysis-plugin-v1
branch
from
b307c7e to
afec839
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1266 +/- ##
==========================================
- Coverage 92.48% 91.86% -0.62%
==========================================
Files 379 376 -3
Lines 24115 21044 -3071
==========================================
- Hits 22302 19332 -2970
+ Misses 1813 1712 -101 ☔ View full report in Codecov by Sentry. |
jstucke
force-pushed
the
elf-analysis-plugin-v1
branch
from
700a7d4 to
64db148
Compare
jstucke
force-pushed
the
elf-analysis-plugin-v1
branch
from
64db148 to
cb519ee
Compare
|
I made some additional changes:
|
| def _deduplicate_functions(lief_functions: Iterable[lief.Function]) -> list[tuple[int, str]]: | ||
| return sorted({(f.address, f.name) for f in lief_functions}) |
There was a problem hiding this comment.
Can you file a bug report upstream and put a link to it in the code?
Then when someone stumbles upon this code again, they can see if it is still needed.
There was a problem hiding this comment.
I'm actually not sure whether or not this is intended. In the old plugin this wasn't an issue, since it also "deduplicated" the results using sets, so in the plugin this is a new bug
| exported_functions: List[ElfSymbol] | ||
| imported_functions: List[ElfSymbol] | ||
| libraries: List[str] | ||
| mod_info: Optional[List[str]] = Field(description='Key value pairs with module information.') |
There was a problem hiding this comment.
Could you reword the description and make more clear that this is related to Linux kernel modules (Isn't it?).
To me the description does not convey much information that is not conveyed by the type and name of the field.
Also, how do 'key value pairs' fit in an Optinal[List[str]] ?
There was a problem hiding this comment.
I agree, a list of key value pairs doesn't make a lot of sense. I converted it to a dict.
jstucke
force-pushed
the
elf-analysis-plugin-v1
branch
from
cb519ee to
ed406c1
Compare
jstucke
force-pushed
the
elf-analysis-plugin-v1
branch
from
ed406c1 to
24c1c7e
Compare
and also removed the redundant normalize_lief_items method
jstucke
force-pushed
the
elf-analysis-plugin-v1
branch
from
24c1c7e to
5b1d342
Compare
| def _deduplicate_functions(lief_functions: Iterable[lief.Function]) -> list[tuple[int, str]]: | ||
| return sorted({(f.address, f.name) for f in lief_functions}) |
| entrypoint=header.entrypoint, | ||
| file_type=header.file_type.__name__, | ||
| header_size=header.header_size, | ||
| identity_abi_version=header.identity_abi_version, | ||
| identity_class=header.identity_class.__name__, | ||
| identity_data=header.identity_data.__name__, | ||
| identity_os_abi=header.identity_os_abi.__name__, | ||
| identity_version=header.identity_version.__name__, | ||
| machine_type=header.machine_type.__name__.lower(), | ||
| numberof_sections=header.numberof_sections, | ||
| numberof_segments=header.numberof_segments, | ||
| object_file_version=header.object_file_version.__name__, | ||
| processor_flag=header.processor_flag, | ||
| program_header_size=header.program_header_size, | ||
| program_headers_offset=header.program_header_offset, | ||
| section_header_size=header.section_header_size, | ||
| section_headers_offset=header.section_header_offset, | ||
| section_name_table_idx=header.section_name_table_idx, |
There was a problem hiding this comment.
Not a huge fan of using __name__ to convert the constants to human readable names.
The lief names are not documented somehwere accessible (are they?).
For example, the header.file_type as defined in the typing stubs does not have any documentation.
Furthermore, the lief names do not match the names in the official specification.
I'm not sure what the right way to proceed is here. From a GUI user perspective human readable names are important. From a API user perspective the numeric values would be preferable.
Also, why does this work at all? It does not work with standard enum.Enum's and is not defined in lief's typing stubs.
There was a problem hiding this comment.
Not a huge fan of using name to convert the constants to human readable names.
The lief names are not documented somehwere accessible (are they?).
neither am I, but sadly this seems to be the only way to reproduce the old output. The alternative would be this: #1266 (comment)
I'm not sure what the right way to proceed is here. From a GUI user perspective human readable names are important. From a API user perspective the numeric values would be preferable.
since the barrier for a GUI user to work with numeric values is arguably higher, IMHO using strings is the better approach
Also, why does this work at all? It does not work with standard enum.Enum's and is not defined in lief's typing stubs.
It probably has something to do with the fact that lief is not actually python code, but rather a shared library (.so) with python bindings
I'm not sure this is the problem here. In cases of duplicate entries, every entry seems to have a duplicate (with the same offset). I'm still not sure why |
No description provided.