Implement Concurrency8 package

Author: MichaelRFairhurst

c/cert/src/rules/CON34-C/AppropriateThreadObjectStorageDurations.ql

@@ -14,30 +14,39 @@
 
 import cpp
 import codingstandards.c.cert
+import codingstandards.c.Objects
 import codingstandards.cpp.Concurrency
 import codingstandards.cpp.dataflow.TaintTracking
 import codingstandards.cpp.dataflow.DataFlow
 import semmle.code.cpp.commons.Alloc
 
-from C11ThreadCreateCall tcc, StackVariable sv, Expr arg, Expr acc
+from C11ThreadCreateCall tcc, Expr arg
 where
   not isExcluded(tcc, Concurrency4Package::appropriateThreadObjectStorageDurationsQuery()) and
   tcc.getArgument(2) = arg and
-  sv.getAnAccess() = acc and
-  // a stack variable that is given as an argument to a thread
-  TaintTracking::localTaint(DataFlow::exprNode(acc), DataFlow::exprNode(arg)) and
-  // or isn't one of the allowed usage patterns
-  not exists(Expr mfc |
-    isAllocationExpr(mfc) and
-    sv.getAnAssignedValue() = mfc and
-    acc.getAPredecessor*() = mfc
-  ) and
-  not exists(TSSGetFunctionCall tsg, TSSSetFunctionCall tss, DataFlow::Node src |
-    sv.getAnAssignedValue() = tsg and
-    acc.getAPredecessor*() = tsg and
-    // there should be dataflow from somewhere (the same somewhere)
-    // into each of the first arguments
-    DataFlow::localFlow(src, DataFlow::exprNode(tsg.getArgument(0))) and
-    DataFlow::localFlow(src, DataFlow::exprNode(tss.getArgument(0)))
+  (
+    exists(ObjectIdentity obj, Expr acc |
+      obj.getASubobjectAccess() = acc and
+      obj.getStorageDuration().isAutomatic() and
+      exists(DataFlow::Node addrNode |
+        (
+          addrNode = DataFlow::exprNode(any(AddressOfExpr e | e.getOperand() = acc))
+          or
+          addrNode = DataFlow::exprNode(acc) and exists(ArrayToPointerConversion c | c.getExpr() = acc)
+        ) and
+        TaintTracking::localTaint(addrNode, DataFlow::exprNode(arg))
+      )
+    )
+    or
+    // TODO: Remove/replace with tss_t type check, see #801.
+    exists(TSSGetFunctionCall tsg |
+      TaintTracking::localTaint(DataFlow::exprNode(tsg), DataFlow::exprNode(arg)) and
+      not exists(TSSSetFunctionCall tss, DataFlow::Node src |
+        // there should be dataflow from somewhere (the same somewhere)
+        // into each of the first arguments
+        DataFlow::localFlow(src, DataFlow::exprNode(tsg.getArgument(0))) and
+        DataFlow::localFlow(src, DataFlow::exprNode(tss.getArgument(0)))
+      )
+    )
   )
 select tcc, "$@ not declared with appropriate storage duration", arg, "Shared object"

c/cert/src/rules/DCL30-C/AppropriateStorageDurationsFunctionReturn.ql

@@ -13,10 +13,15 @@
 
 import cpp
 import codingstandards.c.cert
+import codingstandards.c.Objects
 import codingstandards.cpp.dataflow.DataFlow
 
-class Source extends StackVariable {
-  Source() { not this instanceof Parameter }
+class Source extends Expr {
+  ObjectIdentity rootObject;
+  Source() {
+    rootObject.getStorageDuration().isAutomatic()
+    and this = rootObject.getASubobjectAddressExpr()
+  }
 }
 
 class Sink extends DataFlow::Node {
@@ -40,7 +45,7 @@ from DataFlow::Node src, DataFlow::Node sink
 where
   not isExcluded(sink.asExpr(),
     Declarations8Package::appropriateStorageDurationsFunctionReturnQuery()) and
-  exists(Source s | src.asExpr() = s.getAnAccess()) and
+  exists(Source s | src.asExpr() = s) and
   sink instanceof Sink and
   DataFlow::localFlow(src, sink)
 select sink, "$@ with automatic storage may be accessible outside of its lifetime.", src,

c/cert/src/rules/EXP35-C/DoNotModifyObjectsWithTemporaryLifetime.ql

@@ -13,15 +13,13 @@
 
 import cpp
 import codingstandards.c.cert
-import codingstandards.cpp.lifetimes.CLifetimes
+import codingstandards.c.Objects
 
 // Note: Undefined behavior is possible regardless of whether the accessed field from the returned
 // struct is an array or a scalar (i.e. arithmetic and pointer types) member, according to the standard.
-from FieldAccess fa, FunctionCall fc
+from FieldAccess fa, TemporaryObjectIdentity tempObject
 where
   not isExcluded(fa, InvalidMemory2Package::doNotModifyObjectsWithTemporaryLifetimeQuery()) and
-  not fa.getQualifier().isLValue() and
-  fa.getQualifier().getUnconverted() = fc and
-  fa.getQualifier().getUnconverted().getUnspecifiedType() instanceof StructOrUnionTypeWithArrayField
-select fa, "Field access on $@ qualifier occurs after its temporary object lifetime.", fc,
-  "function call"
+  fa.getQualifier().getUnconverted() = tempObject
+select fa, "Field access on $@ qualifier occurs after its temporary object lifetime.", tempObject,
+  "temporary object"

c/cert/src/rules/MEM33-C/AllocStructsWithAFlexibleArrayMemberDynamically.ql

@@ -14,14 +14,15 @@
 import cpp
 import codingstandards.c.cert
 import codingstandards.c.Variable
+import codingstandards.c.Objects
 import semmle.code.cpp.models.interfaces.Allocation
 import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
 
 abstract class FlexibleArrayAlloc extends Element {
   /**
    * Returns the `Variable` being allocated.
    */
-  abstract Variable getVariable();
+  abstract Element getReportElement();
 }
 
 /**
@@ -53,18 +54,25 @@ class FlexibleArrayStructDynamicAlloc extends FlexibleArrayAlloc, FunctionCall {
       )
   }
 
-  override Variable getVariable() { result = v }
+  override Element getReportElement() { result = v }
 }
 
 /**
  * A `Variable` of type `FlexibleArrayStructType` that is not allocated dynamically.
  */
-class FlexibleArrayNonDynamicAlloc extends FlexibleArrayAlloc, Variable {
+class FlexibleArrayNonDynamicAlloc extends FlexibleArrayAlloc {
+  ObjectIdentity object;
   FlexibleArrayNonDynamicAlloc() {
-    this.getUnspecifiedType().getUnspecifiedType() instanceof FlexibleArrayStructType
+    this = object and
+    not object.getStorageDuration().isAllocated() and
+    // Exclude temporaries. Though they should violate this rule, in practice these results are
+    // often spurious and redundant, such as (*x = *x) which creates an unused temporary object.
+    not object.hasTemporaryLifetime() and
+    object.getType().getUnspecifiedType() instanceof FlexibleArrayStructType
+    and not exists(Variable v | v.getInitializer().getExpr() = this)
   }
 
-  override Variable getVariable() { result = this }
+  override Element getReportElement() { result = object }
 }
 
 from FlexibleArrayAlloc alloc, string message
@@ -77,4 +85,4 @@ where
     alloc instanceof FlexibleArrayNonDynamicAlloc and
     message = "$@ contains a flexible array member but is not dynamically allocated."
   )
-select alloc, message, alloc.getVariable(), alloc.getVariable().getName()
+select alloc, message, alloc.getReportElement(), alloc.getReportElement().toString()

c/cert/test/rules/EXP35-C/DoNotModifyObjectsWithTemporaryLifetime.expected

@@ -1,4 +1,4 @@
-| test.c:65:18:65:18 | a | Field access on $@ qualifier occurs after its temporary object lifetime. | test.c:65:9:65:14 | call to get_s1 | function call |
-| test.c:67:18:67:19 | s1 | Field access on $@ qualifier occurs after its temporary object lifetime. | test.c:67:9:67:14 | call to get_s3 | function call |
-| test.c:68:18:68:19 | i1 | Field access on $@ qualifier occurs after its temporary object lifetime. | test.c:68:9:68:14 | call to get_s3 | function call |
-| test.c:69:18:69:21 | af12 | Field access on $@ qualifier occurs after its temporary object lifetime. | test.c:69:9:69:14 | call to get_s4 | function call |
+| test.c:65:18:65:18 | a | Field access on $@ qualifier occurs after its temporary object lifetime. | test.c:65:9:65:14 | call to get_s1 | temporary object |
+| test.c:67:18:67:19 | s1 | Field access on $@ qualifier occurs after its temporary object lifetime. | test.c:67:9:67:14 | call to get_s3 | temporary object |
+| test.c:68:18:68:19 | i1 | Field access on $@ qualifier occurs after its temporary object lifetime. | test.c:68:9:68:14 | call to get_s3 | temporary object |
+| test.c:69:18:69:21 | af12 | Field access on $@ qualifier occurs after its temporary object lifetime. | test.c:69:9:69:14 | call to get_s4 | temporary object |

c/common/src/codingstandards/c/IdentifierLinkage.qll

@@ -0,0 +1,47 @@
+import cpp
+
+newtype TIdentifierLinkage =
+  TIdentifierLinkageExternal() or
+  TIdentifierLinkageInternal() or
+  TIdentifierLinkageNone()
+
+/**
+ * In C, identifiers have internal linkage, or external linkage, or no linkage (6.2.2.1).
+ *
+ * The linkage of an identifier is used to, among other things, determine the storage duration
+ * and/or lifetime of that identifier. Storage durations and lifetimes are often used to define
+ * rules in the various coding standards.
+ */
+class IdentifierLinkage extends TIdentifierLinkage {
+  predicate isExternal() { this = TIdentifierLinkageExternal() }
+
+  predicate isInternal() { this = TIdentifierLinkageInternal() }
+
+  predicate isNone() { this = TIdentifierLinkageNone() }
+
+  string toString() {
+    this.isExternal() and result = "external linkage"
+    or
+    this.isInternal() and result = "internal linkage"
+    or
+    this.isNone() and result = "no linkage"
+  }
+}
+
+/**
+ * Determine the linkage of a variable: external, or static, or none.
+ *
+ * The linkage of a variable is determined by its scope and storage class. Note that other types of
+ * identifiers (e.g. functions) may also have linkage, but that behavior is not covered in this
+ * predicate.
+ */
+IdentifierLinkage linkageOfVariable(Variable v) {
+  // 6.2.2.3, file scope identifiers marked static have internal linkage.
+  v.isTopLevel() and v.isStatic() and result.isInternal()
+  or
+  // 6.2.2.4 describes generally non-static file scope identifiers, which have external linkage.
+  v.isTopLevel() and not v.isStatic() and result.isExternal()
+  or
+  // Note: Not all identifiers have linkage, see 6.2.2.6
+  not v.isTopLevel() and result.isNone()
+}