Re: [PATCH] run early sprintf warning after SSA (PR 100325)

[Richard Biener <richard.guenther@gmail.com>]

On Fri, May 7, 2021 at 2:12 AM Martin Sebor <msebor@gmail.com> wrote:
>
> On 5/6/21 8:32 AM, Aldy Hernandez wrote:
> >
> >
> > On 5/5/21 9:26 AM, Richard Biener wrote:
> >> On Wed, May 5, 2021 at 1:32 AM Martin Sebor via Gcc-patches
> >> <gcc-patches@gcc.gnu.org> wrote:
> >>>
> >>> With no optimization, -Wformat-overflow and -Wformat-truncation
> >>> runs early to detect a subset of simple bugs.  But as it turns out,
> >>> the pass runs just a tad too early, before SSA.  That causes it to
> >>> miss a class of problems that can easily be detected once code is
> >>> in SSA form, and I would expect might also cause false positives.
> >>>
> >>> The attached change moves the sprintf pass just after pass_build_ssa,
> >>> similar to other early flow-sensitive warnings (-Wnonnull-compare and
> >>> -Wuninitialized).
> >>
> >> Makes sense.  I suppose walloca might also benefit from SSA - it seems
> >> to do range queries which won't work quite well w/o SSA?
> >
> > The early Walloca pass that runs without optimization doesn't do much,
> > as we've never had ranges so early.  All it does is diagnose _every_
> > call to alloca(), if -Walloca is passed:
> >
> >    // The first time this pass is called, it is called before
> >    // optimizations have been run and range information is unavailable,
> >    // so we can only perform strict alloca checking.
> >    if (first_time_p)
> >      return warn_alloca != 0;
> >
> > Though, I suppose we could move the first alloca pass after SSA is
> > available and make it the one and only pass, since ranger only needs
> > SSA.  However, I don't know how well this would work without value
> > numbering or CSE.  For example, for gcc.dg/Walloca-4.c the gimple is:
> >
> >    <bb 2> :
> >    _1 = rear_ptr_9(D) - w_10(D);
> >    _2 = (long unsigned int) _1;
> >    if (_2 <= 4095)
> >      goto <bb 3>; [INV]
> >    else
> >      goto <bb 4>; [INV]
> >
> >    <bb 3> :
> >    _3 = rear_ptr_9(D) - w_10(D);
> >    _4 = (long unsigned int) _3;
> >    src_16 = __builtin_alloca (_4);
> >    goto <bb 5>; [INV]
> >
> > No ranges can be determined for _4.  However, if either FRE or DOM run,
> > as they do value numbering and CSE respectively, we could easily
> > determine a range as the above would become:
> >
> >   <bb 2> :
> >    _1 = rear_ptr_9(D) - w_10(D);
> >    _2 = (long unsigned int) _1;
> >    if (_2 <= 4095)
> >      goto <bb 3>; [INV]
> >    else
> >      goto <bb 4>; [INV]
> >
> >    <bb 3> :
> >    src_16 = __builtin_alloca (_2);
> >    goto <bb 5>; [INV]
> >
> > I'm inclined to leave the first alloca pass before SSA runs, as it
> > doesn't do anything with ranges.  If anyone's open to a simple -O0 CSE
> > type pass, it would be a different story.  Thoughts?
>
> Improving the analysis at -O0 and getting better warnings that are
> more consistent with what is issued with optimization would be very
> helpful (as as long as it doesn't compromise debugging experience
> of course).

I agree.  It shouldn't be too difficult to for example run the VN
propagation part without doing actual elimiation and keep
value-numbers for consumption.  do_rpo_vn (not exported)
might even already support iterate = false, eliminate = false,
it would just need factoring out the init/deinit somewhat.

Of course it will be a lot more expensive to do since it cannot
do "on-demand" value-numbering of interesting SSA names.
I'm not sure that would be possible anyhow.  Though for
the alloca case quickly scanning the function whether there's
any would of course be faster than throwing VN at it.

Oh, and no - we don't want to perform CSE at -O0 (I mean
affecting generated code).

Richard.

> Martin
>
> >
> > Aldy
> >
>