[PATCH] inline-asm: Fix ICE with bitfields in "m" operands [PR100785]

[PATCH] inline-asm: Fix ICE with bitfields in "m" operands [PR100785]

[Jakub Jelinek]

Hi!

Bitfields, while they live in memory, aren't something inline-asm can easily
operate on.
For C and "=m" or "+m", we were diagnosing bitfields in the past in the
FE, where c_mark_addressable had:
      case COMPONENT_REF:
        if (DECL_C_BIT_FIELD (TREE_OPERAND (x, 1)))
          {
            error
              ("cannot take address of bit-field %qD", TREE_OPERAND (x, 1));
            return false;
          }
but that check got moved in GCC 6 to build_unary_op instead and now we
emit an error during expansion and ICE afterwards (i.e. error-recovery).
For "m" it used to be diagnosed in c_mark_addressable too, but since
GCC 6 it is ice-on-invalid.
For C++, this was never diagnosed in the FE, but used to be diagnosed
in the gimplifier and/or during expansion before 4.8.

The following patch does multiple things:
1) diagnoses it in the FEs
2) stops emitting a redundant diagnostic in the gimplifier using the
   usual way, if we already see error_mark_node, we assume error has
   been emitted already and only diagnose if it wasn't error_mark_node;
   this helps diagnosing the same bug with multiple different
   errors
3) simplifies during expansion the inline asm if any errors have been
   reported (similarly how e.g. vregs pass if it detects errors on
   inline-asm either deletes them or simplifies to bare minimum -
   just labels), so that we don't have error-recovery ICEs there

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2021-06-02  Jakub Jelinek  <jakub@redhat.com>

	PR inline-asm/100785
gcc/
	* gimplify.c (gimplify_asm_expr): Don't diagnose errors if
	output or input operands were already error_mark_node.
	* cfgexpand.c (expand_asm_stmt): If errors are emitted,
	remove all inputs, outputs and clobbers from the asm and
	set template to "".
gcc/c/
	* c-typeck.c (build_asm_expr): Diagnose bit-fields in operands
	that have to be in memory.
gcc/cp/
	* semantics.c (finish_asm_stmt): Diagnose bit-fields in operands
	that have to be in memory.
gcc/testsuite/
	* c-c++-common/pr100785.c: New test.
	* gcc.dg/pr48552-1.c: Don't expect invalid lvalue errors.
	* gcc.dg/pr48552-2.c: Likewise.

--- gcc/gimplify.c.jj	2021-05-29 10:05:34.634524244 +0200
+++ gcc/gimplify.c	2021-06-01 13:24:58.710392357 +0200
@@ -6341,12 +6341,14 @@ gimplify_asm_expr (tree *expr_p, gimple_
       if (!allows_reg && allows_mem)
 	mark_addressable (TREE_VALUE (link));
 
+      tree orig = TREE_VALUE (link);
       tret = gimplify_expr (&TREE_VALUE (link), pre_p, post_p,
 			    is_inout ? is_gimple_min_lval : is_gimple_lvalue,
 			    fb_lvalue | fb_mayfail);
       if (tret == GS_ERROR)
 	{
-	  error ("invalid lvalue in %<asm%> output %d", i);
+	  if (orig != error_mark_node)
+	    error ("invalid lvalue in %<asm%> output %d", i);
 	  ret = tret;
 	}
 
@@ -6541,8 +6543,9 @@ gimplify_asm_expr (tree *expr_p, gimple_
 	  mark_addressable (TREE_VALUE (link));
 	  if (tret == GS_ERROR)
 	    {
-	      error_at (EXPR_LOC_OR_LOC (TREE_VALUE (link), input_location),
-			"memory input %d is not directly addressable", i);
+	      if (inputv != error_mark_node)
+		error_at (EXPR_LOC_OR_LOC (TREE_VALUE (link), input_location),
+			  "memory input %d is not directly addressable", i);
 	      ret = tret;
 	    }
 	}
--- gcc/cfgexpand.c.jj	2021-05-20 09:03:54.147700369 +0200
+++ gcc/cfgexpand.c	2021-05-31 20:48:28.078548261 +0200
@@ -3082,6 +3082,7 @@ expand_asm_stmt (gasm *stmt)
   unsigned ninputs = gimple_asm_ninputs (stmt);
   unsigned nlabels = gimple_asm_nlabels (stmt);
   unsigned i;
+  bool error_seen = false;
 
   /* ??? Diagnose during gimplification?  */
   if (ninputs + noutputs + nlabels > MAX_RECOG_OPERANDS)
@@ -3140,6 +3141,7 @@ expand_asm_stmt (gasm *stmt)
 		{
 		  /* ??? Diagnose during gimplification?  */
 		  error ("unknown register name %qs in %<asm%>", regname);
+		  error_seen = true;
 		}
 	      else if (j == -4)
 		{
@@ -3202,7 +3204,10 @@ expand_asm_stmt (gasm *stmt)
 		&& REG_P (DECL_RTL (output_tvec[j]))
 		&& HARD_REGISTER_P (DECL_RTL (output_tvec[j]))
 		&& output_hregno == REGNO (DECL_RTL (output_tvec[j])))
-	      error ("invalid hard register usage between output operands");
+	      {
+		error ("invalid hard register usage between output operands");
+		error_seen = true;
+	      }
 
 	  /* Verify matching constraint operands use the same hard register
 	     and that the non-matching constraint operands do not use the same
@@ -3225,13 +3230,19 @@ expand_asm_stmt (gasm *stmt)
 		  }
 		if (i == match
 		    && output_hregno != input_hregno)
-		  error ("invalid hard register usage between output operand "
-			 "and matching constraint operand");
+		  {
+		    error ("invalid hard register usage between output "
+			   "operand and matching constraint operand");
+		    error_seen = true;
+		  }
 		else if (early_clobber_p
 			 && i != match
 			 && output_hregno == input_hregno)
-		  error ("invalid hard register usage between earlyclobber "
-			 "operand and input operand");
+		  {
+		    error ("invalid hard register usage between "
+			   "earlyclobber operand and input operand");
+		    error_seen = true;
+		  }
 	      }
 	}
 
@@ -3307,7 +3318,10 @@ expand_asm_stmt (gasm *stmt)
 	    op = validize_mem (op);
 
 	  if (! allows_reg && !MEM_P (op))
-	    error ("output number %d not directly addressable", i);
+	    {
+	      error ("output number %d not directly addressable", i);
+	      error_seen = true;
+	    }
 	  if ((! allows_mem && MEM_P (op) && GET_MODE (op) != BLKmode)
 	      || GET_CODE (op) == CONCAT)
 	    {
@@ -3347,6 +3361,19 @@ expand_asm_stmt (gasm *stmt)
 	inout_opnum.safe_push (i);
     }
 
+  const char *str = gimple_asm_string (stmt);
+  if (error_seen)
+    {
+      ninputs = 0;
+      noutputs = 0;
+      inout_opnum.truncate (0);
+      output_rvec.truncate (0);
+      clobber_rvec.truncate (0);
+      constraints.truncate (0);
+      CLEAR_HARD_REG_SET (clobbered_regs);
+      str = "";
+    }
+
   auto_vec<rtx, MAX_RECOG_OPERANDS> input_rvec;
   auto_vec<machine_mode, MAX_RECOG_OPERANDS> input_mode;
 
@@ -3405,7 +3432,7 @@ expand_asm_stmt (gasm *stmt)
     }
 
   /* For in-out operands, copy output rtx to input rtx.  */
-  unsigned ninout = inout_opnum.length();
+  unsigned ninout = inout_opnum.length ();
   for (i = 0; i < ninout; i++)
     {
       int j = inout_opnum[i];
@@ -3459,7 +3486,7 @@ expand_asm_stmt (gasm *stmt)
 
   rtx body = gen_rtx_ASM_OPERANDS ((noutputs == 0 ? VOIDmode
 				    : GET_MODE (output_rvec[0])),
-				   ggc_strdup (gimple_asm_string (stmt)),
+				   ggc_strdup (str),
 				   "", 0, argvec, constraintvec,
 				   labelvec, locus);
   MEM_VOLATILE_P (body) = gimple_asm_volatile_p (stmt);
--- gcc/c/c-typeck.c.jj	2021-05-28 11:26:40.671746525 +0200
+++ gcc/c/c-typeck.c	2021-06-01 12:44:44.818236732 +0200
@@ -10669,6 +10669,14 @@ build_asm_expr (location_t loc, tree str
 	     mark it addressable.  */
 	  if (!allows_reg && !c_mark_addressable (output))
 	    output = error_mark_node;
+	  if (!allows_reg
+	      && TREE_CODE (output) == COMPONENT_REF
+	      && DECL_C_BIT_FIELD (TREE_OPERAND (output, 1)))
+	    {
+	      error_at (loc, "cannot take address of bit-field %qD",
+			TREE_OPERAND (output, 1));
+	      output = error_mark_node;
+	    }
 	  if (!(!allows_reg && allows_mem)
 	      && output != error_mark_node
 	      && VOID_TYPE_P (TREE_TYPE (output)))
@@ -10704,6 +10712,13 @@ build_asm_expr (location_t loc, tree str
 	      STRIP_NOPS (input);
 	      if (!c_mark_addressable (input))
 		input = error_mark_node;
+	      else if (TREE_CODE (input) == COMPONENT_REF
+		       && DECL_C_BIT_FIELD (TREE_OPERAND (input, 1)))
+		{
+		  error_at (loc, "cannot take address of bit-field %qD",
+			    TREE_OPERAND (input, 1));
+		  input = error_mark_node;
+		}
 	    }
 	  else
 	    {
--- gcc/cp/semantics.c.jj	2021-05-29 10:04:30.992447422 +0200
+++ gcc/cp/semantics.c	2021-06-01 13:03:09.209303218 +0200
@@ -1811,6 +1811,11 @@ finish_asm_stmt (location_t loc, int vol
 		 mark it addressable.  */
 	      if (!allows_reg && !cxx_mark_addressable (*op))
 		operand = error_mark_node;
+	      else if (!allows_reg && bitfield_p (*op))
+		{
+		  error_at (loc, "attempt to take address of bit-field");
+		  operand = error_mark_node;
+		}
 	    }
 	  else
 	    operand = error_mark_node;
@@ -1870,6 +1875,11 @@ finish_asm_stmt (location_t loc, int vol
 
 		  if (!cxx_mark_addressable (*op))
 		    operand = error_mark_node;
+		  else if (!allows_reg && bitfield_p (*op))
+		    {
+		      error_at (loc, "attempt to take address of bit-field");
+		      operand = error_mark_node;
+		    }
 		}
 	      else if (!allows_reg && !allows_mem)
 		{
--- gcc/testsuite/c-c++-common/pr100785.c.jj	2021-06-01 13:26:27.602110552 +0200
+++ gcc/testsuite/c-c++-common/pr100785.c	2021-06-01 13:26:14.167304278 +0200
@@ -0,0 +1,21 @@
+/* PR inline-asm/100785 */
+
+struct S { int a : 1; };
+
+void
+foo (struct S *x)
+{
+  __asm__ ("" : "+m" (x->a));	/* { dg-error "address of bit-field" } */
+}
+
+void
+bar (struct S *x)
+{
+  __asm__ ("" : "=m" (x->a));	/* { dg-error "address of bit-field" } */
+}
+
+void
+baz (struct S *x)
+{
+  __asm__ ("" : : "m" (x->a));	/* { dg-error "address of bit-field" } */
+}
--- gcc/testsuite/gcc.dg/pr48552-1.c.jj	2020-01-12 11:54:37.479397326 +0100
+++ gcc/testsuite/gcc.dg/pr48552-1.c	2021-06-02 09:29:51.980149530 +0200
@@ -15,7 +15,7 @@ f2 (void *x)
 {
   __asm volatile ("" : "=r" (*x));	/* { dg-warning "dereferencing" "deref" } */
 }					/* { dg-error "invalid use of void expression" "void expr" { target *-*-* } .-1 } */
-					/* { dg-error "invalid lvalue in 'asm' output 0" "invalid lvalue" { target *-*-* } .-2 } */
+
 void
 f3 (void *x)
 {
@@ -39,7 +39,7 @@ f6 (void *x)
 {
   __asm volatile ("" : "=g" (*x));	/* { dg-warning "dereferencing" "deref" } */
 }					/* { dg-error "invalid use of void expression" "void expr" { target *-*-* } .-1 } */
-					/* { dg-error "invalid lvalue in 'asm' output 0" "invalid lvalue" { target *-*-* } .-2 } */
+
 void
 f7 (struct S *x)
 {
--- gcc/testsuite/gcc.dg/pr48552-2.c.jj	2020-01-12 11:54:37.479397326 +0100
+++ gcc/testsuite/gcc.dg/pr48552-2.c	2021-06-02 09:30:02.779995869 +0200
@@ -15,7 +15,7 @@ f2 (void *x)
 {
   __asm ("" : "=r" (*x));	/* { dg-warning "dereferencing" "deref" } */
 }				/* { dg-error "invalid use of void expression" "void expr" { target *-*-* } .-1 } */
-				/* { dg-error "invalid lvalue in 'asm' output 0" "invalid lvalue" { target *-*-* } .-2 } */
+
 void
 f3 (void *x)
 {
@@ -39,7 +39,7 @@ f6 (void *x)
 {
   __asm ("" : "=g" (*x));	/* { dg-warning "dereferencing" "deref" } */
 }				/* { dg-error "invalid use of void expression" "void expr" { target *-*-* } .-1 } */
-				/* { dg-error "invalid lvalue in 'asm' output 0" "invalid lvalue" { target *-*-* } .-2 } */
+
 void
 f7 (struct S *x)
 {

	Jakub

Re: [PATCH] inline-asm: Fix ICE with bitfields in "m" operands [PR100785]

[Jason Merrill]

On 6/2/21 3:59 AM, Jakub Jelinek wrote:
>   	      if (!allows_reg && !cxx_mark_addressable (*op))
>   		operand = error_mark_node;
> +	      else if (!allows_reg && bitfield_p (*op))
> +		{
> +		  error_at (loc, "attempt to take address of bit-field");

Hmm, why aren't we already catching this in cxx_mark_addressable?

Jason

Re: [PATCH] inline-asm: Fix ICE with bitfields in "m" operands [PR100785]

[Jakub Jelinek]

On Wed, Jun 02, 2021 at 11:09:45AM -0400, Jason Merrill wrote:
> On 6/2/21 3:59 AM, Jakub Jelinek wrote:
> >   	      if (!allows_reg && !cxx_mark_addressable (*op))
> >   		operand = error_mark_node;
> > +	      else if (!allows_reg && bitfield_p (*op))
> > +		{
> > +		  error_at (loc, "attempt to take address of bit-field");
> 
> Hmm, why aren't we already catching this in cxx_mark_addressable?

That is certainly possible, but it goes against Eric's patch
https://gcc.gnu.org/pipermail/gcc-patches/2015-June/421483.html

So, do you want to keep the
  if (bitfield_p (arg))
    {
      if (complain & tf_error)
        error_at (loc, "attempt to take address of bit-field");
      return error_mark_node;
    }
in cp_build_addr_expr_1 and duplicate such check in cxx_mark_addressable
(though, that one doesn't have complain, will it be ok to do it
unconditionally for SFINAE)?

Shall the C FE do the same (i.e. diagnose in both places)?

	Jakub

Re: [PATCH] inline-asm: Fix ICE with bitfields in "m" operands [PR100785]

[Jason Merrill]

On 6/2/21 11:25 AM, Jakub Jelinek wrote:
> On Wed, Jun 02, 2021 at 11:09:45AM -0400, Jason Merrill wrote:
>> On 6/2/21 3:59 AM, Jakub Jelinek wrote:
>>>    	      if (!allows_reg && !cxx_mark_addressable (*op))
>>>    		operand = error_mark_node;
>>> +	      else if (!allows_reg && bitfield_p (*op))
>>> +		{
>>> +		  error_at (loc, "attempt to take address of bit-field");
>>
>> Hmm, why aren't we already catching this in cxx_mark_addressable?
> 
> That is certainly possible, but it goes against Eric's patch
> https://gcc.gnu.org/pipermail/gcc-patches/2015-June/421483.html

Hmm, I wonder what his rationale was?

> So, do you want to keep the
>    if (bitfield_p (arg))
>      {
>        if (complain & tf_error)
>          error_at (loc, "attempt to take address of bit-field");
>        return error_mark_node;
>      }
> in cp_build_addr_expr_1 and duplicate such check in cxx_mark_addressable
> (though, that one doesn't have complain, will it be ok to do it
> unconditionally for SFINAE)?

We would need to add complain; the existing diagnostics can't happen in 
SFINAE context, but this one can.

Alternately, cxx_mark_addressable could abort on a bitfield to require 
the caller to handle it, but that seems less useful.

Jason

> Shall the C FE do the same (i.e. diagnose in both places)?

[PATCH v2] inline-asm: Fix ICE with bitfields in "m" operands [PR100785]

[Jakub Jelinek]

On Wed, Jun 02, 2021 at 12:14:51PM -0400, Jason Merrill wrote:
> On 6/2/21 11:25 AM, Jakub Jelinek wrote:
> > On Wed, Jun 02, 2021 at 11:09:45AM -0400, Jason Merrill wrote:
> > > On 6/2/21 3:59 AM, Jakub Jelinek wrote:
> > > >    	      if (!allows_reg && !cxx_mark_addressable (*op))
> > > >    		operand = error_mark_node;
> > > > +	      else if (!allows_reg && bitfield_p (*op))
> > > > +		{
> > > > +		  error_at (loc, "attempt to take address of bit-field");
> > > 
> > > Hmm, why aren't we already catching this in cxx_mark_addressable?
> > 
> > That is certainly possible, but it goes against Eric's patch
> > https://gcc.gnu.org/pipermail/gcc-patches/2015-June/421483.html
> 
> Hmm, I wonder what his rationale was?

No idea (and see below, nothing in the testsuite seems to require that).

Anyway, sorry for forgetting about this.

Here is a variant of the patch that adds the errors to
{c,cxx}_mark_addressable but keeps them in
build_unary_op/cp_build_addr_expr_1 too where for C++ it handles SFINAE
there etc.

Bootstrapped/regtested on x86_64-linux and i686-linux.

2021-06-11  Jakub Jelinek  <jakub@redhat.com>

	PR inline-asm/100785
gcc/
	* gimplify.c (gimplify_asm_expr): Don't diagnose errors if
	output or input operands were already error_mark_node.
	* cfgexpand.c (expand_asm_stmt): If errors are emitted,
	remove all inputs, outputs and clobbers from the asm and
	set template to "".
gcc/c/
	* c-typeck.c (c_mark_addressable): Diagnose trying to make
	bit-fields addressable.
gcc/cp/
	* semantics.c (cxx_mark_addressable): Diagnose trying to make
	bit-fields addressable.
gcc/testsuite/
	* c-c++-common/pr100785.c: New test.
	* gcc.dg/pr48552-1.c: Don't expect invalid lvalue errors.
	* gcc.dg/pr48552-2.c: Likewise.

--- gcc/gimplify.c.jj	2021-06-10 15:27:35.141299425 +0200
+++ gcc/gimplify.c	2021-06-11 13:27:51.212867419 +0200
@@ -6321,12 +6321,14 @@ gimplify_asm_expr (tree *expr_p, gimple_
       if (!allows_reg && allows_mem)
 	mark_addressable (TREE_VALUE (link));
 
+      tree orig = TREE_VALUE (link);
       tret = gimplify_expr (&TREE_VALUE (link), pre_p, post_p,
 			    is_inout ? is_gimple_min_lval : is_gimple_lvalue,
 			    fb_lvalue | fb_mayfail);
       if (tret == GS_ERROR)
 	{
-	  error ("invalid lvalue in %<asm%> output %d", i);
+	  if (orig != error_mark_node)
+	    error ("invalid lvalue in %<asm%> output %d", i);
 	  ret = tret;
 	}
 
@@ -6521,8 +6523,9 @@ gimplify_asm_expr (tree *expr_p, gimple_
 	  mark_addressable (TREE_VALUE (link));
 	  if (tret == GS_ERROR)
 	    {
-	      error_at (EXPR_LOC_OR_LOC (TREE_VALUE (link), input_location),
-			"memory input %d is not directly addressable", i);
+	      if (inputv != error_mark_node)
+		error_at (EXPR_LOC_OR_LOC (TREE_VALUE (link), input_location),
+			  "memory input %d is not directly addressable", i);
 	      ret = tret;
 	    }
 	}
--- gcc/cfgexpand.c.jj	2021-06-02 10:07:47.632826557 +0200
+++ gcc/cfgexpand.c	2021-06-11 13:27:51.212867419 +0200
@@ -3082,6 +3082,7 @@ expand_asm_stmt (gasm *stmt)
   unsigned ninputs = gimple_asm_ninputs (stmt);
   unsigned nlabels = gimple_asm_nlabels (stmt);
   unsigned i;
+  bool error_seen = false;
 
   /* ??? Diagnose during gimplification?  */
   if (ninputs + noutputs + nlabels > MAX_RECOG_OPERANDS)
@@ -3140,6 +3141,7 @@ expand_asm_stmt (gasm *stmt)
 		{
 		  /* ??? Diagnose during gimplification?  */
 		  error ("unknown register name %qs in %<asm%>", regname);
+		  error_seen = true;
 		}
 	      else if (j == -4)
 		{
@@ -3202,7 +3204,10 @@ expand_asm_stmt (gasm *stmt)
 		&& REG_P (DECL_RTL (output_tvec[j]))
 		&& HARD_REGISTER_P (DECL_RTL (output_tvec[j]))
 		&& output_hregno == REGNO (DECL_RTL (output_tvec[j])))
-	      error ("invalid hard register usage between output operands");
+	      {
+		error ("invalid hard register usage between output operands");
+		error_seen = true;
+	      }
 
 	  /* Verify matching constraint operands use the same hard register
 	     and that the non-matching constraint operands do not use the same
@@ -3225,13 +3230,19 @@ expand_asm_stmt (gasm *stmt)
 		  }
 		if (i == match
 		    && output_hregno != input_hregno)
-		  error ("invalid hard register usage between output operand "
-			 "and matching constraint operand");
+		  {
+		    error ("invalid hard register usage between output "
+			   "operand and matching constraint operand");
+		    error_seen = true;
+		  }
 		else if (early_clobber_p
 			 && i != match
 			 && output_hregno == input_hregno)
-		  error ("invalid hard register usage between earlyclobber "
-			 "operand and input operand");
+		  {
+		    error ("invalid hard register usage between "
+			   "earlyclobber operand and input operand");
+		    error_seen = true;
+		  }
 	      }
 	}
 
@@ -3307,7 +3318,10 @@ expand_asm_stmt (gasm *stmt)
 	    op = validize_mem (op);
 
 	  if (! allows_reg && !MEM_P (op))
-	    error ("output number %d not directly addressable", i);
+	    {
+	      error ("output number %d not directly addressable", i);
+	      error_seen = true;
+	    }
 	  if ((! allows_mem && MEM_P (op) && GET_MODE (op) != BLKmode)
 	      || GET_CODE (op) == CONCAT)
 	    {
@@ -3347,6 +3361,19 @@ expand_asm_stmt (gasm *stmt)
 	inout_opnum.safe_push (i);
     }
 
+  const char *str = gimple_asm_string (stmt);
+  if (error_seen)
+    {
+      ninputs = 0;
+      noutputs = 0;
+      inout_opnum.truncate (0);
+      output_rvec.truncate (0);
+      clobber_rvec.truncate (0);
+      constraints.truncate (0);
+      CLEAR_HARD_REG_SET (clobbered_regs);
+      str = "";
+    }
+
   auto_vec<rtx, MAX_RECOG_OPERANDS> input_rvec;
   auto_vec<machine_mode, MAX_RECOG_OPERANDS> input_mode;
 
@@ -3405,7 +3432,7 @@ expand_asm_stmt (gasm *stmt)
     }
 
   /* For in-out operands, copy output rtx to input rtx.  */
-  unsigned ninout = inout_opnum.length();
+  unsigned ninout = inout_opnum.length ();
   for (i = 0; i < ninout; i++)
     {
       int j = inout_opnum[i];
@@ -3459,7 +3486,7 @@ expand_asm_stmt (gasm *stmt)
 
   rtx body = gen_rtx_ASM_OPERANDS ((noutputs == 0 ? VOIDmode
 				    : GET_MODE (output_rvec[0])),
-				   ggc_strdup (gimple_asm_string (stmt)),
+				   ggc_strdup (str),
 				   "", 0, argvec, constraintvec,
 				   labelvec, locus);
   MEM_VOLATILE_P (body) = gimple_asm_volatile_p (stmt);
--- gcc/c/c-typeck.c.jj	2021-06-10 16:45:43.953519765 +0200
+++ gcc/c/c-typeck.c	2021-06-11 13:32:33.953983430 +0200
@@ -5034,8 +5034,17 @@ c_mark_addressable (tree exp, bool array
 	    && TREE_CODE (TREE_TYPE (x)) == ARRAY_TYPE
 	    && VECTOR_TYPE_P (TREE_TYPE (TREE_OPERAND (x, 0))))
 	  return true;
-	/* FALLTHRU */
+	x = TREE_OPERAND (x, 0);
+	break;
+
       case COMPONENT_REF:
+	if (DECL_C_BIT_FIELD (TREE_OPERAND (x, 1)))
+	  {
+	    error ("cannot take address of bit-field %qD",
+		   TREE_OPERAND (x, 1));
+	    return false;
+	  }
+	/* FALLTHRU */
       case ADDR_EXPR:
       case ARRAY_REF:
       case REALPART_EXPR:
--- gcc/cp/typeck.c.jj	2021-05-31 10:11:15.150978892 +0200
+++ gcc/cp/typeck.c	2021-06-11 13:38:37.850984608 +0200
@@ -7119,9 +7119,14 @@ cxx_mark_addressable (tree exp, bool arr
 	    && TREE_CODE (TREE_TYPE (x)) == ARRAY_TYPE
 	    && VECTOR_TYPE_P (TREE_TYPE (TREE_OPERAND (x, 0))))
 	  return true;
+	x = TREE_OPERAND (x, 0);
+	break;
+
+      case COMPONENT_REF:
+	if (bitfield_p (x))
+	  error ("attempt to take address of bit-field");
 	/* FALLTHRU */
       case ADDR_EXPR:
-      case COMPONENT_REF:
       case ARRAY_REF:
       case REALPART_EXPR:
       case IMAGPART_EXPR:
--- gcc/testsuite/c-c++-common/pr100785.c.jj	2021-06-11 13:27:51.215867378 +0200
+++ gcc/testsuite/c-c++-common/pr100785.c	2021-06-11 13:27:51.215867378 +0200
@@ -0,0 +1,21 @@
+/* PR inline-asm/100785 */
+
+struct S { int a : 1; };
+
+void
+foo (struct S *x)
+{
+  __asm__ ("" : "+m" (x->a));	/* { dg-error "address of bit-field" } */
+}
+
+void
+bar (struct S *x)
+{
+  __asm__ ("" : "=m" (x->a));	/* { dg-error "address of bit-field" } */
+}
+
+void
+baz (struct S *x)
+{
+  __asm__ ("" : : "m" (x->a));	/* { dg-error "address of bit-field" } */
+}
--- gcc/testsuite/gcc.dg/pr48552-1.c.jj	2021-06-02 10:07:47.737825068 +0200
+++ gcc/testsuite/gcc.dg/pr48552-1.c	2021-06-11 13:27:51.228867199 +0200
@@ -15,7 +15,7 @@ f2 (void *x)
 {
   __asm volatile ("" : "=r" (*x));	/* { dg-warning "dereferencing" "deref" } */
 }					/* { dg-error "invalid use of void expression" "void expr" { target *-*-* } .-1 } */
-					/* { dg-error "invalid lvalue in 'asm' output 0" "invalid lvalue" { target *-*-* } .-2 } */
+
 void
 f3 (void *x)
 {
@@ -39,7 +39,7 @@ f6 (void *x)
 {
   __asm volatile ("" : "=g" (*x));	/* { dg-warning "dereferencing" "deref" } */
 }					/* { dg-error "invalid use of void expression" "void expr" { target *-*-* } .-1 } */
-					/* { dg-error "invalid lvalue in 'asm' output 0" "invalid lvalue" { target *-*-* } .-2 } */
+
 void
 f7 (struct S *x)
 {
--- gcc/testsuite/gcc.dg/pr48552-2.c.jj	2021-06-02 10:07:47.773824557 +0200
+++ gcc/testsuite/gcc.dg/pr48552-2.c	2021-06-11 13:27:51.233867130 +0200
@@ -15,7 +15,7 @@ f2 (void *x)
 {
   __asm ("" : "=r" (*x));	/* { dg-warning "dereferencing" "deref" } */
 }				/* { dg-error "invalid use of void expression" "void expr" { target *-*-* } .-1 } */
-				/* { dg-error "invalid lvalue in 'asm' output 0" "invalid lvalue" { target *-*-* } .-2 } */
+
 void
 f3 (void *x)
 {
@@ -39,7 +39,7 @@ f6 (void *x)
 {
   __asm ("" : "=g" (*x));	/* { dg-warning "dereferencing" "deref" } */
 }				/* { dg-error "invalid use of void expression" "void expr" { target *-*-* } .-1 } */
-				/* { dg-error "invalid lvalue in 'asm' output 0" "invalid lvalue" { target *-*-* } .-2 } */
+
 void
 f7 (struct S *x)
 {


	Jakub

Re: [PATCH v2] inline-asm: Fix ICE with bitfields in "m" operands [PR100785]

[Jason Merrill]

On 6/11/21 10:50 AM, Jakub Jelinek wrote:
> On Wed, Jun 02, 2021 at 12:14:51PM -0400, Jason Merrill wrote:
>> On 6/2/21 11:25 AM, Jakub Jelinek wrote:
>>> On Wed, Jun 02, 2021 at 11:09:45AM -0400, Jason Merrill wrote:
>>>> On 6/2/21 3:59 AM, Jakub Jelinek wrote:
>>>>>     	      if (!allows_reg && !cxx_mark_addressable (*op))
>>>>>     		operand = error_mark_node;
>>>>> +	      else if (!allows_reg && bitfield_p (*op))
>>>>> +		{
>>>>> +		  error_at (loc, "attempt to take address of bit-field");
>>>>
>>>> Hmm, why aren't we already catching this in cxx_mark_addressable?
>>>
>>> That is certainly possible, but it goes against Eric's patch
>>> https://gcc.gnu.org/pipermail/gcc-patches/2015-June/421483.html
>>
>> Hmm, I wonder what his rationale was?
> 
> No idea (and see below, nothing in the testsuite seems to require that).
> 
> Anyway, sorry for forgetting about this.
> 
> Here is a variant of the patch that adds the errors to
> {c,cxx}_mark_addressable but keeps them in
> build_unary_op/cp_build_addr_expr_1 too where for C++ it handles SFINAE
> there etc.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux.

C++ change is OK; the whole patch is OK on Tuesday if no other comments.

> 2021-06-11  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR inline-asm/100785
> gcc/
> 	* gimplify.c (gimplify_asm_expr): Don't diagnose errors if
> 	output or input operands were already error_mark_node.
> 	* cfgexpand.c (expand_asm_stmt): If errors are emitted,
> 	remove all inputs, outputs and clobbers from the asm and
> 	set template to "".
> gcc/c/
> 	* c-typeck.c (c_mark_addressable): Diagnose trying to make
> 	bit-fields addressable.
> gcc/cp/
> 	* semantics.c (cxx_mark_addressable): Diagnose trying to make
> 	bit-fields addressable.
> gcc/testsuite/
> 	* c-c++-common/pr100785.c: New test.
> 	* gcc.dg/pr48552-1.c: Don't expect invalid lvalue errors.
> 	* gcc.dg/pr48552-2.c: Likewise.
> 
> --- gcc/gimplify.c.jj	2021-06-10 15:27:35.141299425 +0200
> +++ gcc/gimplify.c	2021-06-11 13:27:51.212867419 +0200
> @@ -6321,12 +6321,14 @@ gimplify_asm_expr (tree *expr_p, gimple_
>         if (!allows_reg && allows_mem)
>   	mark_addressable (TREE_VALUE (link));
>   
> +      tree orig = TREE_VALUE (link);
>         tret = gimplify_expr (&TREE_VALUE (link), pre_p, post_p,
>   			    is_inout ? is_gimple_min_lval : is_gimple_lvalue,
>   			    fb_lvalue | fb_mayfail);
>         if (tret == GS_ERROR)
>   	{
> -	  error ("invalid lvalue in %<asm%> output %d", i);
> +	  if (orig != error_mark_node)
> +	    error ("invalid lvalue in %<asm%> output %d", i);
>   	  ret = tret;
>   	}
>   
> @@ -6521,8 +6523,9 @@ gimplify_asm_expr (tree *expr_p, gimple_
>   	  mark_addressable (TREE_VALUE (link));
>   	  if (tret == GS_ERROR)
>   	    {
> -	      error_at (EXPR_LOC_OR_LOC (TREE_VALUE (link), input_location),
> -			"memory input %d is not directly addressable", i);
> +	      if (inputv != error_mark_node)
> +		error_at (EXPR_LOC_OR_LOC (TREE_VALUE (link), input_location),
> +			  "memory input %d is not directly addressable", i);
>   	      ret = tret;
>   	    }
>   	}
> --- gcc/cfgexpand.c.jj	2021-06-02 10:07:47.632826557 +0200
> +++ gcc/cfgexpand.c	2021-06-11 13:27:51.212867419 +0200
> @@ -3082,6 +3082,7 @@ expand_asm_stmt (gasm *stmt)
>     unsigned ninputs = gimple_asm_ninputs (stmt);
>     unsigned nlabels = gimple_asm_nlabels (stmt);
>     unsigned i;
> +  bool error_seen = false;
>   
>     /* ??? Diagnose during gimplification?  */
>     if (ninputs + noutputs + nlabels > MAX_RECOG_OPERANDS)
> @@ -3140,6 +3141,7 @@ expand_asm_stmt (gasm *stmt)
>   		{
>   		  /* ??? Diagnose during gimplification?  */
>   		  error ("unknown register name %qs in %<asm%>", regname);
> +		  error_seen = true;
>   		}
>   	      else if (j == -4)
>   		{
> @@ -3202,7 +3204,10 @@ expand_asm_stmt (gasm *stmt)
>   		&& REG_P (DECL_RTL (output_tvec[j]))
>   		&& HARD_REGISTER_P (DECL_RTL (output_tvec[j]))
>   		&& output_hregno == REGNO (DECL_RTL (output_tvec[j])))
> -	      error ("invalid hard register usage between output operands");
> +	      {
> +		error ("invalid hard register usage between output operands");
> +		error_seen = true;
> +	      }
>   
>   	  /* Verify matching constraint operands use the same hard register
>   	     and that the non-matching constraint operands do not use the same
> @@ -3225,13 +3230,19 @@ expand_asm_stmt (gasm *stmt)
>   		  }
>   		if (i == match
>   		    && output_hregno != input_hregno)
> -		  error ("invalid hard register usage between output operand "
> -			 "and matching constraint operand");
> +		  {
> +		    error ("invalid hard register usage between output "
> +			   "operand and matching constraint operand");
> +		    error_seen = true;
> +		  }
>   		else if (early_clobber_p
>   			 && i != match
>   			 && output_hregno == input_hregno)
> -		  error ("invalid hard register usage between earlyclobber "
> -			 "operand and input operand");
> +		  {
> +		    error ("invalid hard register usage between "
> +			   "earlyclobber operand and input operand");
> +		    error_seen = true;
> +		  }
>   	      }
>   	}
>   
> @@ -3307,7 +3318,10 @@ expand_asm_stmt (gasm *stmt)
>   	    op = validize_mem (op);
>   
>   	  if (! allows_reg && !MEM_P (op))
> -	    error ("output number %d not directly addressable", i);
> +	    {
> +	      error ("output number %d not directly addressable", i);
> +	      error_seen = true;
> +	    }
>   	  if ((! allows_mem && MEM_P (op) && GET_MODE (op) != BLKmode)
>   	      || GET_CODE (op) == CONCAT)
>   	    {
> @@ -3347,6 +3361,19 @@ expand_asm_stmt (gasm *stmt)
>   	inout_opnum.safe_push (i);
>       }
>   
> +  const char *str = gimple_asm_string (stmt);
> +  if (error_seen)
> +    {
> +      ninputs = 0;
> +      noutputs = 0;
> +      inout_opnum.truncate (0);
> +      output_rvec.truncate (0);
> +      clobber_rvec.truncate (0);
> +      constraints.truncate (0);
> +      CLEAR_HARD_REG_SET (clobbered_regs);
> +      str = "";
> +    }
> +
>     auto_vec<rtx, MAX_RECOG_OPERANDS> input_rvec;
>     auto_vec<machine_mode, MAX_RECOG_OPERANDS> input_mode;
>   
> @@ -3405,7 +3432,7 @@ expand_asm_stmt (gasm *stmt)
>       }
>   
>     /* For in-out operands, copy output rtx to input rtx.  */
> -  unsigned ninout = inout_opnum.length();
> +  unsigned ninout = inout_opnum.length ();
>     for (i = 0; i < ninout; i++)
>       {
>         int j = inout_opnum[i];
> @@ -3459,7 +3486,7 @@ expand_asm_stmt (gasm *stmt)
>   
>     rtx body = gen_rtx_ASM_OPERANDS ((noutputs == 0 ? VOIDmode
>   				    : GET_MODE (output_rvec[0])),
> -				   ggc_strdup (gimple_asm_string (stmt)),
> +				   ggc_strdup (str),
>   				   "", 0, argvec, constraintvec,
>   				   labelvec, locus);
>     MEM_VOLATILE_P (body) = gimple_asm_volatile_p (stmt);
> --- gcc/c/c-typeck.c.jj	2021-06-10 16:45:43.953519765 +0200
> +++ gcc/c/c-typeck.c	2021-06-11 13:32:33.953983430 +0200
> @@ -5034,8 +5034,17 @@ c_mark_addressable (tree exp, bool array
>   	    && TREE_CODE (TREE_TYPE (x)) == ARRAY_TYPE
>   	    && VECTOR_TYPE_P (TREE_TYPE (TREE_OPERAND (x, 0))))
>   	  return true;
> -	/* FALLTHRU */
> +	x = TREE_OPERAND (x, 0);
> +	break;
> +
>         case COMPONENT_REF:
> +	if (DECL_C_BIT_FIELD (TREE_OPERAND (x, 1)))
> +	  {
> +	    error ("cannot take address of bit-field %qD",
> +		   TREE_OPERAND (x, 1));
> +	    return false;
> +	  }
> +	/* FALLTHRU */
>         case ADDR_EXPR:
>         case ARRAY_REF:
>         case REALPART_EXPR:
> --- gcc/cp/typeck.c.jj	2021-05-31 10:11:15.150978892 +0200
> +++ gcc/cp/typeck.c	2021-06-11 13:38:37.850984608 +0200
> @@ -7119,9 +7119,14 @@ cxx_mark_addressable (tree exp, bool arr
>   	    && TREE_CODE (TREE_TYPE (x)) == ARRAY_TYPE
>   	    && VECTOR_TYPE_P (TREE_TYPE (TREE_OPERAND (x, 0))))
>   	  return true;
> +	x = TREE_OPERAND (x, 0);
> +	break;
> +
> +      case COMPONENT_REF:
> +	if (bitfield_p (x))
> +	  error ("attempt to take address of bit-field");
>   	/* FALLTHRU */
>         case ADDR_EXPR:
> -      case COMPONENT_REF:
>         case ARRAY_REF:
>         case REALPART_EXPR:
>         case IMAGPART_EXPR:
> --- gcc/testsuite/c-c++-common/pr100785.c.jj	2021-06-11 13:27:51.215867378 +0200
> +++ gcc/testsuite/c-c++-common/pr100785.c	2021-06-11 13:27:51.215867378 +0200
> @@ -0,0 +1,21 @@
> +/* PR inline-asm/100785 */
> +
> +struct S { int a : 1; };
> +
> +void
> +foo (struct S *x)
> +{
> +  __asm__ ("" : "+m" (x->a));	/* { dg-error "address of bit-field" } */
> +}
> +
> +void
> +bar (struct S *x)
> +{
> +  __asm__ ("" : "=m" (x->a));	/* { dg-error "address of bit-field" } */
> +}
> +
> +void
> +baz (struct S *x)
> +{
> +  __asm__ ("" : : "m" (x->a));	/* { dg-error "address of bit-field" } */
> +}
> --- gcc/testsuite/gcc.dg/pr48552-1.c.jj	2021-06-02 10:07:47.737825068 +0200
> +++ gcc/testsuite/gcc.dg/pr48552-1.c	2021-06-11 13:27:51.228867199 +0200
> @@ -15,7 +15,7 @@ f2 (void *x)
>   {
>     __asm volatile ("" : "=r" (*x));	/* { dg-warning "dereferencing" "deref" } */
>   }					/* { dg-error "invalid use of void expression" "void expr" { target *-*-* } .-1 } */
> -					/* { dg-error "invalid lvalue in 'asm' output 0" "invalid lvalue" { target *-*-* } .-2 } */
> +
>   void
>   f3 (void *x)
>   {
> @@ -39,7 +39,7 @@ f6 (void *x)
>   {
>     __asm volatile ("" : "=g" (*x));	/* { dg-warning "dereferencing" "deref" } */
>   }					/* { dg-error "invalid use of void expression" "void expr" { target *-*-* } .-1 } */
> -					/* { dg-error "invalid lvalue in 'asm' output 0" "invalid lvalue" { target *-*-* } .-2 } */
> +
>   void
>   f7 (struct S *x)
>   {
> --- gcc/testsuite/gcc.dg/pr48552-2.c.jj	2021-06-02 10:07:47.773824557 +0200
> +++ gcc/testsuite/gcc.dg/pr48552-2.c	2021-06-11 13:27:51.233867130 +0200
> @@ -15,7 +15,7 @@ f2 (void *x)
>   {
>     __asm ("" : "=r" (*x));	/* { dg-warning "dereferencing" "deref" } */
>   }				/* { dg-error "invalid use of void expression" "void expr" { target *-*-* } .-1 } */
> -				/* { dg-error "invalid lvalue in 'asm' output 0" "invalid lvalue" { target *-*-* } .-2 } */
> +
>   void
>   f3 (void *x)
>   {
> @@ -39,7 +39,7 @@ f6 (void *x)
>   {
>     __asm ("" : "=g" (*x));	/* { dg-warning "dereferencing" "deref" } */
>   }				/* { dg-error "invalid use of void expression" "void expr" { target *-*-* } .-1 } */
> -				/* { dg-error "invalid lvalue in 'asm' output 0" "invalid lvalue" { target *-*-* } .-2 } */
> +
>   void
>   f7 (struct S *x)
>   {
> 
> 
> 	Jakub
> 

Re: [PATCH v2] inline-asm: Fix ICE with bitfields in "m" operands [PR100785]

[Joseph Myers]

On Fri, 11 Jun 2021, Jakub Jelinek via Gcc-patches wrote:

> gcc/c/
> 	* c-typeck.c (c_mark_addressable): Diagnose trying to make
> 	bit-fields addressable.

The C front-end changes are OK.

-- 
Joseph S. Myers
joseph@codesourcery.com