diff --git a/.vscode/tasks.json b/.vscode/tasks.json index e2b393727f..5915064e54 100644 --- a/.vscode/tasks.json +++ b/.vscode/tasks.json @@ -253,6 +253,7 @@ "Preprocessor3", "Preprocessor4", "Preprocessor5", + "Preprocessor6", "IntegerConversion", "Expressions", "DeadCode", diff --git a/c/common/src/codingstandards/c/IrreplaceableFunctionLikeMacro.qll b/c/common/src/codingstandards/c/IrreplaceableFunctionLikeMacro.qll new file mode 100644 index 0000000000..af62cacfd3 --- /dev/null +++ b/c/common/src/codingstandards/c/IrreplaceableFunctionLikeMacro.qll @@ -0,0 +1,58 @@ +import cpp +import codingstandards.cpp.Macro +import codingstandards.cpp.Naming + +/** + * Macros that cannot be replaced by functions + */ +abstract class IrreplaceableFunctionLikeMacro extends FunctionLikeMacro { } + +/** A function like macro that contains the use of a stringize or tokenize operator should not be replaced by a function. */ +private class StringizeOrTokenizeMacro extends IrreplaceableFunctionLikeMacro { + StringizeOrTokenizeMacro() { + exists(TokenPastingOperator t | t.getMacro() = this) or + exists(StringizingOperator s | s.getMacro() = this) + } +} + +/** A standard library function like macro that should not be replaced by a function. */ +private class StandardLibraryFunctionLikeMacro extends IrreplaceableFunctionLikeMacro { + StandardLibraryFunctionLikeMacro() { Naming::Cpp14::hasStandardLibraryMacroName(this.getName()) } +} + +/** A function like macro invocation as an `asm` argument cannot be replaced by a function. */ +private class AsmArgumentInvoked extends IrreplaceableFunctionLikeMacro { + AsmArgumentInvoked() { + any(AsmStmt s).getLocation().subsumes(this.getAnInvocation().getLocation()) + } +} + +/** A macro that is only invoked with constant arguments is more likely to be compile-time evaluated than a function call so do not suggest replacement. */ +private class OnlyConstantArgsInvoked extends IrreplaceableFunctionLikeMacro { + OnlyConstantArgsInvoked() { + forex(MacroInvocation mi | mi = this.getAnInvocation() | + //int/float literals + mi.getUnexpandedArgument(_).regexpMatch("\\d+") + or + //char literal or string literal, which is a literal surrounded by single quotes or double quotes + mi.getUnexpandedArgument(_).regexpMatch("('[^']*'|\"[^\"]*\")") + ) + } +} + +/** A function like macro invoked to initialize an object with static storage that cannot be replaced with a function call. */ +private class UsedToStaticInitialize extends IrreplaceableFunctionLikeMacro { + UsedToStaticInitialize() { + any(StaticStorageDurationVariable v).getInitializer().getExpr() = + this.getAnInvocation().getExpr() + } +} + +/** A function like macro that is called with an argument that is an operator that cannot be replaced with a function call. */ +private class FunctionLikeMacroWithOperatorArgument extends IrreplaceableFunctionLikeMacro { + FunctionLikeMacroWithOperatorArgument() { + exists(MacroInvocation mi | mi.getMacro() = this | + mi.getUnexpandedArgument(_) = any(Operation op).getOperator() + ) + } +} diff --git a/c/misra/src/rules/DIR-4-9/FunctionOverFunctionLikeMacro.ql b/c/misra/src/rules/DIR-4-9/FunctionOverFunctionLikeMacro.ql new file mode 100644 index 0000000000..e53294fba5 --- /dev/null +++ b/c/misra/src/rules/DIR-4-9/FunctionOverFunctionLikeMacro.ql @@ -0,0 +1,36 @@ +/** + * @id c/misra/function-over-function-like-macro + * @name DIR-4-9: A function should be used in preference to a function-like macro where they are interchangeable + * @description Using a function-like macro instead of a function can lead to unexpected program + * behaviour. + * @kind problem + * @precision medium + * @problem.severity recommendation + * @tags external/misra/id/dir-4-9 + * external/misra/audit + * maintainability + * readability + * external/misra/obligation/advisory + */ + +import cpp +import codingstandards.c.misra +import codingstandards.c.IrreplaceableFunctionLikeMacro + +predicate partOfConstantExpr(MacroInvocation i) { + exists(Expr e | + e.isConstant() and + not i.getExpr() = e and + i.getExpr().getParent+() = e + ) +} + +from FunctionLikeMacro m +where + not isExcluded(m, Preprocessor6Package::functionOverFunctionLikeMacroQuery()) and + not m instanceof IrreplaceableFunctionLikeMacro and + //macros can have empty body + not m.getBody().length() = 0 and + //function call not allowed in a constant expression (where constant expr is parent) + forall(MacroInvocation i | i = m.getAnInvocation() | not partOfConstantExpr(i)) +select m, "Macro used instead of a function." diff --git a/c/misra/test/rules/DIR-4-9/FunctionOverFunctionLikeMacro.expected b/c/misra/test/rules/DIR-4-9/FunctionOverFunctionLikeMacro.expected new file mode 100644 index 0000000000..22d614a183 --- /dev/null +++ b/c/misra/test/rules/DIR-4-9/FunctionOverFunctionLikeMacro.expected @@ -0,0 +1,2 @@ +| test.c:6:1:6:25 | #define MACRO4(x) (x + 1) | Macro used instead of a function. | +| test.c:11:1:11:48 | #define MACRO9() printf_custom("output = %d", 7) | Macro used instead of a function. | diff --git a/c/misra/test/rules/DIR-4-9/FunctionOverFunctionLikeMacro.qlref b/c/misra/test/rules/DIR-4-9/FunctionOverFunctionLikeMacro.qlref new file mode 100644 index 0000000000..831e6e3101 --- /dev/null +++ b/c/misra/test/rules/DIR-4-9/FunctionOverFunctionLikeMacro.qlref @@ -0,0 +1 @@ +rules/DIR-4-9/FunctionOverFunctionLikeMacro.ql \ No newline at end of file diff --git a/c/misra/test/rules/DIR-4-9/test.c b/c/misra/test/rules/DIR-4-9/test.c new file mode 100644 index 0000000000..50e6bdb042 --- /dev/null +++ b/c/misra/test/rules/DIR-4-9/test.c @@ -0,0 +1,40 @@ +#include + +#define MACRO(OP, L, R) ((L)OP(R)) // COMPLIANT +#define MACRO2(L, R) (L + R) // COMPLIANT +#define MACRO3(L, R) (L " " R " " L) // COMPLIANT +#define MACRO4(x) (x + 1) // NON_COMPLIANT +#define MACRO5(L, LR) (LR + 1) // COMPLIANT +#define MACRO6(x) printf_custom("output = %d", test##x) // COMPLIANT +#define MACRO7(x) #x // COMPLIANT +#define MACRO8(x) "NOP" // COMPLIANT +#define MACRO9() printf_custom("output = %d", 7) // NON_COMPLIANT +#define MACRO10(x) // COMPLIANT +#define MY_ASSERT(X) assert(X) // NON_COMPLIANT[FALSE_NEGATIVE] + +const char a1[MACRO2(1, 1) + 6]; +extern printf_custom(); +int test1; + +void f() { + int i = MACRO(+, 1, 1); + int i2 = MACRO2(7, 10); + + static int i3 = MACRO2(1, 1); + + char *i4 = MACRO3("prefix", "suffix"); + + int i5 = MACRO4(1); + + int i6 = MACRO4(MACRO2(1, 1)); + + int i7 = MACRO5(1, 1); + + MACRO6(1); + + char *i10 = MACRO7("prefix"); + + asm(MACRO8(1)); + + MY_ASSERT(1); +} \ No newline at end of file diff --git a/cpp/common/src/codingstandards/cpp/exclusions/c/Preprocessor6.qll b/cpp/common/src/codingstandards/cpp/exclusions/c/Preprocessor6.qll new file mode 100644 index 0000000000..a9fb45b284 --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/exclusions/c/Preprocessor6.qll @@ -0,0 +1,25 @@ +//** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY. **/ +import cpp +import RuleMetadata +import codingstandards.cpp.exclusions.RuleMetadata + +newtype Preprocessor6Query = TFunctionOverFunctionLikeMacroQuery() + +predicate isPreprocessor6QueryMetadata(Query query, string queryId, string ruleId) { + query = + // `Query` instance for the `functionOverFunctionLikeMacro` query + Preprocessor6Package::functionOverFunctionLikeMacroQuery() and + queryId = + // `@id` for the `functionOverFunctionLikeMacro` query + "c/misra/function-over-function-like-macro" and + ruleId = "DIR-4-9" +} + +module Preprocessor6Package { + Query functionOverFunctionLikeMacroQuery() { + //autogenerate `Query` type + result = + // `Query` type for `functionOverFunctionLikeMacro` query + TQueryC(TPreprocessor6PackageQuery(TFunctionOverFunctionLikeMacroQuery())) + } +} diff --git a/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll b/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll index ad05d9b737..4dc785d482 100644 --- a/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll +++ b/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll @@ -29,6 +29,7 @@ import Preprocessor2 import Preprocessor3 import Preprocessor4 import Preprocessor5 +import Preprocessor6 import SideEffects1 import SideEffects2 import Strings1 @@ -65,6 +66,7 @@ newtype TCQuery = TPreprocessor3PackageQuery(Preprocessor3Query q) or TPreprocessor4PackageQuery(Preprocessor4Query q) or TPreprocessor5PackageQuery(Preprocessor5Query q) or + TPreprocessor6PackageQuery(Preprocessor6Query q) or TSideEffects1PackageQuery(SideEffects1Query q) or TSideEffects2PackageQuery(SideEffects2Query q) or TStrings1PackageQuery(Strings1Query q) or @@ -101,6 +103,7 @@ predicate isQueryMetadata(Query query, string queryId, string ruleId) { isPreprocessor3QueryMetadata(query, queryId, ruleId) or isPreprocessor4QueryMetadata(query, queryId, ruleId) or isPreprocessor5QueryMetadata(query, queryId, ruleId) or + isPreprocessor6QueryMetadata(query, queryId, ruleId) or isSideEffects1QueryMetadata(query, queryId, ruleId) or isSideEffects2QueryMetadata(query, queryId, ruleId) or isStrings1QueryMetadata(query, queryId, ruleId) or diff --git a/rule_packages/c/Preprocessor6.json b/rule_packages/c/Preprocessor6.json new file mode 100644 index 0000000000..be0ae84851 --- /dev/null +++ b/rule_packages/c/Preprocessor6.json @@ -0,0 +1,25 @@ +{ + "MISRA-C-2012": { + "DIR-4-9": { + "properties": { + "obligation": "advisory" + }, + "queries": [ + { + "description": "Using a function-like macro instead of a function can lead to unexpected program behaviour.", + "kind": "problem", + "name": "A function should be used in preference to a function-like macro where they are interchangeable", + "precision": "medium", + "severity": "recommendation", + "short_name": "FunctionOverFunctionLikeMacro", + "tags": [ + "external/misra/audit", + "maintainability", + "readability" + ] + } + ], + "title": "A function should be used in preference to a function-like macro where they are interchangeable" + } + } +} \ No newline at end of file diff --git a/rules.csv b/rules.csv index b10e97a9ba..7b283db8b0 100644 --- a/rules.csv +++ b/rules.csv @@ -610,7 +610,7 @@ c,MISRA-C-2012,DIR-4-5,Yes,Advisory,,,Identifiers in the same name space with ov c,MISRA-C-2012,DIR-4-6,Yes,Advisory,,,typedefs that indicate size and signedness should be used in place of the basic numerical types,,Types,Hard, c,MISRA-C-2012,DIR-4-7,Yes,Required,,,"If a function returns error information, then that error information shall be tested",M0-3-2,Contracts,Import, c,MISRA-C-2012,DIR-4-8,Yes,Advisory,,,"If a pointer to a structure or union is never dereferenced within a translation unit, then the implementation of the object should be hidden",,Pointers1,Medium, -c,MISRA-C-2012,DIR-4-9,Yes,Advisory,,,A function should be used in preference to a function-like macro where they are interchangeable,,Preprocessor,Medium, +c,MISRA-C-2012,DIR-4-9,Yes,Advisory,,,A function should be used in preference to a function-like macro where they are interchangeable,,Preprocessor6,Medium,Audit c,MISRA-C-2012,DIR-4-10,Yes,Required,,,Precautions shall be taken in order to prevent the contents of a header file being included more than once,M16-2-3,Preprocessor2,Medium, c,MISRA-C-2012,DIR-4-11,Yes,Required,,,The validity of values passed to library functions shall be checked,,Contracts,Hard, c,MISRA-C-2012,DIR-4-12,Yes,Required,,,Dynamic memory allocation shall not be used,,Banned,Medium,