Determine elf arch by e_machine and bits #2177
Conversation
|
CHANGELOG.md says |
|
Please see why the ROP tests suddenly fail (I think this is something about how your changes influence GOT/PLT resolution). |
Head branch was pushed to by a user without write access
Unfortunately, I can't reproduce any of the failures on my machine. Could you try a reproduction please? |
|
Cool stuff, used it already during hack-a-sat. I have a follow up patch ready which fixes shellcode assembly and debugging. |
The name is picked from gcc target triple or |
|
Alright, this is definitely a new feature, not a bugfix, so it should go in dev. |
Separate 32/64 for riscv and mips Trim some trailing space
Co-authored-by: Arusekk <arek_koz@o2.pl>
|
Also, is rv64 ISA really different from rv32? Or should they just be both treated as |
They may share the same shellcode for some commands, but I don't think all would work. |
| 'riscv': little_32, | ||
| 'riscv32': little_32, | ||
| 'riscv64': little_64, |
There was a problem hiding this comment.
Also, is rv64 ISA really different from rv32? Or should they just be both treated as riscv by pwntools code? I think they can have common shellcodes in the future etc, and bits is used anyway in all places to distinguish architectures.
And what should we do here? If hard-coded riscv as 32 or 64, ppl have to set context.bits manually to work for the other one.
|
Just as they need to do anyway with e.g. mips endianness. The recommended way to set everything automatically is using context.binary anyway.
Or we can add aliases rv32/rv64/riscv32/riscv64 to set both architecture and bits.
|
Alright. Then could you introduce a right approach for adding aliases? Seems to need some refactoring. |
They are broken since the riscv architecture was split into riscv32 and riscv64 internally in Gallopsled#2177.
* Fix debugging and assembly for RISCV They are broken since the riscv architecture was split into riscv32 and riscv64 internally in #2177. * Always look for riscv32. riscv64 or riscv toolchains * Look for riscv64 for riscv64 first
* Determine elf arch by e_machine and bits Separate 32/64 for riscv and mips Trim some trailing space * Fix missing comma Co-authored-by: Arusekk <arek_koz@o2.pl> * Add CHANGELOG * unicorn: Use fixed version * Cosmetic enhancements --------- Co-authored-by: Arusekk <arek_koz@o2.pl>
* Fix debugging and assembly for RISCV They are broken since the riscv architecture was split into riscv32 and riscv64 internally in Gallopsled#2177. * Always look for riscv32. riscv64 or riscv toolchains * Look for riscv64 for riscv64 first
Pwntools Pull Request
Thanks for contributing to Pwntools! Take a moment to look at
CONTRIBUTING.mdto make sure you're familiar with Pwntools development.In context module, riscv32 and riscv64 need to be assigned arch separately.
In elf module, riscv32 and riscv64 share the same e_machine, EM_RISCV, and we used e_machine to determine arch directly, which does not work for RISC-V (and MIPS).
Change to determining elf arch by e_machine and bits.
Separate 32/64 for riscv and mips.
Testing
Pull Requests that introduce new code should try to add doctests for that code. See
TESTING.mdfor more information.Target Branch
Depending on what the PR is for, it needs to target a different branch.
You can always change the branch after you create the PR if it's against the wrong branch.
devdevstablestablebranchbetabetabranch, but notstabledevChangelog
After creating your Pull Request, please add and push a commit that updates the changelog for the appropriate branch.
You can look at the existing changelog for examples of how to do this.