diff --git a/change_notes/2024-02-14-fix-fp-m0-1-4.md b/change_notes/2024-02-14-fix-fp-m0-1-4.md new file mode 100644 index 0000000000..43aa9f5723 --- /dev/null +++ b/change_notes/2024-02-14-fix-fp-m0-1-4.md @@ -0,0 +1,4 @@ +- `M0-1-4` - `SingleUseMemberPODVariable.ql`: + - Address FP reported in #388. Include aggregrate initialization as a use of a member. + - Include indirect initialization of members. For example, casting a pointer to a buffer to a struct pointer. + - Reformat the alert message to adhere to the style-guide. \ No newline at end of file diff --git a/cpp/autosar/src/rules/M0-1-4/SingleUseMemberPODVariable.ql b/cpp/autosar/src/rules/M0-1-4/SingleUseMemberPODVariable.ql index 5ac8f30160..c1dd812e80 100644 --- a/cpp/autosar/src/rules/M0-1-4/SingleUseMemberPODVariable.ql +++ b/cpp/autosar/src/rules/M0-1-4/SingleUseMemberPODVariable.ql @@ -24,5 +24,5 @@ where not isExcluded(v, DeadCodePackage::singleUseMemberPODVariableQuery()) and isSingleUseNonVolatilePODVariable(v) select v, - "Member POD variable " + v.getName() + " in " + v.getDeclaringType().getName() + " is only $@.", - getSingleUse(v), "used once" + "Member POD variable '" + v.getName() + "' in '" + v.getDeclaringType().getName() + + "' is only $@.", getSingleUse(v), "used once" diff --git a/cpp/autosar/src/rules/M0-1-4/SingleUsePODVariable.qll b/cpp/autosar/src/rules/M0-1-4/SingleUsePODVariable.qll index c4e220549a..83d78521a0 100644 --- a/cpp/autosar/src/rules/M0-1-4/SingleUsePODVariable.qll +++ b/cpp/autosar/src/rules/M0-1-4/SingleUsePODVariable.qll @@ -10,23 +10,68 @@ private string getConstExprValue(Variable v) { v.isConstexpr() } +/** + * Gets the number of uses of variable `v` in an opaque assignment, where an opaqua assignment for example a cast from one type to the other and `v` is assumed to be a member of the resulting type. + * e.g., + * struct foo { + * int bar; + * } + * + * struct foo * v = (struct foo*)buffer; + */ +Expr getIndirectSubObjectAssignedValue(MemberVariable subobject) { + // struct foo * ptr = (struct foo*)buffer; + exists(Struct someStruct, Variable instanceOfSomeStruct | someStruct.getAMember() = subobject | + instanceOfSomeStruct.getType().(PointerType).getBaseType() = someStruct and + exists(Cast assignedValue | + // Exclude cases like struct foo * v = nullptr; + not assignedValue.isImplicit() and + // `v` is a subobject of another type that reinterprets another object. We count that as a use of `v`. + assignedValue.getExpr() = instanceOfSomeStruct.getAnAssignedValue() and + result = assignedValue + ) + or + // struct foo; read(..., (char *)&foo); + instanceOfSomeStruct.getType() = someStruct and + exists(Call externalInitializerCall, Cast castToCharPointer, int n | + externalInitializerCall.getArgument(n).(AddressOfExpr).getOperand() = + instanceOfSomeStruct.getAnAccess() and + externalInitializerCall.getArgument(n) = castToCharPointer.getExpr() and + castToCharPointer.getType().(PointerType).getBaseType().getUnspecifiedType() instanceof + CharType and + result = externalInitializerCall + ) + or + // the object this subject is part of is initialized and we assumes this initializes the subobject. + instanceOfSomeStruct.getType() = someStruct and + result = instanceOfSomeStruct.getInitializer().getExpr() + ) +} + /** Gets a "use" count according to rule M0-1-4. */ int getUseCount(Variable v) { - exists(int initializers | - // We enforce that it's a POD type variable, so if it has an initializer it is explicit - (if v.hasInitializer() then initializers = 1 else initializers = 0) and - result = - initializers + - count(VariableAccess access | access = v.getAnAccess() and not access.isCompilerGenerated()) - + count(UserProvidedConstructorFieldInit cfi | cfi.getTarget() = v) + - // For constexpr variables used as template arguments, we don't see accesses (just the - // appropriate literals). We therefore take a conservative approach and count the number of - // template instantiations that use the given constant, and consider each one to be a use - // of the variable - count(ClassTemplateInstantiation cti | - cti.getTemplateArgument(_).(Expr).getValue() = getConstExprValue(v) - ) - ) + // We enforce that it's a POD type variable, so if it has an initializer it is explicit + result = + count(getAUserInitializedValue(v)) + + count(VariableAccess access | access = v.getAnAccess() and not access.isCompilerGenerated()) + + // For constexpr variables used as template arguments, we don't see accesses (just the + // appropriate literals). We therefore take a conservative approach and count the number of + // template instantiations that use the given constant, and consider each one to be a use + // of the variable + count(ClassTemplateInstantiation cti | + cti.getTemplateArgument(_).(Expr).getValue() = getConstExprValue(v) + ) + count(getIndirectSubObjectAssignedValue(v)) +} + +Expr getAUserInitializedValue(Variable v) { + ( + result = v.getInitializer().getExpr() + or + exists(UserProvidedConstructorFieldInit cfi | cfi.getTarget() = v and result = cfi.getExpr()) + or + exists(ClassAggregateLiteral l | not l.isCompilerGenerated() | result = l.getAFieldExpr(v)) + ) and + not result.isCompilerGenerated() } /** Gets a single use of `v`, if `isSingleUseNonVolatilePODVariable` holds. */ diff --git a/cpp/autosar/test/rules/M0-1-4/SingleUseMemberPODVariable.expected b/cpp/autosar/test/rules/M0-1-4/SingleUseMemberPODVariable.expected index f4309e7a4d..bfa053b318 100644 --- a/cpp/autosar/test/rules/M0-1-4/SingleUseMemberPODVariable.expected +++ b/cpp/autosar/test/rules/M0-1-4/SingleUseMemberPODVariable.expected @@ -1,9 +1,9 @@ -| test_global_or_namespace.cpp:16:7:16:7 | x | Member POD variable x in GA is only $@. | test_global_or_namespace.cpp:38:6:38:6 | x | used once | -| test_global_or_namespace.cpp:54:7:54:7 | x | Member POD variable x in N1A is only $@. | test_global_or_namespace.cpp:76:6:76:6 | x | used once | -| test_member.cpp:5:7:5:8 | m2 | Member POD variable m2 in A is only $@. | test_member.cpp:9:21:9:25 | constructor init of field m2 | used once | -| test_member.cpp:6:7:6:8 | m3 | Member POD variable m3 in A is only $@. | test_member.cpp:10:23:10:24 | m3 | used once | -| test_member.cpp:7:7:7:8 | m4 | Member POD variable m4 in A is only $@. | test_member.cpp:14:23:14:24 | m4 | used once | -| test_member.cpp:18:9:18:11 | sm1 | Member POD variable sm1 in s1 is only $@. | test_member.cpp:23:6:23:8 | sm1 | used once | -| test_member.cpp:36:7:36:8 | m1 | Member POD variable m1 in C is only $@. | test_member.cpp:39:21:39:22 | m1 | used once | -| test_member.cpp:37:7:37:8 | m2 | Member POD variable m2 in C is only $@. | test_member.cpp:46:5:46:6 | m2 | used once | -| test_member.cpp:55:5:55:6 | m3 | Member POD variable m3 in E is only $@. | test_member.cpp:56:27:56:32 | constructor init of field m3 | used once | +| test_global_or_namespace.cpp:16:7:16:7 | x | Member POD variable 'x' in 'GA' is only $@. | test_global_or_namespace.cpp:38:6:38:6 | x | used once | +| test_global_or_namespace.cpp:54:7:54:7 | x | Member POD variable 'x' in 'N1A' is only $@. | test_global_or_namespace.cpp:76:6:76:6 | x | used once | +| test_member.cpp:5:7:5:8 | m2 | Member POD variable 'm2' in 'A' is only $@. | test_member.cpp:9:21:9:25 | constructor init of field m2 | used once | +| test_member.cpp:6:7:6:8 | m3 | Member POD variable 'm3' in 'A' is only $@. | test_member.cpp:10:23:10:24 | m3 | used once | +| test_member.cpp:7:7:7:8 | m4 | Member POD variable 'm4' in 'A' is only $@. | test_member.cpp:14:23:14:24 | m4 | used once | +| test_member.cpp:18:9:18:11 | sm1 | Member POD variable 'sm1' in 's1' is only $@. | test_member.cpp:23:6:23:8 | sm1 | used once | +| test_member.cpp:36:7:36:8 | m1 | Member POD variable 'm1' in 'C' is only $@. | test_member.cpp:39:21:39:22 | m1 | used once | +| test_member.cpp:37:7:37:8 | m2 | Member POD variable 'm2' in 'C' is only $@. | test_member.cpp:46:5:46:6 | m2 | used once | +| test_member.cpp:55:5:55:6 | m3 | Member POD variable 'm3' in 'E' is only $@. | test_member.cpp:56:27:56:32 | constructor init of field m3 | used once | diff --git a/cpp/autosar/test/rules/M0-1-4/test_member.cpp b/cpp/autosar/test/rules/M0-1-4/test_member.cpp index a43ee5d799..b82987c8a6 100644 --- a/cpp/autosar/test/rules/M0-1-4/test_member.cpp +++ b/cpp/autosar/test/rules/M0-1-4/test_member.cpp @@ -72,4 +72,62 @@ void test_e() { // Ensure that the template E is fully instantiated e2.getT(); } +void test_fp_reported_in_388() { + struct s1 { + int m1; // COMPLIANT + }; + + s1 l1 = {1}; // m1 is used here + l1.m1; +} + +void test_array_initialized_members() { + struct s1 { + int m1; // COMPLIANT + }; + + struct s1 l1[] = { + {.m1 = 1}, + {.m1 = 2}, + }; + + l1[0].m1; +} + +void test_indirect_assigned_members(void *opaque) { + struct s1 { + int m1; // COMPLIANT + }; + + struct s1 *p = (struct s1 *)opaque; + p->m1; + + struct s2 { + int m1; // COMPLIANT + }; + + char buffer[sizeof(struct s2) + 8] = {0}; + struct s2 *l2 = (struct s2 *)&buffer[8]; + l2->m1; +} + +void test_external_assigned_members(void (*fp)(unsigned char *)) { + + struct s1 { + int m1; // COMPLIANT + }; + + struct s1 l1; + fp((unsigned char *)&l1); + l1.m1; + + struct s2 { + int m1; // COMPLIANT + }; + + struct s2 (*copy_init)(); + struct s2 l2 = copy_init(); + l2.m1; +} + } // namespace test \ No newline at end of file