Skip to content

C++: Fix BMN issue with cpp/integer-multiplication-cast-to-long. #21457

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.

Sign up for GitHub

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
geoffw0 wants to merge 4 commits into
base: main
Choose a base branch
from
+45 −1
Open

C++: Fix BMN issue with cpp/integer-multiplication-cast-to-long. #21457

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
da7da80
C++: Add pseudo-buildless test cases (some missing declarations).
geoffw0 Mar 10, 2026
00d8a10
C++: Add Function.hasAmbiguousReturnType.
geoffw0 Mar 11, 2026
6552c84
C++: Fix BMN issue in cpp/integer-multiplication-cast-to-long.
geoffw0 Mar 11, 2026
4a39055
C++: Change note.
geoffw0 Mar 11, 2026
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
8 changes: 8 additions & 0 deletions cpp/ql/lib/semmle/code/cpp/Function.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,14 @@ class Function extends Declaration, ControlFlowNode, AccessHolder, @function {
not exists(NewOrNewArrayExpr new | e = new.getAllocatorCall().getArgument(0))
)
}

/**
* Holds if this function has ambiguous return type (this occurs sometimes in
* Build Mode None).
Comment on lines +529 to +530
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

The doc comment is a bit unclear/grammatically off: consider “has an ambiguous return type” and clarify what “ambiguous” means here (for example, whether it includes functions with zero extracted return types as well as multiple). Also consider aligning capitalization/wording with existing comments that refer to “build mode none”.

Suggested change
* Holds if this function has ambiguous return type (this occurs sometimes in
* Build Mode None).
* Holds if this function has an ambiguous return type, meaning that zero or
* multiple return types were extracted (this can occur in build mode none).

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@jketema jketema Mar 12, 2026

Choose a reason for hiding this comment

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

We might want to standardize this. In Compilation we use phrasing like "using the "none" build mode", and in the change notes we use build-mode: none.

*/
predicate hasAmbiguousReturnType() {
count(this.getType()) != 1
}
}

pragma[noinline]
Expand Down
4 changes: 3 additions & 1 deletion cpp/ql/src/Likely Bugs/Arithmetic/IntMultToLong.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,9 @@ where
// only report if we cannot prove that the result of the
// multiplication will be less (resp. greater) than the
// maximum (resp. minimum) number we can compute.
overflows(me, t1)
overflows(me, t1) and
// exclude cases where the expression type may not have been extracted accurately
not me.getParent().(Call).getTarget().hasAmbiguousReturnType()
select me,
"Multiplication result may overflow '" + me.getType().toString() + "' before it is converted to '"
+ me.getFullyConverted().getType().toString() + "'."
4 changes: 4 additions & 0 deletions cpp/ql/src/change-notes/2026-03-11-integer-multiplication-cast-to-long.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Fixed an issue with the "Multiplication result converted to larger type" (`cpp/integer-multiplication-cast-to-long`) query causing false positive results in Build Mode Node databases.
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

"Build Mode Node" looks like a typo and doesn’t match the terminology used elsewhere (for example, "build mode none"). Consider changing this to "Build Mode None" (or "build mode none" to match the CLI flag) to avoid confusion in the release note.

Suggested change
* Fixed an issue with the "Multiplication result converted to larger type" (`cpp/integer-multiplication-cast-to-long`) query causing false positive results in Build Mode Node databases.
* Fixed an issue with the "Multiplication result converted to larger type" (`cpp/integer-multiplication-cast-to-long`) query causing false positive results in build mode `none` databases.

Copilot uses AI. Check for mistakes.
28 changes: 28 additions & 0 deletions cpp/ql/test/query-tests/Likely Bugs/Arithmetic/IntMultToLong/Buildless.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// semmle-extractor-options: --expect_errors

void test_float_double1(float f, double d) {
float r1 = f * f; // GOOD
float r2 = f * d; // GOOD
double r3 = f * f; // BAD
double r4 = f * d; // GOOD

float f1 = fabsf(f * f); // GOOD
float f2 = fabsf(f * d); // GOOD
double f3 = fabs(f * f); // BAD [NOT DETECTED]
double f4 = fabs(f * d); // GOOD
}

double fabs(double f);
float fabsf(float f);

void test_float_double2(float f, double d) {
float r1 = f * f; // GOOD
float r2 = f * d; // GOOD
double r3 = f * f; // BAD
double r4 = f * d; // GOOD

float f1 = fabsf(f * f); // GOOD
float f2 = fabsf(f * d); // GOOD
double f3 = fabs(f * f); // BAD [NOT DETECTED]
double f4 = fabs(f * d); // GOOD
}
2 changes: 2 additions & 0 deletions cpp/ql/test/query-tests/Likely Bugs/Arithmetic/IntMultToLong/IntMultToLong.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
| Buildless.c:6:17:6:21 | ... * ... | Multiplication result may overflow 'float' before it is converted to 'double'. |
| Buildless.c:21:17:21:21 | ... * ... | Multiplication result may overflow 'float' before it is converted to 'double'. |
| IntMultToLong.c:4:10:4:14 | ... * ... | Multiplication result may overflow 'int' before it is converted to 'long long'. |
| IntMultToLong.c:7:16:7:20 | ... * ... | Multiplication result may overflow 'int' before it is converted to 'long long'. |
| IntMultToLong.c:18:19:18:23 | ... * ... | Multiplication result may overflow 'float' before it is converted to 'double'. |
Expand Down
Loading