-
Notifications
You must be signed in to change notification settings - Fork 68
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
base: main
Are you sure you want to change the base?
Changes from all commits
9fa5cb2
fb54487
808b5fd
8002997
2ed4443
0710a38
11a277f
0fedbbb
10c3a2c
20a30cc
f3cee16
bb2ad88
a474ded
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,23 +16,237 @@ | |
import cpp | ||
import codingstandards.cpp.autosar | ||
|
||
from Variable v, Expr aexp | ||
newtype TTemplateElement = | ||
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() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you added |
||
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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
|
||
/** | ||
* 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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This member predicate (and |
||
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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice on using |
||
) | ||
} | ||
} | ||
|
||
newtype TImplicitConversionElement = | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. Positive FeedbackNegative Feedback |
||
} | ||
|
||
/** | ||
* 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 | | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can just be |
||
result = this.asInstantiationOfImplicitConversionTemplate(implicitConversion).toString() | ||
) | ||
} | ||
|
||
Location getLocation() { | ||
result = this.asImplicitConversionOutsideTemplate().getLocation() or | ||
exists(ImplicitConversionFromPlainCharType implicitConversion | | ||
result = this.asInstantiationOfImplicitConversionTemplate(implicitConversion).getLocation() | ||
) | ||
} | ||
|
||
Element asElement() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
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 | |
There was a problem hiding this comment.
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 (likeTemplates.qll
or something).I think it also might be less verbose if you made a common
Element
subclass, likeTemplateElement extends Element
with characteristic predicatethis instanceof TemplateClass or this instanceof TemplateFunction or ...
.