diff mbox series

[committed] d: Fix ICE: verify_gimple_failed (conversion of register to a different size in 'view_convert_expr') [PR110712]

Message ID 20231029192524.202262-1-ibuclaw@gdcproject.org
State New
Headers show
Series [committed] d: Fix ICE: verify_gimple_failed (conversion of register to a different size in 'view_convert_expr') [PR110712] | expand

Commit Message

Iain Buclaw Oct. 29, 2023, 7:25 p.m. UTC
Hi,

This patch fixes an ICE cause by the way the D front-end generates its
codegen around va_list types.

Static arrays in D are passed around by value, rather than decaying to a
pointer.  On x86_64 __builtin_va_list is an exception to this rule, but
semantically it's still treated as a static array.

This makes certain assignment operations fail due a mismatch in types.
As all examples in the test program are rejected by C/C++ front-ends,
these are now errors in D too to be consistent.

Bootstrapped and regression tested on x86-64-linux-gnu/-m32, committed
to mainline and backported to releases/gcc-12 and releases/gcc-13.

Regards,
Iain.

---
	PR d/110712

gcc/d/ChangeLog:

	* d-codegen.cc (d_build_call): Update call to convert_for_argument.
	* d-convert.cc (is_valist_parameter_type): New function.
	(check_valist_conversion): New function.
	(convert_for_assignment): Update signature.  Add check whether
	assigning va_list is permissible.
	(convert_for_argument): Likewise.
	* d-tree.h (convert_for_assignment): Update signature.
	(convert_for_argument): Likewise.
	* expr.cc (ExprVisitor::visit (AssignExp *)): Update call to
	convert_for_assignment.

gcc/testsuite/ChangeLog:

	* gdc.dg/pr110712.d: New test.
---
 gcc/d/d-codegen.cc              |   6 +-
 gcc/d/d-convert.cc              | 127 ++++++++++++++++++++++++++------
 gcc/d/d-tree.h                  |   4 +-
 gcc/d/expr.cc                   |  12 +--
 gcc/testsuite/gdc.dg/pr110712.d |  23 ++++++
 5 files changed, 139 insertions(+), 33 deletions(-)
 create mode 100644 gcc/testsuite/gdc.dg/pr110712.d
diff mbox series

Patch

diff --git a/gcc/d/d-codegen.cc b/gcc/d/d-codegen.cc
index 270cb5e2be6..5c53cf78577 100644
--- a/gcc/d/d-codegen.cc
+++ b/gcc/d/d-codegen.cc
@@ -2245,14 +2245,16 @@  d_build_call (TypeFunction *tf, tree callable, tree object,
       for (size_t i = 0; i < arguments->length; ++i)
 	{
 	  Expression *arg = (*arguments)[i];
-	  tree targ = build_expr (arg);
+	  tree targ;
 
 	  if (i - varargs < nparams && i >= varargs)
 	    {
 	      /* Actual arguments for declared formal arguments.  */
 	      Parameter *parg = tf->parameterList[i - varargs];
-	      targ = convert_for_argument (targ, parg);
+	      targ = convert_for_argument (arg, parg);
 	    }
+	  else
+	    targ = build_expr (arg);
 
 	  /* Don't pass empty aggregates by value.  */
 	  if (empty_aggregate_p (TREE_TYPE (targ)) && !TREE_ADDRESSABLE (targ)
diff --git a/gcc/d/d-convert.cc b/gcc/d/d-convert.cc
index 71d7a41374e..4c5375cba9a 100644
--- a/gcc/d/d-convert.cc
+++ b/gcc/d/d-convert.cc
@@ -694,16 +694,86 @@  convert_for_rvalue (tree expr, Type *etype, Type *totype)
   return result ? result : convert_expr (expr, etype, totype);
 }
 
+/* Helper for convert_for_assigment and convert_for_argument.
+   Returns true if EXPR is a va_list static array parameter.  */
+
+static bool
+is_valist_parameter_type (Expression *expr)
+{
+  Declaration *decl = NULL;
+
+  if (VarExp *ve = expr->isVarExp ())
+    decl = ve->var;
+  else if (SymOffExp *se = expr->isSymOffExp ())
+    decl = se->var;
+
+  if (decl != NULL && decl->isParameter () && valist_array_p (decl->type))
+    return true;
+
+  return false;
+}
+
+/* Helper for convert_for_assigment and convert_for_argument.
+   Report erroneous uses of assigning or passing a va_list parameter.  */
+
+static void
+check_valist_conversion (Expression *expr, Type *totype, bool in_assignment)
+{
+  /* Parameter symbol and its converted type.  */
+  Declaration *decl = NULL;
+  /* Type of parameter when evaluated in the expression.  */
+  Type *type = NULL;
+
+  if (VarExp *ve = expr->isVarExp ())
+    {
+      decl = ve->var;
+      type = ve->var->type->nextOf ()->pointerTo ();
+    }
+  else if (SymOffExp *se = expr->isSymOffExp ())
+    {
+      decl = se->var;
+      type = se->var->type->nextOf ()->pointerTo ()->pointerTo ();
+    }
+
+  /* Should not be called unless is_valist_parameter_type also matched.  */
+  gcc_assert (decl != NULL && decl->isParameter ()
+	      && valist_array_p (decl->type));
+
+  /* OK if conversion between types is allowed.  */
+  if (type->implicitConvTo (totype) != MATCH::nomatch)
+    return;
+
+  if (in_assignment)
+    {
+      error_at (make_location_t (expr->loc), "cannot convert parameter %qs "
+		"from type %qs to type %qs in assignment",
+		expr->toChars(), type->toChars (), totype->toChars ());
+    }
+  else
+    {
+      error_at (make_location_t (expr->loc), "cannot convert parameter %qs "
+		"from type %qs to type %qs in argument passing",
+		expr->toChars(), type->toChars (), totype->toChars ());
+    }
+
+  inform (make_location_t (decl->loc), "parameters of type %<va_list%> "
+	  "{aka %qs} are decayed to pointer types, and require %<va_copy%> "
+	  "to be converted back into a static array type",
+	  decl->type->toChars ());
+}
+
 /* Apply semantics of assignment to a value of type TOTYPE to EXPR
-   (e.g., pointer = array -> pointer = &array[0])
+   For example: `pointer = array' gets lowered to `pointer = &array[0]'.
+   If LITERALP is true, then EXPR is a value used in the initialization
+   of another literal.
 
    Return a TREE representation of EXPR implicitly converted to TOTYPE
    for use in assignment expressions MODIFY_EXPR, INIT_EXPR.  */
 
 tree
-convert_for_assignment (tree expr, Type *etype, Type *totype)
+convert_for_assignment (Expression *expr, Type *totype, bool literalp)
 {
-  Type *ebtype = etype->toBasetype ();
+  Type *ebtype = expr->type->toBasetype ();
   Type *tbtype = totype->toBasetype ();
 
   /* Assuming this only has to handle converting a non Tsarray type to
@@ -723,8 +793,8 @@  convert_for_assignment (tree expr, Type *etype, Type *totype)
 	      vec <constructor_elt, va_gc> *ce = NULL;
 	      tree index = build2 (RANGE_EXPR, build_ctype (Type::tsize_t),
 				   size_zero_node, size_int (count - 1));
-	      tree value = convert_for_assignment (expr, etype, sa_type->next);
-
+	      tree value = convert_for_assignment (expr, sa_type->next,
+						   literalp);
 	      /* Can't use VAR_DECLs in CONSTRUCTORS.  */
 	      if (VAR_P (value))
 		{
@@ -745,38 +815,53 @@  convert_for_assignment (tree expr, Type *etype, Type *totype)
   if ((tbtype->ty == TY::Tsarray || tbtype->ty == TY::Tstruct)
       && ebtype->isintegral ())
     {
-      if (!integer_zerop (expr))
-	gcc_unreachable ();
-
-      return expr;
+      tree ret = build_expr (expr, false, literalp);
+      gcc_assert (integer_zerop (ret));
+      return ret;
     }
 
-  return convert_for_rvalue (expr, etype, totype);
+  /* Assigning a va_list by value or reference, check whether RHS is a parameter
+     that has has been lowered by declaration_type or parameter_type.  */
+  if (is_valist_parameter_type (expr))
+    check_valist_conversion (expr, totype, true);
+
+  return convert_for_rvalue (build_expr (expr, false, literalp),
+			     expr->type, totype);
 }
 
 /* Return a TREE representation of EXPR converted to represent
    the parameter type ARG.  */
 
 tree
-convert_for_argument (tree expr, Parameter *arg)
+convert_for_argument (Expression *expr, Parameter *arg)
 {
+  tree targ = build_expr (expr);
+
   /* Lazy arguments: expr should already be a delegate.  */
   if (arg->storageClass & STClazy)
-    return expr;
+    return targ;
 
+  /* Passing a va_list by value, check whether the target requires it to
+     be decayed to a pointer type.  */
   if (valist_array_p (arg->type))
     {
-      /* Do nothing if the va_list has already been decayed to a pointer.  */
-      if (!POINTER_TYPE_P (TREE_TYPE (expr)))
-	return build_address (expr);
-    }
-  else if (parameter_reference_p (arg))
-    {
-      /* Front-end shouldn't automatically take the address.  */
-      return convert (parameter_type (arg), build_address (expr));
+      if (!POINTER_TYPE_P (TREE_TYPE (targ)))
+	return build_address (targ);
+
+      /* Do nothing if the va_list has already been converted.  */
+      return targ;
     }
 
-  return expr;
+  /* Passing a va_list by reference, check if types are really compatible
+     after conversion from static array to pointer type.  */
+  if (is_valist_parameter_type (expr))
+    check_valist_conversion (expr, arg->type, false);
+
+  /* Front-end shouldn't automatically take the address of `ref' parameters.  */
+  if (parameter_reference_p (arg))
+    return convert (parameter_type (arg), build_address (targ));
+
+  return targ;
 }
 
 /* Perform default promotions for data used in expressions.
diff --git a/gcc/d/d-tree.h b/gcc/d/d-tree.h
index 7763695a106..d19c3f50bd9 100644
--- a/gcc/d/d-tree.h
+++ b/gcc/d/d-tree.h
@@ -625,8 +625,8 @@  extern tree d_truthvalue_conversion (tree);
 extern tree d_convert (tree, tree);
 extern tree convert_expr (tree, Type *, Type *);
 extern tree convert_for_rvalue (tree, Type *, Type *);
-extern tree convert_for_assignment (tree, Type *, Type *);
-extern tree convert_for_argument (tree, Parameter *);
+extern tree convert_for_assignment (Expression *, Type *, bool = false);
+extern tree convert_for_argument (Expression *, Parameter *);
 extern tree convert_for_condition (tree, Type *);
 extern tree d_array_convert (Expression *);
 extern tree d_array_convert (Type *, Expression *);
diff --git a/gcc/d/expr.cc b/gcc/d/expr.cc
index 29f114a1b99..ef4ea60ffed 100644
--- a/gcc/d/expr.cc
+++ b/gcc/d/expr.cc
@@ -972,8 +972,7 @@  public:
 	Declaration *decl = e->e1->isVarExp ()->var;
 	if (decl->storage_class & (STCout | STCref))
 	  {
-	    tree t2 = convert_for_assignment (build_expr (e->e2),
-					      e->e2->type, e->e1->type);
+	    tree t2 = convert_for_assignment (e->e2, e->e1->type);
 	    tree t1 = build_expr (e->e1);
 	    /* Want reference to lhs, not indirect ref.  */
 	    t1 = TREE_OPERAND (t1, 0);
@@ -993,8 +992,7 @@  public:
     if (tb1->ty == TY::Tstruct)
       {
 	tree t1 = build_expr (e->e1);
-	tree t2 = convert_for_assignment (build_expr (e->e2, false, true),
-					  e->e2->type, e->e1->type);
+	tree t2 = convert_for_assignment (e->e2, e->e1->type, true);
 	StructDeclaration *sd = tb1->isTypeStruct ()->sym;
 
 	/* Look for struct = 0.  */
@@ -1073,8 +1071,7 @@  public:
 	    || (e->op == EXP::blit || e->e1->type->size () == 0))
 	  {
 	    tree t1 = build_expr (e->e1);
-	    tree t2 = convert_for_assignment (build_expr (e->e2),
-					      e->e2->type, e->e1->type);
+	    tree t2 = convert_for_assignment (e->e2, e->e1->type);
 
 	    this->result_ = build_assign (modifycode, t1, t2);
 	    return;
@@ -1088,8 +1085,7 @@  public:
 
     /* Simple assignment.  */
     tree t1 = build_expr (e->e1);
-    tree t2 = convert_for_assignment (build_expr (e->e2),
-				      e->e2->type, e->e1->type);
+    tree t2 = convert_for_assignment (e->e2, e->e1->type);
 
     this->result_ = build_assign (modifycode, t1, t2);
   }
diff --git a/gcc/testsuite/gdc.dg/pr110712.d b/gcc/testsuite/gdc.dg/pr110712.d
new file mode 100644
index 00000000000..ed24b6c90cc
--- /dev/null
+++ b/gcc/testsuite/gdc.dg/pr110712.d
@@ -0,0 +1,23 @@ 
+// { dg-do compile { target { { i?86-*-* x86_64-*-* } && lp64 } } }
+import gcc.builtins : va_list = __builtin_va_list;
+
+void argpass(va_list *ap);
+
+void pr110712a(va_list ap)
+{
+    argpass(&ap); // { dg-error "cannot convert parameter" }
+}
+
+void pr110712b(va_list ap)
+{
+    va_list ap2 = ap; // { dg-error "cannot convert parameter" }
+}
+
+struct pr110712c
+{
+    this(va_list ap)
+    {
+        this.ap = ap; // { dg-error "cannot convert parameter" }
+    }
+    va_list ap;
+}