From 4a985762f57a4eafa3a65800697949dedcce5d6a Mon Sep 17 00:00:00 2001 From: Remco Vermeulen Date: Tue, 30 Jan 2024 16:18:32 -0800 Subject: [PATCH 1/4] Exclude assignment division and assignment modulo --- .../src/rules/A4-7-1/IntegerExpressionLeadToDataLoss.ql | 4 +++- cpp/autosar/test/rules/A4-7-1/test.cpp | 4 ++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/cpp/autosar/src/rules/A4-7-1/IntegerExpressionLeadToDataLoss.ql b/cpp/autosar/src/rules/A4-7-1/IntegerExpressionLeadToDataLoss.ql index aae951351a..a6d7abc456 100644 --- a/cpp/autosar/src/rules/A4-7-1/IntegerExpressionLeadToDataLoss.ql +++ b/cpp/autosar/src/rules/A4-7-1/IntegerExpressionLeadToDataLoss.ql @@ -30,5 +30,7 @@ where not e instanceof MulExpr and // Not covered by this query - overflow/underflow in division is rare not e instanceof DivExpr and - not e instanceof RemExpr + not e instanceof AssignDivExpr and + not e instanceof RemExpr and + not e instanceof AssignRemExpr select e, "Binary expression ..." + e.getOperator() + "... may overflow." diff --git a/cpp/autosar/test/rules/A4-7-1/test.cpp b/cpp/autosar/test/rules/A4-7-1/test.cpp index 7f6cbb7abe..8370cf0fc3 100644 --- a/cpp/autosar/test/rules/A4-7-1/test.cpp +++ b/cpp/autosar/test/rules/A4-7-1/test.cpp @@ -62,4 +62,8 @@ void test_loop_bound_bad(unsigned int n) { i++) { // NON_COMPLIANT - crement will overflow before loop bound is // reached } +} + +void test_assign_div(int i) { // COMPLIANT + i /= 2; } \ No newline at end of file From 66e3c3b1b22c73417601e69a449ad53f7977c07e Mon Sep 17 00:00:00 2001 From: Remco Vermeulen Date: Tue, 30 Jan 2024 16:19:13 -0800 Subject: [PATCH 2/4] Add changenote --- change_notes/2024-01-30-fix-fp-for-a4-7-1.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 change_notes/2024-01-30-fix-fp-for-a4-7-1.md diff --git a/change_notes/2024-01-30-fix-fp-for-a4-7-1.md b/change_notes/2024-01-30-fix-fp-for-a4-7-1.md new file mode 100644 index 0000000000..2c4a3d7d19 --- /dev/null +++ b/change_notes/2024-01-30-fix-fp-for-a4-7-1.md @@ -0,0 +1,2 @@ +`A4-7-1`: `IntegerExpressionLeadToDataLoss.ql` + - Fix #368: Incorrectly reporting `/=` as a cause for data loss. From 446ff6a43e0d262250be963d858b1600de927373 Mon Sep 17 00:00:00 2001 From: Remco Vermeulen Date: Thu, 8 Feb 2024 15:08:07 -0800 Subject: [PATCH 3/4] Limit operands to integer types --- cpp/common/src/codingstandards/cpp/Overflow.qll | 1 + 1 file changed, 1 insertion(+) diff --git a/cpp/common/src/codingstandards/cpp/Overflow.qll b/cpp/common/src/codingstandards/cpp/Overflow.qll index 130e1bb42d..324230a04c 100644 --- a/cpp/common/src/codingstandards/cpp/Overflow.qll +++ b/cpp/common/src/codingstandards/cpp/Overflow.qll @@ -14,6 +14,7 @@ import semmle.code.cpp.valuenumbering.GlobalValueNumbering */ class InterestingOverflowingOperation extends Operation { InterestingOverflowingOperation() { + forex(Expr operand | operand = this.getAnOperand() | operand.getUnderlyingType() instanceof IntegralType) and // Might overflow or underflow ( exprMightOverflowNegatively(this) From e5881a98e07c90a2a0c43ca711309b5652cfa2fd Mon Sep 17 00:00:00 2001 From: Remco Vermeulen Date: Thu, 8 Feb 2024 15:17:39 -0800 Subject: [PATCH 4/4] Revert "Limit operands to integer types" This reverts commit f9915445ef206ad3a0d91ecb92547abd50b0a804. This is already addressed in PR #490 --- cpp/common/src/codingstandards/cpp/Overflow.qll | 1 - 1 file changed, 1 deletion(-) diff --git a/cpp/common/src/codingstandards/cpp/Overflow.qll b/cpp/common/src/codingstandards/cpp/Overflow.qll index 324230a04c..130e1bb42d 100644 --- a/cpp/common/src/codingstandards/cpp/Overflow.qll +++ b/cpp/common/src/codingstandards/cpp/Overflow.qll @@ -14,7 +14,6 @@ import semmle.code.cpp.valuenumbering.GlobalValueNumbering */ class InterestingOverflowingOperation extends Operation { InterestingOverflowingOperation() { - forex(Expr operand | operand = this.getAnOperand() | operand.getUnderlyingType() instanceof IntegralType) and // Might overflow or underflow ( exprMightOverflowNegatively(this)