Skip to content

Conversation

hvitved
Copy link
Contributor

@hvitved hvitved commented Apr 22, 2020

Stores are currently restricted to target PostUpdateNodes (or any other node in localFlowEntry(), such as OutNodes). However, for C# we are about to model collection-flow using field-flow, so for example

new string[] { "taint" };

will be considered a store from "taint" into the created array, which is not a PostUpdateNode.

(This PR will conflict with #3110, which renames storeDirect to store.) Merge conflict resolved by rebasing onto latest master.

@hvitved hvitved force-pushed the dataflow/impl-no-postupdate branch from 31b1044 to e95cc24 Compare May 5, 2020 12:18
@hvitved hvitved requested review from a team as code owners May 5, 2020 12:18
@dbartol
Copy link

dbartol commented May 5, 2020

@MathiasVP @jbj Does this help with our current challenges around partial stores in C++ field flow?

@MathiasVP
Copy link
Contributor

MathiasVP commented May 5, 2020

@MathiasVP @jbj Does this help with our current challenges around partial stores in C++ field flow?

I haven't looked that much at the details yet, but I guess it would mean that we don't need the target of a store to have a getPreUpdateNode in cases where it doesn't make sense. We might still want it for summarizing setters though (I haven't really kept up on the changes in #3110).

@aschackmull
Copy link
Contributor

For those following along, I think this PR is fairly safe - it merely adds a few starting points for the big-step relation used in the dataflow library, which means that it'll now also start at store-targets that aren't PostUpdateNodes. The rest is just refactorings. @hvitved correct me if I'm wrong here.

@hvitved
Copy link
Contributor Author

hvitved commented May 6, 2020

For those following along, I think this PR is fairly safe - it merely adds a few starting points for the big-step relation used in the dataflow library, which means that it'll now also start at store-targets that aren't PostUpdateNodes. The rest is just refactorings. @hvitved correct me if I'm wrong here.

Yes, that was certainly the intention. Although, it does also remove starting points, namely those PostUpdateNodes that do not correspond to an ArgumentNode.

@jbj jbj merged commit 63f04af into github:master May 6, 2020
@hvitved hvitved deleted the dataflow/impl-no-postupdate branch May 6, 2020 08:17
max-schaefer pushed a commit to max-schaefer/codeql-go that referenced this pull request May 6, 2020
max-schaefer pushed a commit to max-schaefer/codeql-go that referenced this pull request May 6, 2020
owen-mc pushed a commit to owen-mc/codeql-go that referenced this pull request Jun 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants