Fix argument annotations to show only on source registers #24192

[vicky-dx]

Fixes #24192 - Register argument annotations now appear only on source operands, not on destinations being overwritten.

Problem

In AT&T syntax, inline ; argN comments were appearing ambiguously on both:

• Source registers (being read - contains the actual argument value) ✓ correct
• Destination registers (being overwritten - loses the argument) ✗ wrong

Example of the bug:

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

This made it unclear which operand actually contains the argument value.

Solution

Modified libr/anal/var.c to only create R_META_TYPE_VARTYPE metadata when the register appears in the source position (op->srcs), not in the destination position.

After fix:

movq %rdi, %rdx    ; arg1    ← clear! %rdi (source) contains arg1
movl %esi, %ecx    ; arg2    ← clear! %esi (source) contains arg2

Changes

1. Line 1364: Removed destination register check from condition

   • Before: if (is_reg_in_src() || STR_EQUAL(regname, opdreg))
   • After: if (is_reg_in_src())

2. Lines 1369-1371: Wrapped r_meta_set_string() in is_reg_in_src() guard

3. Lines 1391-1393: Same guard for selfreg annotation case

Why This Works

• Uses structured data (op->srcs vector) instead of string parsing
• Syntax-agnostic: Works for both AT&T and Intel syntax
• Correctly tracks which registers are being read vs written

Testing

New regression tests in test/db/cmd/arg_annotations:

• Test 1: AT&T syntax annotations on sources
• Test 2: Intel syntax annotations on sources

No regressions in existing tests:

• test/db/cmd/feat_variables: 23/23 passing
• test/db/cmd/cmd_print: 70/71 passing (1 broken)
• test/db/anal/vars: 10/13 passing (2 ARM tests already broken before this fix)

Manual testing with 6 different test binaries - all correct

Example Output

AT&T Syntax:

movq %rdi, %rdx             ; arg1    ← %rdi is source (has arg1)
movl %esi, %ecx             ; arg2    ← %esi is source (has arg2)
movslq %esi, %rax           ; arg2    ← %esi is source
leal (%rsi, %rsi), %r9d     ; arg2    ← %rsi in memory ref

Intel Syntax:

mov rdx, rdi                ; arg1    ← rdi is 2nd operand (source in Intel)
mov ecx, esi                ; arg2    ← esi is 2nd operand (source in Intel)

• Commit message references issue Comments and color problems in AT&T mode #24192

---

This fix makes argument tracking much clearer in disassembly, especially for AT&T syntax users!

[trufae]

update the broken tests

[trufae]

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

[vicky-dx]

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.

2. 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

3. 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.

4. 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

[vicky-dx]

@trufae Hello, am i officially a contributor now?😆 just asking,
i always wanted to be the part of this community

[trufae]

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

[trufae]

.

[trufae]

@trufae Hello, am i officially a contributor now?😆 just asking, i always wanted to be the part of this community

You are contributing, so yes you are :p

[vicky-dx]

@trufae, I understand the concern. While this fix addresses the ambiguity in x86 AT&T syntax (issue #24192), it removes valuable information for ARM/MIPS architectures where storing arguments to stack is a common prologue pattern.

In ARM/MIPS:

str x0, [sp, 0x10]  ; arg1
str x1, [sp, 0x18]  ; arg2

Showing annotations on destination (stack locations) is actually useful because it documents where arguments are stored for later use.

The original issue reporter's complaint was specific to register-to-register moves in x86 where it's ambiguous which operand contains the argument.
in my opinion a better solution would need to:

1. Distinguish between memory destinations (useful) vs register destinations (confusing)
2. Or be architecture-specific

Since this requires more thought and design, I'm closing this PR for now.
Thanks for the feedback!