Uplifting 8051 architecture to new IL #1609

Basstorm · 2021-09-07T09:10:34Z

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

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

...

XVilka · 2021-09-07T17:27:31Z

Please rebase on top of the latest IL branch, there are way too many conflicts.

XVilka · 2021-10-09T10:15:51Z

@Basstorm please rebase on top of the latest dev and resolve the conflicts.

XVilka · 2021-10-19T04:45:14Z

There are many failing tests. I recommend to start addressing them one by one

ret2libc · 2021-10-19T07:20:51Z

librz/analysis/p/analysis_8051.c

+	return oplist;
+}
+
+RzPVector *i8051_movx(RzILVM *vm, ut64 id, const ut8 *buf, _8051_op_t op) {


how is this called?

XVilka · 2021-10-19T07:26:48Z

librz/analysis/p/analysis_8051.c

@@ -1257,7 +3613,7 @@ static int i8051_op(RzAnalysis *analysis, RzAnalysisOp *op, ut64 addr, const ut8
 	if (mask & RZ_ANALYSIS_OP_MASK_ESIL) {


I think ESIL here can be removed completely.

yes, will remove that when the uplifing work is done

XVilka · 2021-10-19T07:27:27Z

librz/analysis/p/analysis_8051.c

+		return true;
+	}
+
+	// create core theory VM


Better to refer to it as RZIL VM outside of the RZIL module.

XVilka · 2021-10-19T07:27:52Z

librz/analysis/p/analysis_8051.c

+	RzAnalysisRzil *rzil = analysis->rzil;
+
+	if (rzil->inited) {
+		eprintf("Already init\n");


Probably RZ_LOG_WARNING()

XVilka · 2021-10-19T07:29:46Z

librz/analysis/p/analysis_8051.c

+	RzILOp *var_c = rz_il_new_op(RZIL_OP_VAR);
+	var_c->op.var->v = "PSW";
+
+	RzILOp *int_ = rz_il_new_op(RZIL_OP_INT);


I wonder if we can shorten this pattern of:

RzILOp *op = rz_il_new_op(RZIL_OP_TYPE); op->op.something = x; op->op.another = y;

@ret2libc @thestr4ng3r @wargio any ideas?

static inline RzILOp *rz_il_new_op_int(ut64 /* or what the actual type is */ value, size_t length) { RzILOp *r = rz_il_new_op(RZIL_OP_INT); if (!r) { return NULL; } r->op.int_->value = value; r->op.int_->length = length; return r; }

and add that for all op types

I guess you mean to have this function for each type of il_op, right? If so, I agree with that.

So you'd have rz_il_new_op_int, rz_il_new_op_logand, rz_il_new_op_shiftl, rz_il_new_op_shiftr, etc.

Yes. You could also compose them like rz_il_new_op_shiftl(stuff, rz_il_new_op_int(42, 8)).

Agree, add family functions of new_op

just add the family functons in this 8051? or add them into RZIL module?

XVilka · 2021-10-19T07:31:14Z

librz/analysis/p/analysis_8051.c

+	RzILOp *perform_b = rz_il_new_op(RZIL_OP_PERFORM);
+	perform_b->op.perform->eff = set_b;
+
+	RzPVector *oplist = rz_il_make_oplist(id, 8, var_a, var_b, div, mod, set_a, perform_a, set_b, perform_b);


I wonder if we want to add also the rz_il_oplist_push() that will allow to add the ops one by one. cc @Heersin @thestr4ng3r @ret2libc

I think it makes sense

Wouldn't that be just rz_pvector_push()?

Yes, but I think it would be nice to have a semantically-linked wrapper.

ret2libc · 2021-10-19T07:31:57Z

librz/analysis/p/analysis_8051.c

+	RzILOp *var_a = rz_il_new_op(RZIL_OP_VAR);
+	var_a->op.var->v = "ACC";
+
+	char *regname = get_regname_bybase(buf[0], 0xE2);


where is this 0xE2 coming from?

from the Opcode table in the description of this PR.

ret2libc

Few things:

how are we testing these conversions? As said in the past, I think we should configure a system to compare a piece of assembly code executed under the debugger and one executed with RzIL(generated from the original code) to make sure that the IL actually applies operation in the right way. Until we have such a system, these conversion will be hard/impossible to really review.
shall we have all these lifters somewhere else instead of p/analysis_x.c? I expect they will be quite big...

XVilka · 2021-10-20T07:21:09Z

@ret2libc full testing can be done once whole architecture converted, then we can compare the debugger results with the emulated results of some trace.

ret2libc · 2021-10-20T07:41:09Z

@ret2libc full testing can be done once whole architecture converted, then we can compare the debugger results with the emulated results of some trace.

Not really necessary to convert a whole arch to start testing.

The idea is to have things like:

; START INSTRUCTIONS
inc eax
; END INSTRUCTIONS

This would be a test. You can start by converting just the inc instruction to make sure this test works.

Then you could have another test:

; START INSTRUCTIONS
mov eax, ebx
; END INSTRUCTIONS

And so on and so on. This way you have a very good way to develop without having to "hope" that everything will be alright at the end of the day. Until we have something like that that allows us to test pieces of assemblies, we are just blindly writing code hoping that it will work well.

I'm not saying this PR is not ok, but before we move too much forward with the IL I think testing is foundamental.

kazarmy · 2021-10-20T11:41:27Z

Not really necessary to convert a whole arch to start testing.

I agree with @ret2libc that there should be a way to uplift progressively. Otherwise the uplifting of big, complex arches like x86 might become non-starters.

Heersin

LGTM. Is there any test file to check some of the instructions ?

wargio · 2021-11-18T11:39:39Z

to test the code, please use ao @ <offset> ~rzil for RzIL semantic and aezse for execution changes.

example:

[0x00000000]> ao @ 0x00 ~rzil
rzil: [store(key:var(v:ptr), value:add(x:load(key:var(v:ptr), mem:0), y:bitv(bits:0x01, len:8)), mem:0)]
[0x00000000]> aezi
[0x00000000]> aezse
var_read(name: ptr, value: 0x0000000000000000)
var_read(name: ptr, value: 0x0000000000000000)
mem_read(addr: 0x0000000000000000, value: uninitialized memory)
mem_write(addr: 0x0000000000000000, old: 0x00, new: 0x01)
pc_write(old: 0x0000000000000000, new: 0x0000000000000001)

XVilka · 2022-05-19T11:08:16Z

@Basstorm do you have plans to finish this?

XVilka · 2022-06-08T13:15:11Z

@imbillow assigned you, but I think it's probably better to start a new PR when you will work on it. Once you open a new one, feel free to close this.

Basstorm · 2022-07-11T11:50:50Z

Will start a new one later.

Basstorm requested a review from thestr4ng3r as a code owner September 7, 2021 09:10

Basstorm marked this pull request as draft September 7, 2021 09:10

github-actions bot added the RzAnalysis label Sep 7, 2021

Heersin force-pushed the asan_integrate_new_il branch from ce05b8c to bb72753 Compare September 7, 2021 12:25

github-actions bot added the command label Sep 7, 2021

Basstorm force-pushed the asan-fuzz-uplifting-8051 branch 3 times, most recently from f541f68 to 7b22784 Compare September 10, 2021 08:23

Heersin force-pushed the asan_integrate_new_il branch 3 times, most recently from 32f7093 to f0fc2f0 Compare September 15, 2021 13:51

XVilka added this to In progress in RzIL via automation Sep 15, 2021

Heersin force-pushed the asan_integrate_new_il branch from f0fc2f0 to 6f1372a Compare September 16, 2021 16:18

Heersin force-pushed the asan_integrate_new_il branch from 6f1372a to 7da8ec3 Compare September 24, 2021 06:09

Heersin force-pushed the asan_integrate_new_il branch 2 times, most recently from 343cc78 to 570cffc Compare October 9, 2021 07:21

Base automatically changed from asan_integrate_new_il to dev October 9, 2021 10:14

XVilka added the RZIL label Oct 9, 2021

XVilka requested a review from Heersin October 9, 2021 10:16

github-actions bot added the ESIL label Oct 9, 2021

Basstorm force-pushed the asan-fuzz-uplifting-8051 branch 2 times, most recently from c47b6c7 to c147ae5 Compare October 18, 2021 19:49

XVilka requested review from ret2libc and wargio October 19, 2021 05:42

ret2libc reviewed Oct 19, 2021

View reviewed changes

XVilka requested changes Oct 19, 2021

View reviewed changes

RzIL automation moved this from In progress to Review in progress Oct 19, 2021

ret2libc reviewed Oct 19, 2021

View reviewed changes

Heersin reviewed Oct 26, 2021

View reviewed changes

Uplifting 8051 architecture to new IL

b151f21

Basstorm force-pushed the asan-fuzz-uplifting-8051 branch from c147ae5 to 4b3c7b4 Compare November 18, 2021 07:58

Sync RZIL changes

1dc6426

Basstorm force-pushed the asan-fuzz-uplifting-8051 branch from 4b3c7b4 to 1dc6426 Compare November 18, 2021 08:02

Using bitvector instead of imm

f64f917

Basstorm force-pushed the asan-fuzz-uplifting-8051 branch from 6d7537f to f64f917 Compare November 18, 2021 09:09

XVilka mentioned this pull request Dec 10, 2021

ESIL to RZIL conversion #2080

Open

37 tasks

XVilka moved this from Review in progress to In progress in RzIL Feb 14, 2022

XVilka added the waiting-for-author Used to mark PRs where more work is needed label May 19, 2022

XVilka assigned imbillow Jun 8, 2022

Basstorm closed this Jul 11, 2022

RzIL automation moved this from In progress to Done Jul 11, 2022

imbillow mentioned this pull request Sep 2, 2022

Uplifting 8051 architecture to new IL #2999

Merged

5 tasks

Basstorm deleted the asan-fuzz-uplifting-8051 branch September 9, 2022 02:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uplifting 8051 architecture to new IL #1609

Uplifting 8051 architecture to new IL #1609

Basstorm commented Sep 7, 2021 •

edited

XVilka commented Sep 7, 2021

XVilka commented Oct 9, 2021

XVilka commented Oct 19, 2021

ret2libc Oct 19, 2021

XVilka Oct 19, 2021

Basstorm Nov 11, 2021

XVilka Oct 19, 2021

XVilka Oct 19, 2021

XVilka Oct 19, 2021

thestr4ng3r Oct 20, 2021

ret2libc Oct 20, 2021

thestr4ng3r Oct 20, 2021

Heersin Oct 26, 2021

Basstorm Nov 11, 2021

XVilka Oct 19, 2021

ret2libc Oct 20, 2021

thestr4ng3r Oct 20, 2021

XVilka Oct 26, 2021

ret2libc Oct 19, 2021

Basstorm Nov 11, 2021

ret2libc left a comment

XVilka commented Oct 20, 2021

ret2libc commented Oct 20, 2021

kazarmy commented Oct 20, 2021

Heersin left a comment

wargio commented Nov 18, 2021 •

edited

XVilka commented May 19, 2022

XVilka commented Jun 8, 2022

Basstorm commented Jul 11, 2022 •

edited

		@@ -1257,7 +3613,7 @@ static int i8051_op(RzAnalysis analysis, RzAnalysisOp op, ut64 addr, const ut8
		if (mask & RZ_ANALYSIS_OP_MASK_ESIL) {

Uplifting 8051 architecture to new IL #1609

Uplifting 8051 architecture to new IL #1609

Conversation

Basstorm commented Sep 7, 2021 • edited

XVilka commented Sep 7, 2021

XVilka commented Oct 9, 2021

XVilka commented Oct 19, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ret2libc left a comment

Choose a reason for hiding this comment

XVilka commented Oct 20, 2021

ret2libc commented Oct 20, 2021

kazarmy commented Oct 20, 2021

Heersin left a comment

Choose a reason for hiding this comment

wargio commented Nov 18, 2021 • edited

XVilka commented May 19, 2022

XVilka commented Jun 8, 2022

Basstorm commented Jul 11, 2022 • edited

Basstorm commented Sep 7, 2021 •

edited

wargio commented Nov 18, 2021 •

edited

Basstorm commented Jul 11, 2022 •

edited