Skip to content

Conversation

MathiasVP
Copy link
Contributor

@MathiasVP MathiasVP commented Jul 30, 2025

In #18490 we fixed a number of discrepancies between IR generated from C code, and IR generated from C++ code. For example, the following code has a int-to-bool conversion in the AST on x at the conditional when compiled as C++, but not when compiled as C:

void test(int x) {
  if(x) { /* ... */ }
}

#18490 removed this discrepancy in the IR by synthesizing the equivalent int-to-bool conversion when generating IR. One of the things we did was adjust the IRType returned by various operations (see here)

However, when I was working on something else I noticed that this change introduced a type error in the IR on examples such as:

void test(int a, int b) {
  int x = a < b;
  /* ... */
}

Because a < b has been overwritten to return a boolean, the StoreInstruction corresponding to the initialization of x looks like:

r1(glval<int>) = VariableAddress[x]
r4(bool)       = CompareLT           : r2, r3
m(int)         = Store[x]            : &:1, r4

In particular, notice that we are storing a result of type bool into a memory address of type glval<int>. That's not allowed in the IR, and it gives some false negatives in the guards library.

This PR fixes that by adding the necessary bool-to-int conversions when such type errors would have occurred in the IR.

Commit-by-commit review highly encouraged.

DCA shows a couple new results for cpp/missing-check-scanf. They look like FPs caused by insufficient guards logic (which we now need now that those conditionals are properly handled in the IR). I will take a look at once this PR has been merged.

@github-actions github-actions bot added the C++ label Jul 30, 2025
@MathiasVP MathiasVP added the no-change-note-required This PR does not need a change note label Jul 30, 2025
@MathiasVP MathiasVP force-pushed the fix-type-error-in-ir branch from a0f296d to 6e209d0 Compare July 31, 2025 14:49
@MathiasVP MathiasVP force-pushed the fix-type-error-in-ir branch from 6e209d0 to 14345a8 Compare August 1, 2025 15:09
@MathiasVP MathiasVP marked this pull request as ready for review August 1, 2025 17:56
@MathiasVP MathiasVP requested a review from a team as a code owner August 1, 2025 17:56
@Copilot Copilot AI review requested due to automatic review settings August 1, 2025 17:56
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes missing bool to int conversions in C code that were causing type errors in the IR. When comparisons and other boolean operations in C code store their results to integer variables, the IR previously showed incorrect type mismatches where boolean values were being stored to integer memory locations.

  • Adds TranslatedSyntheticBoolToIntConversion class to handle bool-to-int conversions
  • Introduces hasTranslatedSyntheticBoolToIntConversion predicate to identify expressions needing conversion
  • Updates instruction tags and test expectations to reflect the new conversion instructions

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedExpr.qll Adds TranslatedSyntheticBoolToIntConversion class and updates core expression handling
cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedElement.qll Adds predicate logic to identify expressions needing bool-to-int conversion
cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/InstructionTag.qll Adds BoolToIntConversionTag for the new conversion instruction
cpp/ql/test/library-tests/ir/ir/ir.c Adds test case for double negation assignment
cpp/ql/test/library-tests/ir/ir/raw_ir.expected Updates expected IR output to include new conversion instructions
cpp/ql/test/library-tests/ir/ir/aliased_ir.expected Updates expected aliased IR output with conversion instructions
cpp/ql/test/library-tests/ir/ir/PrintAST.expected Updates expected AST output for new test case
Comments suppressed due to low confidence (1)

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedElement.qll:530

  • The predicate name parentExpectsBool is misleading. It should be named something like parentExpectsBoolValue or usedInBooleanContext since it checks if the child expression is used in a context where a boolean value is expected, not just any parent.
private predicate parentExpectsBool(Expr child) {
  any(NotExpr notExpr).getOperand() = child
  or
  usedAsCondition(child)
}

@MathiasVP MathiasVP marked this pull request as draft August 1, 2025 17:58
@MathiasVP MathiasVP marked this pull request as ready for review August 1, 2025 18:06
@MathiasVP MathiasVP merged commit 56aacb1 into github:main Aug 11, 2025
17 checks passed
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.

2 participants