Skip to content

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

Open
geoffw0 wants to merge 4 commits intogithub:mainfrom
geoffw0:intmultlong
Open

C++: Fix BMN issue with cpp/integer-multiplication-cast-to-long.#21457
geoffw0 wants to merge 4 commits intogithub:mainfrom
geoffw0:intmultlong

Conversation

@geoffw0
Copy link
Contributor

@geoffw0 geoffw0 commented Mar 11, 2026

Fix an issue with cpp/integer-multiplication-cast-to-long in Build Mode Node databases. The test was inspired by a case in bminor_glibc where fabsf had two return types in the database (float and int, implying interference from an implicit definition perhaps?). The fix has been made a new predicate of Function, since I anticipate this issue may surface again in a couple of other queries involving types.

@geoffw0 geoffw0 requested a review from a team as a code owner March 11, 2026 18:41
@geoffw0 geoffw0 added the C++ label Mar 11, 2026
Copilot AI review requested due to automatic review settings March 11, 2026 18:41
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a reusable helper on Function to detect functions with non-unique return-type extraction (seen in build-mode-none databases), and documents the fix via a C++ change note.

Changes:

  • Add Function::hasAmbiguousReturnType() predicate to detect non-unique extracted return types.
  • Add a C++ change note entry describing the fix for cpp/integer-multiplication-cast-to-long.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

File Description
cpp/ql/lib/semmle/code/cpp/Function.qll Introduces a new Function predicate for identifying ambiguous/non-unique return types.
cpp/ql/src/change-notes/2026-03-11-integer-multiplication-cast-to-long.md Adds a release note for the query fix (but contains a build-mode wording typo).

You can also share your feedback on Copilot code review. Take the survey.

---
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.
Comment on lines +529 to +530
* Holds if this function has ambiguous return type (this occurs sometimes in
* Build Mode None).
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

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.

Copy link
Contributor

@jketema jketema left a comment

Choose a reason for hiding this comment

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

LGTM, with formatting fix of course. One thing we probably want to improve slightly below.

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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants