On Tue, Apr 30, 2024 at 10:23 AM Roger Sayle <ro...@nextmovesoftware.com> wrote: > > > 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)).
I see. Can we catch this when we build (NOP_EXPR:sizetype (VAR_DECL:error_mark_node)) and instead have it "build" 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 > > > -- > > > >
