Skip to content

Conversation

alexet
Copy link
Contributor

@alexet alexet commented Sep 28, 2023

By adding at the instruction level we lose a bunch of the GVN complexity but it only applies to one query.

@github-actions github-actions bot added the C++ label Sep 28, 2023
@MathiasVP
Copy link
Contributor

The changes LGTM! Based on the discussions we've had with Schack I think this seem like a sensible approach to getting rid of these FPs. Is there a reason why this is still in a draft state?

@MathiasVP
Copy link
Contributor

I've started a DCA run to see the impact of this change.

@MathiasVP
Copy link
Contributor

Ouch. Something looks really bad on a couple of projects @alexet 😱

@alexet alexet force-pushed the reverse-range-analysis-bounds branch from c2bcc63 to 9c0bcc2 Compare October 10, 2023 12:40
@alexet
Copy link
Contributor Author

alexet commented Oct 12, 2023

So I have fixed the performance issues by avoiding constants. However this showns more issues: We rely on the bounds only going one direction as a psudo flow to make the query make sense. Essentially the query has the following flow:

  • Allocation -> Pointer addition
  • Alocation length -> Other length
  • Check pointer addition is out of bounds
  • Find an instruction >= pointer addition
  • Find flow from that instruction to a deref.

The weird new results come from the following:

buffer = alloc(len) // A

buffer[len-1] = 0 // E

buffer[len-1] = 0; // B

if (cond)
  len--; // C

use(len) // D

Here we get that len at D (which I will write len(D) is the length of the buffer by existential flow.

We also get that len(C) >= len(B) - 1 (we work it out based on len(A) but gvn also gives us bounds with len(B)). From that we get that len(B) <= len(C) + 1. But as len(C) is the length of the buffer this is out of bounds. Finally we work out that buffer[len-1](E) >= buffer[len-1](B) we end up reporting at E that it is out of bounds but we point to the use of len-1 at B byt it is based on the buffer length at D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants