Merge pull request #1102 from github/michaelrfairhurst/declarations-10-1-1-properly-qualify-lvalue-refs-pointers

Michaelrfairhurst/declarations 10-1-1 properly qualify lvalue refs pointers

Author: knewbury01

change_notes/2026-04-16-alias-parameter-side-effects-in-compond-assignment.md

@@ -0,0 +1,2 @@
+- All queries related to side effects:
+  - Compound assignments of pointer parameters (e.g. `p += 1`) are no longer treated as a modification of the pointed-to object. This was previously only handled for simple assignments (e.g. `p = ...`).

cpp/common/src/codingstandards/cpp/Call.qll

@@ -17,3 +17,48 @@ FunctionType getExprCallFunctionType(ExprCall call) {
   // Returns a RoutineType
   result = call.(ExprCall).getChild(-1).getType().(PointerToMemberType).getBaseType()
 }
+
+/**
+ * An `Expr` that is used as an argument to a `Call`, and has helpers to handle with the differences
+ * between `ExprCall` and `FunctionCall` cases.
+ */
+class CallArgumentExpr extends Expr {
+  Call call;
+  Type paramType;
+  int argIndex;
+
+  CallArgumentExpr() {
+    this = call.getArgument(argIndex) and
+    (
+      paramType = call.getTarget().getParameter(argIndex).getType()
+      or
+      paramType = getExprCallFunctionType(call).getParameterType(argIndex)
+    )
+  }
+
+  /**
+   * Gets the `FunctionExpr` or `FunctionCall` that this argument appears in.
+   */
+  Call getCall() { result = call }
+
+  /**
+   * Gets the `Type` of the parameter corresponding to this argument, whether its based on the
+   * target function or the function pointer type.
+   */
+  Type getParamType() { result = paramType }
+
+  /**
+   * Gets the argument index of this argument in the call.
+   */
+  int getArgIndex() { result = argIndex }
+
+  /**
+   * Gets the target `Function` if this is an argument to a `FunctionCall`.
+   */
+  Function getKnownFunction() { result = call.getTarget() }
+
+  /**
+   * Gets the target `Parameter` if this is an argument to a `FunctionCall`.
+   */
+  Parameter getKnownParameter() { result = call.getTarget().getParameter(argIndex) }
+}

cpp/common/src/codingstandards/cpp/SideEffect.qll

@@ -295,7 +295,7 @@ class AliasParameter extends Parameter {
         this.getUnspecifiedType() instanceof ReferenceType
         or
         // Exclude effects that change the who we point to, since we care about changes to the referenced object.
-        not result.(AssignExpr).getLValue() = va and
+        not result.(Assignment).getLValue() = va and
         not result.(CrementOperation).getOperand() = va
       )
     )

cpp/common/src/codingstandards/cpp/exclusions/cpp/Declarations6.qll

@@ -0,0 +1,26 @@
+//** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY.  **/
+import cpp
+import RuleMetadata
+import codingstandards.cpp.exclusions.RuleMetadata
+
+newtype Declarations6Query = TPointerOrRefParamNotConstQuery()
+
+predicate isDeclarations6QueryMetadata(Query query, string queryId, string ruleId, string category) {
+  query =
+    // `Query` instance for the `pointerOrRefParamNotConst` query
+    Declarations6Package::pointerOrRefParamNotConstQuery() and
+  queryId =
+    // `@id` for the `pointerOrRefParamNotConst` query
+    "cpp/misra/pointer-or-ref-param-not-const" and
+  ruleId = "RULE-10-1-1" and
+  category = "advisory"
+}
+
+module Declarations6Package {
+  Query pointerOrRefParamNotConstQuery() {
+    //autogenerate `Query` type
+    result =
+      // `Query` type for `pointerOrRefParamNotConst` query
+      TQueryCPP(TDeclarations6PackageQuery(TPointerOrRefParamNotConstQuery()))
+  }
+}

cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll

@@ -39,6 +39,7 @@ import Declarations1
 import Declarations2
 import Declarations3
 import Declarations4
+import Declarations6
 import Declarations7
 import ExceptionSafety
 import Exceptions1
@@ -143,6 +144,7 @@ newtype TCPPQuery =
   TDeclarations2PackageQuery(Declarations2Query q) or
   TDeclarations3PackageQuery(Declarations3Query q) or
   TDeclarations4PackageQuery(Declarations4Query q) or
+  TDeclarations6PackageQuery(Declarations6Query q) or
   TDeclarations7PackageQuery(Declarations7Query q) or
   TExceptionSafetyPackageQuery(ExceptionSafetyQuery q) or
   TExceptions1PackageQuery(Exceptions1Query q) or
@@ -247,6 +249,7 @@ predicate isQueryMetadata(Query query, string queryId, string ruleId, string cat
   isDeclarations2QueryMetadata(query, queryId, ruleId, category) or
   isDeclarations3QueryMetadata(query, queryId, ruleId, category) or
   isDeclarations4QueryMetadata(query, queryId, ruleId, category) or
+  isDeclarations6QueryMetadata(query, queryId, ruleId, category) or
   isDeclarations7QueryMetadata(query, queryId, ruleId, category) or
   isExceptionSafetyQueryMetadata(query, queryId, ruleId, category) or
   isExceptions1QueryMetadata(query, queryId, ruleId, category) or

cpp/common/src/codingstandards/cpp/types/Pointers.qll

@@ -125,3 +125,89 @@ Type baseType(Type t) {
   // Make sure that the type has a size and that it isn't ambiguous.
   strictcount(result.getSize()) = 1
 }
+
+/**
+ * A `Type` that may be a pointer, array, or reference, to a const or a non-const type.
+ *
+ * For example, `const int*`, `int* const`, `const int* const`, `int*`, `int&`, `const int&` are all
+ * `PointerLikeType`s, while `int`, `int&&`, and `const int` are not.
+ *
+ * To check if a `PointerLikeType` points/refers to a const-qualified type, use the `pointsToConst()`
+ * predicate.
+ */
+class PointerLikeType extends Type {
+  Type innerType;
+  Type outerType;
+
+  PointerLikeType() {
+    innerType = this.(UnspecifiedPointerOrArrayType).getBaseType() and
+    outerType = this
+    or
+    innerType = this.(LValueReferenceType).getBaseType() and
+    outerType = this
+    or
+    exists(PointerLikeType stripped |
+      stripped = this.stripTopLevelSpecifiers() and not stripped = this
+    |
+      innerType = stripped.getInnerType() and
+      outerType = stripped.getOuterType()
+    )
+  }
+
+  /**
+   * Gets the pointed to or referred to type, for instance `int` for `int*` or `const int&`.
+   */
+  Type getInnerType() { result = innerType }
+
+  /**
+   * Gets the resolved pointer, array, or reference type itself, for instance `int*` in `int* const`.
+   *
+   * Removes cv-qualification and resolves typedefs and decltypes and specifiers via
+   * `stripTopLevelSpecifiers()`.
+   */
+  Type getOuterType() { result = outerType }
+
+  /**
+   * Holds when this type points to const -- for example, `const int*` and `const int&` point to
+   * const, while `int*`, `int *const` and `int&` do not.
+   */
+  predicate pointsToConst() { innerType.isConst() }
+
+  /**
+   * Holds when this type points to non-const -- for example, `int*` and `int&` and `int *const`
+   * point to non-const, while `const int*`, `const int&` do not.
+   */
+  predicate pointsToNonConst() { not innerType.isConst() }
+}
+
+/**
+ * Gets usages of this parameter that maintain pointer-like semantics -- typically this means
+ * either a normal access, or switching between pointers and reference semantics.
+ *
+ * Examples of accesses with pointer-like semantics include:
+ * - `ref` in `int &x = ref`, or `&ref` in `int *x = &ref`;
+ * - `ptr` in `int *x = ptr`, or `*ptr` in `int &x = *ptr`;
+ *
+ * In the above examples, we can still access the value pointed to by `ref` or `ptr` through the
+ * expression.
+ *
+ * Examples of non-pointer-like semantics include:
+ * - `ref` in `int x = ref` and `*ptr` in `int x = *ptr`;
+ *
+ * In the above examples, the value pointed to by `ref` or `ptr` is copied and the expression
+ * refers to a new/different object.
+ */
+Expr getAPointerLikeAccessOf(Expr expr) {
+  exists(PointerLikeType pointerLikeType | pointerLikeType = expr.getType() |
+    result = expr
+    or
+    // For reference parameters, also consider accesses to the parameter itself as accesses to the referent
+    pointerLikeType.getOuterType() instanceof ReferenceType and
+    result.(AddressOfExpr).getOperand() = expr
+    or
+    // A pointer is dereferenced, but the result is not copied
+    pointerLikeType.getOuterType() instanceof PointerType and
+    result.(PointerDereferenceExpr).getOperand() = expr and
+    not any(ReferenceDereferenceExpr rde).getExpr() = result.getConversion+()
+  )
+}

cpp/common/test/includes/standard-library/vector.h

@@ -20,8 +20,8 @@ template <class T, class Allocator = std::allocator<T>> class vector {
 
   iterator begin();
   iterator end();
-  const_iterator cbegin();
-  const_iterator cend();
+  const_iterator cbegin() const;
+  const_iterator cend() const;
   size_type size() const noexcept;
   void resize(size_type sz);
   void resize(size_type sz, const T &c);

cpp/misra/src/rules/RULE-10-1-1/PointerOrRefParamNotConst.ql

@@ -0,0 +1,145 @@
+/**
+ * @id cpp/misra/pointer-or-ref-param-not-const
+ * @name RULE-10-1-1: The target type of a pointer or lvalue reference parameter should be const-qualified appropriately
+ * @description Pointer or lvalue reference parameters that do not modify the target object should
+ *              be const-qualified to accurately reflect function behavior and prevent unintended
+ *              modifications.
+ * @kind problem
+ * @precision high
+ * @problem.severity warning
+ * @tags external/misra/id/rule-10-1-1
+ *       correctness
+ *       readability
+ *       performance
+ *       scope/single-translation-unit
+ *       external/misra/enforcement/decidable
+ *       external/misra/obligation/advisory
+ */
+
+import cpp
+import codingstandards.cpp.misra
+import codingstandards.cpp.types.Pointers
+import codingstandards.cpp.Call
+import codingstandards.cpp.SideEffect
+
+/**
+ * Holds if `f` is a `Function` in a template scope and should be excluded.
+ */
+predicate isInTemplateScope(Function f) {
+  f.isFromTemplateInstantiation(_)
+  or
+  f.isFromUninstantiatedTemplate(_)
+}
+
+/**
+ * A `Parameter` whose type is a `PointerLikeType` such as a pointer or reference.
+ */
+class PointerLikeParam extends Parameter {
+  PointerLikeType pointerLikeType;
+
+  PointerLikeParam() { pointerLikeType = this.getType() }
+
+  /**
+   * Gets the pointer like type of this parameter.
+   */
+  PointerLikeType getPointerLikeType() { result = pointerLikeType }
+
+  /**
+   * Gets usages of this parameter that maintain pointer-like semantics -- typically this means
+   * either a normal access, or switching between pointers and reference semantics.
+   *
+   * Examples of accesses with pointer-like semantics include:
+   * - `ref` in `int &x = ref`, or `&ref` in `int *x = &ref`;
+   * - `ptr` in `int *x = ptr`, or `*ptr` in `int &x = *ptr`;
+   *
+   * In the above examples, we can still access the value pointed to by `ref` or `ptr` through the
+   * expression.
+   *
+   * Examples of non-pointer-like semantics include:
+   * - `ref` in `int x = ref` and `*ptr` in `int x = *ptr`;
+   *
+   * In the above examples, the value pointed to by `ref` or `ptr` is copied and the expression
+   * refers to a new/different object.
+   */
+  Expr getAPointerLikeAccess() { result = getAPointerLikeAccessOf(this.getAnAccess()) }
+}
+
+/**
+ * A candidate parameter that could have its target type const-qualified.
+ */
+class NonConstParam extends PointerLikeParam {
+  NonConstParam() {
+    not pointerLikeType.pointsToConst() and
+    // Ignore parameters in functions without bodies
+    exists(this.getFunction().getBlock()) and
+    // Ignore unnamed parameters
+    this.isNamed() and
+    // Ignore functions that use ASM statements
+    not exists(AsmStmt a | a.getEnclosingFunction() = this.getFunction()) and
+    // Must have a pointer, array, or lvalue reference type with non-const target
+    // Exclude pointers to non-object types
+    not pointerLikeType.getInnerType+().getUnderlyingType() instanceof RoutineType and
+    not pointerLikeType.getInnerType+().getUnderlyingType() instanceof VoidType and
+    // Exclude virtual functions
+    not this.getFunction().isVirtual() and
+    // Exclude functions in template scope
+    not isInTemplateScope(this.getFunction()) and
+    // Exclude main
+    not this.getFunction().hasGlobalName("main") and
+    // Exclude deleted functions
+    not this.getFunction().isDeleted() and
+    // Exclude any parameter whose underlying data is modified
+    not exists(AliasParameter alias | alias = this | alias.isModified()) and
+    // Exclude parameters passed as arguments to non-const pointer/ref params
+    not exists(CallArgumentExpr arg |
+      arg = this.getAPointerLikeAccess() and
+      arg.getParamType().(PointerLikeType).pointsToNonConst()
+    ) and
+    // Exclude parameters used as qualifier for a non-const member function
+    not exists(FunctionCall fc |
+      fc.getQualifier() = [this.getAnAccess(), this.getAPointerLikeAccess()] and
+      not fc.getTarget().hasSpecifier("const") and
+      not fc.getTarget().isStatic()
+    ) and
+    // Exclude parameters assigned to a non-const pointer/reference alias
+    not exists(Variable v |
+      v.getAnAssignedValue() = this.getAPointerLikeAccess() and
+      v.getType().(PointerLikeType).pointsToNonConst()
+    ) and
+    // Exclude parameters returned as non-const pointer/reference
+    not exists(ReturnStmt ret |
+      ret.getExpr() = this.getAPointerLikeAccess() and
+      ret.getEnclosingFunction().getType().(PointerLikeType).pointsToNonConst()
+    ) and
+    not exists(FieldAccess fa |
+      fa.getQualifier() = [this.getAPointerLikeAccess(), this.getAnAccess()] and
+      fa.isLValueCategory()
+    ) and
+    not exists(AddressOfExpr addrOf |
+      // exclude pointer to pointer and reference to pointer cases.
+      addrOf.getOperand() = this.getAPointerLikeAccess() and
+      addrOf.getType().(PointerLikeType).getInnerType() instanceof PointerLikeType
+    ) and
+    not exists(PointerArithmeticOperation pointerManip |
+      pointerManip.getAnOperand() = this.getAPointerLikeAccess() and
+      pointerManip.getType().(PointerLikeType).pointsToNonConst()
+    )
+  }
+}
+
+from NonConstParam param, Type innerType
+where
+  not isExcluded(param, Declarations6Package::pointerOrRefParamNotConstQuery()) and
+  innerType = param.getPointerLikeType().getInnerType() and
+  not param.isAffectedByMacro() and
+  // There are some odd database patterns where a function has multiple parameters with the same
+  // index and different names, due to strange extraction+linker scenarios. These give wrong
+  // results, and should be excluded.
+  count(Parameter p |
+    p.getFunction() = param.getFunction() and
+    p.getIndex() = param.getIndex()
+  ) = 1
+select param,
+  "Parameter '" + param.getName() + "' points/refers to a non-const type '" + innerType.toString() +
+    "' but does not modify the target object in the $@.", param.getFunction().getDefinition(),
+  "function definition"