Merge pull request #20145 from MathiasVP/fix-type-error-in-ir

MathiasVP · web-flow · commit 56aacb1e559c · 2025-08-11T10:26:54.000+02:00
C++: Fix missing `bool` -&gt; `int` conversions in C code
diff --git a/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/InstructionTag.qll b/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/InstructionTag.qll
@@ -96,7 +96,8 @@ newtype TInstructionTag =
     exists(Expr e | exists(e.getImplicitDestructorCall(index))) or
     exists(Stmt s | exists(s.getImplicitDestructorCall(index)))
   } or
-  CoAwaitBranchTag()
+  CoAwaitBranchTag() or
+  BoolToIntConversionTag()
 
 class InstructionTag extends TInstructionTag {
   final string toString() { result = getInstructionTagId(this) }
@@ -286,4 +287,6 @@ string getInstructionTagId(TInstructionTag tag) {
   )
   or
   tag = CoAwaitBranchTag() and result = "CoAwaitBranch"
+  or
+  tag = BoolToIntConversionTag() and result = "BoolToIntConversion"
 }
diff --git a/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedElement.qll b/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedElement.qll
@@ -509,6 +509,41 @@ predicate hasTranslatedSyntheticTemporaryObject(Expr expr) {
   not expr.hasLValueToRValueConversion()
 }
 
+Opcode comparisonOpcode(ComparisonOperation expr) {
+  expr instanceof EQExpr and result instanceof Opcode::CompareEQ
+  or
+  expr instanceof NEExpr and result instanceof Opcode::CompareNE
+  or
+  expr instanceof LTExpr and result instanceof Opcode::CompareLT
+  or
+  expr instanceof GTExpr and result instanceof Opcode::CompareGT
+  or
+  expr instanceof LEExpr and result instanceof Opcode::CompareLE
+  or
+  expr instanceof GEExpr and result instanceof Opcode::CompareGE
+}
+
+private predicate parentExpectsBool(Expr child) {
+  any(NotExpr notExpr).getOperand() = child
+  or
+  usedAsCondition(child)
+}
+
+/**
+ * Holds if `expr` should have a `TranslatedSyntheticBoolToIntConversion` on it.
+ */
+predicate hasTranslatedSyntheticBoolToIntConversion(Expr expr) {
+  not ignoreExpr(expr) and
+  not isIRConstant(expr) and
+  not parentExpectsBool(expr) and
+  expr.getUnspecifiedType() instanceof IntType and
+  (
+    expr instanceof NotExpr
+    or
+    exists(comparisonOpcode(expr))
+  )
+}
+
 class StaticInitializedStaticLocalVariable extends StaticLocalVariable {
   StaticInitializedStaticLocalVariable() {
     this.hasInitializer() and
@@ -647,6 +682,9 @@ newtype TTranslatedElement =
   // A temporary object that we had to synthesize ourselves, so that we could do a field access or
   // method call on a prvalue.
   TTranslatedSyntheticTemporaryObject(Expr expr) { hasTranslatedSyntheticTemporaryObject(expr) } or
+  TTranslatedSyntheticBoolToIntConversion(Expr expr) {
+    hasTranslatedSyntheticBoolToIntConversion(expr)
+  } or
   // For expressions that would not otherwise generate an instruction.
   TTranslatedResultCopy(Expr expr) {
     not ignoreExpr(expr) and
diff --git a/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedExpr.qll b/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedExpr.qll
@@ -216,7 +216,8 @@ abstract class TranslatedCoreExpr extends TranslatedExpr {
     not hasTranslatedLoad(expr) and
     not hasTranslatedSyntheticTemporaryObject(expr) and
     // If there's a result copy, then this expression's result is the copy.
-    not exprNeedsCopyIfNotLoaded(expr)
+    not exprNeedsCopyIfNotLoaded(expr) and
+    not hasTranslatedSyntheticBoolToIntConversion(expr)
   }
 }
 
@@ -358,11 +359,12 @@ class TranslatedConditionValue extends TranslatedCoreExpr, ConditionContext,
 }
 
 /**
- * The IR translation of a node synthesized to adjust the value category of its operand.
+ * The IR translation of a node synthesized to adjust the value category or type of its operand.
  * One of:
  * - `TranslatedLoad` - Convert from glvalue to prvalue by loading from the location.
  * - `TranslatedSyntheticTemporaryObject` - Convert from prvalue to glvalue by storing to a
  *   temporary variable.
+ * - `TranslatedSyntheticBoolToIntConversion` - Convert a prvalue Boolean to a prvalue integer.
  */
 abstract class TranslatedValueCategoryAdjustment extends TranslatedExpr {
   final override Instruction getFirstInstruction(EdgeKind kind) {
@@ -513,6 +515,45 @@ class TranslatedSyntheticTemporaryObject extends TranslatedValueCategoryAdjustme
   }
 }
 
+class TranslatedSyntheticBoolToIntConversion extends TranslatedValueCategoryAdjustment,
+  TTranslatedSyntheticBoolToIntConversion
+{
+  TranslatedSyntheticBoolToIntConversion() { this = TTranslatedSyntheticBoolToIntConversion(expr) }
+
+  override string toString() { result = "Bool-to-int conversion of " + expr.toString() }
+
+  override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType resultType) {
+    opcode instanceof Opcode::Convert and
+    tag = BoolToIntConversionTag() and
+    resultType = getIntType()
+  }
+
+  override predicate isResultGLValue() { none() }
+
+  override Instruction getInstructionSuccessorInternal(InstructionTag tag, EdgeKind kind) {
+    tag = BoolToIntConversionTag() and
+    result = this.getParent().getChildSuccessor(this, kind)
+  }
+
+  override Instruction getALastInstructionInternal() {
+    result = this.getInstruction(BoolToIntConversionTag())
+  }
+
+  override Instruction getChildSuccessorInternal(TranslatedElement child, EdgeKind kind) {
+    child = this.getOperand() and
+    result = this.getInstruction(BoolToIntConversionTag()) and
+    kind instanceof GotoEdge
+  }
+
+  override Instruction getResult() { result = this.getInstruction(BoolToIntConversionTag()) }
+
+  override Instruction getInstructionRegisterOperand(InstructionTag tag, OperandTag operandTag) {
+    tag = BoolToIntConversionTag() and
+    operandTag instanceof UnaryOperandTag and
+    result = this.getOperand().getResult()
+  }
+}
+
 /**
  * IR translation of an expression that simply returns its result. We generate an otherwise useless
  * `CopyValue` instruction for these expressions so that there is at least one instruction
@@ -1794,20 +1835,6 @@ private Opcode binaryArithmeticOpcode(BinaryArithmeticOperation expr) {
   expr instanceof PointerDiffExpr and result instanceof Opcode::PointerDiff
 }
 
-private Opcode comparisonOpcode(ComparisonOperation expr) {
-  expr instanceof EQExpr and result instanceof Opcode::CompareEQ
-  or
-  expr instanceof NEExpr and result instanceof Opcode::CompareNE
-  or
-  expr instanceof LTExpr and result instanceof Opcode::CompareLT
-  or
-  expr instanceof GTExpr and result instanceof Opcode::CompareGT
-  or
-  expr instanceof LEExpr and result instanceof Opcode::CompareLE
-  or
-  expr instanceof GEExpr and result instanceof Opcode::CompareGE
-}
-
 private Opcode spaceShipOpcode(SpaceshipExpr expr) {
   exists(expr) and
   result instanceof Opcode::Spaceship
diff --git a/cpp/ql/test/library-tests/ir/ir/PrintAST.expected b/cpp/ql/test/library-tests/ir/ir/PrintAST.expected
@@ -4932,7 +4932,20 @@ ir.c:
 #  103|             Type = [IntType] int
 #  103|             ValueCategory = prvalue(load)
 #  103|       getThen(): [BlockStmt] { ... }
-#  104|     getStmt(16): [ReturnStmt] return ...
+#  105|     getStmt(16): [DeclStmt] declaration
+#  105|       getDeclarationEntry(0): [VariableDeclarationEntry] definition of double_negation
+#  105|           Type = [IntType] int
+#  105|         getVariable().getInitializer(): [Initializer] initializer for double_negation
+#  105|           getExpr(): [NotExpr] ! ...
+#  105|               Type = [IntType] int
+#  105|               ValueCategory = prvalue
+#  105|             getOperand(): [NotExpr] ! ...
+#  105|                 Type = [IntType] int
+#  105|                 ValueCategory = prvalue
+#  105|               getOperand(): [VariableAccess] x1
+#  105|                   Type = [IntType] int
+#  105|                   ValueCategory = prvalue(load)
+#  106|     getStmt(17): [ReturnStmt] return ...
 ir.cpp:
 #    1| [TopLevelFunction] void Constants()
 #    1|   <params>: 
diff --git a/cpp/ql/test/library-tests/ir/ir/aliased_ir.expected b/cpp/ql/test/library-tests/ir/ir/aliased_ir.expected
@@ -3706,9 +3706,10 @@ ir.c:
 #   88|     r88_3(int)        = Load[x1]            : &:r88_2, m84_6
 #   88|     r88_4(int)        = Constant[0]         : 
 #   88|     r88_5(bool)       = CompareEQ           : r88_3, r88_4
-#   88|     m88_6(int)        = Store[y]            : &:r88_1, r88_5
+#   88|     r88_6(int)        = Convert             : r88_5
+#   88|     m88_7(int)        = Store[y]            : &:r88_1, r88_6
 #   89|     r89_1(glval<int>) = VariableAddress[y]  : 
-#   89|     r89_2(int)        = Load[y]             : &:r89_1, m88_6
+#   89|     r89_2(int)        = Load[y]             : &:r89_1, m88_7
 #   89|     r89_3(int)        = Constant[0]         : 
 #   89|     r89_4(bool)       = CompareNE           : r89_2, r89_3
 #   89|     v89_5(void)       = ConditionalBranch   : r89_4
@@ -3721,7 +3722,7 @@ ir.c:
 
 #   90|   Block 6
 #   90|     r90_1(glval<int>) = VariableAddress[y] : 
-#   90|     r90_2(int)        = Load[y]            : &:r90_1, m88_6
+#   90|     r90_2(int)        = Load[y]            : &:r90_1, m88_7
 #   90|     r90_3(int)        = Constant[0]        : 
 #   90|     r90_4(bool)       = CompareEQ          : r90_2, r90_3
 #   90|     v90_5(void)       = ConditionalBranch  : r90_4
@@ -3969,11 +3970,19 @@ ir.c:
 #  103|     v103_6(void) = NoOp : 
 #-----|   Goto -> Block 40
 
-#  104|   Block 40
-#  104|     v104_1(void) = NoOp         : 
-#   84|     v84_9(void)  = ReturnVoid   : 
-#   84|     v84_10(void) = AliasedUse   : m84_3
-#   84|     v84_11(void) = ExitFunction : 
+#  105|   Block 40
+#  105|     r105_1(glval<int>) = VariableAddress[double_negation] : 
+#  105|     r105_2(glval<int>) = VariableAddress[x1]              : 
+#  105|     r105_3(int)        = Load[x1]                         : &:r105_2, m84_6
+#  105|     r105_4(int)        = Constant[0]                      : 
+#  105|     r105_5(bool)       = CompareEQ                        : r105_3, r105_4
+#  105|     r105_6(bool)       = LogicalNot                       : r105_5
+#  105|     r105_7(int)        = Convert                          : r105_6
+#  105|     m105_8(int)        = Store[double_negation]           : &:r105_1, r105_7
+#  106|     v106_1(void)       = NoOp                             : 
+#   84|     v84_9(void)        = ReturnVoid                       : 
+#   84|     v84_10(void)       = AliasedUse                       : m84_3
+#   84|     v84_11(void)       = ExitFunction                     : 
 
 ir.cpp:
 #    1| void Constants()
diff --git a/cpp/ql/test/library-tests/ir/ir/ir.c b/cpp/ql/test/library-tests/ir/ir/ir.c
@@ -101,6 +101,8 @@ void branch_on_integral_in_c(int x1, int x2) {
   int x_1_and_2 = x1 && x2;
   if(x_1_and_2) {}
   if(!x_1_and_2) {}
+
+  int double_negation = !!x1;
 }
 
 // semmle-extractor-options: --microsoft
diff --git a/cpp/ql/test/library-tests/ir/ir/raw_ir.expected b/cpp/ql/test/library-tests/ir/ir/raw_ir.expected
@@ -3343,7 +3343,8 @@ ir.c:
 #   88|     r88_3(int)        = Load[x1]            : &:r88_2, ~m?
 #   88|     r88_4(int)        = Constant[0]         : 
 #   88|     r88_5(bool)       = CompareEQ           : r88_3, r88_4
-#   88|     mu88_6(int)       = Store[y]            : &:r88_1, r88_5
+#   88|     r88_6(int)        = Convert             : r88_5
+#   88|     mu88_7(int)       = Store[y]            : &:r88_1, r88_6
 #   89|     r89_1(glval<int>) = VariableAddress[y]  : 
 #   89|     r89_2(int)        = Load[y]             : &:r89_1, ~m?
 #   89|     r89_3(int)        = Constant[0]         : 
@@ -3605,11 +3606,19 @@ ir.c:
 #  103|     v103_6(void) = NoOp : 
 #-----|   Goto -> Block 40
 
-#  104|   Block 40
-#  104|     v104_1(void) = NoOp         : 
-#   84|     v84_8(void)  = ReturnVoid   : 
-#   84|     v84_9(void)  = AliasedUse   : ~m?
-#   84|     v84_10(void) = ExitFunction : 
+#  105|   Block 40
+#  105|     r105_1(glval<int>) = VariableAddress[double_negation] : 
+#  105|     r105_2(glval<int>) = VariableAddress[x1]              : 
+#  105|     r105_3(int)        = Load[x1]                         : &:r105_2, ~m?
+#  105|     r105_4(int)        = Constant[0]                      : 
+#  105|     r105_5(bool)       = CompareEQ                        : r105_3, r105_4
+#  105|     r105_6(bool)       = LogicalNot                       : r105_5
+#  105|     r105_7(int)        = Convert                          : r105_6
+#  105|     mu105_8(int)       = Store[double_negation]           : &:r105_1, r105_7
+#  106|     v106_1(void)       = NoOp                             : 
+#   84|     v84_8(void)        = ReturnVoid                       : 
+#   84|     v84_9(void)        = AliasedUse                       : ~m?
+#   84|     v84_10(void)       = ExitFunction                     : 
 
 ir.cpp:
 #    1| void Constants()

Original file line number	Diff line number	Diff line change
`@@ -101,6 +101,8 @@ void branch_on_integral_in_c(int x1, int x2) {`
`101`	`101`	`int x_1_and_2 = x1 && x2;`
`102`	`102`	`if(x_1_and_2) {}`
`103`	`103`	`if(!x_1_and_2) {}`
	`104`	`+`
	`105`	`+ int double_negation = !!x1;`
`104`	`106`	`}`
`105`	`107`
`106`	`108`	`// semmle-extractor-options: --microsoft`