Skip to content

Conversation

@geoffw0
Copy link
Contributor

@geoffw0 geoffw0 commented Sep 14, 2020

New tests for CWE-190:

  • the first commit (test6.cpp) is one I had lying around from a previous investigation that never got put in a PR. We currently pass it.
  • the second commit (test2.cpp) diagnoses the latest SAMATE-cpp regression (at least one of the new failures; it's theoretically possible there could be multiple new issues, but this is unlikely as they look superficially similar).

@MathiasVP - I'm interested in whether you might have an idea what's going on here, and how widely it's likely to cause problems in real-world scenarios. The SAMATE regression itself is quite small.

@geoffw0 geoffw0 added the C++ label Sep 14, 2020
@geoffw0 geoffw0 requested a review from a team as a code owner September 14, 2020 17:28
@MathiasVP
Copy link
Contributor

@MathiasVP - I'm interested in whether you might have an idea what's going on here, and how widely it's likely to cause problems in real-world scenarios. The SAMATE regression itself is quite small.

Interestingly, the result comes back if we add another member to the single-field struct MyStruct. So I suspect this has something to do with the missing implementation of getPreUpdateNode for store steps without a ChiInstruction. I don't know why it's only a problem for pass-by-value, though.

@@ -0,0 +1,28 @@

typedef signed long long int s64;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I forgot to simplify this. The type doesn't matter, I get the same results with int.

Copy link
Contributor

@MathiasVP MathiasVP left a comment

Choose a reason for hiding this comment

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

LGTM! I've found the underlying issue for the missing result and I'll have a fix for it very soon.

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