From f32b4abd9f66d39508baa04570bf74b1531ca265 Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Mon, 18 Nov 2024 20:15:26 +0000
Subject: [PATCH] C++: Always convert the condition of a 'two' operand
 conditional expression to a boolean before performing the negation.

---
 .../raw/internal/InstructionTag.qll           |  2 +
 .../raw/internal/TranslatedExpr.qll           | 45 ++++++++++++++++++-
 2 files changed, 45 insertions(+), 2 deletions(-)

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
index 12af98cfda10c..737a6378affd4 100644
--- 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
@@ -41,6 +41,8 @@ newtype TInstructionTag =
   ValueConditionCompareTag() or
   ValueConditionConstantTag() or
   ValueConditionConditionalBranchTag() or
+  ValueConditionConditionalConstantTag() or
+  ValueConditionConditionalCompareTag() or
   ConditionValueTrueTempAddressTag() or
   ConditionValueTrueConstantTag() or
   ConditionValueTrueStoreTag() or
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
index fa5b0ef98cac6..d68eb97fb958c 100644
--- 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
@@ -2969,11 +2969,33 @@ class TranslatedBinaryConditionalExpr extends TranslatedConditionalExpr {
     tag = ValueConditionConditionalBranchTag() and
     opcode instanceof Opcode::ConditionalBranch and
     resultType = getVoidType()
+    or
+    not hasBooleanConversion(expr.getCondition()) and
+    (
+      tag = ValueConditionConditionalConstantTag() and
+      opcode instanceof Opcode::Constant and
+      resultType = getIntType()
+      or
+      tag = ValueConditionConditionalCompareTag() and
+      opcode instanceof Opcode::CompareNE and
+      resultType = getBoolType()
+    )
   }
 
   override Instruction getInstructionSuccessorInternal(InstructionTag tag, EdgeKind kind) {
     result = super.getInstructionSuccessorInternal(tag, kind)
     or
+    not hasBooleanConversion(expr.getCondition()) and
+    (
+      tag = ValueConditionConditionalConstantTag() and
+      kind instanceof GotoEdge and
+      result = this.getInstruction(ValueConditionConditionalCompareTag())
+      or
+      tag = ValueConditionConditionalCompareTag() and
+      kind instanceof GotoEdge and
+      result = this.getInstruction(ValueConditionConditionalBranchTag())
+    )
+    or
     tag = ValueConditionConditionalBranchTag() and
     (
       kind instanceof TrueEdge and
@@ -2989,7 +3011,19 @@ class TranslatedBinaryConditionalExpr extends TranslatedConditionalExpr {
     or
     tag = ValueConditionConditionalBranchTag() and
     operandTag instanceof ConditionOperandTag and
-    result = this.getCondition().getResult()
+    if hasBooleanConversion(expr.getCondition())
+    then result = this.getCondition().getResult()
+    else result = this.getInstruction(ValueConditionConditionalCompareTag())
+    or
+    not hasBooleanConversion(expr.getCondition()) and
+    tag = ValueConditionConditionalCompareTag() and
+    (
+      operandTag instanceof LeftOperandTag and
+      result = this.getCondition().getResult()
+      or
+      operandTag instanceof RightOperandTag and
+      result = this.getInstruction(ValueConditionConditionalConstantTag())
+    )
   }
 
   override Instruction getChildSuccessorInternal(TranslatedElement child, EdgeKind kind) {
@@ -2997,7 +3031,9 @@ class TranslatedBinaryConditionalExpr extends TranslatedConditionalExpr {
     or
     kind instanceof GotoEdge and
     child = this.getCondition() and
-    result = this.getInstruction(ValueConditionConditionalBranchTag())
+    if hasBooleanConversion(expr.getCondition())
+    then result = this.getInstruction(ValueConditionConditionalBranchTag())
+    else result = this.getInstruction(ValueConditionConditionalConstantTag())
   }
 
   private TranslatedExpr getCondition() {
@@ -3014,6 +3050,11 @@ class TranslatedBinaryConditionalExpr extends TranslatedConditionalExpr {
     // always converting the "then" operand to `bool`, which is almost always the wrong type.
     result = getTranslatedExpr(expr.getThen().getExplicitlyConverted())
   }
+
+  override string getInstructionConstantValue(InstructionTag tag) {
+    tag = ValueConditionConditionalConstantTag() and
+    result = "0"
+  }
 }
 
 /**