* Re: PR109439 - Terminate analysis path when OOB detected
2023-05-01 12:43 PR109439 - Terminate analysis path when OOB detected Benjamin Priour
@ 2023-05-01 13:33 ` Mark Wielaard
2023-05-02 19:14 ` David Malcolm
1 sibling, 0 replies; 3+ messages in thread
From: Mark Wielaard @ 2023-05-01 13:33 UTC (permalink / raw)
To: Benjamin Priour; +Cc: David Malcolm, gcc
Hi Benjamin,
On Mon, May 01, 2023 at 02:43:13PM +0200, Benjamin Priour via Gcc wrote:
> 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.
An account has been setup for you, you can try it out by doing:
ssh vultkayn@gcc.gnu.org alive
Some other helpful info can be found at:
https://sourceware.org/sourceware/accountinfo.html
If anything seems wrong, please contact admin-requests@sourceware.org
And please coordinate actually pushing patches with David and the
other gcc developers on gcc-patches@gcc.gnu.org
Cheers,
Mark
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: PR109439 - Terminate analysis path when OOB detected
2023-05-01 12:43 PR109439 - Terminate analysis path when OOB detected Benjamin Priour
2023-05-01 13:33 ` Mark Wielaard
@ 2023-05-02 19:14 ` David Malcolm
1 sibling, 0 replies; 3+ messages in thread
From: David Malcolm @ 2023-05-02 19:14 UTC (permalink / raw)
To: Benjamin Priour, gcc
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
^ permalink raw reply [flat|nested] 3+ messages in thread