Skip to content

Fix False Positives of M5-0-12 #925

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -16,23 +16,237 @@
import cpp
import codingstandards.cpp.autosar

from Variable v, Expr aexp
newtype TTemplateElement =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love that you're solving this directly. I think this type (and TemplateElement) could be moved into cpp/common/src/codinstandards/cpp under a qll (like Templates.qll or something).

I think it also might be less verbose if you made a common Element subclass, like TemplateElement extends Element with characteristic predicate this instanceof TemplateClass or this instanceof TemplateFunction or ....

TTemplateClass(TemplateClass c) or
TTemplateFunction(TemplateFunction f) or
TTemplateVariable(TemplateVariable v)

/**
* A templated element. These are either templated classes, templated functions,
* or templated variables.
*/
class TemplateElement extends TTemplateElement {
TemplateClass asTemplateClass() { this = TTemplateClass(result) }

TemplateFunction asTemplateFunction() { this = TTemplateFunction(result) }

TemplateVariable asTemplateVariable() { this = TTemplateVariable(result) }

string toString() {
result = this.asTemplateClass().toString() or
result = this.asTemplateFunction().toString() or
result = this.asTemplateVariable().toString()
}

Location getLocation() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you added Element getElement() then this could be getElement().getLocation(), and toString() could be getElement().toString()

result = this.asTemplateClass().getLocation() or
result = this.asTemplateFunction().getLocation() or
result = this.asTemplateVariable().getLocation()
}

string getName() {
result = this.asTemplateClass().getName() or
result = this.asTemplateFunction().getName() or
result = this.asTemplateVariable().getName()
}
}

newtype TTemplateInstantiation =
TClassTemplateInstantiation(ClassTemplateInstantiation c) or
TFunctionTemplateInstantiation(FunctionTemplateInstantiation f) or
TVariableTemplateInstantiation(VariableTemplateInstantiation v)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again, awesome and could be in a shared qll! And I think these could also be a class that extends Element?


/**
* An instantiation of a templated element, either a templated class, templated
* function, or templated variable.
*/
class TemplateInstantiation extends TTemplateInstantiation {
ClassTemplateInstantiation asClassTemplateInstantiation() {
this = TClassTemplateInstantiation(result)
}

FunctionTemplateInstantiation asFunctionTemplateInstantiation() {
this = TFunctionTemplateInstantiation(result)
}

VariableTemplateInstantiation asVariableTemplateInstantiation() {
this = TVariableTemplateInstantiation(result)
}

string toString() {
result = this.asClassTemplateInstantiation().toString() or
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This member predicate (and getLocation()) could be result = this.asElement().toString()/.getLocation().

result = this.asFunctionTemplateInstantiation().toString() or
result = this.asVariableTemplateInstantiation().toString()
}

Location getLocation() {
result = this.asClassTemplateInstantiation().getLocation() or
result = this.asFunctionTemplateInstantiation().getLocation() or
result = this.asVariableTemplateInstantiation().getLocation()
}

Element asElement() {
result = this.asClassTemplateInstantiation() or
result = this.asFunctionTemplateInstantiation() or
result = this.asVariableTemplateInstantiation()
}

/**
* Gets the template this instantiation is from, depending on the kind of the element
* this instantiation is for.
*/
TemplateElement getTemplate() {
result.asTemplateClass() = this.asClassTemplateInstantiation().getTemplate() or
result.asTemplateFunction() = this.asFunctionTemplateInstantiation().getTemplate() or
result.asTemplateVariable() = this.asVariableTemplateInstantiation().getTemplate()
}

/**
* Gets a use of an instantiation of this template. i.e.
* 1. For a class template, it's where the instantiated type is used by the name.
* 2. For a function template, it's where the instantiated function is called.
* 3. For a variable template, it's where the instantiated variable is initialized.
*/
Element getAUse() {
result = this.asClassTemplateInstantiation().getATypeNameUse() or
result = this.asFunctionTemplateInstantiation().getACallToThisFunction() or
result = this.asVariableTemplateInstantiation()
}
}

/**
* An implicit conversion from a plain char type to an explicitly signed or unsigned char
* type. `std::uint8_t` and `std::int8_t` are also considered as these char types.
*
* Note that this class only includes implicit conversions and does not include explicit
* type conversions, i.e. casts.
*/
class ImplicitConversionFromPlainCharType extends Conversion {
ImplicitConversionFromPlainCharType() {
this.isImplicit() and
this.getExpr().getUnspecifiedType() instanceof PlainCharType and
(
this.getUnspecifiedType() instanceof SignedCharType or
this.getUnspecifiedType() instanceof UnsignedCharType
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice on using .getUnspecifiedType()! I forgot to mention this quirk of writing cpp queries.

)
}
}

newtype TImplicitConversionElement =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is cool, nicely done.

In theory I think what you've written here could be used even more widely too. In the future, this could be a module under cpp/common/src/codingstandards/cpp/alertreporting with a parameterized module e.g.

signature class ElementSig = Element;
module TemplatableElement<ElementSig Elem> {
  newtype TTemplatableElement = TElementOutsideTemplate(Elem elem) { ... }
  or
  TElementInsideTemplate(TemplateInstantiation templateInstantiation, Elem elem) { ... };
  ...

TImplicitConversionOutsideTemplate(ImplicitConversionFromPlainCharType implicitConversion) {
not exists(TemplateInstantiation instantiation |
implicitConversion.isFromTemplateInstantiation(instantiation.asElement())
)
} or
TInstantiationOfImplicitConversionTemplate(
TemplateInstantiation templateInstantiation,
ImplicitConversionFromPlainCharType implicitConversion
) {
implicitConversion.getEnclosingElement+() = templateInstantiation.asElement()
Copy link
Preview

Copilot AI Jul 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of getEnclosingElement+() (transitive closure) could be expensive and may match unintended nested elements. Consider using a more specific predicate or adding bounds to limit the scope.

Copilot uses AI. Check for mistakes.

}

/**
* The locations where the implicit conversion from a plain char to an explicitly signed / unsigned
* char is taking place on a high level. It splits case on whether the conversion is caused by
* instantiating a template:
*
* - For conversions not due to template usage (i.e. outside a templated element), this refers to
* the same element as the one associated with the conversion.
* - For conversions due to template usage, this refers to the element that uses the instantiation
* of a template where an implicit char conversion happens.
*/
class ImplicitConversionLocation extends TImplicitConversionElement {
ImplicitConversionFromPlainCharType asImplicitConversionOutsideTemplate() {
this = TImplicitConversionOutsideTemplate(result)
}

TemplateInstantiation asInstantiationOfImplicitConversionTemplate(
ImplicitConversionFromPlainCharType implicitConversion
) {
this = TInstantiationOfImplicitConversionTemplate(result, implicitConversion)
}

/**
* Holds if this is a location of a conversion happening outside of a template.
*/
predicate isImplicitConversionOutsideTemplate() {
exists(this.asImplicitConversionOutsideTemplate())
}

/**
* Holds if this is a location of a conversion happening due to instantiating a
* template.
*/
predicate isInstantiationOfImplicitConversionTemplate() {
exists(
TemplateInstantiation templateInstantiation,
ImplicitConversionFromPlainCharType implicitConversion
|
templateInstantiation = this.asInstantiationOfImplicitConversionTemplate(implicitConversion)
)
}

/**
* Gets the implicit conversion that this location is associated with.
* - In cases of conversions not involving a template, this is the same as the
* location associated with the conversion.
* - In cases of conversions due to using a template, this is the conversion that
* happens in the instantiated template.
*/
ImplicitConversionFromPlainCharType getImplicitConversion() {
result = this.asImplicitConversionOutsideTemplate() or
exists(TemplateInstantiation templateInstantiation |
this = TInstantiationOfImplicitConversionTemplate(templateInstantiation, result)
)
}

string toString() {
result = this.asImplicitConversionOutsideTemplate().toString() or
exists(ImplicitConversionFromPlainCharType implicitConversion |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can just be result = this.asInstantiationOfImplicitConversionTemplate(_).toString(), both here and in a few other spots

result = this.asInstantiationOfImplicitConversionTemplate(implicitConversion).toString()
)
}

Location getLocation() {
result = this.asImplicitConversionOutsideTemplate().getLocation() or
exists(ImplicitConversionFromPlainCharType implicitConversion |
result = this.asInstantiationOfImplicitConversionTemplate(implicitConversion).getLocation()
)
}

Element asElement() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this name is a bit confusing, since I wouldn't expect it to call getAUse in the templated case. I might just name this getAUse() or maybe getAnOccurrence() or something like that?

A comment would suffice too!

result = this.asImplicitConversionOutsideTemplate() or
exists(ImplicitConversionFromPlainCharType implicitConversion |
result = this.asInstantiationOfImplicitConversionTemplate(implicitConversion).getAUse()
)
}
}

string getMessageTemplate(ImplicitConversionLocation implicitConversionLocation) {
exists(ImplicitConversionFromPlainCharType implicitConversion |
implicitConversion = implicitConversionLocation.getImplicitConversion()
|
implicitConversionLocation.isImplicitConversionOutsideTemplate() and
result =
"Implicit conversion of plain char $@ to '" + implicitConversion.getType().getName() + "'."
or
implicitConversionLocation.isInstantiationOfImplicitConversionTemplate() and
result =
"Implicit conversion of plain char $@ to '" + implicitConversion.getType().getName() +
"' from instantiating template '" +
implicitConversionLocation
.asInstantiationOfImplicitConversionTemplate(implicitConversion)
.getTemplate()
.getName() + "'."
)
}

from
ImplicitConversionLocation implicitConversionLocation,
ImplicitConversionFromPlainCharType implicitConversion
where
not isExcluded(v,
not isExcluded(implicitConversionLocation.asElement(),
StringsPackage::signedCharAndUnsignedCharTypeShallOnlyBeUsedForTheStorageAndUseOfNumericValuesQuery()) and
// We find cases where it is an explicitly signed char type with an assignment
// to a non-numeric type. NOTE: This rule addresses cases where the char type
// is used character data only, the rule does not explicitly cover this.
// Please see M5-0-11 for explicit handling of this case. Get types that are
// char, except for ones that are 'plain', meaning the sign is explicit.
(
v.getUnspecifiedType() instanceof SignedCharType or
v.getUnspecifiedType() instanceof UnsignedCharType
) and
// Identify places where these explicitly signed types are being assigned to a
// non-numeric type.
aexp = v.getAnAssignedValue() and
aexp.getUnspecifiedType() instanceof CharType
select aexp,
"Assignment of an non-integer type to variable $@ which is a variable with an explicitly signed char type",
v, v.getName()
implicitConversion = implicitConversionLocation.getImplicitConversion()
select implicitConversionLocation.asElement(), getMessageTemplate(implicitConversionLocation),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

select implicitConversionLocation, ... might also work, since you define getLocation()

implicitConversion.getExpr(), "expression"
Original file line number Diff line number Diff line change
@@ -1,4 +1,28 @@
| test.cpp:4:22:4:24 | 99 | Assignment of an non-integer type to variable $@ which is a variable with an explicitly signed char type | test.cpp:4:17:4:18 | a1 | a1 |
| test.cpp:6:20:6:22 | 99 | Assignment of an non-integer type to variable $@ which is a variable with an explicitly signed char type | test.cpp:6:15:6:16 | a3 | a3 |
| test.cpp:9:20:9:22 | 99 | Assignment of an non-integer type to variable $@ which is a variable with an explicitly signed char type | test.cpp:9:15:9:16 | a5 | a5 |
| test.cpp:12:21:12:23 | 99 | Assignment of an non-integer type to variable $@ which is a variable with an explicitly signed char type | test.cpp:12:16:12:17 | a7 | a7 |
| test.cpp:93:7:93:9 | (unsigned char)... | Implicit conversion of plain char $@ to 'unsigned char'. | test.cpp:93:7:93:9 | 118 | expression |
| test.cpp:94:21:94:23 | (signed char)... | Implicit conversion of plain char $@ to 'signed char'. | test.cpp:94:21:94:23 | 118 | expression |
| test.cpp:102:7:102:9 | (unsigned char)... | Implicit conversion of plain char $@ to 'unsigned char'. | test.cpp:102:7:102:9 | 118 | expression |
| test.cpp:103:21:103:23 | (signed char)... | Implicit conversion of plain char $@ to 'signed char'. | test.cpp:103:21:103:23 | 118 | expression |
| test.cpp:121:7:121:8 | (unsigned char)... | Implicit conversion of plain char $@ to 'unsigned char'. | test.cpp:121:7:121:8 | x3 | expression |
| test.cpp:124:20:124:21 | (signed char)... | Implicit conversion of plain char $@ to 'signed char'. | test.cpp:124:20:124:21 | x4 | expression |
| test.cpp:134:21:134:22 | (uint8_t)... | Implicit conversion of plain char $@ to 'uint8_t'. | test.cpp:134:21:134:22 | x7 | expression |
| test.cpp:137:20:137:21 | (int8_t)... | Implicit conversion of plain char $@ to 'int8_t'. | test.cpp:137:20:137:21 | x8 | expression |
| test.cpp:147:17:147:18 | definition of c3 | Implicit conversion of plain char $@ to 'unsigned char' from instantiating template 'C1<T, y>'. | test.cpp:5:12:5:12 | 120 | expression |
| test.cpp:150:17:150:18 | definition of c4 | Implicit conversion of plain char $@ to 'signed char' from instantiating template 'C2<T, y>'. | test.cpp:13:12:13:12 | 120 | expression |
| test.cpp:160:15:160:16 | definition of c7 | Implicit conversion of plain char $@ to 'uint8_t' from instantiating template 'C5<T, y>'. | test.cpp:22:12:22:12 | 1 | expression |
| test.cpp:163:15:163:16 | definition of c8 | Implicit conversion of plain char $@ to 'int8_t' from instantiating template 'C6<T, y>'. | test.cpp:30:12:30:12 | 1 | expression |
| test.cpp:180:7:180:10 | (unsigned char)... | Implicit conversion of plain char $@ to 'unsigned char'. | test.cpp:180:7:180:10 | * ... | expression |
| test.cpp:185:7:185:10 | (signed char)... | Implicit conversion of plain char $@ to 'signed char'. | test.cpp:185:7:185:10 | * ... | expression |
| test.cpp:200:7:200:10 | (uint8_t)... | Implicit conversion of plain char $@ to 'uint8_t'. | test.cpp:200:7:200:10 | * ... | expression |
| test.cpp:205:7:205:10 | (int8_t)... | Implicit conversion of plain char $@ to 'int8_t'. | test.cpp:205:7:205:10 | * ... | expression |
| test.cpp:219:6:219:7 | (unsigned char)... | Implicit conversion of plain char $@ to 'unsigned char'. | test.cpp:219:6:219:7 | a3 | expression |
| test.cpp:222:6:222:7 | (signed char)... | Implicit conversion of plain char $@ to 'signed char'. | test.cpp:222:6:222:7 | a4 | expression |
| test.cpp:232:6:232:7 | (uint8_t)... | Implicit conversion of plain char $@ to 'uint8_t'. | test.cpp:232:6:232:7 | a7 | expression |
| test.cpp:235:7:235:8 | (int8_t)... | Implicit conversion of plain char $@ to 'int8_t'. | test.cpp:235:7:235:8 | a8 | expression |
| test.cpp:249:3:249:4 | call to f5 | Implicit conversion of plain char $@ to 'unsigned char' from instantiating template 'f5'. | test.cpp:43:56:43:56 | x | expression |
| test.cpp:253:3:253:4 | call to f6 | Implicit conversion of plain char $@ to 'signed char' from instantiating template 'f6'. | test.cpp:44:54:44:54 | x | expression |
| test.cpp:266:3:266:5 | call to f13 | Implicit conversion of plain char $@ to 'uint8_t' from instantiating template 'f13'. | test.cpp:47:56:47:56 | x | expression |
| test.cpp:270:3:270:5 | call to f14 | Implicit conversion of plain char $@ to 'int8_t' from instantiating template 'f14'. | test.cpp:48:55:48:55 | x | expression |
| test.cpp:287:12:287:14 | definition of c11 | Implicit conversion of plain char $@ to 'unsigned char' from instantiating template 'C9<T>'. | test.cpp:52:15:52:15 | y | expression |
| test.cpp:292:13:292:15 | definition of c12 | Implicit conversion of plain char $@ to 'signed char' from instantiating template 'C10<T>'. | test.cpp:60:16:60:16 | y | expression |
| test.cpp:307:13:307:15 | definition of c15 | Implicit conversion of plain char $@ to 'uint8_t' from instantiating template 'C13<T>'. | test.cpp:69:16:69:16 | y | expression |
| test.cpp:311:13:311:15 | definition of c16 | Implicit conversion of plain char $@ to 'int8_t' from instantiating template 'C14<T>'. | test.cpp:77:16:77:16 | y | expression |
Loading
Loading