Skip to content

C++: move logic from DefaultTaintTracking into TaintTrackingUtil #3320

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 27, 2020

Conversation

rdmarsh2
Copy link
Contributor

No description provided.

@rdmarsh2 rdmarsh2 requested a review from a team as a code owner April 22, 2020 17:05
@rdmarsh2 rdmarsh2 added the C++ label Apr 22, 2020
@jbj
Copy link
Contributor

jbj commented Apr 23, 2020

The code LGTM, but how do you plan to ensure that performance is good on real snapshots? The new additions are essentially dead code until we can retarget DefaultTaintTracking to use TaintTracking or until we write queries directly on TaintTracking.

@rdmarsh2
Copy link
Contributor Author

I had planned to modify DefaultTaintTracking to use some of the TaintTrackingUtil predicates, but the semantics don't quite line up:

  • I didn't copy the pointer/object conflation that DefaultTaintTracking does for modeled functions. I think C++: Wire up param/arg indirections in data flow #3123 will make that unnecessary, but I'll add it for the moment.
  • the existing taint steps in TaintTrackingUtil don't use the predictability constraint that DefaultTaintTracking has. I think that's a change we want to make eventually, but it's a big/scary change, and I don't think I can have DefaultTaintTracking use TaintTracking::Configuration without making it.

I'll go ahead and do those, but make the latter change on a separate branch

Copy link
Contributor

@jbj jbj left a comment

Choose a reason for hiding this comment

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

On balance, I prefer merging this code now rather than later. It seems better to have dead code merge than to have it sitting in a PR.

@jbj jbj merged commit 20c956e into master Apr 27, 2020
@rdmarsh2 rdmarsh2 deleted the rdmarsh/cpp/taint-tracking-util-port branch February 15, 2022 18:19
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