Attasm #25028

trufae · 2025-12-12T09:24:05Z

Mark this if you consider it ready to merge
I've added tests (optional)
I wrote some lines in the book (optional)

Description

Copilot

Pull request overview

This PR implements AT&T to Intel syntax conversion for x86 assembly in radare2. The implementation extracts and refactors the conversion logic into a reusable utility function, enabling assembly operations to automatically convert AT&T syntax to Intel when needed.

Key changes:

Adds a new shared AT&T to Intel converter in libr/util/str_att.c with comprehensive instruction coverage
Refactors the existing att2intel plugin to use the new shared implementation
Integrates automatic syntax conversion in the assembler when AT&T syntax is specified but encoders only support Intel
Adds comprehensive test coverage with 35+ test cases covering various x86 instructions and edge cases

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
`libr/util/str_att.c`	New implementation of AT&T to Intel syntax converter with instruction tables and conversion logic
`libr/arch/p/x86_nz/att2intel.c`	Refactored to delegate to the shared `r_str_att2intel()` function
`libr/asm/asm.c`	Integration of automatic syntax conversion in assembly process with arch-specific checks
`libr/include/r_util/r_str.h`	API declaration for `r_str_att2intel()`
`libr/include/r_asm.h`	API declaration for `r_asm_att2intel()` wrapper
`libr/include/r_arch.h`	New constants and bitmasks for encoder syntax support
`libr/util/meson.build`	Added `str_att.c` to meson build
`libr/util/Makefile`	Added `str_att.o` to makefile build
`test/db/cmd/attasm`	Comprehensive test suite with 35+ tests for AT&T syntax conversion
`test/db/cmd/cmd_asm_syntax`	Empty placeholder test file

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-12T09:44:38Z

libr/util/str_att.c

+		while (start > buf && (isdigit ((unsigned char)*(start - 1)) || *(start - 1) == '-' || *(start - 1) == '+')) {
+			start--;
+		}
+		if (start < ptr && start > buf && *(start - 1) != ' ' && *(start - 1) != ',') {
+			// skip this case, it's part of an operand name
+		} else if (start < ptr) {
+			// there's a displacement
+			int disp = atoi (start);


The function uses 'isdigit' to detect displacement values, but this won't correctly handle hexadecimal values like '0x10' in AT&T syntax. When the code checks for digits before the bracket at line 252, it will only match decimal digits. For hex values like '0x10[rbp]', the 'x' character will stop the backward scan prematurely. Consider supporting hexadecimal displacement values or documenting this limitation.

Suggested change

while (start > buf && (isdigit ((unsigned char)*(start - 1)) || *(start - 1) == '-' || *(start - 1) == '+')) {

start--;

}

if (start < ptr && start > buf && *(start - 1) != ' ' && *(start - 1) != ',') {

// skip this case, it's part of an operand name

} else if (start < ptr) {

// there's a displacement

int disp = atoi (start);

while (start > buf) {

char c = *(start - 1);

if (isdigit((unsigned char)c) || c == '-' || c == '+' ||

(c >= 'a' && c <= 'f') || (c >= 'A' && c <= 'F') ||

c == 'x' || c == 'X') {

start--;

} else {

break;

}

}

if (start < ptr && start > buf && *(start - 1) != ' ' && *(start - 1) != ',') {

// skip this case, it's part of an operand name

} else if (start < ptr) {

// there's a displacement

int disp = (int)strtol(start, NULL, 0);

Copilot · 2025-12-12T09:44:38Z

libr/util/str_att.c

+	}
+
+	// skip directives and labels
+	if (*buf == '.' || (strlen (buf) > 0 && buf[strlen (buf) - 1] == ':')) {


When checking 'strlen(buf) > 0' at line 232, this is redundant since if '*buf' is checked first with the condition '*buf == '.'', then 'strlen(buf)' must be > 0. The condition could be simplified to just checking if 'buf[strlen(buf) - 1] == ':''. However, consider that calling 'strlen' twice (once for comparison, once for indexing) is inefficient. Store the length in a variable or use a pointer to check the last character.

Suggested change

if (*buf == '.' || (strlen (buf) > 0 && buf[strlen (buf) - 1] == ':')) {

size_t len = strlen (buf);

if (*buf == '.' || (len > 0 && buf[len - 1] == ':')) {

Copilot · 2025-12-12T09:44:39Z

libr/util/str_att.c

+	if (argc >= 2 && (is_in_list (argv[0], ops_1) || is_in_list (op, ops_1))) {
+		// 1 operand instruction
+		if (newstr) {
+			snprintf (newstr, 256, "%s %s", op, argv[1]);


The function uses hardcoded buffer size 256 for snprintf calls, but allocates the buffer with strlen(att_str) + 256 on line 356. If the operands are long (especially memory addressing expressions), the snprintf calls could exceed 256 bytes. Consider using a consistent buffer size calculation or using the actual allocated size.

Copilot · 2025-12-12T09:44:39Z

libr/util/str_att.c

+
+	// no operands fallback
+	if (argc == 1 && newstr) {
+		strcpy (newstr, op);


Using strcpy without bounds checking. Although 'op' is limited to 32 bytes and 'newstr' is allocated with strlen(att_str) + 256 on line 356, it's safer to use a bounds-checked copy function like strncpy or verify the size explicitly.

Copilot · 2025-12-12T09:44:39Z

libr/util/str_att.c

+	if (is_in_list (argv[0], ops_0) || is_in_list (op, ops_0)) {
+		// 0 operand instruction
+		if (newstr) {
+			strcpy (newstr, op);


Using strcpy without bounds checking. Although 'op' is limited to 32 bytes and 'newstr' is allocated with strlen(att_str) + 256 on line 356, it's safer to use a bounds-checked copy function like strncpy or verify the size explicitly.

Copilot · 2025-12-12T09:44:41Z

libr/util/str_att.c

+						// rebuild: [inner+disp]rest
+						size_t avail = strlen (att_str) + 64 - (start - buf);
+						snprintf (start, avail, "[%s%+d]%s", inner, disp, rest ? rest : "");


If 'strdup(close + 1)' fails and returns NULL on line 269, the code continues and passes 'rest ? rest : ""' to snprintf. While this prevents a crash, if strdup failed due to memory pressure, the subsequent malloc on line 356 might also fail. Consider checking for allocation failures and returning early to avoid inconsistent state.

Suggested change

// rebuild: [inner+disp]rest

size_t avail = strlen (att_str) + 64 - (start - buf);

snprintf (start, avail, "[%s%+d]%s", inner, disp, rest ? rest : "");

if (!rest) {

free (buf);

return NULL;

}

// rebuild: [inner+disp]rest

size_t avail = strlen (att_str) + 64 - (start - buf);

snprintf (start, avail, "[%s%+d]%s", inner, disp, rest);

Copilot · 2025-12-12T09:44:42Z

libr/util/str_att.c

+	if (argc >= 3 && (is_in_list (argv[0], ops_2swap) || is_in_list (op, ops_2swap))) {
+		// 2 operand instruction - swap operands (ATT: src, dst -> Intel: dst, src)
+		if (newstr) {
+			snprintf (newstr, 256, "%s %s, %s", op, argv[2], argv[1]);


The function uses hardcoded buffer size 256 for snprintf calls, but allocates the buffer with strlen(att_str) + 256 on line 356. If the operands are long (especially memory addressing expressions), the snprintf calls could exceed 256 bytes. Consider using a consistent buffer size calculation or using the actual allocated size.

Copilot · 2025-12-12T09:44:42Z

libr/util/str_att.c

+		snprintf (newstr, 256, "%s %s, %s", op, argv[2], argv[1]);
+		return true;
+	}
+
+	// single operand fallback
+	if (argc == 2 && newstr) {
+		snprintf (newstr, 256, "%s %s", op, argv[1]);


The function uses hardcoded buffer size 256 for snprintf calls, but allocates the buffer with strlen(att_str) + 256 on line 356. If the operands are long (especially memory addressing expressions), the snprintf calls could exceed 256 bytes. Consider using a consistent buffer size calculation or using the actual allocated size.

Copilot · 2025-12-12T09:44:42Z

libr/util/str_att.c

+						size_t avail = strlen (att_str) + 64 - (start - buf);
+						snprintf (start, avail, "[%s%+d]%s", inner, disp, rest ? rest : "");


The buffer size calculation is incorrect. 'avail' is computed as strlen(att_str) + 64 - (start - buf), but 'buf' may have been modified (characters replaced) so its length may be different from att_str. The available space should be based on the actual allocated size of 'buf' or the remaining space from 'start' to the end of 'buf'. This could lead to buffer overflow.

Copilot · 2025-12-12T09:44:43Z

test/db/cmd/cmd_asm_syntax

@@ -0,0 +1 @@
+


This test file appears to be empty (contains only whitespace). If this is intentional as a placeholder, consider either removing it or adding a comment explaining its purpose. Empty test files can be confusing for maintainers.

Suggested change

# This file is intentionally left empty as a placeholder for future tests.

radare added 3 commits December 12, 2025 07:30

Fix the att assembler support ##asm

dd6bdd7

ii

1c78b28

asmatt

a94db68

Copilot AI review requested due to automatic review settings December 12, 2025 09:24

Copilot started reviewing on behalf of trufae December 12, 2025 09:34 View session

Copilot AI reviewed Dec 12, 2025

View reviewed changes

trufae mentioned this pull request Dec 14, 2025

-S Option is Ignored When Assembling in rasm2 #25034

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Attasm #25028

Attasm #25028

Uh oh!

trufae commented Dec 12, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	if (*buf == '.' \|\| (strlen (buf) > 0 && buf[strlen (buf) - 1] == ':')) {
	size_t len = strlen (buf);
	if (*buf == '.' \|\| (len > 0 && buf[len - 1] == ':')) {

		size_t avail = strlen (att_str) + 64 - (start - buf);
		snprintf (start, avail, "[%s%+d]%s", inner, disp, rest ? rest : "");


	# This file is intentionally left empty as a placeholder for future tests.

Uh oh!

Attasm #25028

Are you sure you want to change the base?

Attasm #25028

Uh oh!

Conversation

trufae commented Dec 12, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants