New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Uplifting 8051 architecture to new IL #2999

Merged

thestr4ng3r merged 16 commits into dev from asan-fuzz-uplifting-8051_

Dec 14, 2022

Contributor

imbillow commented Sep 2, 2022 •

edited by XVilka

Loading

SQUASH ME

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 documented or updated the documentation of every function and struct this PR changes. If not so I've explained why.
I've added tests that prove my fix is effective or that my feature works (if possible)
I've updated the rizin book with the relevant information (if needed)

Detailed description

continue #1609

Instrunction set: https://www.win.tue.nl/~aeb/comp/8051/set8051.html

/	0x00	0x01	0x02	0x03	0x04	0x05	0x06	0x07	0x08	0x09	0x0A	0x0B	0x0C	0x0D	0x0E	0x0F
0x00	NOP	AJMP	LJMP	RR	INC	INC	INC	INC	INC	INC	INC	INC	INC	INC	INC	INC
0x10	JBC	ACALL	LCALL	RRC	DEC	DEC	DEC	DEC	DEC	DEC	DEC	DEC	DEC	DEC	DEC	DEC
0x20	JB	AJMP	RET	RL	ADD	ADD	ADD	ADD	ADD	ADD	ADD	ADD	ADD	ADD	ADD	ADD
0x30	JNB	ACALL	RETI	RLC	ADDC	ADDC	ADDC	ADDC	ADDC	ADDC	ADDC	ADDC	ADDC	ADDC	ADDC	ADDC
0x40	JC	AJMP	ORL	ORL	ORL	ORL	ORL	ORL	ORL	ORL	ORL	ORL	ORL	ORL	ORL	ORL
0x50	JNC	ACALL	ANL	ANL	ANL	ANL	ANL	ANL	ANL	ANL	ANL	ANL	ANL	ANL	ANL	ANL
0x60	JZ	AJMP	XRL	XRL	XRL	XRL	XRL	XRL	XRL	XRL	XRL	XRL	XRL	XRL	XRL	XRL
0x70	JNZ	ACALL	ORL	JMP	MOV	MOV	MOV	MOV	MOV	MOV	MOV	MOV	MOV	MOV	MOV	MOV
0x80	SJMP	AJMP	ANL	MOVC	DIV	MOV	MOV	MOV	MOV	MOV	MOV	MOV	MOV	MOV	MOV	MOV
0x90	MOV	ACALL	MOV	MOVC	SUBB	SUBB	SUBB	SUBB	SUBB	SUBB	SUBB	SUBB	SUBB	SUBB	SUBB	SUBB
0xA0	ORL	AJMP	MOV	INC	MUL	?	MOV	MOV	MOV	MOV	MOV	MOV	MOV	MOV	MOV	MOV
0xB0	ANL	ACALL	CPL	CPL	CJNE	CJNE	CJNE	CJNE	CJNE	CJNE	CJNE	CJNE	CJNE	CJNE	CJNE	CJNE
0xC0	PUSH	AJMP	CLR	CLR	SWAP	XCH	XCH	XCH	XCH	XCH	XCH	XCH	XCH	XCH	XCH	XCH
0xD0	POP	ACALL	SETB	SETB	DA	DJNZ	XCHD	XCHD	DJNZ	DJNZ	DJNZ	DJNZ	DJNZ	DJNZ	DJNZ	DJNZ
0xE0	MOVX	AJMP	MOVX	MOVX	CLR	MOV	MOV	MOV	MOV	MOV	MOV	MOV	MOV	MOV	MOV	MOV
0xF0	MOVX	ACALL	MOVX	MOVX	CPL	MOV	MOV	MOV	MOV	MOV	MOV	MOV	MOV	MOV	MOV	MOV

...

Test plan

...

Closing issues

...

github-actions bot added the RzAnalysis label

Member

XVilka commented Sep 4, 2022

Please add also yourself in SPDX copyright section of that file and @Basstorm as well.

XVilka mentioned this pull request

ESIL to RZIL conversion #2080

Open

38 tasks

github-actions bot added API RzAsm-Disassembler RZIL RzReg and removed API labels

github-actions bot added RzAsm-Assembler and removed RzAsm-Assembler RzAsm-Disassembler labels

github-actions bot removed the RZIL label

imbillow force-pushed the asan-fuzz-uplifting-8051_ branch from e485b0a to 6e22f31 Compare

October 24, 2022 12:11

imbillow force-pushed the asan-fuzz-uplifting-8051_ branch from 6e22f31 to defdb34 Compare

November 12, 2022 09:33

github-actions bot added the rz-test label

imbillow force-pushed the asan-fuzz-uplifting-8051_ branch from 61d0f5a to ac55c0d Compare

November 20, 2022 13:20

imbillow marked this pull request as ready for review

November 20, 2022 15:04

imbillow requested review from XVilka, wargio, thestr4ng3r and kazarmy as code owners

November 20, 2022 15:04

XVilka reviewed

View reviewed changes

librz/analysis/arch/8051/8051_il.c Outdated Show resolved Hide resolved

Contributor Author

imbillow commented Nov 21, 2022 •

edited

Loading

The format error seems to be because the clang-format version in my system is 15

thestr4ng3r requested changes

View reviewed changes

Member

thestr4ng3r left a comment

Looks very nice already!

librz/analysis/arch/8051/8051_il.c Outdated Show resolved Hide resolved

librz/analysis/meson.build Outdated Show resolved Hide resolved

imbillow force-pushed the asan-fuzz-uplifting-8051_ branch from a21efd6 to 52bf8d0 Compare

November 27, 2022 12:42


          Uplifting 8051 architecture to new IL

ce35ba7

imbillow force-pushed the asan-fuzz-uplifting-8051_ branch from 52bf8d0 to ce35ba7 Compare

November 27, 2022 12:45

thestr4ng3r approved these changes

View reviewed changes

Member

thestr4ng3r left a comment

Great job!

wargio approved these changes

View reviewed changes

XVilka approved these changes

View reviewed changes

Member

XVilka left a comment

Looks good, apart from a couple of comments. The remaining part would be the tracetest.

librz/analysis/arch/8051/8051_il.c Outdated Show resolved Hide resolved

librz/analysis/arch/8051/8051_il.c Outdated Show resolved Hide resolved

DMaroo requested changes

View reviewed changes

Member

DMaroo left a comment

Looks pretty neat to me. Great work!

librz/analysis/arch/8051/8051_il.c Outdated Show resolved Hide resolved

librz/analysis/arch/8051/8051_il.c Outdated Show resolved Hide resolved

librz/analysis/arch/8051/8051_il.c

+              		return STOREW(U16(a->d.addr), BOOL_TO_BV(v, 1));
+              	}
+              	case I8051_ADDRESSING_DIRECT:
+              		return STORE(U16(a->d.addr), v);

Member

DMaroo Nov 27, 2022

Instead of duplicating the address calculation logic, just use return STOREW(get_any(a), v).

Contributor Author

imbillow Dec 11, 2022

But it should not be equivalent

DMaroo added the RZIL label

Rot127 approved these changes

View reviewed changes

Member

Rot127 left a comment

Could only skim over it briefly. But LGTM. Good job!

librz/analysis/arch/8051/8051_il.c Show resolved Hide resolved

librz/analysis/arch/8051/8051_il.c Show resolved Hide resolved

librz/analysis/arch/8051/8051_op.h Outdated Show resolved Hide resolved

librz/analysis/arch/8051/8051_op.h Outdated

+               * 0x05 iram addr
+               * 0x06-0x07 @(R0-R1)
+               * 0x08-0x0f, R0-R7
+               */

Member

Rot127 Nov 28, 2022

Is the meaning here common knowledge for people who know 8051? Otherwise I would suggest to add more context (this is the offset into the buffer?).

Member

Rot127 Nov 28, 2022

Same for comments below.

librz/analysis/arch/8051/8051_op.h Outdated Show resolved Hide resolved

Basstorm approved these changes

View reviewed changes


          Merge branch 'dev' into asan-fuzz-uplifting-8051_

6675a8c

github-actions bot removed the RZIL label

imbillow added 2 commits

December 11, 2022 15:46

fix

56322ae


          fix test

c79832e

DMaroo approved these changes

View reviewed changes


          8051_op.h -> 8051_il.h

beda9bc

XVilka requested changes

View reviewed changes

Member

XVilka left a comment

@imbillow many tests are broken on big endian platform:

[XX] db/asm/8051 <asm> djnz 0x40, 0x118b
-- <asm> djnz 0x40, 0x118b <--> d54088 ---> <IL>
-- assembly
-- IL
--- expected
+++ actual

@@ -1,1 +1,1 @@
-(seq (set res (- (load 0 (bv 16 0x40)) (bv 8 0x1))) (store 0 (bv 16 0x40) (var res)) (branch (! (is_zero (var res))) (jmp (bv 16 0x1288)) nop))
+(seq (set res (- (load 0 (bv 16 0x0)) (bv 8 0x1))) (store 0 (bv 16 0x0) (var res)) (branch (! (is_zero (var res))) (jmp (bv 16 0x1288)) nop))

[XX] db/asm/8051 <asm> dec 0x35
-- <asm> dec 0x35 <--> 1535 ---> <IL>
-- assembly
-- IL
--- expected
+++ actual
@@ -1,1 +1,1 @@
-(seq (set arg0 (load 0 (bv 16 0x35))) (store 0 (bv 16 0x35) (- (var arg0) (bv 8 0x1))))
+(seq (set arg0 (load 0 (bv 16 0x0))) (store 0 (bv 16 0x0) (- (var arg0) (bv 8 0x1))))

See all at https://app.travis-ci.com/github/rizinorg/rizin/jobs/590853977

imbillow added 7 commits

December 11, 2022 20:35


          doxygen

0cd6abc

d_0

77621de

d_1

78bab02

d_2

d9c65e4

d_3

663e9f4


          format

ded1361

fix

5a655b0

XVilka reviewed

View reviewed changes

librz/analysis/arch/8051/8051_parse.c Outdated Show resolved Hide resolved

imbillow added 4 commits

December 13, 2022 22:03

fix

69d4c94

fix

cd53a3d

fix

8920c5d


          Merge branch 'dev' into asan-fuzz-uplifting-8051_

f859f43

XVilka approved these changes

View reviewed changes

Member

XVilka left a comment

LGTM

thestr4ng3r merged commit 291d2a6 into dev

thestr4ng3r deleted the asan-fuzz-uplifting-8051_ branch

December 14, 2022 12:13

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

DMaroo DMaroo approved these changes

Rot127 Rot127 approved these changes

Basstorm Basstorm approved these changes

wargio wargio approved these changes

XVilka XVilka approved these changes

ret2libc Awaiting requested review from ret2libc ret2libc is a code owner

kazarmy Awaiting requested review from kazarmy kazarmy is a code owner

thestr4ng3r Awaiting requested review from thestr4ng3r thestr4ng3r is a code owner

Labels

rz-test RzAnalysis RzReg