-Wanalyzer-use-of-uninitialized-value always shadows -Wanalyzer-out-of-bounds

David Malcolm dmalcolm@redhat.com
Thu Apr 6 21:59:50 GMT 2023


On Thu, 2023-04-06 at 13:02 +0200, Benjamin Priour wrote:
> Hi David,
> I haven't yet looked into your suggestions, probably won't have time
> until
> tomorrow actually :/
> Still, here are some updates
> 
> On Thu, Apr 6, 2023 at 2:32 AM David Malcolm <dmalcolm@redhat.com>
> wrote:
> 
> > On Wed, 2023-04-05 at 19:50 +0200, Benjamin Priour wrote:
> > > Hi David,
> 

[...snip...]

> > There's quite a bit of duplication here.  My recollection is that
> > there's code in the analyzer that's meant to be eliminating some of
> > this e.g. we want to show the OOB when consecutive_oob_in_frame is
> > called directly; we *don't* want to show it when
> > consecutive_oob_in_frame is called by goo.  Perhaps this
> > deduplication
> > code isn't working?  Can you reproduce similar behavior with C, or
> > is
> > it specific to C++?
> > 
> > 
> Identical behavior both in C and C++. I will look at this code, any
> hint at
> where it starts ?
> Otherwise I would find it the good old way.

It's in diagnostic-manager.cc:
diagnostic_manager::emit_saved_diagnostics uses class dedupe_winners to
try to deduplicate the various saved_diagnostic instances, using
dedupe_key::operator== (and also its hash function).  One of the things
dedupe_key::operator== uses is saved_diagnostic::operator==, which does
the bulk of the work to decide "are these two really the 'same'
diagnotic?"


> > > 
> > > First, as the subject line reads, I get a
> > > -Wanalyzer-use-of-uninitialized-value for each -Wanalyzer-out-of-
> > > bounds. I
> > > feel it might be too much, as fixing the OOB would fix the
> > > former.
> > > So maybe only OOB could result in a warning here ?
> > 
> > Yes, that's a good point.  Can you file a bug about this in
> > bugzilla
> > please?  (and feel free to assign it to yourself if you want to
> > have a
> > go at fixing it)
> > 
> 
> Unfortunately the Assignee field is grayed out for me in both
> enter_bug.cgi
> and show_bug.cgi.
> I've also created a new tracker bug for out-of-bounds, as there is a
> number
> of related bugs.

I think you get more access rights in our bugzilla if you log in with a
gcc.gnu.org email address.  You can request one here:
  https://sourceware.org/cgi-bin/pdw/ps_form.cgi
and cite me as approving the request.


> > 
> > Maybe we could fix this by having region_model::check_region_bounds
> > return a bool that signifies if the access is valid, and
> > propagating
> > that value up through callers so that we can return a non-
> > poisoned_svalue at the point where we'd normally return an
> > "uninitialized" poisoned_svalue.
> > 
> > Alternatively, we could simply terminate any analysis path in which
> > an
> > OOB access is detected (by implementing the
> > pending_diagnostic::terminate_path_p virtual function for class
> > out_of_bounds).
> > 
> 
> I'm adding your suggestions as comment to the filed bugs so as to not
> forget them.

Thanks.


> > > Second, it seems that if a frame was a cause for a OOB (either by
> > > containing the spurious code or by being a caller to such code),
> > > it
> > > will
> > > only emit one set of warning, rather than at each unique
> > > compromising
> > > statements.
> > 
> > Maybe.  There's a pending_diagnostic::supercedes_p virtual function
> > that perhaps we could implement for out_of_bounds (or its
> > subclasses).
> > 
> > 
> 
> > > 
> > > Finally, I think the diagnostic path should only go at deep as
> > > the
> > > declaration of the injurious index.
> > 
> > I'm not quite sure what you mean by this, sorry.
> > 
> > 
> Indeed not the best explanation so far. I was actually sort of
> suggesting
> to only emit OOB only on direct call sites,
> you did too, so in a way you have answered me on this.
> 
> Just an addition though: if there is an OOB independent of its
> enclosing
> function's parameters, I think
> it might make sense to not emit for this particular OOB outside the
> function definition itself.
> Meaning that no OOB should be emitted on call sites to this function
> for
> this particular array access.
> (Typically, consecutive_oob_in_frame () shouldn't have resulted in
> more
> than one warning, since the OOB within is independent of its
> parameters).

Yes; I think we're in agreement here.

> 
> I believe right now the expected behavior is to issue warnings only
> on
> actual function calls, so that a function never called
> won't result in warnings. 

IIRC that was the case in GCC 10, but I changed it in
8a2750086d57d1a2251d9239fa4e6c2dc9ec3a86.

> As a result, the initial analysis of each
> functions should never results in warnings -actually the case for
> malloc-leak,
> not for OOB though-.
> Thus we would need to tweak this into actually diagnosing the issues
> on
> initial analysis -those that can be at least-, so that they are saved
> for a
> later
> use whenever the function is actually called. Then we would emit them
> once,
> and only once, because by nature these diagnostics are parameters
> independent.
> I hope I made it clearer, not more convoluted.
> 
> > 
> > > Also, have you considered extending the current call summaries
> > > model
> > > (symbolic execution from what you told me), to implement a
> > > partial
> > > VASCO
> > > model ? There would still be no need for distributive problems.
> > > Maybe a start could be that functions without parameters working
> > > on
> > > non-mutable global data should not generate EN more than once,
> > > rather
> > > than
> > > at each call sites.
> > 
> > Please forgive my ignorance; do you have a link to a paper
> > describing
> > this?
> > 
> > 
> > Thanks
> > Dave
> > 
> > 
> See https://dl.acm.org/doi/pdf/10.1145/2487568.2487569 and I would
> also
> recommend watching
> https://www.youtube.com/watch?v=_RI7sTxX2_o&list=PLamk8lFsMyPXrUIQm5naAQ08aK2ctv6gE&index=29
> along or before the read,
> it genuinely made it way more digest for me :-)

Thanks
Dave



More information about the Gcc mailing list