From: Utkarsh Bahuguna <[email protected]>
{} in asm! was numbered by where the operand sits in the call, but gcc
gets the outputs first and then the inputs. So an input before an output
pointed at the wrong operand, mov {}, {} moved x into y and only worked
when both got the same register. A placeholder past the last operand is
an error now.
Fixes Rust-GCC/gccrs#5057
Assisted-by: Claude Opus (Anthropic)
gcc/rust/ChangeLog:
* expand/rust-macro-builtins-asm.cc (is_output_operand): New.
(gcc_operand_numbers): New.
(expand_inline_asm_strings): Renumber the placeholders and reject
one past the last operand.
gcc/testsuite/ChangeLog:
* rust/execute/torture/issue-5057.rs: New test.
* rust/compile/issues/issue-5057-2.rs: New test.
Signed-off-by: Utkarsh Bahuguna <[email protected]>
Tested on x86_64-pc-linux-gnu.
---
gcc/rust/expand/rust-macro-builtins-asm.cc | 62 ++++++++++++++++++-
.../rust/compile/issues/issue-5057-2.rs | 19 ++++++
.../rust/execute/torture/issue-5057.rs | 27 ++++++++
3 files changed, 106 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/rust/compile/issues/issue-5057-2.rs
create mode 100644 gcc/testsuite/rust/execute/torture/issue-5057.rs
diff --git a/gcc/rust/expand/rust-macro-builtins-asm.cc
b/gcc/rust/expand/rust-macro-builtins-asm.cc
index a3683a0ca5b..bde990db7fb 100644
--- a/gcc/rust/expand/rust-macro-builtins-asm.cc
+++ b/gcc/rust/expand/rust-macro-builtins-asm.cc
@@ -774,12 +774,52 @@ parse_asm_arg (InlineAsmContext inline_asm_ctx)
return tl::expected<InlineAsmContext, InlineAsmParseError> (inline_asm_ctx);
}
+// same split as the backend, these operands go in the output list
+static bool
+is_output_operand (const AST::InlineAsmOperand &operand)
+{
+ switch (operand.get_register_type ())
+ {
+ case AST::InlineAsmOperand::RegisterType::Out:
+ case AST::InlineAsmOperand::RegisterType::InOut:
+ case AST::InlineAsmOperand::RegisterType::SplitInOut:
+ return true;
+ default:
+ return false;
+ }
+}
+
+// asm! numbers its operands as written, but the backend hands gcc all the
+// outputs first and then the inputs, so a placeholder has to be renumbered
+// to where its operand ends up.
+static std::vector<size_t>
+gcc_operand_numbers (const std::vector<AST::InlineAsmOperand> &operands)
+{
+ size_t outputs = 0;
+ for (auto &operand : operands)
+ if (is_output_operand (operand))
+ outputs++;
+
+ std::vector<size_t> numbers;
+ size_t out_seen = 0;
+ size_t in_seen = 0;
+ for (auto &operand : operands)
+ {
+ if (is_output_operand (operand))
+ numbers.push_back (out_seen++);
+ else
+ numbers.push_back (outputs + in_seen++);
+ }
+ return numbers;
+}
+
tl::expected<InlineAsmContext, InlineAsmParseError>
expand_inline_asm_strings (InlineAsmContext inline_asm_ctx)
{
auto &inline_asm = inline_asm_ctx.inline_asm;
auto str_vec = inline_asm.get_template_strs ();
+ auto numbers = gcc_operand_numbers (inline_asm.get_operands ());
decltype (str_vec) resulting_template_vec;
for (auto &template_str : str_vec)
@@ -809,6 +849,14 @@ expand_inline_asm_strings (InlineAsmContext inline_asm_ctx)
case Fmt::ffi::Position::Tag::ArgumentImplicitlyIs:
{
auto idx = next_argument.position.argument_implicitly_is._0;
+ if (idx >= numbers.size ())
+ {
+ rust_error_at (inline_asm.get_locus (),
+ "invalid reference to argument at "
+ "index %lu",
+ (unsigned long) idx);
+ break;
+ }
/*auto trait = next_argument.format;*/
/*auto arg = arguments.at (idx);*/
@@ -819,7 +867,8 @@ expand_inline_asm_strings (InlineAsmContext inline_asm_ctx)
/* args.push_back ({arg.get_expr ().clone_expr (),
* trait});*/
- transformed_template_str += "%" + std::to_string (idx);
+ transformed_template_str
+ += "%" + std::to_string (numbers[idx]);
// std::cout << "argument implicitly is: " << idx <<
// std::endl; std::cout << "transformed template str is:"
// << transformed_template_str << std::endl;
@@ -831,7 +880,16 @@ expand_inline_asm_strings (InlineAsmContext inline_asm_ctx)
case Fmt::ffi::Position::Tag::ArgumentIs:
{
auto idx = next_argument.position.argument_is._0;
- transformed_template_str += "%" + std::to_string (idx);
+ if (idx >= numbers.size ())
+ {
+ rust_error_at (inline_asm.get_locus (),
+ "invalid reference to argument at "
+ "index %lu",
+ (unsigned long) idx);
+ break;
+ }
+ transformed_template_str
+ += "%" + std::to_string (numbers[idx]);
break;
}
case Fmt::ffi::Position::Tag::ArgumentNamed:
diff --git a/gcc/testsuite/rust/compile/issues/issue-5057-2.rs
b/gcc/testsuite/rust/compile/issues/issue-5057-2.rs
new file mode 100644
index 00000000000..43bd1e0f989
--- /dev/null
+++ b/gcc/testsuite/rust/compile/issues/issue-5057-2.rs
@@ -0,0 +1,19 @@
+#![feature(no_core)]
+#![no_core]
+#![feature(rustc_attrs)]
+
+// Rust-GCC/gccrs#5057: a placeholder past the last operand is an error,
+// not something to hand gcc
+
+#[rustc_builtin_macro]
+macro_rules! asm {
+ () => {};
+}
+
+fn main() {
+ let y: i32 = 9;
+ let mut x: i32 = 0;
+ unsafe {
+ asm!("mov {2}, {}", in(reg) y, out(reg) x); // { dg-error "invalid
reference to argument at index 2" }
+ }
+}
diff --git a/gcc/testsuite/rust/execute/torture/issue-5057.rs
b/gcc/testsuite/rust/execute/torture/issue-5057.rs
new file mode 100644
index 00000000000..f612704e3f7
--- /dev/null
+++ b/gcc/testsuite/rust/execute/torture/issue-5057.rs
@@ -0,0 +1,27 @@
+/* { dg-do run { target x86_64*-*-* } } */
+/* { dg-output "9\r*\n" } */
+#![feature(no_core)]
+#![no_core]
+#![feature(rustc_attrs)]
+
+// Rust-GCC/gccrs#5057: an input listed before an output got the output's
+// operand number, so this moved x into y
+
+#[rustc_builtin_macro]
+macro_rules! asm {
+ () => {};
+}
+
+extern "C" {
+ fn printf(s: *const i8, ...);
+}
+
+fn main() -> i32 {
+ let y: i32 = 9;
+ let mut x: i32 = 0;
+ unsafe {
+ asm!("mov {}, {}", in(reg) y, out(reg) x);
+ printf("%d\n\0" as *const str as *const i8, x);
+ }
+ 0
+}
--
2.55.0
