Fix the LoongArch support code and some more #483
Conversation
xen0n
force-pushed
the
fix-loong-support
branch
from
bacac9b
to
fb3c8d2
Compare
|
We don't have any LoongArch binaries to autotest against, do we? |
|
I tried to add some tests but IIRC the readelf test failed because the prebuilt binary is too old to recognize some of the new LoongArch relocs (or the architecture itself), and at that time I don't have enough time to look into this. I may be able to follow up a few days later though. |
|
It may be time to bump the pre-built readelf to recognize LoongArch; otherwise I feel like we're stretching it without real files to test against. |
|
The latest readelf will break a whole bunch of autotests, at least on the DWARF side. There is a reason we carry around our own. That said, that bullet needs to be eventually bitten. |
|
@xen0n: Try grabbing an updated copy of |
|
I'm rebasing this PR, hoping to post an updated revision this week. |
|
@xen0n if you want this PR to make it into the next release of pyelftools, please address the comments soon |
Thanks for the reminder; I'll try to do this tonight. |
|
Update: |
xen0n
force-pushed
the
fix-loong-support
branch
from
fb3c8d2
to
c181bde
Compare
The e_machine constant is EM_LOONGARCH, and the emulation name is just
elf{32,64}-loongarch without the endian prefix.
Fixes: 6c36d79 ("add support for loongarch64 to dwarfdump (eliben#458)")
Signed-off-by: WANG Xuerui <[email protected]>
The current code gets the logic right, but not the symbol names. Fix them for consistency with the canonical definition that's binutils. Fixes: 2059475 ("Add support for LoongArch (eliben#470)") Signed-off-by: WANG Xuerui <[email protected]>
Fixes: 2059475 ("Add support for LoongArch (eliben#470)") Signed-off-by: WANG Xuerui <[email protected]>
Signed-off-by: WANG Xuerui <[email protected]>
Signed-off-by: WANG Xuerui <[email protected]>
xen0n
force-pushed
the
fix-loong-support
branch
from
c181bde
to
05e4b53
Compare
|
The basic ELF/LoongArch binary is a part of the autotest, the relocations is the only architecture specific part that needed any nontrivial treatment and that's done. Friendly register names on RISC platforms, as we've concluded, are nowhere on GNU binutils' radar and therefore shouldn't be on pyelftools', either. :) :( I think this is good to go. 謝謝 |
elftools/elf/relocation.py
Outdated
| @@ -322,6 +331,12 @@ def _reloc_calc_sym_plus_addend(value, sym_value, offset, addend=0): | |||
| def _reloc_calc_sym_plus_addend_pcrel(value, sym_value, offset, addend=0): | |||
| return sym_value + addend - offset | |||
|
|
|||
| def _reloc_calc_value_plus_sym_addend(value, sym_value, offset, addend=0): | |||
There was a problem hiding this comment.
If you rebase this PR, one of the recent commits made _reloc_calc_sym_plus_value do the same thing -- can that be used instead?
| @@ -95,6 +96,11 @@ def _get_cu_base(cu): | |||
| else: | |||
| raise ValueError("Can't find the base IP (low_pc) for a CU") | |||
|
|
|||
| _CONTROL_CHAR_RE = re.compile(r'[\x01-\x1f]') | |||
There was a problem hiding this comment.
Please add a comment explaining what this is for
There was a problem hiding this comment.
I've added an explanation for it (meant for use in _format_symbol_name).
Signed-off-by: WANG Xuerui <[email protected]>
This is necessary to match readelf behavior on fake symbol names, that usually look like "L0^A" when rendered (being "L0\x01" in reality). Signed-off-by: WANG Xuerui <[email protected]>
…interp According to binutils sources (function frame_display_row in binutils/dwarf.c), the apparent ordering of the ra register after other registers is merely a side effect of most architectures allocating a larger DWARF register number for their respective ra registers. This has no effect on all readelf test cases, but is necessary for a future LoongArch test binary to pass comparisons. Signed-off-by: WANG Xuerui <[email protected]>
Signed-off-by: WANG Xuerui <[email protected]>
xen0n
force-pushed
the
fix-loong-support
branch
from
05e4b53
to
a7ecce7
Compare
There were #458 and #470, but unfortunately both were partly incorrect. I've fixed them and added some more support for LoongArch relocations while at it.