Skip to content

C++: Add more alias and side effect models#17034

Merged
MathiasVP merged 8 commits into
github:mainfrom
MathiasVP:more-alias-and-side-effect-models
Jul 25, 2024
Merged

C++: Add more alias and side effect models#17034
MathiasVP merged 8 commits into
github:mainfrom
MathiasVP:more-alias-and-side-effect-models

Conversation

@MathiasVP

Copy link
Copy Markdown
Contributor

I added these as part of investigating a performance problem. By itself, this doesn't actually solve the performance issues. However, it does reduce the number of CallSideEffect instructions on certain databases by quite a lot.

@MathiasVP MathiasVP requested a review from a team as a code owner July 22, 2024 15:27
@MathiasVP MathiasVP added the no-change-note-required This PR does not need a change note label Jul 22, 2024
@github-actions github-actions Bot added the C++ label Jul 22, 2024
Comment thread cpp/ql/lib/semmle/code/cpp/models/implementations/Iterator.qll Fixed
Comment thread cpp/ql/lib/semmle/code/cpp/models/implementations/Iterator.qll Outdated

@jketema jketema left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Most of this looks sensible, but I've not checked everything in detail. Some comments though.

Comment thread cpp/ql/lib/semmle/code/cpp/models/implementations/Iterator.qll
Comment thread cpp/ql/lib/semmle/code/cpp/models/implementations/Printf.qll
Comment thread cpp/ql/lib/semmle/code/cpp/models/implementations/StdException.qll Outdated
Comment thread cpp/ql/lib/semmle/code/cpp/models/implementations/StdFunction.qll Outdated
@jketema

jketema commented Jul 23, 2024

Copy link
Copy Markdown
Contributor

DCA shows a lot more IR inconsistencies. Is that expected?

@MathiasVP

Copy link
Copy Markdown
Contributor Author

DCA shows a lot more IR inconsistencies. Is that expected?

Nope, that's certainly not expected. I'll take a look 👀

@MathiasVP MathiasVP force-pushed the more-alias-and-side-effect-models branch from 0d84b23 to 43df4a9 Compare July 23, 2024 16:22
@MathiasVP

Copy link
Copy Markdown
Contributor Author

43df4a9 fixes the inconsistencies on Samate. I'll run another DCA to see if there are still any left 🤞

@MathiasVP

Copy link
Copy Markdown
Contributor Author

Latest DCA showed a nice reduction in new inconsistencies, but there were still some left. b7542ee fixes another case of inconsistencies caused by std::vector<bool>'s push_back not taking a reference parameter, but instead simply taking a bool.

Running yet another DCA now 🤞

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

Labels

C++ no-change-note-required This PR does not need a change note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants