diff --git a/c/common/test/rules/functionnoreturnattributecondition/FunctionNoReturnAttributeCondition.expected b/c/common/test/rules/functionnoreturnattributecondition/FunctionNoReturnAttributeCondition.expected new file mode 100644 index 0000000000..5aede0a5ba --- /dev/null +++ b/c/common/test/rules/functionnoreturnattributecondition/FunctionNoReturnAttributeCondition.expected @@ -0,0 +1,3 @@ +| test.c:9:16:9:31 | test_noreturn_f2 | The function test_noreturn_f2 declared with attribute _Noreturn returns a value. | +| test.c:34:16:34:31 | test_noreturn_f5 | The function test_noreturn_f5 declared with attribute _Noreturn returns a value. | +| test.c:49:32:49:47 | test_noreturn_f7 | The function test_noreturn_f7 declared with attribute _Noreturn returns a value. | diff --git a/c/common/test/rules/functionnoreturnattributecondition/FunctionNoReturnAttributeCondition.ql b/c/common/test/rules/functionnoreturnattributecondition/FunctionNoReturnAttributeCondition.ql new file mode 100644 index 0000000000..4af4aeceaf --- /dev/null +++ b/c/common/test/rules/functionnoreturnattributecondition/FunctionNoReturnAttributeCondition.ql @@ -0,0 +1,4 @@ +// GENERATED FILE - DO NOT MODIFY +import codingstandards.cpp.rules.functionnoreturnattributecondition.FunctionNoReturnAttributeCondition + +class TestFileQuery extends FunctionNoReturnAttributeConditionSharedQuery, TestQuery { } diff --git a/c/common/test/rules/functionnoreturnattributecondition/test.c b/c/common/test/rules/functionnoreturnattributecondition/test.c new file mode 100644 index 0000000000..1b0ba759e1 --- /dev/null +++ b/c/common/test/rules/functionnoreturnattributecondition/test.c @@ -0,0 +1,88 @@ +#include "setjmp.h" +#include "stdlib.h" +#include "threads.h" + +_Noreturn void test_noreturn_f1(int i) { // COMPLIANT + abort(); +} + +_Noreturn void test_noreturn_f2(int i) { // NON_COMPLIANT + if (i > 0) { + abort(); + } + if (i < 0) { + abort(); + } +} + +_Noreturn void test_noreturn_f3(int i) { // COMPLIANT + if (i > 0) { + abort(); + } + exit(1); +} + +void test_noreturn_f4(int i) { // COMPLIANT + if (i > 0) { + abort(); + } + if (i < 0) { + abort(); + } +} + +_Noreturn void test_noreturn_f5(int i) { // NON_COMPLIANT + if (i > 0) { + abort(); + } +} + +_Noreturn void test_noreturn_f6(int i) { // COMPLIANT + if (i > 0) { + abort(); + } + while (1) { + i = 5; + } +} + +__attribute__((noreturn)) void test_noreturn_f7(int i) { // NON_COMPLIANT + if (i > 0) { + abort(); + } +} + +__attribute__((noreturn)) void test_noreturn_f8(int i) { // COMPLIANT + abort(); +} + +_Noreturn void test_noreturn_f9(int i) { // COMPLIANT + test_noreturn_f1(i); +} + +_Noreturn void test_noreturn_f10(int i) { // COMPLIANT + switch (i) { + case 0: + abort(); + break; + case 1: + exit(0); + break; + case 2: + _Exit(0); + break; + case 3: + quick_exit(0); + break; + case 4: + thrd_exit(0); + break; + default: + jmp_buf jb; + longjmp(jb, 0); + } +} + +_Noreturn void test_noreturn_f11(int i) { // COMPLIANT + return test_noreturn_f11(i); +} \ No newline at end of file diff --git a/c/misra/src/rules/RULE-17-10/NonVoidReturnTypeOfNoreturnFunction.ql b/c/misra/src/rules/RULE-17-10/NonVoidReturnTypeOfNoreturnFunction.ql new file mode 100644 index 0000000000..68c5faeb1b --- /dev/null +++ b/c/misra/src/rules/RULE-17-10/NonVoidReturnTypeOfNoreturnFunction.ql @@ -0,0 +1,26 @@ +/** + * @id c/misra/non-void-return-type-of-noreturn-function + * @name RULE-17-10: A function declared with _noreturn shall have a return type of void + * @description Function declared with _noreturn will by definition not return a value, and should + * be declared to return void. + * @kind problem + * @precision very-high + * @problem.severity recommendation + * @tags external/misra/id/rule-17-10 + * correctness + * external/misra/obligation/required + */ + +import cpp +import codingstandards.c.misra +import codingstandards.cpp.Noreturn + +from NoreturnFunction f, Type returnType +where + not isExcluded(f, NoReturnPackage::nonVoidReturnTypeOfNoreturnFunctionQuery()) and + returnType = f.getType() and + not returnType instanceof VoidType and + not f.isCompilerGenerated() +select f, + "The function " + f.getName() + " is declared _noreturn but has a return type of " + + returnType.toString() + "." diff --git a/c/misra/src/rules/RULE-17-11/FunctionWithNoReturningBranchShouldBeNoreturn.ql b/c/misra/src/rules/RULE-17-11/FunctionWithNoReturningBranchShouldBeNoreturn.ql new file mode 100644 index 0000000000..90cb1af7c2 --- /dev/null +++ b/c/misra/src/rules/RULE-17-11/FunctionWithNoReturningBranchShouldBeNoreturn.ql @@ -0,0 +1,28 @@ +/** + * @id c/misra/function-with-no-returning-branch-should-be-noreturn + * @name RULE-17-11: A function without a branch that returns shall be declared with _Noreturn + * @description Functions which cannot return should be declared with _Noreturn. + * @kind problem + * @precision very-high + * @problem.severity recommendation + * @tags external/misra/id/rule-17-11 + * correctness + * external/misra/obligation/advisory + */ + +import cpp +import codingstandards.c.misra +import codingstandards.cpp.Noreturn + +from Function f +where + not isExcluded(f, NoReturnPackage::functionWithNoReturningBranchShouldBeNoreturnQuery()) and + not f instanceof NoreturnFunction and + not mayReturn(f) and + f.hasDefinition() and + not f.getName() = "main" and // Allowed exception; _Noreturn main() is undefined behavior. + // Harden against c++ cases. + not f.isFromUninstantiatedTemplate(_) and + not f.isDeleted() and + not f.isCompilerGenerated() +select f, "The function " + f.getName() + " cannot return and should be declared as _Noreturn." diff --git a/c/misra/src/rules/RULE-17-9/ReturnStatementInNoreturnFunction.ql b/c/misra/src/rules/RULE-17-9/ReturnStatementInNoreturnFunction.ql new file mode 100644 index 0000000000..360be01b7c --- /dev/null +++ b/c/misra/src/rules/RULE-17-9/ReturnStatementInNoreturnFunction.ql @@ -0,0 +1,21 @@ +/** + * @id c/misra/return-statement-in-noreturn-function + * @name RULE-17-9: Verify that a function declared with _Noreturn does not return + * @description Returning inside a function declared with _Noreturn is undefined behavior. + * @kind problem + * @precision very-high + * @problem.severity error + * @tags external/misra/id/rule-17-9 + * correctness + * external/misra/obligation/mandatory + */ + +import cpp +import codingstandards.c.misra +import codingstandards.cpp.rules.functionnoreturnattributecondition.FunctionNoReturnAttributeCondition + +class ReturnStatementInNoreturnFunctionQuery extends FunctionNoReturnAttributeConditionSharedQuery { + ReturnStatementInNoreturnFunctionQuery() { + this = NoReturnPackage::returnStatementInNoreturnFunctionQuery() + } +} diff --git a/c/misra/test/rules/RULE-17-10/NonVoidReturnTypeOfNoreturnFunction.expected b/c/misra/test/rules/RULE-17-10/NonVoidReturnTypeOfNoreturnFunction.expected new file mode 100644 index 0000000000..a94e37baa4 --- /dev/null +++ b/c/misra/test/rules/RULE-17-10/NonVoidReturnTypeOfNoreturnFunction.expected @@ -0,0 +1,4 @@ +| test.c:6:15:6:16 | f4 | The function f4 is declared _noreturn but has a return type of int. | +| test.c:19:15:19:16 | f8 | The function f8 is declared _noreturn but has a return type of int. | +| test.c:24:17:24:18 | f9 | The function f9 is declared _noreturn but has a return type of void *. | +| test.c:26:31:26:33 | f10 | The function f10 is declared _noreturn but has a return type of int. | diff --git a/c/misra/test/rules/RULE-17-10/NonVoidReturnTypeOfNoreturnFunction.qlref b/c/misra/test/rules/RULE-17-10/NonVoidReturnTypeOfNoreturnFunction.qlref new file mode 100644 index 0000000000..6726b6957a --- /dev/null +++ b/c/misra/test/rules/RULE-17-10/NonVoidReturnTypeOfNoreturnFunction.qlref @@ -0,0 +1 @@ +rules/RULE-17-10/NonVoidReturnTypeOfNoreturnFunction.ql \ No newline at end of file diff --git a/c/misra/test/rules/RULE-17-10/test.c b/c/misra/test/rules/RULE-17-10/test.c new file mode 100644 index 0000000000..b5fc988af2 --- /dev/null +++ b/c/misra/test/rules/RULE-17-10/test.c @@ -0,0 +1,26 @@ +#include "stdlib.h" + +void f1(); // COMPLIANT +int f2(); // COMPLIANT +_Noreturn void f3(); // COMPLIANT +_Noreturn int f4(); // NON-COMPLIANT + +void f5() { // COMPLIANT +} + +int f6() { // COMPLIANT + return 0; +} + +_Noreturn void f7() { // COMPLIANT + abort(); +} + +_Noreturn int f8() { // NON-COMPLIANT + abort(); + return 0; +} + +_Noreturn void *f9(); // NON-COMPLIANT + +__attribute__((noreturn)) int f10(); // NON-COMPLIANT \ No newline at end of file diff --git a/c/misra/test/rules/RULE-17-11/FunctionWithNoReturningBranchShouldBeNoreturn.expected b/c/misra/test/rules/RULE-17-11/FunctionWithNoReturningBranchShouldBeNoreturn.expected new file mode 100644 index 0000000000..ecb77a477c --- /dev/null +++ b/c/misra/test/rules/RULE-17-11/FunctionWithNoReturningBranchShouldBeNoreturn.expected @@ -0,0 +1,6 @@ +| test.c:7:6:7:21 | test_noreturn_f2 | The function test_noreturn_f2 cannot return and should be declared as _Noreturn. | +| test.c:18:6:18:21 | test_noreturn_f4 | The function test_noreturn_f4 cannot return and should be declared as _Noreturn. | +| test.c:47:6:47:21 | test_noreturn_f8 | The function test_noreturn_f8 cannot return and should be declared as _Noreturn. | +| test.c:63:6:63:22 | test_noreturn_f10 | The function test_noreturn_f10 cannot return and should be declared as _Noreturn. | +| test.c:97:6:97:22 | test_noreturn_f15 | The function test_noreturn_f15 cannot return and should be declared as _Noreturn. | +| test.c:101:6:101:22 | test_noreturn_f16 | The function test_noreturn_f16 cannot return and should be declared as _Noreturn. | diff --git a/c/misra/test/rules/RULE-17-11/FunctionWithNoReturningBranchShouldBeNoreturn.qlref b/c/misra/test/rules/RULE-17-11/FunctionWithNoReturningBranchShouldBeNoreturn.qlref new file mode 100644 index 0000000000..feb6f40804 --- /dev/null +++ b/c/misra/test/rules/RULE-17-11/FunctionWithNoReturningBranchShouldBeNoreturn.qlref @@ -0,0 +1 @@ +rules/RULE-17-11/FunctionWithNoReturningBranchShouldBeNoreturn.ql \ No newline at end of file diff --git a/c/misra/test/rules/RULE-17-11/test.c b/c/misra/test/rules/RULE-17-11/test.c new file mode 100644 index 0000000000..7baaea5821 --- /dev/null +++ b/c/misra/test/rules/RULE-17-11/test.c @@ -0,0 +1,104 @@ +#include "stdlib.h" + +_Noreturn void test_noreturn_f1(int i) { // COMPLIANT + abort(); +} + +void test_noreturn_f2(int i) { // NON_COMPLIANT + abort(); +} + +_Noreturn void test_noreturn_f3(int i) { // COMPLIANT + if (i > 0) { + abort(); + } + exit(1); +} + +void test_noreturn_f4(int i) { // NON_COMPLIANT + if (i > 0) { + abort(); + } + exit(1); +} + +void test_noreturn_f5(int i) { // COMPLIANT + if (i > 0) { + return; + } + exit(1); +} + +void test_noreturn_f6(int i) { // COMPLIANT + if (i > 0) { + abort(); + } + if (i < 0) { + abort(); + } +} + +void test_noreturn_f7(int i) { // COMPLIANT + if (i > 0) { + abort(); + } +} + +void test_noreturn_f8(int i) { // NON_COMPLIANT + if (i > 0) { + abort(); + } else { + abort(); + } +} + +_Noreturn void test_noreturn_f9(int i) { // COMPLIANT + if (i > 0) { + abort(); + } else { + abort(); + } +} + +void test_noreturn_f10(int i) { // NON_COMPLIANT + if (i > 0) { + abort(); + } + while (1) { + i = 5; + } +} + +_Noreturn void test_noreturn_f11(int i) { // COMPLIANT + if (i > 0) { + abort(); + } + while (1) { + i = 5; + } +} + +void test_noreturn_f12(); // COMPLIANT + +__attribute__((noreturn)) void test_noreturn_f13(int i) { // COMPLIANT + abort(); +} + +// Allowed by exception. It is undefined behavior for main() to be declared with +// noreturn. +int main(char **argv, int argc) { // COMPLIANT + abort(); +} + +_Noreturn void test_noreturn_f14(int i) { // COMPLIANT + test_noreturn_f1(i); +} + +void test_noreturn_f15(int i) { // NON_COMPLIANT + test_noreturn_f1(i); +} + +void test_noreturn_f16(int i) { // NON_COMPLIANT + // Infinite tail recursion + test_noreturn_f16(i); +} \ No newline at end of file diff --git a/c/misra/test/rules/RULE-17-9/ReturnStatementInNoreturnFunction.testref b/c/misra/test/rules/RULE-17-9/ReturnStatementInNoreturnFunction.testref new file mode 100644 index 0000000000..09a6d90538 --- /dev/null +++ b/c/misra/test/rules/RULE-17-9/ReturnStatementInNoreturnFunction.testref @@ -0,0 +1 @@ +c/common/test/rules/functionnoreturnattributecondition/FunctionNoReturnAttributeCondition.ql \ No newline at end of file diff --git a/change_notes/2024-09-28-improved-noreturn-rules.md b/change_notes/2024-09-28-improved-noreturn-rules.md new file mode 100644 index 0000000000..99fb4a0f46 --- /dev/null +++ b/change_notes/2024-09-28-improved-noreturn-rules.md @@ -0,0 +1,3 @@ + - `A7-6-1`, `MSC53-CPP`, `RULE-9-6-4` - `FunctionNoReturnAttbrituteCondition.qll` + - Analysis expanded from functions with "noreturn" attribute, now includes the "noreturn" specifier as well to handle new c rules. No difference in C++ results expected. + - Exclude compiler generated functions from being reported. \ No newline at end of file diff --git a/cpp/common/src/codingstandards/cpp/Noreturn.qll b/cpp/common/src/codingstandards/cpp/Noreturn.qll new file mode 100644 index 0000000000..eabe86b56e --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/Noreturn.qll @@ -0,0 +1,22 @@ +import cpp + +/** + * A function marked with _Noreturn or __attribute((noreturn)) + */ +class NoreturnFunction extends Function { + NoreturnFunction() { + this.getASpecifier().getName() = "noreturn" or + this.getAnAttribute().getName() = "noreturn" + } +} + +/** + * A function that may complete normally, and/or contains an explicit reachable + * return statement. + */ +predicate mayReturn(Function function) { + exists(ReturnStmt s | + function = s.getEnclosingFunction() and + s.getBasicBlock().isReachable() + ) +} diff --git a/cpp/common/src/codingstandards/cpp/exclusions/c/NoReturn.qll b/cpp/common/src/codingstandards/cpp/exclusions/c/NoReturn.qll new file mode 100644 index 0000000000..07b9360213 --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/exclusions/c/NoReturn.qll @@ -0,0 +1,61 @@ +//** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY. **/ +import cpp +import RuleMetadata +import codingstandards.cpp.exclusions.RuleMetadata + +newtype NoReturnQuery = + TNonVoidReturnTypeOfNoreturnFunctionQuery() or + TFunctionWithNoReturningBranchShouldBeNoreturnQuery() or + TReturnStatementInNoreturnFunctionQuery() + +predicate isNoReturnQueryMetadata(Query query, string queryId, string ruleId, string category) { + query = + // `Query` instance for the `nonVoidReturnTypeOfNoreturnFunction` query + NoReturnPackage::nonVoidReturnTypeOfNoreturnFunctionQuery() and + queryId = + // `@id` for the `nonVoidReturnTypeOfNoreturnFunction` query + "c/misra/non-void-return-type-of-noreturn-function" and + ruleId = "RULE-17-10" and + category = "required" + or + query = + // `Query` instance for the `functionWithNoReturningBranchShouldBeNoreturn` query + NoReturnPackage::functionWithNoReturningBranchShouldBeNoreturnQuery() and + queryId = + // `@id` for the `functionWithNoReturningBranchShouldBeNoreturn` query + "c/misra/function-with-no-returning-branch-should-be-noreturn" and + ruleId = "RULE-17-11" and + category = "advisory" + or + query = + // `Query` instance for the `returnStatementInNoreturnFunction` query + NoReturnPackage::returnStatementInNoreturnFunctionQuery() and + queryId = + // `@id` for the `returnStatementInNoreturnFunction` query + "c/misra/return-statement-in-noreturn-function" and + ruleId = "RULE-17-9" and + category = "mandatory" +} + +module NoReturnPackage { + Query nonVoidReturnTypeOfNoreturnFunctionQuery() { + //autogenerate `Query` type + result = + // `Query` type for `nonVoidReturnTypeOfNoreturnFunction` query + TQueryC(TNoReturnPackageQuery(TNonVoidReturnTypeOfNoreturnFunctionQuery())) + } + + Query functionWithNoReturningBranchShouldBeNoreturnQuery() { + //autogenerate `Query` type + result = + // `Query` type for `functionWithNoReturningBranchShouldBeNoreturn` query + TQueryC(TNoReturnPackageQuery(TFunctionWithNoReturningBranchShouldBeNoreturnQuery())) + } + + Query returnStatementInNoreturnFunctionQuery() { + //autogenerate `Query` type + result = + // `Query` type for `returnStatementInNoreturnFunction` query + TQueryC(TNoReturnPackageQuery(TReturnStatementInNoreturnFunctionQuery())) + } +} diff --git a/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll b/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll index c2771f4171..b3ed02f204 100644 --- a/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll +++ b/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll @@ -42,6 +42,7 @@ import Memory1 import Memory2 import Memory3 import Misc +import NoReturn import OutOfBounds import Pointers1 import Pointers2 @@ -113,6 +114,7 @@ newtype TCQuery = TMemory2PackageQuery(Memory2Query q) or TMemory3PackageQuery(Memory3Query q) or TMiscPackageQuery(MiscQuery q) or + TNoReturnPackageQuery(NoReturnQuery q) or TOutOfBoundsPackageQuery(OutOfBoundsQuery q) or TPointers1PackageQuery(Pointers1Query q) or TPointers2PackageQuery(Pointers2Query q) or @@ -184,6 +186,7 @@ predicate isQueryMetadata(Query query, string queryId, string ruleId, string cat isMemory2QueryMetadata(query, queryId, ruleId, category) or isMemory3QueryMetadata(query, queryId, ruleId, category) or isMiscQueryMetadata(query, queryId, ruleId, category) or + isNoReturnQueryMetadata(query, queryId, ruleId, category) or isOutOfBoundsQueryMetadata(query, queryId, ruleId, category) or isPointers1QueryMetadata(query, queryId, ruleId, category) or isPointers2QueryMetadata(query, queryId, ruleId, category) or diff --git a/cpp/common/src/codingstandards/cpp/rules/functionnoreturnattributecondition/FunctionNoReturnAttributeCondition.qll b/cpp/common/src/codingstandards/cpp/rules/functionnoreturnattributecondition/FunctionNoReturnAttributeCondition.qll index 2b910612cb..bb54a31df6 100644 --- a/cpp/common/src/codingstandards/cpp/rules/functionnoreturnattributecondition/FunctionNoReturnAttributeCondition.qll +++ b/cpp/common/src/codingstandards/cpp/rules/functionnoreturnattributecondition/FunctionNoReturnAttributeCondition.qll @@ -5,20 +5,30 @@ import cpp import codingstandards.cpp.Customizations import codingstandards.cpp.Exclusions +import codingstandards.cpp.Noreturn abstract class FunctionNoReturnAttributeConditionSharedQuery extends Query { } Query getQuery() { result instanceof FunctionNoReturnAttributeConditionSharedQuery } +/** + * `noreturn` functions are declared differently in c/c++. Attempt to match + * the description to the file; low risk if it chooses incorrectly. + */ +string describeNoreturn(Function f) { + if f.getFile().getExtension() = ["c", "C", "h", "H"] + then result = "_Noreturn" + else result = "[[noreturn]]" +} + /** * This checks that the return statement is reachable from the function entry point */ -query predicate problems(Function f, string message) { +query predicate problems(NoreturnFunction f, string message) { not isExcluded(f, getQuery()) and - f.getAnAttribute().getName() = "noreturn" and - exists(ReturnStmt s | - f = s.getEnclosingFunction() and - s.getBasicBlock().isReachable() - ) and - message = "The function " + f.getName() + " declared with attribute [[noreturn]] returns a value." + mayReturn(f) and + not f.isCompilerGenerated() and + message = + "The function " + f.getName() + " declared with attribute " + describeNoreturn(f) + + " returns a value." } diff --git a/rule_packages/c/NoReturn.json b/rule_packages/c/NoReturn.json new file mode 100644 index 0000000000..49cdb4c255 --- /dev/null +++ b/rule_packages/c/NoReturn.json @@ -0,0 +1,56 @@ +{ + "MISRA-C-2012": { + "RULE-17-10": { + "properties": { + "obligation": "required" + }, + "queries": [ + { + "description": "Function declared with _noreturn will by definition not return a value, and should be declared to return void.", + "kind": "problem", + "name": "A function declared with _noreturn shall have a return type of void", + "precision": "very-high", + "severity": "recommendation", + "short_name": "NonVoidReturnTypeOfNoreturnFunction", + "tags": ["correctness"] + } + ], + "title": "A function declared with _noreturn shall have a return type of void" + }, + "RULE-17-11": { + "properties": { + "obligation": "advisory" + }, + "queries": [ + { + "description": "Functions which cannot return should be declared with _Noreturn.", + "kind": "problem", + "name": "A function without a branch that returns shall be declared with _Noreturn", + "precision": "very-high", + "severity": "recommendation", + "short_name": "FunctionWithNoReturningBranchShouldBeNoreturn", + "tags": ["correctness"] + } + ], + "title": "A function without a branch that returns shall be declared with _Noreturn" + }, + "RULE-17-9": { + "properties": { + "obligation": "mandatory" + }, + "queries": [ + { + "description": "Returning inside a function declared with _Noreturn is undefined behavior.", + "kind": "problem", + "name": "Verify that a function declared with _Noreturn does not return", + "precision": "very-high", + "severity": "error", + "short_name": "ReturnStatementInNoreturnFunction", + "tags": ["correctness"], + "shared_implementation_short_name": "FunctionNoReturnAttributeCondition" + } + ], + "title": "Verify that a function declared with _Noreturn does not return" + } + } +} \ No newline at end of file