c++: Fix up ICEs on constexpr inline asm strings in templates [PR118277]
Checks
Context |
Check |
Description |
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 |
success
|
Build passed
|
linaro-tcwg-bot/tcwg_gcc_build--master-arm |
success
|
Build passed
|
linaro-tcwg-bot/tcwg_gcc_check--master-arm |
fail
|
Patch failed to apply
|
Commit Message
Hi!
The following patch fixes ICEs when the new inline asm syntax
to use C++26 static_assert-like constant expressions in place
of string literals is used in templates.
As finish_asm_stmt doesn't do any checking for
processing_template_decl, this patch also just defers handling
those strings in templates rather than say trying fold_non_dependent_expr
and if the result is non-dependent and usable, try to extract.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
There is one case I didn't handle and I'd like to discuss.
The initial commit to enable this new extension also changed
cp_parser_asm_specification_opt to use cp_parser_asm_string_expression.
That function doesn't have anything to do with asm statements though,
it is about asm redirection of declarations.
I couldn't find any testcase coverage for that nor documentation.
It is basically whether one can use something like
void foo () asm ((std::string_view ("bar")));
or not. GCC 14 didn't support that obviously, now it works
when not used in templates (both before and after this patch), but
will crash when used on templates. Say
template <int N>
void baz () asm ((std::string_view ("qux")));
(or even worse when it is dependent).
Do we want to revert the cp_parser_asm_specification_opt
changes and only support void foo () asm ("bar");, or was it intentional
to add support for constant expressions even for those?
The problem with supporting those in templates is that
cp_finish_decl/grokfield really assume asmspec_tree is NULL,
error_mark_node or STRING_CST, they perform e.g. pragma renaming on those
and there is no obvious place to store the not yet finalized
constant expression for the asmspec, those are really stored as const char *
later transformed into INDENTIFIER_NODEs. Or shall we sorry if it is
used in templates for now? Or say temporarily turn into some internal
attribute?
2025-01-07 Jakub Jelinek <jakub@redhat.com>
PR c++/118277
* cp-tree.h (finish_asm_string_expression): Declare.
* semantics.cc (finish_asm_string_expression): New function.
(finish_asm_stmt): Use it.
* parser.cc (cp_parser_asm_string_expression): Likewise.
* g++.dg/cpp1z/constexpr-asm-4.C: New test.
Jakub
Comments
On Tue, Jan 07, 2025 at 08:36:29PM +0100, Jakub Jelinek wrote:
> Hi!
>
> The following patch fixes ICEs when the new inline asm syntax
> to use C++26 static_assert-like constant expressions in place
> of string literals is used in templates.
> As finish_asm_stmt doesn't do any checking for
> processing_template_decl, this patch also just defers handling
> those strings in templates rather than say trying fold_non_dependent_expr
> and if the result is non-dependent and usable, try to extract.
Thanks. I've been looking at a similar patch, but you beat me to it.
Your patch looks good to me, except for two comments below.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> There is one case I didn't handle and I'd like to discuss.
> The initial commit to enable this new extension also changed
> cp_parser_asm_specification_opt to use cp_parser_asm_string_expression.
> That function doesn't have anything to do with asm statements though,
> it is about asm redirection of declarations.
I don't know of a use case for this, so i guess it can be rejected
(like this patchlet)
@@ -30067,7 +30063,11 @@ cp_parser_asm_specification_opt (cp_parser* parser)
parens.require_open (parser);
/* Look for the string-literal. */
+ token = cp_lexer_peek_token (parser->lexer);
tree asm_specification = cp_parser_asm_string_expression (parser);
+ if (TREE_CODE (asm_specification) != STRING_CST)
+ error_at (token->location,
+ "%<asm%> specification for declaration must be string");
/* Look for the `)'. */
parens.require_close (parser);
Since you add a return of error_mark_node to finish_asm_stmt you
also need this patchlet:
@@ -19170,7 +19170,8 @@ tsubst_stmt (tree t, tree args, tsubst_flags_t complain, tree in_decl)
tree asm_expr = tmp;
if (TREE_CODE (asm_expr) == CLEANUP_POINT_EXPR)
asm_expr = TREE_OPERAND (asm_expr, 0);
- ASM_BASIC_P (asm_expr) = ASM_BASIC_P (t);
+ if (asm_expr != error_mark_node)
+ ASM_BASIC_P (asm_expr) = ASM_BASIC_P (t);
}
break;
> else if (!cp_parser_is_string_literal (tok))
> {
> --- gcc/testsuite/g++.dg/cpp1z/constexpr-asm-4.C.jj 2025-01-07 12:19:34.472033295 +0100
> +++ gcc/testsuite/g++.dg/cpp1z/constexpr-asm-4.C 2025-01-07 12:28:59.178178486 +0100
It needs a test case with constexpr errors too. In my version I had
a lot of trouble with them.
-Andi
@@ -7946,6 +7946,7 @@ enum {
extern tree begin_compound_stmt (unsigned int);
extern void finish_compound_stmt (tree);
+extern tree finish_asm_string_expression (location_t, tree);
extern tree finish_asm_stmt (location_t, int, tree, tree,
tree, tree, tree, bool, bool);
extern tree finish_label_stmt (tree);
@@ -2133,6 +2133,26 @@ finish_compound_stmt (tree stmt)
add_stmt (stmt);
}
+/* Finish an asm string literal, which can be a string literal
+ or parenthesized constant expression. Extract the string literal
+ from the latter. */
+
+tree
+finish_asm_string_expression (location_t loc, tree string)
+{
+ if (string == error_mark_node
+ || TREE_CODE (string) == STRING_CST
+ || processing_template_decl)
+ return string;
+ string = cxx_constant_value (string, tf_error);
+ cexpr_str cstr (string);
+ if (!cstr.type_check (loc))
+ return error_mark_node;
+ if (!cstr.extract (loc, string))
+ string = error_mark_node;
+ return string;
+}
+
/* Finish an asm-statement, whose components are a STRING, some
OUTPUT_OPERANDS, some INPUT_OPERANDS, some CLOBBERS and some
LABELS. Also note whether the asm-statement should be
@@ -2159,6 +2179,26 @@ finish_asm_stmt (location_t loc, int vol
oconstraints = XALLOCAVEC (const char *, noutputs);
+ string = finish_asm_string_expression (cp_expr_loc_or_loc (string, loc),
+ string);
+ if (string == error_mark_node)
+ return error_mark_node;
+ for (int i = 0; i < 2; ++i)
+ for (t = i ? input_operands : output_operands; t; t = TREE_CHAIN (t))
+ {
+ tree s = TREE_VALUE (TREE_PURPOSE (t));
+ s = finish_asm_string_expression (cp_expr_loc_or_loc (s, loc), s);
+ if (s == error_mark_node)
+ return error_mark_node;
+ TREE_VALUE (TREE_PURPOSE (t)) = s;
+ }
+ for (t = clobbers; t; t = TREE_CHAIN (t))
+ {
+ tree s = TREE_VALUE (t);
+ s = finish_asm_string_expression (cp_expr_loc_or_loc (s, loc), s);
+ TREE_VALUE (t) = s;
+ }
+
string = resolve_asm_operand_names (string, output_operands,
input_operands, labels);
@@ -23107,15 +23107,8 @@ cp_parser_asm_string_expression (cp_pars
matching_parens parens;
parens.consume_open (parser);
tree string = cp_parser_constant_expression (parser);
- if (string != error_mark_node)
- string = cxx_constant_value (string, tf_error);
- cexpr_str cstr (string);
- if (!cstr.type_check (tok->location))
- return error_mark_node;
- if (!cstr.extract (tok->location, string))
- string = error_mark_node;
parens.require_close (parser);
- return string;
+ return finish_asm_string_expression (tok->location, string);
}
else if (!cp_parser_is_string_literal (tok))
{
@@ -0,0 +1,71 @@
+// PR c++/118277
+// { dg-do compile }
+// { dg-options "-std=gnu++17" }
+
+using size_t = decltype (sizeof (0));
+struct string_view {
+ size_t s;
+ const char *d;
+ constexpr string_view () : s (0), d (nullptr) {}
+ constexpr string_view (const char *p) : s (__builtin_strlen (p)), d (p) {}
+ constexpr string_view (size_t l, const char *p) : s (l), d (p) {}
+ constexpr size_t size () const noexcept { return s; }
+ constexpr const char *data () const noexcept { return d; }
+};
+
+template <typename T>
+constexpr T
+gen (int n)
+{
+ switch (n)
+ {
+ case 0: return "foo %3,%2,%1,%0";
+ case 1: return "=r";
+ case 2: return "r";
+ case 3: return "memory";
+ case 4: return "cc";
+ case 5: return "goo %3,%2,%1,%0";
+ case 6: return "hoo %3,%2,%1,%0";
+ }
+}
+
+void
+bar ()
+{
+ int a, b;
+ asm ((gen <string_view> (0))
+ : (gen <string_view> (1)) (a), (gen <string_view> (1)) (b)
+ : (gen <string_view> (2)) (1), (gen <string_view> (2)) (2)
+ : (gen <string_view> (3)), (gen <string_view> (4)));
+}
+
+template <typename T, typename U>
+void
+baz ()
+{
+ U a, b;
+ asm ((gen <T> (5))
+ : (gen <T> (1)) (a), (gen <T> (1)) (b)
+ : (gen <T> (2)) (U(1)), (gen <T> (2)) (U(2))
+ : (gen <T> (3)), (gen <T> (4)));
+}
+
+template <typename T, typename U>
+void
+qux ()
+{
+ U a, b;
+ asm ((gen <T> (6))
+ : (gen <T> (1)) (a), (gen <T> (1)) (b)
+ : (gen <T> (2)) (U(1)), (gen <T> (2)) (U(2))
+ : (gen <T> (3)), (gen <T> (4)));
+}
+
+void
+corge ()
+{
+ qux <string_view, int> ();
+}
+
+/* { dg-final { scan-assembler "foo" } } */
+/* { dg-final { scan-assembler "hoo" } } */