From f5c52ac496f01041afdf96e55a561c0cb614af59 Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Mon, 18 Dec 2023 12:02:33 +0100
Subject: [PATCH] C++: Fix joins in 'isModifiableAtImpl'.

---
 .../code/cpp/ir/dataflow/internal/DataFlowUtil.qll |  3 ++-
 .../ir/dataflow/internal/SsaInternalsCommon.qll    | 12 ++++++++----
 cpp/ql/lib/semmle/code/cpp/ir/internal/CppType.qll | 14 +++++++++++++-
 3 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll
index 4e8acfe8187..5221276ccd7 100644
--- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll
+++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll
@@ -997,7 +997,8 @@ private Type getTypeImpl0(Type t, int indirectionIndex) {
  *
  * If `indirectionIndex` cannot be stripped off `t`, an `UnknownType` is returned.
  */
-bindingset[indirectionIndex]
+bindingset[t, indirectionIndex]
+pragma[inline_late]
 Type getTypeImpl(Type t, int indirectionIndex) {
   result = getTypeImpl0(t, indirectionIndex)
   or
diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll
index ce3a5fbccd3..e349367fed4 100644
--- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll
+++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll
@@ -418,6 +418,11 @@ class BaseCallVariable extends AbstractBaseSourceVariable, TBaseCallVariable {
 }
 
 private module IsModifiableAtImpl {
+  pragma[nomagic]
+  private predicate isUnderlyingIndirectionType(Type t) {
+    t = any(Indirection ind).getUnderlyingType()
+  }
+
   /**
    * Holds if the `indirectionIndex`'th dereference of a value of type
    * `cppType` is a type that can be modified (either by modifying the value
@@ -445,10 +450,9 @@ private module IsModifiableAtImpl {
   bindingset[cppType, indirectionIndex]
   pragma[inline_late]
   private predicate impl(CppType cppType, int indirectionIndex) {
-    exists(Type pointerType, Type base, Type t |
-      pointerType = t.getUnderlyingType() and
-      pointerType = any(Indirection ind).getUnderlyingType() and
-      cppType.hasType(t, _) and
+    exists(Type pointerType, Type base |
+      isUnderlyingIndirectionType(pointerType) and
+      cppType.hasUnderlyingType(pointerType, _) and
       base = getTypeImpl(pointerType, indirectionIndex)
     |
       // The value cannot be modified if it has a const specifier,
diff --git a/cpp/ql/lib/semmle/code/cpp/ir/internal/CppType.qll b/cpp/ql/lib/semmle/code/cpp/ir/internal/CppType.qll
index 315db83a5cc..c1d97d36360 100644
--- a/cpp/ql/lib/semmle/code/cpp/ir/internal/CppType.qll
+++ b/cpp/ql/lib/semmle/code/cpp/ir/internal/CppType.qll
@@ -227,7 +227,7 @@ class CppType extends TCppType {
   predicate hasType(Type type, boolean isGLValue) { none() }
 
   /**
-   * Holds if this type represents the C++ type `type`. If `isGLValue` is `true`, then this type
+   * Holds if this type represents the C++ unspecified type `type`. If `isGLValue` is `true`, then this type
    * represents a glvalue of type `type`. Otherwise, it represents a prvalue of type `type`.
    */
   final predicate hasUnspecifiedType(Type type, boolean isGLValue) {
@@ -236,6 +236,18 @@ class CppType extends TCppType {
       type = specifiedType.getUnspecifiedType()
     )
   }
+
+  /**
+   * Holds if this type represents the C++ type `type` (after resolving
+   * typedefs). If `isGLValue` is `true`, then this type represents a glvalue
+   * of type `type`. Otherwise, it represents a prvalue of type `type`.
+   */
+  final predicate hasUnderlyingType(Type type, boolean isGLValue) {
+    exists(Type typedefType |
+      this.hasType(typedefType, isGLValue) and
+      type = typedefType.getUnderlyingType()
+    )
+  }
 }
 
 /**