Split afvx to two commands - one for variables and one for function arguments

[XVilka]

Is your feature request related to a problem? Please describe.
Currently it's afvx[j] returns the output for both local variables and function arguments which is not always desirable.

[0x00004d80]> afv?
Usage: afv  [rbs]
| afv*                          output rizin command to add args/locals to flagspace
| afv-([name])                  remove all or given var
| afv=                          list function variables and arguments with disasm refs
| afva                          analyze function arguments/locals
| afvb[?]                       manipulate bp based arguments/locals
| afvd name                     output rizin command for displaying the value of args/locals in the debugger
| afvf                          show BP relative stackframe variables
| afvn [new_name] ([old_name])  rename argument/local
| afvr[?]                       manipulate register based arguments
| afvR [varname]                list addresses where vars are accessed (READ)
| afvs[?]                       manipulate sp based arguments/locals
| afvt [name] [new_type]        change type for given argument/local
| afvW [varname]                list addresses where vars are accessed (WRITE)
| afvx                          show function variable xrefs (same as afvR+afvW)
[0x00004d80]> s main
[0x00004d80]> pd 10
            ; DATA XREF from entry0 @ 0x6b81
╭ 6697: int main (int argc, char **argv);
│ bp: 0 (vars 0, args 0)
│ sp: 17 (vars 17, args 0)
│ rg: 2 (vars 0, args 2)
│           0x00004d80      endbr64
│           0x00004d84      push  r15
│           0x00004d86      push  r14
│           0x00004d88      push  r13
│           0x00004d8a      push  r12
│           0x00004d8c      push  rbp
│           0x00004d8d      push  rbx
│           0x00004d8e      sub   rsp, 0x68
│           0x00004d92      mov   r12, qword [rsi]                     ; argv
│           0x00004d95      mov   rax, qword fs:[0x28]
[0x00004d80]> afvx
afvR
      argv  0x4d92,0x4db0
      argc  0x4dae
   var_58h
   var_40h  0x4fae,0x501c,0x6143,0x685f,0x688d
   var_10h  0x4ffe,0x5cec,0x613b,0x6833,0x6892
   var_1ch  0x53b0,0x53be,0x5ecd
   var_38h  0x5030,0x6091,0x684a,0x6876
    var_8h  0x5279,0x639b,0x6653,0x66a7,0x6933
   var_55h  0x60fb
   var_57h
   var_56h
  var_8h_2
 var_10h_2  0x5aab
   var_28h  0x5ae1,0x5cdb
 var_40h_2  0x5d04
 var_1ch_2
 var_38h_2  0x6aca
 var_55h_2  0x6b44
   var_42h  0x4fd1
afvW
      argv
      argc
   var_58h  0x4d9e
   var_40h  0x608c
   var_10h  0x4fbd,0x5ab9
   var_1ch  0x5028,0x529c
   var_38h  0x505e,0x605a,0x60b7,0x60d1,0x60e2,0x681f,0x687f
    var_8h  0x503c,0x6377,0x6645,0x6929
   var_55h  0x6074,0x60bf
   var_57h  0x607b
   var_56h  0x60da
  var_8h_2  0x5603,0x5915,0x5938,0x5a21,0x5a7a,0x5a8e,0x5b0f
 var_10h_2
   var_28h  0x5ad2,0x5cd1
 var_40h_2
 var_1ch_2  0x6013
 var_38h_2
 var_55h_2  0x6b3d
   var_42h
[0x00004d80]> afvxj
{"reads":[{"name":"argv","addrs":[19858,19888]},{"name":"argc","addrs":[19886]},{"name":"var_58h","addrs":[]},{"name":"var_40h","addrs":[20398,20508,24899,26719,26765]},{"name":"var_10h","addrs":[20478,23788,24891,26675,26770]},{"name":"var_1ch","addrs":[21424,21438,24269]},{"name":"var_38h","addrs":[20528,24721,26698,26742]},{"name":"var_8h","addrs":[21113,25499,26195,26279,26931]},{"name":"var_55h","addrs":[24827]},{"name":"var_57h","addrs":[]},{"name":"var_56h","addrs":[]},{"name":"var_8h_2","addrs":[]},{"name":"var_10h_2","addrs":[23211]},{"name":"var_28h","addrs":[23265,23771]},{"name":"var_40h_2","addrs":[23812]},{"name":"var_1ch_2","addrs":[]},{"name":"var_38h_2","addrs":[27338]},{"name":"var_55h_2","addrs":[27460]},{"name":"var_42h","addrs":[20433]}],"writes":[{"name":"argv","addrs":[]},{"name":"argc","addrs":[]},{"name":"var_58h","addrs":[19870]},{"name":"var_40h","addrs":[24716]},{"name":"var_10h","addrs":[20413,23225]},{"name":"var_1ch","addrs":[20520,21148]},{"name":"var_38h","addrs":[20574,24666,24759,24785,24802,26655,26751]},{"name":"var_8h","addrs":[20540,25463,26181,26921]},{"name":"var_55h","addrs":[24692,24767]},{"name":"var_57h","addrs":[24699]},{"name":"var_56h","addrs":[24794]},{"name":"var_8h_2","addrs":[22019,22805,22840,23073,23162,23182,23311]},{"name":"var_10h_2","addrs":[]},{"name":"var_28h","addrs":[23250,23761]},{"name":"var_40h_2","addrs":[]},{"name":"var_1ch_2","addrs":[24595]},{"name":"var_38h_2","addrs":[]},{"name":"var_55h_2","addrs":[27453]},{"name":"var_42h","addrs":[]}]}

Describe the solution you'd like
I suggest to split the command in two:

• afvx will show both
• afvxa will show only arguments
• afvxv will show only local variables
• all commands have JSON output.

[officialcjunior]

How should the check to see whether the variable is a function argument or not, be implemented?
This is where all the names are printed.

[XVilka]

@officialcjunior you can use the bool isarg field of the RzAnalysisVar to check if it's the argument.
For example, like in https://github.com/rizinorg/rizin/blob/dev/librz/core/cmd_analysis.c#L832:

rz_list_foreach (all_vars, iter, var) {
		if (var->isarg) {
			if (!rz_strbuf_setf (&key, "func.%s.arg.%d", fcn->name, arg_count) ||
				!rz_strbuf_setf (&value, "%s,%s", var->type, var->name)) {
				goto exit;
			}
			sdb_set (core->analysis->sdb_types, rz_strbuf_get (&key), rz_strbuf_get (&value), 0);
			arg_count++;
		}
	}

[officialcjunior]

Great. Currently, cmd_afvx() is a function. Should a new function be created or incorporated into the same function with the help of a boolean variable?

[XVilka]

@officialcjunior I think it's better to use two separate functions. Just please avoid code duplication.

[ret2libc]

Please note that afvx and afv were already converted to newshell.

[fangxlmr]

@fhgkdhg This issue is asigned to you as RSoC micro-task, so you could work on this from now on. Feel free to ask questions and send Pull Requests.

In case for anyone who is not aware about RSoC.

CC @XVilka

[fhgkdhg]

@XVilka My idea is add a parameter of boolean in function list_vars(), then add two functions named cmd_afvxa() and cmd_afvxv() in file cmd_analysis.c. And both of them will be called in function cmd_afvx(). But I can't avoid code duplication.

static void list_vars(RzCore *core, RzAnalysisFunction *fcn, PJ *pj, int type, const char *name, bool is_arg){
    ...
    rz_list_foreach (list, iter, var) {
        if(var->isarg == is_arg)
	    var_accesses_list(fcn, var, pj, access_type, var->name);
    }
    ...
}

static void cmd_afvx(RzCore *core, RzAnalysisFunction *fcn, bool json) {
	cmd_afvxa(core, fcn, json);
	cmd_afvxv(core, fcn, json);
}

static void cmd_afvxa(RzCore *core, RzAnalysisFunction *fcn, bool json){
    ...
    list_vars(core, fcn, pj, 'R/W', NULL, true);
    ...
}

// is similar to function cmd_afvxa()
static void cmd_afvxv(RzCore *core, RzAnalysisFunction *fcn, bool json){
    ...
    list_vars(core, fcn, pj, 'R/W', NULL, false);
    ...
}

sorry, I can't figure out a better way to solve the problem.

[ret2libc]

@fhgkdhg you have to consider the code in rz_analysis_function_vars_xrefs_handler, not cmd_afvx (that is not used anymore). Also, to add new commands you have to use cmd_analysis.yaml. I suggest to look at it, at cmd_descs.yaml (and the comments there) and also to look at doc/newshell.md. Please let us know if something is not clear.

[fhgkdhg]

@ret2libc thanks, I've added two subcommands below command afvx in file cmd_analysis.yaml, and added two functions in file cmd_analysis.c following instructions in new_shell.md. And next, I'd like to modify funciton list_vars() like I said yesterday and finish the code in the function I added.
I noticed that functions named "*handler" in file cmd_analysis.c, they all have the prefix "rz", shoud I add this prefix to function name too?

[ret2libc]

I noticed that functions named "*handler" in file cmd_analysis.c, they all have the prefix "rz", shoud I add this prefix to function name too?

Yes, use rz_ prefix as said in https://github.com/rizinorg/rizin/blob/dev/librz/core/cmd_descs/cmd_descs.yaml#L31 .

[fangxlmr]

@no1rr This issue is asigned to you as RSoC micro-task. Since @fhgkdhg isn't going to send any PR, you could continue this work. Feel free to ask questions and send Pull Requests.

In case for anyone who is not aware about RSoC.

CC @XVilka

[no1rr]

@ret2libc I can't find any function that calls rz_analysis_function_vars_xrefs_handler, where are the parameters passed to rz_analysis_function_vars_xrefs_handler?

[ret2libc]

@ret2libc I can't find any function that calls rz_analysis_function_vars_xrefs_handler, where are the parameters passed to rz_analysis_function_vars_xrefs_handler?

Those command handler functions are called automatically by the command parser (RzCmd). You can see in the YAML files in librz/core/cmd_descs/ which handler is called for each command. In particular, afvx command is defined in https://github.com/rizinorg/rizin/blob/dev/librz/core/cmd_descs/cmd_analysis.yaml#L227 and the handler is defined with cname: analysis_function_vars_xrefs (the rz_ prefix and the _handler suffix are automatically added).

The parameters passed to that handler are simply the arguments passed by the user when calling the afvx command. It works in the same way as main in a regular C program (so argv[0] contains the command name, and then there are the arguments starting from argv[1:]).

[fangxlmr]

@no1rr Make a PR for review please. Thanks.