On Mon, 2023-05-01 at 14:43 +0200, Benjamin Priour wrote: > Hi David, Hi Benjamin
> > Sorry for not being active these past two weeks I've been overwhelmed > at my > university with creating a new club and other uni stuff. Fair enough. > > I just went back to solving these 3 bugs we discussed last time. > > <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109439> > PR109439 <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109439> is the > one > about the double warnings emitted (both OOB and use of uninitialized > value). > Your second suggestion of terminating the diagnostic path on an OOB > proved > to interfere with PR109437. > I might actually close PR109437 as solving PR109439 will probably > solve it > too. Aha - so is the "uninit" warning terminating the path? That could explain it. > I'm going with your first suggestion of bubbling a boolean from > check_region_bounds up to get_rvalue. > I'm considering two approaches > 1. preventing the check_for_poison call if there is an OOB Read. > 2. or marking the OOB values as Unknown rather than Poisoned, but > then we > are semantically incorrect. What do you mean by "semantically incorrect" here? If we have e.g.: int would_like_only_oob () { int arr[] = {1,2,3,4,5,6,7}; int y1 = arr[9]; // 2 warnings instead of only OOB warning are emitted here return y1; } then we emit the OOB warning at the get_rvalue that accesses arr[9], but we do need some svalue instance to return back from get_value, so that we have something to put into "y1" in the store (or else y1 will be falsely considered as uninit at the "return y1;". The unknown_svalue for 'int' seems to me to be the most appropriate svalue to use here, but maybe there's some issue I haven't thought of with that. > > Another unrelated question, I felt like the use of an uninitialized > value > terminating the path was a bit strong. No other warnings will be > considered > for the remaining of the function if there is such use, even for > unrelated > stuff. Like a double free on a completely different variable. > Couldn't we tune that so we only ignore everything related to any > variable > tainted by this uninitialized value ? Maybe; FWIW, I implemented this termination logic in 8f6369157917a4371b5d66dfe82b84aded3b8268, which has some notes on my reasoning. > > Sorry again for the past weeks, the club is finally running > (somewhat). > Benjamin. > > PS: I submitted a patch, bootstrapped and regtested, Great! When you say "submitted", did you post it to a mailing list? > for the bug I was > solving on gcc-request. Sorry, I'm not sure what you mean by "gcc-request" here. > I guess I'm not too clear on the process of > submitting a patch, as I probably had to commit and push it > afterwards, > sadly there was no feedback on the previous RFC as well as on the > patch > submission - no blaming at all, people are busy and the flow of mails > is > massive. I'm swamped in email, and often miss things; sorry. Please make sure that stuff for the analyzer has my email address (as well as the mailing list), in the recipient list, as it's more likely to be detected by my filters. If you haven't had a reply on a patch after a week, send a followup with "PING" in the title. Are you waiting on patch reviews from me? If so, please resend (sorry). > I believe I still don't have the right to commit it directly to the > repo, > and honestly even if I would using my fresh gcc account, I would > prefer not > to commit it myself for the first patch, I don't wanna mess with > anything > because of an oversight. I can talk you through the various steps as you familiarize yourself with the process; thanks for your caution; sorry again if you're waiting on me. Dave
