[Bug sanitizer/80536] [6/7/8 Regression] UBSAN: compile time segfault

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80536

--- Comment #15 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #14)
> (In reply to Marek Polacek from comment #13)
> > This is true, but it happens very rarely.  It can happen e.g. when the
> > fold() call in save_expr() folds away the first operand of a COMPOUND_EXPR,
> > and the second operand is e.g.
> 
> Can't it happen say if you have save_expr called with (0 * i) + (0 * j) + (0
> * k) or whatever similar initially complex, but after folding very simple
> and obviously invariant?

In C I don't think so, because we mostly call c_save_expr and c_fully_fold
therein would fold that expression to 0.  And when we call save_expr, it's when
in_late_binary_op so the operands have already been folded.  There's one case,
though, where we call save_expr without previous folding, and that's when
constructing a VLA whose size is a sizeof of another VLA in grokdeclarator:

 6097                     /* Arrange for the SAVE_EXPR on the inside of the
 6098                        MINUS_EXPR, which allows the -1 to get folded
 6099                        with the +1 that happens when building TYPE_SIZE. 
*/
 6100                     if (size_varies)
 6101                       size = save_expr (size);

void
f (int i)
{
  int (*a)[i];
  int x[sizeof (*a)];
}

I wouldn't worry much about that.

> > But even if I add this to fold or c_fully_fold, we don't have any guarantees
> > that any of these will be called before gimplification, right?  So most
> > likely we'd end up with the new SAVE_EXPR in the gimplifier, which, as you
> > point out, is not that bad.
> 
> I think cp_fold should handle SAVE_EXPR (by cp_folding the operand, and if
> it is invariant or invariant after skipping simple arith, returning that
> folded operand, otherwise making sure to add the SAVE_EXPR into the
> fold_cache giving 
> the SAVE_EXPR itself.  Right now cp_fold ignores SAVE_EXPR, but cp_fold_r
> handles it, but that one doesn't do much good, because it cp_folds the
> operands only after folding the containing trees.

I'm testing this.  Judging by running the C++ testsuite, it basically never
happens that we're able to cp_fold the content of a SAVE_EXPR to an invariant,
although it happens e.g. with this test:

int
foo (int i)
{
  return ((0 * i * (unsigned long long) (-0 + 1UL)) * 2) % 1;
}

so it probably makes sense to add the cp_fold bits.

Thanks.