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]>
---
This change was merged into the gccrs repository and is posted here for
upstream visibility and potential drive-by review, as requested by GCC
release managers.
Each commit email contains a link to its details on github from where you can
find the Pull-Request and associated discussions.
Commit on github:
https://github.com/Rust-GCC/gccrs/commit/82ceec7dc1173f6c626015ca9ef252998c1632e6
The commit has been mentioned in the following issue(s):
- Rust-GCC/gccrs#5057: https://github.com/Rust-GCC/gccrs/issues/5057
The commit has been mentioned in the following pull-request(s):
- https://github.com/Rust-GCC/gccrs/pull/5058
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 a3683a0ca..bde990db7 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 000000000..43bd1e0f9
--- /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 000000000..f612704e3
--- /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
+}
base-commit: 52a684bea62ea5d0832beaec0904467d4b1574f5
--
2.56.0
