Address feedback comments

Author: knewbury01

cpp/common/src/codingstandards/cpp/lifetimes/CppObjects.qll

@@ -144,30 +144,29 @@ abstract class ObjectIdentityBase extends Element {
  * Note that this does not hold for `e->x` or `e[x]` where `e` is a pointer.
  */
 Expr getASubobjectAccessOf(Expr e) {
-  exists(Field f |
-    not f.getUnderlyingType() instanceof ReferenceType and
-    f.getAnAccess().(FieldAccess) = result.getAChild*()
-  ) and
-  (
-    result = e
-    or
-    result.(DotFieldAccess).getQualifier() = getASubobjectAccessOf(e)
-  )
+  result = e
+  or
+  result.(DotFieldAccess).getQualifier() = getASubobjectAccessOf(e) and
+  not result.(DotFieldAccess).getTarget().getUnderlyingType() instanceof ReferenceType
   or
   result.(ArrayExpr).getArrayBase() = getASubobjectAccessOf(e) and
   not result.(ArrayExpr).getArrayBase().getUnspecifiedType() instanceof PointerType
 }
 
 /**
- * gets an access where the pointee is the subobject
+ * Finds subobjects of the pointee for expression `e`,
+ * where `e` has the type pointer type.
+ *
+ * For `e` this will be subobject accesses of `*e`.
+ * Or for `e->x` the subobject access is `x`.
  */
 Expr getASubobjectAccessOfPointee(Expr e) {
-  e.getParent() instanceof AddressOfExpr and
+  e.getParent() instanceof PointerDereferenceExpr and
   result = getASubobjectAccessOf(e.getParent())
   or
-  // the accessed field is a pointer to a subobject
-  e.getParent().(PointerFieldAccess).getTarget().getUnspecifiedType() instanceof PointerType and
-  result = getASubobjectAccessOf(e.getParent())
+  // for e1->e2 : getASubobjectAccessOfPointee(e1) = e2 and or subobjects of e2
+  result = getASubobjectAccessOf(e.getParent().(PointerFieldAccess)) and
+  not result.(PointerFieldAccess).getTarget().getUnderlyingType() instanceof ReferenceType
 }
 
 /**

cpp/misra/src/rules/RULE-6-8-4/MemberFunctionsRefqualified.ql

@@ -36,14 +36,11 @@ class MembersReturningObject extends MembersReturningObjectOrSubobject {
     exists(ReturnStmt r, ThisExpr t |
       r.getEnclosingFunction() = this and
       (
-        //direct access only
+        //return `this`
         r.getAChild() = t
         or
-        //or one level of indirection
-        exists(PointerDereferenceExpr p |
-          p.getAChild() = t and
-          r.getAChild() = p
-        )
+        //accesses of subobjects through the `this` pointer
+        r.getAChild() = getASubobjectAccessOf(t)
       ) and
       t.getActualType().stripType() = this.getDeclaringType()
     )
@@ -60,16 +57,16 @@ class MembersReturningSubObject extends MembersReturningObjectOrSubobject {
   MembersReturningSubObject() {
     exists(ReturnStmt r, FieldAccess access, Expr e |
       r.getEnclosingFunction() = this and
-      //direct access only
-      r.getAChild() = e and
       (
-        //pointer or reference to pointer subobject returned
-        e = getASubobjectAccessOfPointee(access) and
-        (e.getType() instanceof PointerType or e.getType() instanceof ReferenceType)
+        //subobject returned by address
+        r.getAChild() = access.getParent() and
+        e = getASubobjectAccessOf(access) and
+        access.getParent() instanceof AddressOfExpr
         or
         //reference to subobject returned
-        (this.getType() instanceof ReferenceType or e.getType() instanceof ReferenceType) and
-        not access.getTarget().getType() instanceof PointerType
+        r.getAChild() = e and
+        e = getASubobjectAccessOf(access) and
+        this.getType() instanceof ReferenceType
       )
     )
   }
@@ -117,3 +114,6 @@ where
   not f instanceof AppropriatelyQualified and
   not f instanceof DefaultedAssignmentOperator
 select f, "Member function is not properly ref qualified."
+// from Expr e, PointerFieldAccess p
+// where e.getParent() = p
+// select e, p, p.getTarget(), p.getQualifier(), p.getParent()

cpp/misra/test/rules/RULE-6-8-4/MemberFunctionsRefqualified.expected

@@ -1,20 +1,13 @@
 | test.cpp:12:14:12:20 | Members returning object or subobject | Member function is not properly ref qualified. |
 | test.cpp:24:12:24:23 | Members returning object or subobject | Member function is not properly ref qualified. |
-| test.cpp:28:6:28:18 | Members returning object or subobject | Member function is not properly ref qualified. |
 | test.cpp:42:16:42:16 | Members returning object or subobject | Member function is not properly ref qualified. |
 | test.cpp:42:16:42:16 | Members returning object or subobject | Member function is not properly ref qualified. |
 | test.cpp:42:16:42:16 | Members returning object or subobject | Member function is not properly ref qualified. |
 | test.cpp:61:8:61:8 | Members returning object or subobject | Member function is not properly ref qualified. |
 | test.cpp:71:9:71:10 | Members returning object or subobject | Member function is not properly ref qualified. |
 | test.cpp:79:8:79:9 | Members returning object or subobject | Member function is not properly ref qualified. |
-| test.cpp:82:8:82:9 | Members returning object or subobject | Member function is not properly ref qualified. |
-| test.cpp:85:8:85:9 | Members returning object or subobject | Member function is not properly ref qualified. |
 | test.cpp:89:9:89:10 | Members returning object or subobject | Member function is not properly ref qualified. |
-| test.cpp:93:9:93:10 | Members returning object or subobject | Member function is not properly ref qualified. |
 | test.cpp:103:8:103:8 | Members returning object or subobject | Member function is not properly ref qualified. |
 | test.cpp:113:9:113:10 | Members returning object or subobject | Member function is not properly ref qualified. |
 | test.cpp:121:8:121:9 | Members returning object or subobject | Member function is not properly ref qualified. |
-| test.cpp:124:8:124:9 | Members returning object or subobject | Member function is not properly ref qualified. |
-| test.cpp:127:8:127:9 | Members returning object or subobject | Member function is not properly ref qualified. |
 | test.cpp:131:9:131:10 | Members returning object or subobject | Member function is not properly ref qualified. |
-| test.cpp:135:9:135:10 | Members returning object or subobject | Member function is not properly ref qualified. |

cpp/misra/test/rules/RULE-6-8-4/test.cpp

@@ -80,18 +80,18 @@ class B {
     return i; // NON_COMPLIANT
   }
   int &f6() {
-    return *ip; // COMPLIANT[FALSE_POSITIVE] -- reads pointer
+    return *ip; // COMPLIANT -- reads pointer
   }
   int &f7() {
-    return **ip2; // COMPLIANT[FALSE_POSITIVE]-- reads pointer
+    return **ip2; // COMPLIANT -- reads pointer
   }
 
   int *&f8() {
     // return &p; // won't compile
     return ip; // NON_COMPLIANT
   }
   int *&f9() {
-    return *ip2; // COMPLIANT[FALSE_POSITIVE] -- reads pointer
+    return *ip2; // COMPLIANT -- reads pointer
   }
 };
 
@@ -122,17 +122,17 @@ class D {
     return this->i; // NON_COMPLIANT
   }
   int &f6() {
-    return *this->ip; // COMPLIANT[FALSE_POSITIVE] -- reads pointer
+    return *this->ip; // COMPLIANT -- reads pointer
   }
   int &f7() {
-    return **this->ip2; // COMPLIANT[FALSE_POSITIVE] -- reads pointer
+    return **this->ip2; // COMPLIANT -- reads pointer
   }
 
   int *&f8() {
     // return &p; // won't compile
     return this->ip; // NON_COMPLIANT
   }
   int *&f9() {
-    return *this->ip2; // COMPLIANT[FALSE_POSITIVE] -- reads pointer
+    return *this->ip2; // COMPLIANT -- reads pointer
   }
 };