llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Akash Manna (akash-manna-sky)
<details>
<summary>Changes</summary>
Fixes #<!-- -->204773
The asm template in the report wraps the whole instruction sequence in `{...}`
and then uses `{l}` inside it. GCC's dialect-alternative syntax `{att|intel}`
does not nest, and GCC rejects this with a plain error. Clang never looked at
the brace structure: `AnalyzeAsmString` rewrote every `{`, `|` and `}` into the
backend markers `$(`, `$|` and `$)` and passed the result on. The X86
AsmPrinter is the first thing that notices the nesting, and at that stage its
only option is `report_fatal_error`, which the driver then reports as a crash.
An opened `{` that is never closed was worse, since nothing caught it at all
and the backend silently emitted whichever text happened to fall in variant 0.
`AnalyzeAsmString` now remembers where the current alternative was opened. A
second `{` before the closing `}` is reported as nested dialect alternatives,
and reaching the end of the string inside an alternative is reported as
unterminated, both pointing at the offending brace in the source string. The
check only applies on targets where braces carry dialect meaning; targets that
set `NoAsmVariants` keep treating them as ordinary characters, and the `%{` /
`%}` escapes are unaffected.
---
Full diff: https://github.com/llvm/llvm-project/pull/225306.diff
5 Files Affected:
- (modified) clang/docs/ReleaseNotes.md (+4)
- (modified) clang/include/clang/Basic/DiagnosticASTKinds.td (+4)
- (modified) clang/lib/AST/Stmt.cpp (+27-2)
- (modified) clang/test/Sema/asm.c (+13)
- (modified) clang/test/Sema/inline-asm-validate-aarch64.c (+4)
``````````diff
diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md
index f4a34a37aff52..61dacc99a9f2b 100644
--- a/clang/docs/ReleaseNotes.md
+++ b/clang/docs/ReleaseNotes.md
@@ -451,6 +451,10 @@ features cannot lower the translation-unit ABI level;
- Clang now rejects inline asm constraints and clobbers that contain an
embedded null character, instead of silently truncating them. (#GH173900)
+- Clang now rejects nested or unterminated assembler dialect alternatives
+ (`{att|intel}`) in an inline asm string, instead of producing a backend fatal
+ error or silently emitting the wrong dialect. (#GH204773)
+
- Added `-Wstringop-overread` to warn when `memcpy`, `memmove`, `memcmp`,
and related builtins read more bytes than the source buffer size (#GH83728).
diff --git a/clang/include/clang/Basic/DiagnosticASTKinds.td
b/clang/include/clang/Basic/DiagnosticASTKinds.td
index 0aca1f75428f8..ac8e7ed7c2f6d 100644
--- a/clang/include/clang/Basic/DiagnosticASTKinds.td
+++ b/clang/include/clang/Basic/DiagnosticASTKinds.td
@@ -461,6 +461,10 @@ let CategoryName = "Inline Assembly Issue" in {
"empty symbolic operand name in inline assembly string">;
def err_asm_invalid_operand_number : Error<
"invalid operand number in inline asm string">;
+ def err_asm_nested_dialect_alternatives : Error<
+ "nested assembler dialect alternatives in inline assembly string">;
+ def err_asm_unterminated_dialect_alternative : Error<
+ "unterminated assembler dialect alternative in inline assembly string">;
}
// vtable related.
diff --git a/clang/lib/AST/Stmt.cpp b/clang/lib/AST/Stmt.cpp
index 15d0e6435aaf3..4513e8d6f665f 100644
--- a/clang/lib/AST/Stmt.cpp
+++ b/clang/lib/AST/Stmt.cpp
@@ -699,12 +699,19 @@ unsigned
GCCAsmStmt::AnalyzeAsmString(SmallVectorImpl<AsmStringPiece>&Pieces,
bool HasVariants = !C.getTargetInfo().hasNoAsmVariants();
+ // Offset of the '{' opening the current {a|b|c} dialect alternative, if any.
+ std::optional<unsigned> VariantStartOffs;
+
unsigned LastAsmStringToken = 0;
unsigned LastAsmStringOffset = 0;
while (true) {
// Done with the string?
if (CurPtr == StrEnd) {
+ if (VariantStartOffs) {
+ DiagOffs = *VariantStartOffs;
+ return diag::err_asm_unterminated_dialect_alternative;
+ }
if (!CurStringPiece.empty())
Pieces.push_back(AsmStringPiece(CurStringPiece));
return 0;
@@ -713,9 +720,27 @@ unsigned
GCCAsmStmt::AnalyzeAsmString(SmallVectorImpl<AsmStringPiece>&Pieces,
char CurChar = *CurPtr++;
switch (CurChar) {
case '$': CurStringPiece += "$$"; continue;
- case '{': CurStringPiece += (HasVariants ? "$(" : "{"); continue;
+ case '{':
+ if (!HasVariants) {
+ CurStringPiece += '{';
+ continue;
+ }
+ if (VariantStartOffs) {
+ DiagOffs = CurPtr - StrStart - 1;
+ return diag::err_asm_nested_dialect_alternatives;
+ }
+ VariantStartOffs = CurPtr - StrStart - 1;
+ CurStringPiece += "$(";
+ continue;
case '|': CurStringPiece += (HasVariants ? "$|" : "|"); continue;
- case '}': CurStringPiece += (HasVariants ? "$)" : "}"); continue;
+ case '}':
+ if (!HasVariants) {
+ CurStringPiece += '}';
+ continue;
+ }
+ VariantStartOffs.reset();
+ CurStringPiece += "$)";
+ continue;
case '%':
break;
default:
diff --git a/clang/test/Sema/asm.c b/clang/test/Sema/asm.c
index cc9acac1e169d..4434adfeee5e7 100644
--- a/clang/test/Sema/asm.c
+++ b/clang/test/Sema/asm.c
@@ -404,3 +404,16 @@ void test20(char x) {
asm ("fabs" : "=t" (d): "0" (v)); // expected-error {{unsupported inline
asm: input with type 'int2' (vector of 2 'int' values) matching output with
type 'double'}}
asm ("fabs" : "=t" (v): "0" (d)); // expected-error {{unsupported inline
asm: input with type 'double' matching output with type 'int2' (vector of 2
'int' values)}}
}
+
+// GH204773
+void test21(int x) {
+ asm ("{cmpl{l}\t$d,%c0;je\t1f;addw{l}\t$d,%c0;jmp\t2f;1:decl\t%c0;2:}" : :
"g"(x)); // expected-error {{nested assembler dialect alternatives in inline
assembly string}}
+ asm ("{addl %0, %0|add %0, %0}" : : "r"(x)); // ok
+ asm ("{a|b}{c|d}" : : "r"(x)); // ok
+ asm ("{a|b" : : "r"(x)); // expected-error {{unterminated assembler dialect
alternative in inline assembly string}}
+ asm ("{" : : "r"(x)); // expected-error {{unterminated assembler dialect
alternative in inline assembly string}}
+ asm ("a}b|c" : : "r"(x)); // ok
+ asm ("%{%{%}%}" : : "r"(x)); // ok, escaped braces
+ asm ("{%{a%}|b}" : : "r"(x)); // ok
+ asm ("{{"); // ok, simple asm
+}
diff --git a/clang/test/Sema/inline-asm-validate-aarch64.c
b/clang/test/Sema/inline-asm-validate-aarch64.c
index 1e753d40d8ca0..da4e59859743d 100644
--- a/clang/test/Sema/inline-asm-validate-aarch64.c
+++ b/clang/test/Sema/inline-asm-validate-aarch64.c
@@ -10,6 +10,10 @@ void test_s(int i) {
/// Codegen error
asm("" :: "S"(i));
asm("" :: "S"(test_s(i))); // expected-error{{invalid type 'void' in asm
input for constraint 'S'}}
+
+ // GH204773: braces are ordinary characters on this target.
+ asm("{a{b}}" :: "r"(i));
+ asm("{" :: "r"(i));
}
#else
uint8_t constraint_r(uint8_t *addr) {
``````````
</details>
https://github.com/llvm/llvm-project/pull/225306
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
