Merge pull request github#12982 from rdmarsh2/rdmarsh2/ir-guards-unreached

C++: Handle nonreturning functions in IR generation

Author: jketema

cpp/ql/lib/change-notes/2023-05-02-ir-noreturn-calls.md

@@ -0,0 +1,4 @@
+---
+category: majorAnalysis
+---
+* In the intermediate representation, handling of control flow after non-returning calls has been improved. This should remove false positives in queries that use the intermedite representation or libraries based on it, including the new data flow library.

cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSAConstruction.qll

@@ -34,9 +34,13 @@ private module Cached {
 
   cached
   predicate hasUnreachedInstructionCached(IRFunction irFunc) {
-    exists(OldInstruction oldInstruction |
+    exists(OldIR::Instruction oldInstruction |
       irFunc = oldInstruction.getEnclosingIRFunction() and
-      Reachability::isInfeasibleInstructionSuccessor(oldInstruction, _)
+      (
+        Reachability::isInfeasibleInstructionSuccessor(oldInstruction, _)
+        or
+        oldInstruction.getOpcode() instanceof Opcode::Unreached
+      )
     )
   }
 
@@ -366,21 +370,19 @@ private module Cached {
     then
       result = getChi(getOldInstruction(instruction)) and
       kind instanceof GotoEdge
-    else (
+    else
       exists(OldInstruction oldInstruction |
-        oldInstruction = getOldInstruction(instruction) and
+        (
+          oldInstruction = getOldInstruction(instruction)
+          or
+          instruction = getChi(oldInstruction)
+        ) and
         (
           if Reachability::isInfeasibleInstructionSuccessor(oldInstruction, kind)
           then result = unreachedInstruction(instruction.getEnclosingIRFunction())
           else result = getNewInstruction(oldInstruction.getSuccessor(kind))
         )
       )
-      or
-      exists(OldInstruction oldInstruction |
-        instruction = getChi(oldInstruction) and
-        result = getNewInstruction(oldInstruction.getSuccessor(kind))
-      )
-    )
   }
 
   cached

cpp/ql/lib/semmle/code/cpp/ir/implementation/internal/TInstruction.qll

@@ -19,6 +19,9 @@ newtype TInstruction =
   ) {
     IRConstruction::Raw::hasInstruction(tag1, tag2)
   } or
+  TRawUnreachedInstruction(IRFunctionBase irFunc) {
+    IRConstruction::hasUnreachedInstruction(irFunc)
+  } or
   TUnaliasedSsaPhiInstruction(
     TRawInstruction blockStartInstr, UnaliasedSsa::Ssa::MemoryLocation memoryLocation
   ) {

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/IRConstruction.qll

@@ -178,9 +178,9 @@ module Raw {
   }
 }
 
-class TStageInstruction = TRawInstruction;
+class TStageInstruction = TRawInstruction or TRawUnreachedInstruction;
 
-predicate hasInstruction(TRawInstruction instr) { any() }
+predicate hasInstruction(TStageInstruction instr) { any() }
 
 predicate hasModeledMemoryResult(Instruction instruction) { none() }
 
@@ -368,6 +368,11 @@ private predicate isStrictlyForwardGoto(GotoStmt goto) {
 
 Locatable getInstructionAst(TStageInstruction instr) {
   result = getInstructionTranslatedElement(instr).getAst()
+  or
+  exists(IRFunction irFunc |
+    instr = TRawUnreachedInstruction(irFunc) and
+    result = irFunc.getFunction()
+  )
 }
 
 /** DEPRECATED: Alias for getInstructionAst */
@@ -377,14 +382,22 @@ deprecated Locatable getInstructionAST(TStageInstruction instr) {
 
 CppType getInstructionResultType(TStageInstruction instr) {
   getInstructionTranslatedElement(instr).hasInstruction(_, getInstructionTag(instr), result)
+  or
+  instr instanceof TRawUnreachedInstruction and
+  result = getVoidType()
 }
 
 predicate getInstructionOpcode(Opcode opcode, TStageInstruction instr) {
   getInstructionTranslatedElement(instr).hasInstruction(opcode, getInstructionTag(instr), _)
+  or
+  instr instanceof TRawUnreachedInstruction and
+  opcode instanceof Opcode::Unreached
 }
 
 IRFunctionBase getInstructionEnclosingIRFunction(TStageInstruction instr) {
   result.getFunction() = getInstructionTranslatedElement(instr).getFunction()
+  or
+  instr = TRawUnreachedInstruction(result)
 }
 
 Instruction getPrimaryInstructionForSideEffect(SideEffectInstruction instruction) {
@@ -393,6 +406,16 @@ Instruction getPrimaryInstructionForSideEffect(SideEffectInstruction instruction
         .getPrimaryInstructionForSideEffect(getInstructionTag(instruction))
 }
 
+predicate hasUnreachedInstruction(IRFunction func) {
+  exists(Call c |
+    c.getEnclosingFunction() = func.getFunction() and
+    any(Options opt).exits(c.getTarget())
+  ) and
+  not exists(TranslatedUnreachableReturnStmt return |
+    return.getEnclosingFunction().getFunction() = func.getFunction()
+  )
+}
+
 import CachedForDebugging
 
 cached

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/InstructionTag.qll

@@ -34,6 +34,7 @@ newtype TInstructionTag =
   CallTargetTag() or
   CallTag() or
   CallSideEffectTag() or
+  CallNoReturnTag() or
   AllocationSizeTag() or
   AllocationElementSizeTag() or
   AllocationExtentConvertTag() or

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedCall.qll

@@ -8,6 +8,7 @@ private import SideEffects
 private import TranslatedElement
 private import TranslatedExpr
 private import TranslatedFunction
+private import DefaultOptions as DefaultOptions
 
 /**
  * Gets the `CallInstruction` from the `TranslatedCallExpr` for the specified expression.
@@ -66,7 +67,13 @@ abstract class TranslatedCall extends TranslatedExpr {
     )
     or
     child = getSideEffects() and
-    result = getParent().getChildSuccessor(this)
+    if this.isNoReturn()
+    then
+      result =
+        any(UnreachedInstruction instr |
+          this.getEnclosingFunction().getFunction() = instr.getEnclosingFunction()
+        )
+    else result = this.getParent().getChildSuccessor(this)
   }
 
   override Instruction getInstructionSuccessor(InstructionTag tag, EdgeKind kind) {
@@ -161,6 +168,8 @@ abstract class TranslatedCall extends TranslatedExpr {
    */
   abstract predicate hasArguments();
 
+  predicate isNoReturn() { none() }
+
   final TranslatedSideEffects getSideEffects() { result.getExpr() = expr }
 }
 
@@ -266,6 +275,8 @@ abstract class TranslatedCallExpr extends TranslatedNonConstantExpr, TranslatedC
   }
 
   final override int getNumberOfArguments() { result = expr.getNumberOfArguments() }
+
+  final override predicate isNoReturn() { any(Options opt).exits(expr.getTarget()) }
 }
 
 /**

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/reachability/ReachableBlock.qll

@@ -7,6 +7,9 @@ predicate isInfeasibleInstructionSuccessor(Instruction instr, EdgeKind kind) {
     conditionValue = getConstantValue(instr.(ConditionalBranchInstruction).getCondition()) and
     if conditionValue = 0 then kind instanceof TrueEdge else kind instanceof FalseEdge
   )
+  or
+  instr.getSuccessor(kind) instanceof UnreachedInstruction and
+  kind instanceof GotoEdge
 }
 
 pragma[noinline]
@@ -41,7 +44,9 @@ class ReachableBlock extends IRBlockBase {
  * An instruction that is contained in a reachable block.
  */
 class ReachableInstruction extends Instruction {
-  ReachableInstruction() { this.getBlock() instanceof ReachableBlock }
+  ReachableInstruction() {
+    this.getBlock() instanceof ReachableBlock and not this instanceof UnreachedInstruction
+  }
 }
 
 module Graph {

cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSAConstruction.qll

@@ -34,9 +34,13 @@ private module Cached {
 
   cached
   predicate hasUnreachedInstructionCached(IRFunction irFunc) {
-    exists(OldInstruction oldInstruction |
+    exists(OldIR::Instruction oldInstruction |
       irFunc = oldInstruction.getEnclosingIRFunction() and
-      Reachability::isInfeasibleInstructionSuccessor(oldInstruction, _)
+      (
+        Reachability::isInfeasibleInstructionSuccessor(oldInstruction, _)
+        or
+        oldInstruction.getOpcode() instanceof Opcode::Unreached
+      )
     )
   }
 
@@ -366,21 +370,19 @@ private module Cached {
     then
       result = getChi(getOldInstruction(instruction)) and
       kind instanceof GotoEdge
-    else (
+    else
       exists(OldInstruction oldInstruction |
-        oldInstruction = getOldInstruction(instruction) and
+        (
+          oldInstruction = getOldInstruction(instruction)
+          or
+          instruction = getChi(oldInstruction)
+        ) and
         (
           if Reachability::isInfeasibleInstructionSuccessor(oldInstruction, kind)
           then result = unreachedInstruction(instruction.getEnclosingIRFunction())
           else result = getNewInstruction(oldInstruction.getSuccessor(kind))
         )
       )
-      or
-      exists(OldInstruction oldInstruction |
-        instruction = getChi(oldInstruction) and
-        result = getNewInstruction(oldInstruction.getSuccessor(kind))
-      )
-    )
   }
 
   cached

cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/reachability/ReachableBlock.qll

@@ -7,6 +7,9 @@ predicate isInfeasibleInstructionSuccessor(Instruction instr, EdgeKind kind) {
     conditionValue = getConstantValue(instr.(ConditionalBranchInstruction).getCondition()) and
     if conditionValue = 0 then kind instanceof TrueEdge else kind instanceof FalseEdge
   )
+  or
+  instr.getSuccessor(kind) instanceof UnreachedInstruction and
+  kind instanceof GotoEdge
 }
 
 pragma[noinline]
@@ -41,7 +44,9 @@ class ReachableBlock extends IRBlockBase {
  * An instruction that is contained in a reachable block.
  */
 class ReachableInstruction extends Instruction {
-  ReachableInstruction() { this.getBlock() instanceof ReachableBlock }
+  ReachableInstruction() {
+    this.getBlock() instanceof ReachableBlock and not this instanceof UnreachedInstruction
+  }
 }
 
 module Graph {

cpp/ql/test/library-tests/ir/ir/PrintAST.expected

@@ -14408,6 +14408,60 @@ ir.cpp:
 # 1894|             Conversion = [IntegralConversion] integral conversion
 # 1894|             Type = [IntType] int
 # 1894|             ValueCategory = prvalue
+# 1897| [TopLevelFunction] void noreturnFunc()
+# 1897|   <params>: 
+# 1899| [TopLevelFunction] int noreturnTest(int)
+# 1899|   <params>: 
+# 1899|     getParameter(0): [Parameter] x
+# 1899|         Type = [IntType] int
+# 1899|   getEntryPoint(): [BlockStmt] { ... }
+# 1900|     getStmt(0): [IfStmt] if (...) ... 
+# 1900|       getCondition(): [LTExpr] ... < ...
+# 1900|           Type = [BoolType] bool
+# 1900|           ValueCategory = prvalue
+# 1900|         getLesserOperand(): [VariableAccess] x
+# 1900|             Type = [IntType] int
+# 1900|             ValueCategory = prvalue(load)
+# 1900|         getGreaterOperand(): [Literal] 10
+# 1900|             Type = [IntType] int
+# 1900|             Value = [Literal] 10
+# 1900|             ValueCategory = prvalue
+# 1900|       getThen(): [BlockStmt] { ... }
+# 1901|         getStmt(0): [ReturnStmt] return ...
+# 1901|           getExpr(): [VariableAccess] x
+# 1901|               Type = [IntType] int
+# 1901|               ValueCategory = prvalue(load)
+# 1902|       getElse(): [BlockStmt] { ... }
+# 1903|         getStmt(0): [ExprStmt] ExprStmt
+# 1903|           getExpr(): [FunctionCall] call to noreturnFunc
+# 1903|               Type = [VoidType] void
+# 1903|               ValueCategory = prvalue
+# 1905|     getStmt(1): [ReturnStmt] return ...
+# 1907| [TopLevelFunction] int noreturnTest2(int)
+# 1907|   <params>: 
+# 1907|     getParameter(0): [Parameter] x
+# 1907|         Type = [IntType] int
+# 1907|   getEntryPoint(): [BlockStmt] { ... }
+# 1908|     getStmt(0): [IfStmt] if (...) ... 
+# 1908|       getCondition(): [LTExpr] ... < ...
+# 1908|           Type = [BoolType] bool
+# 1908|           ValueCategory = prvalue
+# 1908|         getLesserOperand(): [VariableAccess] x
+# 1908|             Type = [IntType] int
+# 1908|             ValueCategory = prvalue(load)
+# 1908|         getGreaterOperand(): [Literal] 10
+# 1908|             Type = [IntType] int
+# 1908|             Value = [Literal] 10
+# 1908|             ValueCategory = prvalue
+# 1908|       getThen(): [BlockStmt] { ... }
+# 1909|         getStmt(0): [ExprStmt] ExprStmt
+# 1909|           getExpr(): [FunctionCall] call to noreturnFunc
+# 1909|               Type = [VoidType] void
+# 1909|               ValueCategory = prvalue
+# 1911|     getStmt(1): [ReturnStmt] return ...
+# 1911|       getExpr(): [VariableAccess] x
+# 1911|           Type = [IntType] int
+# 1911|           ValueCategory = prvalue(load)
 perf-regression.cpp:
 #    4| [CopyAssignmentOperator] Big& Big::operator=(Big const&)
 #    4|   <params>: