Update Hexagon asm and analysis plugins #1614

Rot127 · 2021-09-07T10:23:42Z

Replaces #1338

Your checklist for this pull request

I've read the guidelines for contributing to this repository
I made sure to follow the project's coding style
I've added tests that prove my fix is effective or that my feature works (if possible)
I've updated the documentation and the rizin book with the relevant information (if needed)

Detailed description

Updates Hexagon analysis and asm plugin.

Supports

Add asm support for v5-v68 and HVX instructions
- Recognize instruction packets.
Basic analysis support
- set instruction type (return, call, jumps hardware loop etc.)
- Enables search for immediate operands.
Patching of relocs

Not yet implemented

~~System registers~~
~~System/Monitor instructions~~
Can not analyse >1024bit vector registers
No ESIL support
~~No tests yet for HVX instructions and hardware loops.~~
Test files for reloc patching

Test plan

See test files.

Closing issues

closes rizinorg/rz-hexagon#13
closes rizinorg/rz-hexagon#15
closes rizinorg/rz-hexagon#17
closes rizinorg/rz-hexagon#18
closes rizinorg/rz-hexagon#19
closes rizinorg/rz-hexagon#21
closes rizinorg/rz-hexagon#22
closes rizinorg/rz-hexagon#24

Rot127 · 2021-09-08T14:52:32Z

I have absolutely no idea what the cause of the failing test is. I just created a VM and built rizin as the ubuntu-tcc-test Action does it. Basically copied and pasted the commands to build TinyCC and meson.

All tests succeeded. No error.

The only difference is, that I use a debian-10 in the VM. No Ubuntu. Unfortunately I can't install a Ubuntu VM at the moment and my enthusiasm debugging it is meanwhile really low also.

Do you have any advice how to proceed?

Just in case I'm a blockhead. The commands I compiled rizin with TinyCC:

git clone https://repo.or.cz/tinycc.git
cd tinycc
#git checkout mob
# Well-tested commit
git checkout 675046bd59bc6977bb2016c7d2e115ace8a6ae6c
chmod +x ./configure
./configure --prefix=/usr
make
sudo make install
sudo apt-get --assume-yes install python3-wheel python3-setuptools python3-pip
sudo python3 -m pip install ninja
git clone https://github.com/ret2libc/meson.git
cd meson
git checkout tiny-cc
python3 setup.py build
sudo python3 setup.py install
cd ~
git clone https://github.com/Rot127/rizin.git
cd ~/rizin/
git checkout Hexagon
env CC=tcc meson --prefix=/usr build
ninja -C build && sudo ninja -C build install
mkdir -p test/bins/elf/analysis/
cd test/bins/elf/analysis/
wget https://raw.githubusercontent.com/rizinorg/rizin-testbins/master/elf/analysis/hexagon-hello-loop
cd ~/rizin
rz-test test/db/analysis/hexagon 

Running from /home/user/rizin/test
Loaded 3 tests.
Skipping json tests because jq is not available.
[3/3]                       3 OK         0 BR        0 XX        0 FX
Finished in 0 seconds.

Rot127 · 2021-09-11T20:14:20Z

So I ran the tests on an Ubuntu 20.04.3 LTS VM and they succeeded. It must be something with Github Actions than.

wargio · 2021-09-11T20:53:56Z

So I ran the tests on an Ubuntu 20.04.3 LTS VM and they succeeded. It must be something with Github Actions than.

also with tinycc?

Rot127 · 2021-09-11T21:50:32Z

Oh, I was stupid. Sorry.
Ran meson --prefix=/usr build instead of env CC=tcc meson --prefix=/usr build.
Now the tests fail.

Will debug it once I have more time.

ret2libc

I haven't really followed here... but if @wargio things is good, I'm ok to merge this as it is anyway quite confined to hexagon code. Please address just this thing first.

librz/bin/p/bin_elf.inc

XVilka

Overall looks good. What bothers me - why you don't use predicates? They are pretty important.
Also, for relocations - could you please add a test specifically for relocations? Like it's done in https://github.com/rizinorg/rizin/pull/1699/files#diff-815fd301b119045a3c4aef6ecbb05174a1d997c1c43cf2b65d4a461ea57bd7b4R114
So for every different type of relocation you implemented, if it's in the test binary - print bytes, print instruction. It's recommended to cross-check your results against readelf, objdump, IDA Pro, Binary Ninja, Ghidra.

Good job!

librz/analysis/p/analysis_hexagon.c

librz/analysis/arch/hexagon/hexagon_analysis.h

librz/asm/arch/hexagon/hexagon.c

librz/asm/arch/hexagon/hexagon.h

librz/bin/p/bin_elf.inc

Rot127 · 2021-09-29T11:12:22Z

@XVilka

Good job!

Thank you. I really appreciate the work you guys put into reviewing this.

What bothers me - why you don't use predicates? They are pretty important.

I quite frankly forgot about them. Will start with the implementation today.

Can you give me a hint where rizin uses this information? I can't find a member in RzAnalysisOp which represents predicates properly. But maybe my understanding is limited.

It's recommended to cross-check your results against readelf, objdump, IDA Pro, Binary Ninja, Ghidra.

Tool	comment
objdump	I checked the disassembly and analysis test against the SDKs `objdump`. And actively advice it in the test documentation in the rz-hexagon docs.
IDA Pro	I can't effort a license for IDA Pro, therefore not test against it. That's the reason why I started developing the Hexagon plugin for rizin in the first place.
Binary Ninja	Same like IDA Pro. I can't effort a license and the demos can't load the Hexagon plugin.
Ghidra	Ghidra does not support the Hexagon architecture yet.

The problem for me is, that tests implementations need a lot of time which I currently not have (e.g. compiling HVX code with the SDK is buggy, building a ELF binary with all implemented relocations is quite hard, as I just start learning about relocations details etc.).
At the moment I can, unfortunately, only promise that I will add them gradually once I use it more myself. For me it is absolutely fine if this PR is not merged yet for this reason. The plugin will be under development at least for the next 5-6 months anyways.

Rot127 · 2021-09-29T11:55:25Z

Almost forgot. Testing the reloc patching is quite hard at the moment since the rebasing is so broken.

XVilka · 2021-09-29T12:04:55Z

@Rot127 do you mean this one? #1661
cc @ret2libc

Rot127 · 2021-09-29T12:12:20Z

@Rot127 do you mean this one? #1661

Yes. It does correctly patch if the binary is loaded with rizin -m <baddr>. But because of #1661 and building a binary which uses all implemented relocations is quite time consuming I postponed the tests.

librz/bin/p/bin_elf.inc

librz/include/rz_bin.h

XVilka · 2021-09-30T07:06:22Z

Please also rebase on top of the latest dev to resolve the conflicts.

thestr4ng3r · 2021-10-31T16:31:19Z

Hmm I see. What about, instead of generating code for each property, generating a lookup table in C for some properties? That would shift a bit of the work to the runtime, but probably also compress the entire thing a bit.
By the way, what is the code size after the file has been compiled?

Rot127 · 2021-10-31T16:40:03Z

By the way, what is the code size after the file has been compiled?

ls -l for ./build/librz/analysis/librz_analysis.a.p/.._asm_arch_hexagon_hexagon_disas.c.o gives me:

compiler	target	size
tcc	x86_64	5.3M
gcc	x86_64	1.9M
clang	aarch64	1.4M

Rot127 · 2021-10-31T16:49:58Z

What about, instead of generating code for each property, generating a lookup table in C for some properties?

Mh, I will ponder about this a bit. But for now I will remove unnecessary function calls from it (~3 per instruction). This should drive the memory usage of gcc down quite a bit.

@thestr4ng3r Just in case you have time and want to take a quick look at it yourself, in librz/asm/arch/hexagon/hexagon_disas.c @ line 56088 is an instruction which is pretty complex and has a lot of assignments. Maybe you see something right away.

Rot127 · 2021-11-10T14:48:55Z

It's weird. But I no longer get the problem with >4gb memory usage of gcc (top shows something around 16MB).
So if you like, you can review it.

XVilka · 2021-11-25T14:58:05Z

There are a lot of unrelated changes. Thus, I propose:

Squash all commits into one - this way rebasing should be easier
Rebase on top of the latest dev
Fix clang-format warning:

Ubuntu clang-format version 13.0.1-++20211110063012+9dc7d6d5e326-1~exp1~20211110183538.22
librz/asm/arch/hexagon/hexagon_disas.c:18:26: error: code should be clang-formatted [-Wclang-format-violations]
#include "hexagon_arch.h"
                         ^

XVilka

Looks good to merge. Please use the better commit name when merging.

…gin design to monolithic; Basic ELF-reloc patching. - Update instruction set to v5-v67 + HVX. - Introduce monolithic plugin architecture to reverse opcodes in the same functions. - Analysis and Asm functionality was previously seperated and let to inconsistensies and incorrectly reversed opcodes. - Add missing system instructions and registers. - Enable search for immediates. - Mark packets of instructions. - Set more analysis data in RzAnalysisOp. - Add basic ELF-reloc patching. - Fix several disassembly and analysis bugs. - Jumps were not aligned to packet begin. - Endloop packets were not marked as such. - Rs.New registers were incorrectly disassembled. - Calling convention was out of date. - Add several formatting configs for the produced disassembly. - Dis/Enable UTF-8. - Print or omit # prefix of immediates. - Print signed immediates with signe or as unsigned integers.

XVilka · 2021-11-26T17:01:26Z

Merge if #2033 is green (excluding the known debug test ASAN failure)

XVilka · 2021-11-26T17:13:54Z

Fails to build on Windows AppVeyor:

FAILED: librz/asm/librz_asm.a.p/arch_hexagon_hexagon_arch.c.obj 
"cl" "-Ilibrz/asm\librz_asm.a.p" "-I." "-I.." "-I..\librz\include" "-Ishlr" "-I..\shlr" "-I..\librz\asm\arch\include" "-I..\librz\asm\arch" "-I..\librz\asm\arch\h8300" "-I..\librz\asm\arch\hexagon" "-I..\librz\asm\arch\msp430" "-I..\librz\asm\arch\rsp" "-I..\librz\asm\arch\mcore" "-I..\librz\asm\arch\v850" "-I..\librz\asm\arch\propeller" "-I..\librz\asm\arch\ebc" "-I..\librz\asm\arch\cr16" "-I..\librz\asm\arch\8051" "-I..\librz\asm\arch\v810" "-I..\librz\asm\arch\or1k" "-Isubprojects\sdb" "-I..\subprojects\sdb" "-Isubprojects\sdb\src" "-I..\subprojects\sdb\src" "-I..\librz\bin\format" "-I..\subprojects\capstone-bundled\include" "-I..\subprojects\capstone-bundled\include\capstone" "-I..\shlr\spp" "/MT" "/nologo" "/showIncludes" "/utf-8" "/W2" "/O2" "/Gw" "-DRZ_PLUGIN_INCORE=1" "/Fdlibrz/asm\librz_asm.a.p\arch_hexagon_hexagon_arch.c.pdb" /Folibrz/asm/librz_asm.a.p/arch_hexagon_hexagon_arch.c.obj "/c" ../librz/asm/arch/hexagon/hexagon_arch.c
../librz/asm/arch/hexagon/hexagon_arch.c(613): error C2375: 'hex_extend_op': redefinition; different linkage
c:\projects\rizin\librz\asm\arch\hexagon\hexagon.h(550): note: see declaration of 'hex_extend_op'
[1116/1727] Compiling C object librz/asm/librz_asm.a.p/arch_h8300_h8300_disas.c.obj
[1117/1727] Compiling C object librz/asm/librz_asm.a.p/arch_hexagon_hexagon_disas.c.obj
ninja: build stopped: subcommand failed.

https://ci.appveyor.com/project/rizinorg/rizin/builds/41680793/job/6m3ufxlgdbrqrbi0

XVilka · 2021-11-26T17:24:51Z

Failing test with ASAN:



[XX] db/analysis/hexagon hexagon jumps
RZ_NOPLUGINS=1 rizin -escr.utf8=0 -escr.color=0 -escr.interactive=0 -N -Qc 'b 0x100000
e analysis.jmp.cref=true
s sym.main
aaa
afx
' bins/elf/analysis/hexagon-hello-loop
-- stdout
--- expected
+++ actual
@@ -5,3 +5,7 @@
 C 0x0000513c -> 0x000050f8 [   call sym.pWorld
 c 0x00005140 -> 0x00005144 [   jump 0x5144
 c 0x00005150 -> 0x00005128 ?   jump 0x5128
+WARNING: rz_reg_set_value: assertion 'reg && item' failed (line 145)
+WARNING: rz_reg_set_value: assertion 'reg && item' failed (line 145)
+WARNING: rz_reg_set_value: assertion 'reg && item' failed (line 145)
+WARNING: rz_reg_set_value: assertion 'reg && item' failed (line 145)

-- stderr
Cannot determine entrypoint, using 0x00005000.
[ ] Analyze all flags starting with sym. and entry0 (aa)

Rot127 · 2021-11-27T13:03:17Z

6942f6c should fix the Windows build.

XVilka · 2021-11-27T17:40:24Z

The other PR was green.

Rot127 requested review from kazarmy, ret2libc, thestr4ng3r, wargio and XVilka as code owners September 7, 2021 10:23

Rot127 mentioned this pull request Sep 7, 2021

Hexagon update #1338

Closed

4 tasks

github-actions bot added RzAnalysis RzBin labels Sep 7, 2021

XVilka added this to the 0.4.0 milestone Sep 8, 2021

Rot127 mentioned this pull request Sep 8, 2021

Search up to the last byte in a block. #1621

Merged

4 tasks

Rot127 mentioned this pull request Sep 15, 2021

Track the state and context of the asm and analysis plugin. rizinorg/rz-hexagon#24

Closed

wargio approved these changes Sep 15, 2021

View reviewed changes

Rot127 force-pushed the Hexagon branch from cf7ab55 to fce9a9f Compare September 22, 2021 10:44

XVilka mentioned this pull request Sep 23, 2021

Patches: rizin pull request rizinorg/rz-hexagon#16

Merged

ret2libc requested changes Sep 24, 2021

View reviewed changes

librz/bin/p/bin_elf.inc Outdated Show resolved Hide resolved

XVilka requested changes Sep 29, 2021

View reviewed changes

XVilka requested a review from a user September 29, 2021 05:07

XVilka requested changes Sep 29, 2021

View reviewed changes

librz/bin/p/bin_elf.inc Outdated Show resolved Hide resolved

github-actions bot added the ELF label Sep 29, 2021

XVilka requested changes Sep 29, 2021

View reviewed changes

librz/include/rz_bin.h Outdated Show resolved Hide resolved

Rot127 requested a review from XVilka October 1, 2021 11:06

Rot127 mentioned this pull request Nov 3, 2021

Fix: Conditional jumps are treated as data xrefs if analysis.jmp.cref=false #1934

Closed

4 tasks

github-actions bot added API rz-test labels Nov 18, 2021

Rot127 requested a review from yossizap as a code owner November 24, 2021 18:42

github-actions bot added ESIL infrastructure RzCore RZIL ARM labels Nov 24, 2021

Rot127 force-pushed the Hexagon branch 2 times, most recently from cb564e0 to 2c00b89 Compare November 26, 2021 13:33

XVilka approved these changes Nov 26, 2021

View reviewed changes

Rot127 force-pushed the Hexagon branch from 2c00b89 to 14ba744 Compare November 26, 2021 14:49

wargio approved these changes Nov 26, 2021

View reviewed changes

This was referenced Nov 27, 2021

Check for plugin ESIL support before running ESIL code in aaa. #2039

Merged

Register sizes >256bit are not supported by rizin rizinorg/rz-hexagon#20

Closed

Add missing RZ_API in declaration.

6942f6c

XVilka merged commit 26103ee into rizinorg:dev Nov 27, 2021

Rot127 mentioned this pull request Dec 4, 2021

Search RzAnalysisOp.analysis_vals[] for xrefs. #2067

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Hexagon asm and analysis plugins #1614

Update Hexagon asm and analysis plugins #1614

Rot127 commented Sep 7, 2021 •

edited

Loading

Rot127 commented Sep 8, 2021 •

edited

Loading

Rot127 commented Sep 11, 2021

wargio commented Sep 11, 2021

Rot127 commented Sep 11, 2021

ret2libc left a comment

XVilka left a comment

Rot127 commented Sep 29, 2021

Rot127 commented Sep 29, 2021

XVilka commented Sep 29, 2021

Rot127 commented Sep 29, 2021 •

edited

Loading

XVilka commented Sep 30, 2021

thestr4ng3r commented Oct 31, 2021

Rot127 commented Oct 31, 2021

Rot127 commented Oct 31, 2021 •

edited

Loading

Rot127 commented Nov 10, 2021 •

edited

Loading

XVilka commented Nov 25, 2021

XVilka left a comment

XVilka commented Nov 26, 2021

XVilka commented Nov 26, 2021

XVilka commented Nov 26, 2021

Rot127 commented Nov 27, 2021

XVilka commented Nov 27, 2021

Update Hexagon asm and analysis plugins #1614

Update Hexagon asm and analysis plugins #1614

Conversation

Rot127 commented Sep 7, 2021 • edited Loading

Rot127 commented Sep 8, 2021 • edited Loading

Rot127 commented Sep 11, 2021

wargio commented Sep 11, 2021

Rot127 commented Sep 11, 2021

ret2libc left a comment

Choose a reason for hiding this comment

XVilka left a comment

Choose a reason for hiding this comment

Rot127 commented Sep 29, 2021

Rot127 commented Sep 29, 2021

XVilka commented Sep 29, 2021

Rot127 commented Sep 29, 2021 • edited Loading

XVilka commented Sep 30, 2021

thestr4ng3r commented Oct 31, 2021

Rot127 commented Oct 31, 2021

Rot127 commented Oct 31, 2021 • edited Loading

Rot127 commented Nov 10, 2021 • edited Loading

XVilka commented Nov 25, 2021

XVilka left a comment

Choose a reason for hiding this comment

XVilka commented Nov 26, 2021

XVilka commented Nov 26, 2021

XVilka commented Nov 26, 2021

Rot127 commented Nov 27, 2021

XVilka commented Nov 27, 2021

Rot127 commented Sep 7, 2021 •

edited

Loading

Rot127 commented Sep 8, 2021 •

edited

Loading

Rot127 commented Sep 29, 2021 •

edited

Loading

Rot127 commented Oct 31, 2021 •

edited

Loading

Rot127 commented Nov 10, 2021 •

edited

Loading