On Wed, 2023-04-05 at 19:50 +0200, Benjamin Priour wrote: > Hi David, > > I used the below code snippet to experiment with out-of-bounds (OOB) > on > trunk. Three things occurred that I believe could see some > improvement. See > https://godbolt.org/z/57n459cEs for the warnings. > > int consecutive_oob_in_frame () > { > int arr[] = {1,2,3,4,5,6,7}; > int y1 = arr[9]; // only this one get warnings (3*2 actually), > expect > only 1 OOB though > int y2 = arr[10]; // expect a warning too, despite fooling with > asm > int y3 = arr[50]; // expect a warning for that one too (see asm) > return (y1+y2+y3); > } > > int goo () { > int x = consecutive_oob_in_frame (); // causes another pair of > warnings > return 2 * x; > } > > int main () { > goo (); // causes another pair of warning > consecutive_oob_in_frame (); // silent > int x [] = {1,2}; > x[5]; /* silent, probably because another set of OOB warnings > has already been issued with this frame being the source */ > return 0; > }
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++? > > 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) 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). > > 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. > > What do you think ? If they also make sense to you I will open a RFE > and > try my hands at fixing them. Please do. > > 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
