Skip to content

Fix argument annotations to show only on source registers #24192 #24719

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

Closed
wants to merge 2 commits into from
Closed

Fix argument annotations to show only on source registers #24192 #24719

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
66cb4f5
Fix argument annotations to show only on source registers
vicky-dx Oct 20, 2025
6f14d56
Update test expectations for argument annotation fix
vicky-dx Oct 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions libr/anal/var.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1361,12 +1361,14 @@ R_API void r_anal_extract_rarg(RAnal *anal, RAnalOp *op, RAnalFunction *fcn, int
}
continue;
}
if (is_reg_in_src (regname, anal, op) || STR_EQUAL (regname, opdreg)) {
if (is_reg_in_src (regname, anal, op)) {
reg_set[i] = 1;
}
if (var) {
r_anal_var_set_access (anal, var, var->regname, op->addr, R_PERM_R, 0);
r_meta_set_string (anal, R_META_TYPE_VARTYPE, op->addr, var->name);
if (is_reg_in_src (regname, anal, op)) {
r_meta_set_string (anal, R_META_TYPE_VARTYPE, op->addr, var->name);
}
}
}
}
Expand All @@ -1386,7 +1388,9 @@ R_API void r_anal_extract_rarg(RAnal *anal, RAnalOp *op, RAnalFunction *fcn, int
if (newvar) {
r_anal_var_set_access (anal, newvar, newvar->regname, op->addr, R_PERM_R, 0);
}
r_meta_set_string (anal, R_META_TYPE_VARTYPE, op->addr, vname);
if (is_reg_in_src (selfreg, anal, op)) {
r_meta_set_string (anal, R_META_TYPE_VARTYPE, op->addr, vname);
}
free (vname);
(*count)++;
} else {
Expand Down
6 changes: 3 additions & 3 deletions test/db/anal/arm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -683,7 +683,7 @@ EXPECT=<<EOF
| 0x00010420 00482de9 push {fp, lr}
| 0x00010424 04b08de2 add fp, sp, 4
| 0x00010428 08d04de2 sub sp, sp, 8
| 0x0001042c 08000be5 str r0, [var_8h] ; 8 ; arg1
| 0x0001042c 08000be5 str r0, [var_8h] ; 8
| 0x00010430 14009fe5 ldr r0, [0x0001044c] ; [0x1044c:4]=0x104f0 "%d"
| 0x00010434 08101be5 ldr r1, [var_8h] ; 8
| 0x00010438 a2ffffeb bl sym.imp.printf
Expand All @@ -697,7 +697,7 @@ EXPECT=<<EOF
| 0x00010420 00482de9 push {fp, lr}
| 0x00010424 04b08de2 add fp, sp, 4
| 0x00010428 08d04de2 sub sp, sp, 8
| 0x0001042c 08000be5 str r0, [var_8h] ; 8 ; arg1
| 0x0001042c 08000be5 str r0, [var_8h] ; 8
| 0x00010430 14009fe5 ldr r0, [0x0001044c] ; [0x1044c:4]=0x104f0 "%d"
| 0x00010434 08101be5 ldr r1, [var_8h] ; 8
| 0x00010438 a2ffffeb bl sym.imp.printf
Expand Down Expand Up @@ -1211,7 +1211,7 @@ EXPECT=<<EOF
| 0x000083c0 04109de4 pop {r1}
| 0x000083c4 0d20a0e1 mov r2, sp
| 0x000083c8 04202de5 str r2, [sp, -4]!
| 0x000083cc 04002de5 str r0, [sp, -4]! ; arg1
| 0x000083cc 04002de5 str r0, [sp, -4]!
| 0x000083d0 10c09fe5 ldr ip, fcn.00008664 ; [0x83e8:4]=0x8664 fcn.00008664
| 0x000083d4 04c02de5 str ip, [sp, -4]!
| 0x000083d8 0c009fe5 ldr r0, main ; [0x83ec:4]=0x8470 main
Expand Down
4 changes: 2 additions & 2 deletions test/db/anal/jmptbl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ EXPECT=<<EOF
0x100004148 ldr x8, [x8, 0x390]
0x10000414c ldr x8, [x8]
0x100004150 stur x8, [x29, -0x60]
0x100004154 str x1, [var_80h] ; arg2
0x100004154 str x1, [var_80h]
0x100004158 lsr x25, x1, 0x3e ; arg2
0x10000415c adrp x24, reloc.Foundation.__DataStorage.bytes.allocator_...itcfc_ ; 0x10000c000
0x100004160 ldr x24, [x24, 0x398]
Expand Down Expand Up @@ -195,7 +195,7 @@ EXPECT=<<EOF
0x1000041b8 stur x24, [x29, -0x68]
0x1000041bc cmp x26, 0
0x1000041c0 csel x1, x26, xzr, gt
0x1000041c4 str x0, [arg_140hx88] ; arg1
0x1000041c4 str x0, [arg_140hx88]
0x1000041c8 sub x20, x29, 0x68
0x1000041cc mov w0, 0
0x1000041d0 mov w2, 0
Expand Down
33 changes: 33 additions & 0 deletions test/db/cmd/arg_annotations
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
NAME=Register argument annotations show on sources not destinations (AT&T)
FILE=-
CMDS=<<EOF
e asm.arch=x86
e asm.bits=64
e asm.syntax=att
wx 4889fa89f1
aaa
e asm.comments=true
pd 2~mov
EOF
EXPECT=<<EOF
| 0x00000000 4889fa movq %rdi, %rdx ; arg3
| 0x00000003 89f1 movl %esi, %ecx ; arg4
EOF
RUN

NAME=Register argument annotations show on sources not destinations (Intel)
FILE=-
CMDS=<<EOF
e asm.arch=x86
e asm.bits=64
e asm.syntax=intel
wx 4889fa89f1
aaa
e asm.comments=true
pd 2~mov
EOF
EXPECT=<<EOF
| 0x00000000 4889fa mov rdx, rdi ; arg1
| 0x00000003 89f1 mov ecx, esi ; arg2
EOF
RUN
4 changes: 2 additions & 2 deletions test/db/cmd/cmd_graph
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,10 +421,10 @@ digraph code {
graph [fontsize=8 fontname="Courier" bgcolor=azure splines="ortho"];
node [fillcolor=white style=filled shape=box];
edge [arrowhead="normal"];
"0x0000044b" [URL="fcn.0000044b/0x0000044b", fontcolor="#767676", fontname="Courier", label="54: fcn.0000044b (int64_t arg1, int64_t arg2, int64_t arg_4h, int64_t arg_8h, int64_t arg_34h, int64_t arg_38h);\l; arg int64_t arg1 @ rdi\l; arg int64_t arg2 @ rsi\l; arg int64_t arg_4h @ rsp+0x4\l; arg int64_t arg_8h @ rsp+0x8\l; arg int64_t arg_34h @ rsp+0x34\l; arg int64_t arg_38h @ rsp+0x38\l0x0000044b sar esi, 2 ; arg2\l0x0000044e test esi, esi ; arg2\l0x00000450 je 0x479\l"]
"0x0000044b" [URL="fcn.0000044b/0x0000044b", fontcolor="#767676", fontname="Courier", label="54: fcn.0000044b (int64_t arg1, int64_t arg2, int64_t arg_4h, int64_t arg_8h, int64_t arg_34h, int64_t arg_38h);\l; arg int64_t arg1 @ rdi\l; arg int64_t arg2 @ rsi\l; arg int64_t arg_4h @ rsp+0x4\l; arg int64_t arg_8h @ rsp+0x8\l; arg int64_t arg_34h @ rsp+0x34\l; arg int64_t arg_38h @ rsp+0x38\l0x0000044b sar esi, 2\l0x0000044e test esi, esi ; arg2\l0x00000450 je 0x479\l"]
"0x00000479" [URL="fcn.0000044b/0x00000479", fontcolor="#767676", fontname="Courier", label="0x00000479 add esp, 0x1c\l0x0000047c pop rbx\l0x0000047d pop rsi\l0x0000047e pop rdi\l0x0000047f pop rbp\l0x00000480 ret\l"]
"0x00000452" [URL="fcn.0000044b/0x00000452", fontcolor="#767676", fontname="Courier", label="0x00000452 lea esi, [rsi] ; arg2\l"]
"0x00000458" [URL="fcn.0000044b/0x00000458", fontcolor="#767676", fontname="Courier", label="0x00000458 mov eax, dword [arg_38h]\l0x0000045c mov dword [rsp], ebp\l0x0000045f mov dword [arg_8h], eax\l0x00000463 mov eax, dword [arg_34h]\l0x00000467 mov dword [arg_4h], eax\l0x0000046b call qword [rbx + rdi*4 - 0xf8]\l0x00000472 add edi, 1 ; arg1\l0x00000475 cmp edi, esi ; arg2\l0x00000477 jne 0x458\l"]
"0x00000458" [URL="fcn.0000044b/0x00000458", fontcolor="#767676", fontname="Courier", label="0x00000458 mov eax, dword [arg_38h]\l0x0000045c mov dword [rsp], ebp\l0x0000045f mov dword [arg_8h], eax\l0x00000463 mov eax, dword [arg_34h]\l0x00000467 mov dword [arg_4h], eax\l0x0000046b call qword [rbx + rdi*4 - 0xf8]\l0x00000472 add edi, 1\l0x00000475 cmp edi, esi ; arg2\l0x00000477 jne 0x458\l"]
"0x0000044b" -> "0x00000479" [color="#13a10e"];
"0x0000044b" -> "0x00000452" [color="#c50f1f"];
"0x00000452" -> "0x00000458" [color="cyan"];
Expand Down
4 changes: 2 additions & 2 deletions test/db/cmd/dwarf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -495,8 +495,8 @@ EXPECT=<<EOF
| : 0x100012e4 7c283840 cmpld r8, r7
| `=< 0x100012e8 4082ffe0 bne 0x100012c8
| 0x100012ec 39200000 li r9, 0
| 0x100012f0 91230144 stw r9, 0x144(r3) ; arg1
| 0x100012f4 91230148 stw r9, 0x148(r3) ; arg1
| 0x100012f0 91230144 stw r9, 0x144(r3)
| 0x100012f4 91230148 stw r9, 0x148(r3)
\ 0x100012f8 4e800020 blr
EOF
RUN
Expand Down
24 changes: 12 additions & 12 deletions test/db/formats/elf/random
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,18 +36,18 @@ EXPECT=<<EOF
| 0x08000814 e91300f9 str x9, [var_20h]
| 0x08000818 e90308aa mov x9, x8
| 0x0800081c e81340f9 ldr x8, [var_20h]
| 0x08000820 e21700f9 str x2, [var_28h] ; arg3
Copy link
Collaborator

@trufae trufae Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this code is storing the values passed as arguments into the stack. thats common on arm and mips programs because this way they can reuse registers for other purposes. so imho the pr is not correct

| 0x08000824 002100f9 str x0, [x8, 0x40] ; arg1
| 0x08000828 011d00f9 str x1, [x8, 0x38] ; arg2
| 0x0800082c 031900f9 str x3, [x8, 0x30] ; arg4
| 0x08000830 041500f9 str x4, [x8, 0x28] ; arg5
| 0x08000820 e21700f9 str x2, [var_28h]
| 0x08000824 002100f9 str x0, [x8, 0x40]
| 0x08000828 011d00f9 str x1, [x8, 0x38]
| 0x0800082c 031900f9 str x3, [x8, 0x30]
| 0x08000830 041500f9 str x4, [x8, 0x28]
| 0x08000834 08000090 adrp x8, segment.ehdr ; 0x8000000; RELOC 32 .rodata @ 0x080057f8 + 0x18
| 0x08000838 08010091 add x8, x8, 0 ; RELOC 32 .rodata @ 0x080057f8 + 0x18 ; segment.ehdr
| 0x0800083c 0001c03d ldr q0, [x8] ; segment.ehdr
| 0x08000840 2001803d str q0, [x9]
| 0x08000844 080940f9 ldr x8, [x8, 0x10]
| 0x08000848 280900f9 str x8, [x9, 0x10]
| 0x0800084c 5f0000b9 str wzr, [x2] ; arg3
| 0x0800084c 5f0000b9 str wzr, [x2]
EOF
EXPECT_ERR=<<EOF
WARN: Poorly supported AARCH64 instruction reloc type 275 at 0x08000074
Expand Down Expand Up @@ -117,18 +117,18 @@ EXPECT=<<EOF
| 0x00000fa8 e91300f9 str x9, [var_20h]
| 0x00000fac e90308aa mov x9, x8
| 0x00000fb0 e81340f9 ldr x8, [var_20h]
| 0x00000fb4 e21700f9 str x2, [var_28h] ; arg3
Copy link
Collaborator

@trufae trufae Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually this is correct. the function is saving the arguments passed as parameters into local variables. im unsure about this pr.. ill think about it before merging or not

Copy link
Contributor Author

@vicky-dx vicky-dx Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand your point that showing which argument is being saved can be informative. However, there are a few reasons why annotations on source registers are clearer:

  1. Original Issue Reporter's Complaint
    The issue reporter @l0kh specifically stated in Comments and color problems in AT&T mode #24192

"The arg3 and arg4 are totally false as ecx and rdx are the destination of the mov."

For this AT&T code:

leaq obj.VYeXkgjLLMrczyw7i7dJPkyAbxqgCahe, %rdx ; arg3
movb %cl, (%rax, %rdx)

They found it confusing that %rdx (destination) shows; arg3when it's being overwritten, not read.

  1. Ambiguity Problem
    When both source and destination can have annotations, it's unclear which operand contains the actual argument:

Before (ambiguous):

movq %rdi, %rdx    ; arg3    ← Which one has arg3? Both shown!

After (clear):

movq %rdi, %rdx    ; arg1    ← Only %rdi (source) has arg1
  1. For Saving Arguments
    When saving arguments to stack, the annotation still appears - on the source register being read:

This is actually more accurate because:

x2 (source) currently contains arg3
[var_28h] (destination) will contain arg3 after execution
The annotation shows the current state, not the future state.

  1. Consistency with Analysis
    The destination register will be overwritten immediately, so it no longer contains the argument after this instruction. Annotating sources maintains consistency with what's true at this line.

But if you disagree with this approach , i will be happy to close this PR

| 0x00000fb8 002100f9 str x0, [x8, 0x40] ; arg1
| 0x00000fbc 011d00f9 str x1, [x8, 0x38] ; arg2
| 0x00000fc0 031900f9 str x3, [x8, 0x30] ; arg4
| 0x00000fc4 041500f9 str x4, [x8, 0x28] ; arg5
| 0x00000fb4 e21700f9 str x2, [var_28h]
| 0x00000fb8 002100f9 str x0, [x8, 0x40]
| 0x00000fbc 011d00f9 str x1, [x8, 0x38]
| 0x00000fc0 031900f9 str x3, [x8, 0x30]
| 0x00000fc4 041500f9 str x4, [x8, 0x28]
| 0x00000fc8 280000b0 adrp x8, 0x5000
| 0x00000fcc 08e13e91 add x8, x8, 0xfb8 ; str.O_n_
| 0x00000fd0 0001c03d ldr q0, [x8] ; str.O_n_
| 0x00000fd4 2001803d str q0, [x9]
| 0x00000fd8 080940f9 ldr x8, [x8, 0x10]
| 0x00000fdc 280900f9 str x8, [x9, 0x10]
| 0x00000fe0 5f0000b9 str wzr, [x2] ; arg3
| 0x00000fe0 5f0000b9 str wzr, [x2]
0x00005fb8 0x5a8a51be3e7c0a4f O.|>.Q.Z
EOF
EXPECT_ERR=<<EOF
Expand Down
Loading