Hi Richard,
Thanks for looking into this.

It’s not the call to size_binop_loc (for CEIL_DIV_EXPR) that's problematic, but 
the
call to fold_convert_loc (loc, size_type_node, value) on line 4009 of 
c-common.cc.
At this point, value is (NOP_EXPR:sizetype (VAR_DECL:error_mark_node)).

Ultimately, it's the code in match.pd /* Handle cases of two conversions in a 
row.  */
with the problematic line being (match.pd:4748):
      unsigned int inside_prec = element_precision (inside_type); 

Here inside_type is error_mark_node, and so tree type checking in 
element_precision
throws an internal_error.

There doesn’t seem to be a good way to fix this in element_precision, and it's
complicated to reorganize the logic in match.pd's "with clause" inside the
(ocvt (icvt@1 @0)), but perhaps a (ocvt (icvt:non_error_type@1 @0))?

The last place/opportunity the front-end could sanitize this operand before
passing the dubious tree to the middle-end is c_sizeof_or_alignof_type (which
alas doesn't appear in the backtrace due to inlining).

#5  0x000000000227b0e9 in internal_error (
    gmsgid=gmsgid@entry=0x249c7b8 "tree check: expected class %qs, have %qs 
(%s) in %s, at %s:%d") at ../../gcc/gcc/diagnostic.cc:2232
#6  0x000000000081e32a in tree_class_check_failed (node=0x7ffff6c1ef30,
    cl=cl@entry=tcc_type, file=file@entry=0x2495f3f "../../gcc/gcc/tree.cc",
    line=line@entry=6795, function=function@entry=0x24961fe "element_precision")
    at ../../gcc/gcc/tree.cc:9005
#7  0x000000000081ef4c in tree_class_check (__t=<optimized out>, 
__class=tcc_type,
    __f=0x2495f3f "../../gcc/gcc/tree.cc", __l=6795,
    __g=0x24961fe "element_precision") at ../../gcc/gcc/tree.h:4067
#8  element_precision (type=<optimized out>, type@entry=0x7ffff6c1ef30)
    at ../../gcc/gcc/tree.cc:6795
#9  0x00000000017f66a4 in generic_simplify_CONVERT_EXPR (loc=201632,
    code=<optimized out>, type=0x7ffff6c3e7e0, _p0=0x7ffff6dc95c0)
    at generic-match-6.cc:3386
#10 0x0000000000c1b18c in fold_unary_loc (loc=201632, code=NOP_EXPR,
    type=0x7ffff6c3e7e0, op0=0x7ffff6dc95c0) at ../../gcc/gcc/fold-const.cc:9523
#11 0x0000000000c1d94a in fold_build1_loc (loc=201632, code=NOP_EXPR,
    type=0x7ffff6c3e7e0, op0=0x7ffff6dc95c0) at 
../../gcc/gcc/fold-const.cc:14165
#12 0x000000000094068c in c_expr_sizeof_expr (loc=loc@entry=201632, expr=...)
    at ../../gcc/gcc/tree.h:3771
#13 0x000000000097f06c in c_parser_sizeof_expression (parser=<optimized out>)
    at ../../gcc/gcc/c/c-parser.cc:9932


I hope this explains what's happening.  The size_binop_loc call is a bit of a 
red
herring that returns the same tree it is given (as TYPE_PRECISION 
(char_type_node)
== BITS_PER_UNIT), so it's the "TYPE_SIZE_UNIT (type)" which needs to be checked
for the embedded VAR_DECL with a TREE_TYPE of error_mark_node.

As Andrew Pinski writes in comment #3, this one is trickier than average.

A more comprehensive fix might be to write deep_error_operand_p which does
more of a tree traversal checking error_operand_p within the unary and binary
operators of an expression tree.

Please let me know what you think/recommend.
Best regards,
Roger
--

> -----Original Message-----
> From: Richard Biener <richard.guent...@gmail.com>
> Sent: 30 April 2024 08:38
> To: Roger Sayle <ro...@nextmovesoftware.com>
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [C PATCH] PR c/109618: ICE-after-error from error_mark_node.
> 
> On Tue, Apr 30, 2024 at 1:06 AM Roger Sayle <ro...@nextmovesoftware.com>
> wrote:
> >
> >
> > This patch solves another ICE-after-error problem in the C family
> > front-ends.  Upon a conflicting type redeclaration, the ambiguous type
> > is poisoned with an error_mark_node to indicate to the middle-end that
> > the type is suspect, but care has to be taken by the front-end to
> > avoid passing these malformed trees into the middle-end during error
> > recovery. In this case, a var_decl with a poisoned type appears within
> > a sizeof() expression (wrapped in NOP_EXPR) which causes problems.
> >
> > This revision of the patch tests seen_error() to avoid tree traversal
> > (STRIP_NOPs) in the most common case that an error hasn't occurred.
> > Both this version (and an earlier revision that didn't test
> > seen_error) have survived bootstrap and regression testing on 
> > x86_64-pc-linux-
> gnu.
> > As a consolation, this code also contains a minor performance
> > improvement, by avoiding trying to create (and folding away) a
> > CEIL_DIV_EXPR in the common case that "char" is a single-byte.  The
> > current code relies on the middle-end's tree folding to recognize that
> > CEIL_DIV_EXPR of integer_one_node is a no-op, that can be optimized away.
> >
> > Ok for mainline?
> 
> Where does it end up ICEing?  I see size_binop_loc guards against
> error_mark_node operands already, maybe it should use error_operand_p
> instead?
> 
> >
> > 2024-04-30  Roger Sayle  <ro...@nextmovesoftware.com>
> >
> > gcc/c-family/ChangeLog
> >         PR c/109618
> >         * c-common.cc (c_sizeof_or_alignof_type): If seen_error() check
> >         whether value is (a VAR_DECL) of type error_mark_node, or a
> >         NOP_EXPR thereof.  Avoid folding CEIL_DIV_EXPR for the common
> >         case where char_type is a single byte.
> >
> > gcc/testsuite/ChangeLog
> >         PR c/109618
> >         * gcc.dg/pr109618.c: New test case.
> >
> >
> > Thanks in advance,
> > Roger
> > --
> >

Reply via email to