Skip to content

Conversation

rdmarsh2
Copy link
Contributor

This PR adds value numbering for the std::vector::size function, based on the value of the this pointer and object. This fixes some false positives in an experimental range analysis query.

Opened as draft for discussion - I intend to generalize this with either a model or some heuristics before merging.

@github-actions github-actions bot added the C++ label Jul 12, 2022
int a = v.size();
int b = v.size();
v.set(10);
int c = v.size();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not exactly sure how it works, but I'm impressed we match the result of size() on line 178 to line 179, but not 181.

How easy would it be to fool it, by doing something subtler that changes the number of elements in v?

Copy link
Contributor Author

@rdmarsh2 rdmarsh2 Jul 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This currently relies on the fact that size is declared as a const member function, so we don't generate a call side-effect that writes to v. We could potentially do a deeper analysis to discover that functions don't mutate their arguments, but I'm not sure it's necessary. Right now we'd be sound if something subtler changes the result, but if size weren't declared as const we'd have those side-effects and would value-number the call on 179 differently.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like the right call to me.

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