Skip to content

Conversation

MathiasVP
Copy link
Contributor

This PR does two things with the end goal of getting flow through this example:

void test() {
  unsigned long x = source();
  double d = __builtin_bit_cast(double, x);
  sink(d);
}
  • First, we fix IR generation for builtins. We were not skipping children we could not translate, which caused the IR generation to fail since the first argument of __builtin_bit_cast is a TypeName which has no IR representation. This meant that __builtin_bit_cast had no operands in the IR (see the first commit which adds a test for this).

    We could have added IR generation for TypeNames, but that would involve introducing another IR instruction which I didn't really feel like doing since we currently have no use-cases for doing anything with TypeNames in any queries.

  • Second, we add flow through __builtin_bit_cast by treating it as a conversion in dataflow.

Commit-by-commit review recommended

@MathiasVP MathiasVP requested a review from a team as a code owner June 6, 2024 09:05
@github-actions github-actions bot added the C++ label Jun 6, 2024
@MathiasVP MathiasVP added the no-change-note-required This PR does not need a change note label Jun 6, 2024
Copy link
Contributor

@jketema jketema left a comment

Choose a reason for hiding this comment

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

LGTM, but could we do a DCA run with uninterpreted queries enabled to see the impact this has on existing inconsistencies?

@MathiasVP
Copy link
Contributor Author

MathiasVP commented Jun 6, 2024

LGTM, but could we do a DCA run with uninterpreted queries enabled to see the impact this has on existing inconsistencies?

I think I did? Although, it looked like these changes had 0 impact on the existing inconsistencies 😢

@jketema
Copy link
Contributor

jketema commented Jun 6, 2024

Do'h. I'm expecting to always see changes in that table, but that's with non-cached databases. Apologies.

@MathiasVP MathiasVP merged commit 314eb5d into github:main Jun 6, 2024
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