Add Rule-19-1 and converted M0-2-1 to shared query

mbaluda · mbaluda · commit ab4360237708 · 2023-03-16T22:51:49.000+01:00
diff --git a/c/common/test/rules/overlappingobjectassignment/OverlappingObjectAssignment.expected b/c/common/test/rules/overlappingobjectassignment/OverlappingObjectAssignment.expected
@@ -0,0 +1 @@
+| test.c:9:3:9:11 | ... = ... | An object $@ assigned to overlapping object $@. | test.c:9:5:9:5 | l | l | test.c:9:11:9:11 | i | i |
diff --git a/c/common/test/rules/overlappingobjectassignment/OverlappingObjectAssignment.ql b/c/common/test/rules/overlappingobjectassignment/OverlappingObjectAssignment.ql
@@ -0,0 +1,2 @@
+// GENERATED FILE - DO NOT MODIFY
+import codingstandards.cpp.rules.overlappingobjectassignment.OverlappingObjectAssignment
diff --git a/c/common/test/rules/overlappingobjectassignment/test.c b/c/common/test/rules/overlappingobjectassignment/test.c
@@ -0,0 +1,10 @@
+#include <stdint.h>
+
+void f(void) {
+  union {
+    int i;
+    long l;
+  } u = {0};
+
+  u.l = u.i; // NON_COMPLIANT
+}
diff --git a/c/misra/src/rules/RULE-19-1/ObjectAssignedToAnOverlappingObject.ql b/c/misra/src/rules/RULE-19-1/ObjectAssignedToAnOverlappingObject.ql
@@ -0,0 +1,21 @@
+/**
+ * @id c/misra/object-assigned-to-an-overlapping-object
+ * @name RULE-19-1: An object shall not be assigned to an overlapping object
+ * @description An object shall not be assigned to an overlapping object.
+ * @kind problem
+ * @precision high
+ * @problem.severity error
+ * @tags external/misra/id/rule-19-1
+ *       correctness
+ *       external/misra/obligation/mandatory
+ */
+
+import cpp
+import codingstandards.c.misra
+import codingstandards.cpp.rules.overlappingobjectassignment.OverlappingObjectAssignment
+
+class ObjectAssignedToAnOverlappingObjectQuery extends OverlappingObjectAssignmentSharedQuery {
+  ObjectAssignedToAnOverlappingObjectQuery() {
+    this = Contracts7Package::objectAssignedToAnOverlappingObjectQuery()
+  }
+}
diff --git a/c/misra/src/rules/RULE-19-1/ObjectCopiedToAnOverlappingObject.ql b/c/misra/src/rules/RULE-19-1/ObjectCopiedToAnOverlappingObject.ql
@@ -0,0 +1,93 @@
+/**
+ * @id c/misra/object-copied-to-an-overlapping-object
+ * @name RULE-19-1: An object shall not be copied to an overlapping object
+ * @description An object shall not be copied to an overlapping object.
+ * @kind problem
+ * @precision high
+ * @problem.severity error
+ * @tags external/misra/id/rule-19-1
+ *       correctness
+ *       external/misra/obligation/mandatory
+ */
+
+import cpp
+import codingstandards.c.misra
+import semmle.code.cpp.valuenumbering.GlobalValueNumbering
+import semmle.code.cpp.dataflow.DataFlow
+
+/**
+ * Models calls to memcpy on overlapping objects
+ */
+class MemcpyCall extends Locatable {
+  Expr src;
+  Expr dst;
+
+  MemcpyCall() {
+    this.(MacroInvocation).getMacroName() = "memcpy" and
+    src = this.(MacroInvocation).getExpr().getChild(1) and
+    dst = this.(MacroInvocation).getExpr().getChild(0)
+    or
+    this.(FunctionCall).getTarget().hasGlobalName("memcpy") and
+    src = this.(FunctionCall).getArgument(1) and
+    dst = this.(FunctionCall).getArgument(0)
+  }
+
+  Expr getSrc() { result = src }
+
+  Expr getDst() { result = dst }
+
+  Expr getBase(Expr e) {
+    result =
+      [
+        e.(VariableAccess), e.(PointerAddExpr).getLeftOperand(),
+        e.(AddressOfExpr).getOperand().(ArrayExpr).getArrayBase()
+      ]
+  }
+
+  int getOffset(Expr e) {
+    result =
+      [
+        e.(PointerAddExpr).getRightOperand().getValue().toInt(),
+        e.(AddressOfExpr).getOperand().(ArrayExpr).getArrayOffset().getValue().toInt()
+      ]
+    or
+    e instanceof VariableAccess and result = 0
+  }
+
+  // maximum amount of element copied
+  int getCount() {
+    result =
+      upperBound([this.(MacroInvocation).getExpr().getChild(2), this.(FunctionCall).getArgument(2)])
+  }
+
+  // source and destination overlap
+  predicate overlap() {
+    globalValueNumber(this.getBase(src)) = globalValueNumber(this.getBase(dst)) and
+    exists(int dstStart, int dstEnd, int srcStart, int srcEnd |
+      dstStart = this.getOffset(dst) and
+      dstEnd = dstStart + this.getCount() - 1 and
+      srcStart = this.getOffset(src) and
+      srcEnd = srcStart + this.getCount() - 1 and
+      (
+        srcStart >= dstStart and srcEnd <= dstEnd
+        or
+        srcStart <= dstStart and srcEnd > dstStart
+        or
+        srcStart < dstEnd and srcEnd >= dstStart
+      ) and
+      // Exception 1: exact overlap and compatible type
+      not (
+        srcStart = dstStart and
+        srcEnd = dstEnd and
+        this.getBase(src).getUnspecifiedType() = this.getBase(dst).getUnspecifiedType()
+      )
+    )
+  }
+}
+
+from MemcpyCall memcpy
+where
+  not isExcluded(memcpy, Contracts7Package::objectCopiedToAnOverlappingObjectQuery()) and
+  memcpy.overlap()
+select memcpy, "The object to copy $@ overlaps the object to copy $@.", memcpy.getSrc(), "from",
+  memcpy.getDst(), "to"
diff --git a/c/misra/test/rules/RULE-12-2/test.c b/c/misra/test/rules/RULE-12-2/test.c
@@ -21,9 +21,9 @@ void f1() {
   ul << 64; // NON_COMPLIANT
 
   // 1UL essential type is essentially unsigned char
-  1UL << 7;        // COMPLIANT
-  1UL << 8;        // NON_COMPLIANT
-  1UL << 64;       // NON_COMPLIANT
+  1UL << 7;  // COMPLIANT
+  1UL << 8;  // NON_COMPLIANT
+  1UL << 64; // NON_COMPLIANT
 
   // ULONG_MAX essential type is essentially unsigned long
   ULONG_MAX << 8;  // COMPLIANT
diff --git a/c/misra/test/rules/RULE-19-1/ObjectAssignedToAnOverlappingObject.testref b/c/misra/test/rules/RULE-19-1/ObjectAssignedToAnOverlappingObject.testref
@@ -0,0 +1 @@
+c/common/test/rules/overlappingobjectassignment/OverlappingObjectAssignment.ql
diff --git a/c/misra/test/rules/RULE-19-1/ObjectCopiedToAnOverlappingObject.expected b/c/misra/test/rules/RULE-19-1/ObjectCopiedToAnOverlappingObject.expected
@@ -0,0 +1,4 @@
+| test.c:5:3:5:8 | call to memcpy | The object to copy $@ overlaps the object to copy $@. | test.c:5:17:5:21 | & ... | from | test.c:5:10:5:14 | & ... | to |
+| test.c:7:3:7:8 | call to memcpy | The object to copy $@ overlaps the object to copy $@. | test.c:7:17:7:21 | & ... | from | test.c:7:10:7:14 | & ... | to |
+| test.c:8:3:8:8 | call to memcpy | The object to copy $@ overlaps the object to copy $@. | test.c:8:17:8:17 | o | from | test.c:8:10:8:14 | ... + ... | to |
+| test.c:10:3:10:8 | call to memcpy | The object to copy $@ overlaps the object to copy $@. | test.c:10:17:10:21 | ... + ... | from | test.c:10:10:10:14 | ... + ... | to |
diff --git a/c/misra/test/rules/RULE-19-1/ObjectCopiedToAnOverlappingObject.qlref b/c/misra/test/rules/RULE-19-1/ObjectCopiedToAnOverlappingObject.qlref
@@ -0,0 +1 @@
+rules/RULE-19-1/ObjectCopiedToAnOverlappingObject.ql
diff --git a/c/misra/test/rules/RULE-19-1/test.c b/c/misra/test/rules/RULE-19-1/test.c
@@ -0,0 +1,22 @@
+#include <string.h>
+
+int o[10];
+void g(void) {
+  memcpy(&o[1], &o[0], 2); // NON_COMPLIANT
+  memcpy(&o[2], &o[0], 2); // COMPLIANT
+  memcpy(&o[2], &o[1], 2); // NON_COMPLIANT
+  memcpy(o + 1, o, 2);     // NON_COMPLIANT
+  memcpy(o + 2, o, 2);     // COMPLIANT
+  memcpy(o + 2, o + 1, 2); // NON_COMPLIANT
+
+  // Exception 1
+  int *p = &o[0];
+  int *q = &o[0];
+
+  *p = *q;                 // COMPLIANT
+  memcpy(&o[0], &o[0], 2); // COMPLIANT
+  memcpy(o, o, 2);         // COMPLIANT
+
+  // Exception 2
+  memmove(&o[1], &o[0], 2u * sizeof(o[0])); // COMPLIANT
+}
diff --git a/cpp/autosar/src/rules/M0-2-1/ObjectAssignedToAnOverlappingObject.ql b/cpp/autosar/src/rules/M0-2-1/ObjectAssignedToAnOverlappingObject.ql
@@ -12,46 +12,12 @@
  *       external/autosar/obligation/required
  */
 
-//Assignment between different active members of same union instance
 import cpp
 import codingstandards.cpp.autosar
-import semmle.code.cpp.valuenumbering.GlobalValueNumbering
+import codingstandards.cpp.rules.overlappingobjectassignment.OverlappingObjectAssignment
 
-VariableAccess getAQualifier(VariableAccess va) { result = va.getQualifier+() }
-
-int getAccessByteOffset(FieldAccess fa) {
-  not fa.getQualifier() instanceof FieldAccess and result = fa.getTarget().getByteOffset()
-  or
-  result = fa.getTarget().getByteOffset() + getAccessByteOffset(fa.getQualifier())
-}
-
-predicate overlaps(FieldAccess fa1, FieldAccess fa2) {
-  exists(int startfa1, int endfa1, int startfa2, int endfa2 |
-    startfa1 = getAccessByteOffset(fa1) and
-    endfa1 = startfa1 + fa1.getTarget().getType().getSize() - 1 and
-    startfa2 = getAccessByteOffset(fa2) and
-    endfa2 = startfa2 + fa2.getTarget().getType().getSize() - 1
-  |
-    startfa1 = startfa2 and endfa1 = endfa2
-    or
-    startfa1 > startfa2 and endfa1 < endfa2
-    or
-    startfa1 < startfa2 and endfa1 < endfa2 and endfa1 > startfa2
-    or
-    startfa1 > startfa2 and endfa1 > endfa2 and startfa1 < endfa2
-  )
+class ObjectAssignedToAnOverlappingObjectQuery extends OverlappingObjectAssignmentSharedQuery {
+  ObjectAssignedToAnOverlappingObjectQuery() {
+    this = RepresentationPackage::objectAssignedToAnOverlappingObjectQuery()
+  }
 }
-
-from AssignExpr assignExpr, Expr lhs, Expr rhs, ValueFieldAccess valuelhs, ValueFieldAccess valuerhs
-where
-  not isExcluded(assignExpr, RepresentationPackage::objectAssignedToAnOverlappingObjectQuery()) and
-  lhs.getType() instanceof Union and
-  rhs.getType() instanceof Union and
-  lhs = getAQualifier(assignExpr.getLValue()) and
-  rhs = getAQualifier(assignExpr.getRValue()) and
-  globalValueNumber(lhs) = globalValueNumber(rhs) and
-  valuerhs = assignExpr.getRValue() and
-  valuelhs = assignExpr.getLValue() and // a.b.c == ((a.b).c)
-  overlaps(valuelhs, valuerhs)
-select assignExpr, "An object $@ assigned to overlapping object $@.", valuelhs,
-  valuelhs.getTarget().getName(), valuerhs, valuerhs.getTarget().getName()
diff --git a/cpp/autosar/test/rules/M0-2-1/ObjectAssignedToAnOverlappingObject.qlref b/cpp/autosar/test/rules/M0-2-1/ObjectAssignedToAnOverlappingObject.qlref
diff --git a/cpp/autosar/test/rules/M0-2-1/ObjectAssignedToAnOverlappingObject.testref b/cpp/autosar/test/rules/M0-2-1/ObjectAssignedToAnOverlappingObject.testref
@@ -0,0 +1 @@
+cpp/common/test/rules/overlappingobjectassignment/OverlappingObjectAssignment.ql
diff --git a/cpp/common/src/codingstandards/cpp/exclusions/c/Contracts7.qll b/cpp/common/src/codingstandards/cpp/exclusions/c/Contracts7.qll
@@ -7,7 +7,8 @@ newtype Contracts7Query =
   TDoNotPassInvalidDataToTheAsctimeFunctionQuery() or
   TDoNotCallVaArgOnAVaListThatHasAnIndeterminateValueQuery() or
   TRightHandOperandOfAShiftRangeQuery() or
-  TObjectAssignedOrCopiedToAnOverlappingObjectQuery()
+  TObjectAssignedToAnOverlappingObjectQuery() or
+  TObjectCopiedToAnOverlappingObjectQuery()
 
 predicate isContracts7QueryMetadata(Query query, string queryId, string ruleId, string category) {
   query =
@@ -38,11 +39,20 @@ predicate isContracts7QueryMetadata(Query query, string queryId, string ruleId,
   category = "required"
   or
   query =
-    // `Query` instance for the `objectAssignedOrCopiedToAnOverlappingObject` query
-    Contracts7Package::objectAssignedOrCopiedToAnOverlappingObjectQuery() and
+    // `Query` instance for the `objectAssignedToAnOverlappingObject` query
+    Contracts7Package::objectAssignedToAnOverlappingObjectQuery() and
   queryId =
-    // `@id` for the `objectAssignedOrCopiedToAnOverlappingObject` query
-    "c/misra/object-assigned-or-copied-to-an-overlapping-object" and
+    // `@id` for the `objectAssignedToAnOverlappingObject` query
+    "c/misra/object-assigned-to-an-overlapping-object" and
+  ruleId = "RULE-19-1" and
+  category = "mandatory"
+  or
+  query =
+    // `Query` instance for the `objectCopiedToAnOverlappingObject` query
+    Contracts7Package::objectCopiedToAnOverlappingObjectQuery() and
+  queryId =
+    // `@id` for the `objectCopiedToAnOverlappingObject` query
+    "c/misra/object-copied-to-an-overlapping-object" and
   ruleId = "RULE-19-1" and
   category = "mandatory"
 }
@@ -69,10 +79,17 @@ module Contracts7Package {
       TQueryC(TContracts7PackageQuery(TRightHandOperandOfAShiftRangeQuery()))
   }
 
-  Query objectAssignedOrCopiedToAnOverlappingObjectQuery() {
+  Query objectAssignedToAnOverlappingObjectQuery() {
+    //autogenerate `Query` type
+    result =
+      // `Query` type for `objectAssignedToAnOverlappingObject` query
+      TQueryC(TContracts7PackageQuery(TObjectAssignedToAnOverlappingObjectQuery()))
+  }
+
+  Query objectCopiedToAnOverlappingObjectQuery() {
     //autogenerate `Query` type
     result =
-      // `Query` type for `objectAssignedOrCopiedToAnOverlappingObject` query
-      TQueryC(TContracts7PackageQuery(TObjectAssignedOrCopiedToAnOverlappingObjectQuery()))
+      // `Query` type for `objectCopiedToAnOverlappingObject` query
+      TQueryC(TContracts7PackageQuery(TObjectCopiedToAnOverlappingObjectQuery()))
   }
 }
diff --git a/cpp/common/src/codingstandards/cpp/rules/overlappingobjectassignment/OverlappingObjectAssignment.qll b/cpp/common/src/codingstandards/cpp/rules/overlappingobjectassignment/OverlappingObjectAssignment.qll
@@ -0,0 +1,55 @@
+/**
+ * Provides a library which includes a `problems` predicate for reporting....
+ */
+
+import cpp
+import codingstandards.cpp.Customizations
+import codingstandards.cpp.Exclusions
+import semmle.code.cpp.valuenumbering.GlobalValueNumbering
+
+abstract class OverlappingObjectAssignmentSharedQuery extends Query { }
+
+Query getQuery() { result instanceof OverlappingObjectAssignmentSharedQuery }
+
+VariableAccess getAQualifier(VariableAccess va) { result = va.getQualifier+() }
+
+int getAccessByteOffset(FieldAccess fa) {
+  not fa.getQualifier() instanceof FieldAccess and result = fa.getTarget().getByteOffset()
+  or
+  result = fa.getTarget().getByteOffset() + getAccessByteOffset(fa.getQualifier())
+}
+
+predicate overlaps(FieldAccess fa1, FieldAccess fa2) {
+  exists(int startfa1, int endfa1, int startfa2, int endfa2 |
+    startfa1 = getAccessByteOffset(fa1) and
+    endfa1 = startfa1 + fa1.getTarget().getType().getSize() - 1 and
+    startfa2 = getAccessByteOffset(fa2) and
+    endfa2 = startfa2 + fa2.getTarget().getType().getSize() - 1
+  |
+    startfa2 >= startfa1 and endfa2 <= endfa1
+    or
+    startfa2 <= startfa1 and endfa2 > startfa1
+    or
+    startfa2 < endfa1 and endfa2 >= startfa1
+  )
+}
+
+query predicate problems(
+  AssignExpr assignExpr, string message, ValueFieldAccess valuelhs, string valuelhsDesc,
+  ValueFieldAccess valuerhs, string valuerhsDesc
+) {
+  not isExcluded(assignExpr, getQuery()) and
+  exists(Expr lhs, Expr rhs |
+    lhs.getType() instanceof Union and
+    rhs.getType() instanceof Union and
+    lhs = getAQualifier(assignExpr.getLValue()) and
+    rhs = getAQualifier(assignExpr.getRValue()) and
+    globalValueNumber(lhs) = globalValueNumber(rhs) and
+    valuerhs = assignExpr.getRValue() and
+    valuelhs = assignExpr.getLValue() and // a.b.c == ((a.b).c)
+    overlaps(valuelhs, valuerhs) and
+    message = "An object $@ assigned to overlapping object $@." and
+    valuelhsDesc = valuelhs.getTarget().getName() and
+    valuerhsDesc = valuerhs.getTarget().getName()
+  )
+}
diff --git a/cpp/common/test/rules/overlappingobjectassignment/OverlappingObjectAssignment.expected b/cpp/common/test/rules/overlappingobjectassignment/OverlappingObjectAssignment.expected
diff --git a/cpp/common/test/rules/overlappingobjectassignment/OverlappingObjectAssignment.ql b/cpp/common/test/rules/overlappingobjectassignment/OverlappingObjectAssignment.ql
@@ -0,0 +1,2 @@
+// GENERATED FILE - DO NOT MODIFY
+import codingstandards.cpp.rules.overlappingobjectassignment.OverlappingObjectAssignment
diff --git a/cpp/common/test/rules/overlappingobjectassignment/test.cpp b/cpp/common/test/rules/overlappingobjectassignment/test.cpp
@@ -51,4 +51,4 @@ void internal_shift() {
 void separate_access() {
   UnionSecret_t hash1, hash2;
   hash2.diff.suffix = hash1.fnv.suffix; // COMPLIANT, different union.
-}
+}
diff --git a/rule_packages/c/Contracts7.json b/rule_packages/c/Contracts7.json
@@ -66,12 +66,24 @@
       },
       "queries": [
         {
-          "description": "An object shall not be assigned or copied to an overlapping object.",
+          "description": "An object shall not be assigned to an overlapping object.",
           "kind": "problem",
-          "name": "An object shall not be assigned or copied to an overlapping object",
+          "name": "An object shall not be assigned to an overlapping object",
           "precision": "high",
           "severity": "error",
-          "short_name": "ObjectAssignedOrCopiedToAnOverlappingObject",
+          "short_name": "ObjectAssignedToAnOverlappingObject",
+          "shared_implementation_short_name": "OverlappingObjectAssignment",
+          "tags": [
+            "correctness"
+          ]
+        },
+        {
+          "description": "An object shall not be copied to an overlapping object.",
+          "kind": "problem",
+          "name": "An object shall not be copied to an overlapping object",
+          "precision": "high",
+          "severity": "error",
+          "short_name": "ObjectCopiedToAnOverlappingObject",
           "tags": [
             "correctness"
           ]
diff --git a/rule_packages/cpp/Representation.json b/rule_packages/cpp/Representation.json
@@ -50,6 +50,7 @@
           "precision": "high",
           "severity": "error",
           "short_name": "ObjectAssignedToAnOverlappingObject",
+          "shared_implementation_short_name": "OverlappingObjectAssignment",
           "tags": [
             "correctness"
           ]

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+\| test.c:9:3:9:11 \| ... = ... \| An object $@ assigned to overlapping object $@. \| test.c:9:5:9:5 \| l \| l \| test.c:9:11:9:11 \| i \| i \|`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+// GENERATED FILE - DO NOT MODIFY`
	`2`	`+import codingstandards.cpp.rules.overlappingobjectassignment.OverlappingObjectAssignment`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+c/common/test/rules/overlappingobjectassignment/OverlappingObjectAssignment.ql`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+rules/RULE-19-1/ObjectCopiedToAnOverlappingObject.ql`