[PATCH v2] inline-asm: Fix ICE with bitfields in "m" operands [PR100785]
Jason Merrill
jason@redhat.com
Fri Jun 11 15:19:45 GMT 2021
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
>
More information about the Gcc-patches
mailing list