cfg.bigendian not honored if set before asm.arch

[queueRAM]

It appears that cfg.bigendian isn't propagated to the new arch when the architecture is changed and must be set again after switching asm.arch.

As an example, set e cfg.bigendian=true then e asm.arch=mips and write a MIPS assembly instruction addiu sp, sp, -24 which should be 27bdffe8 in big-endian. The first wa and pd commands appear to both be operating under little-endian mode, despite e cfg.bigendian confirmed to be true

$ radare2 malloc://16
[0x00000000]> e cfg.bigendian=true
[0x00000000]> e asm.arch=mips
[0x00000000]> e asm.bits=32
[0x00000000]> e cfg.bigendian
true
[0x00000000]> wa addiu sp, sp, -24
Written 4 bytes (addiu sp, sp, -24) = wx e8ffbd27
[0x00000000]> pd 1
            0x00000000      e8ffbd27       addiu sp, sp, -0x18
[0x00000000]> e cfg.bigendian=true
[0x00000000]> pd 1
            0x00000000      e8ffbd27       swc2 31, -0x42d9(a3)
[0x00000000]> wa addiu sp, sp, -24
Written 4 bytes (addiu sp, sp, -24) = wx 27bdffe8
[0x00000000]> pd 1
            0x00000000      27bdffe8       addiu sp, sp, -0x18

[Maijin]

Hello,

Ensure you are using radare2 from git, if you're unsure paste output of r2 -v here. To install radare2 from git, first uninstall your version of radare2 and clean your distro. Then use git clone https://github.com/radare/radare2 && cd radare2 && ./sys/install.sh, verify your version and check if there is no error using r2 -v.

[queueRAM]

As requested:

$ radare2 -v
radare2 1.3.0-git 13931 @ linux-x86-64 git.1.2.0-260-g0744e3344
commit: 0744e334429cc00cef937a87f89b8c3429152d11 build: 2017-02-28__13:34:49

[radare]

Can we have a bunch of tests for this? Mark them as broken. And try with different archs

On 28 Feb 2017, at 22:00, Q [email protected] wrote:

As requested:

$ radare2 -v radare2 1.3.0-git 13931 @ linux-x86-64 git.1.2.0-260-g0744e3344 commit: 0744e334429cc00cef937a87f89b8c3429152d11 build: 2017-02-28__13:34:49 — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[queueRAM]

I tracked it down the discrepancy in the code and it affects all archs. Setting asm.arch sets r_asm_set_big_endian() to default r_bin_is_big_endian() instead of cfg.bigendian. I am unsure of the expected result since I could see use cases for defaulting to the binary's endianness instead of cfg.bigendian.

From lines 342-354 of libr/core/cconfig.c

 // set a default endianness
   int bigbin = r_bin_is_big_endian (core->bin);
   if (bigbin == -1 /* error: no endianness detected in binary */) {
      // try to set RAsm to LE
      r_asm_set_big_endian (core->assembler, false);
      // set endian of display to LE
      core->print->big_endian = false;
   } else {
      // try to set endian of RAsm to match binary
      r_asm_set_big_endian (core->assembler, bigbin);
      // set endian of display to match binary
      core->print->big_endian = bigbin;
   }

I've also noticed this affects saving and opening in projects as I will always have to manually set cfg.bigendian after opening a project.

$ radare2 -w /tmp/test.bin
[0x00000000]> e asm.arch=mips
[0x00000000]> e asm.bits=32
[0x00000000]> e cfg.bigendian=true
[0x00000000]> wa addiu sp, sp, -24
Written 4 bytes (addiu sp, sp, -24) = wx 27bdffe8
[0x00000000]> Ps test
test
[0x00000000]> pd 1@0
            0x00000000      27bdffe8       addiu sp, sp, -0x18
[0x00000000]> q
$ radare2 -p test
[0x00000000]> pd 1@0
            0x00000000      27bdffe8       swc2 31, -0x42d9(a3)
[0x00000000]> e cfg.bigendian
true
[0x00000000]> e cfg.bigendian=true
[0x00000000]> pd 1@0
            0x00000000      27bdffe8       addiu sp, sp, -0x18 

I looked into radare2-regressions and the tests all seem to set cfg.bigendian after asm.arch or after -a [arch] command line argument so this issue isn't exposed. Can I get some guidance on what your expectations are for defaulting to binary's endianness vs. cfg.bigendian?

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. Considering a lot has changed since its creation, we kindly ask you to check again if the issue you reported is still relevant in the current version of radare2. If it is, update this issue with a comment, otherwise it will be automatically closed if no further activity occurs. Thank you for your contributions.

[queueRAM]

This is still an issue. Please remove stale tag. Same test case from above is failing demonstrating discrepancy between cfg.bigendian=true and pd:

# create MIPS 32-bit big endian file and save to project
$ radare2 -w /tmp/test.bin
[0x00000000]> e asm.arch=mips
[0x00000000]> e asm.bits=32
[0x00000000]> e cfg.bigendian=true
[0x00000000]> wa addiu sp, sp, -24
Written 4 byte(s) (addiu sp, sp, -24) = wx 27bdffe8
[0x00000000]> pd 1@0
            0x00000000      27bdffe8       addiu sp, sp, -0x18
[0x00000000]> Ps test
test
[0x00000000]> q

# reload project, confirm correct cfg.bigendian setting but wrong disassembly. re-set bigendian to current value and disassembly is correct
$ radare2 -p test
[0x00000000]> pd 1@0
            0x00000000      27bdffe8       swc2 31, -0x42d9(a3)
[0x00000000]> e cfg.bigendian
true
[0x00000000]> e cfg.bigendian=true
[0x00000000]> pd 1@0
            0x00000000      27bdffe8       addiu sp, sp, -0x18

Version from git today:

$ radare2 -v
radare2 4.5.0-git 24827 @ linux-x86-64 git.4.4.0-340-g765f2b6bd4
commit: 765f2b6bd4098b8e90e46a45c174a71099bc0d56 build: 2020-06-28__22:42:22

[ret2libc]

It seems in your original issue you were not using projects. Projects, unfortunately, are known to be very buggy for a while and a complete rewrite is ongoing. Can you reproduce without using projects?

[queueRAM]

Yes, the original test case fails as well without using projects. Please find below the log from that test with additional commentary.

$ radare2 malloc://16
[0x00000000]> e cfg.bigendian=true # set bigendian
[0x00000000]> e asm.arch=mips
[0x00000000]> e asm.bits=32
[0x00000000]> e cfg.bigendian      # confirm bigendian is still true!
true
[0x00000000]> wa addiu sp, sp, -24 # in big endian should be 27bdffe8!
Written 4 byte(s) (addiu sp, sp, -24) = wx e8ffbd27
[0x00000000]> pd 1                 # pd also still using little endian
            0x00000000      e8ffbd27       addiu sp, sp, -0x18
[0x00000000]> e cfg.bigendian=true # set it again?
[0x00000000]> pd 1
            0x00000000      e8ffbd27       swc2 31, -0x42d9(a3)
[0x00000000]> wa addiu sp, sp, -24 # this time it works
Written 4 byte(s) (addiu sp, sp, -24) = wx 27bdffe8
[0x00000000]> pd 1
            0x00000000      27bdffe8       addiu sp, sp, -0x18
[0x00000000]>

$ radare2 -v
radare2 4.5.0-git 24827 @ linux-x86-64 git.4.4.0-340-g765f2b6bd4
commit: 765f2b6bd4098b8e90e46a45c174a71099bc0d56 build: 2020-06-28__22:42:22

[ret2libc]

Thanks @queueRAM ! It seems your original analysis at https://github.com/radareorg/radare2/issues/6867#issuecomment-283386997 still holds. I think the issue is that there is duplicated code in cb_bigendian and cb_asmarch. If changing asm.arch automagically changes cfg.bigendian as well, that change should be at least reflected everywhere, so that you don't end up with e cfg.bigendian showing a value while the assembler plugin considering another one.

Would you be willing to send a PR for this? I guess a good solution could be to refactor the code in cb_asmarch and cb_bigendian and ensuring that when cb_asmarch changes the bigendian value it changes so properly, by using the common function that also sets the value of the env var cfg.bigendian.

Once we have that we can discuss in the future whether we want cfg.bigendian to be preferred or the binary endianess to be preferred.

[ret2libc]

cc @trufae

[queueRAM]

Would you be willing to send a PR for this?

Thank you for your guidance. I have a commit ready for this in my bigendian_asmarch branch. I'm still going through the PR checklist and looks like I have to remove some C99 variable declarations and add tests for this issue. I'll likely have this worked out within a few days.

[ret2libc]

Would you be willing to send a PR for this?

Thank you for your guidance. I have a commit ready for this in my bigendian_asmarch branch. I'm still going through the PR checklist and looks like I have to remove some C99 variable declarations and add tests for this issue. I'll likely have this worked out within a few days.

Thanks a lot! If you need help with anything, please don't hesitate to ask here on IRC/Telegram.

[ret2libc]

@queueRAM hey! How are things going? Can I help with anything?