Skip to content

C++: Provide BarrierGuard API without a Unit column when instantiating non-parameterized BarrierGuards#21360

Merged
MathiasVP merged 3 commits intogithub:mainfrom
microsoft:unbreak-changes
Feb 24, 2026
Merged

C++: Provide BarrierGuard API without a Unit column when instantiating non-parameterized BarrierGuards#21360
MathiasVP merged 3 commits intogithub:mainfrom
microsoft:unbreak-changes

Conversation

@MathiasVP
Copy link
Contributor

This PR fixes an accidental syntactical breakage I introduced in #21185.

In that PR I added barrier guards via MaD parameters to match the feature in other languages.

As part of that work we needed to support BarrierGuards parameterize by an arbitrary state column (see this commit). In order to reduce code duplication I then implemented the non-parameterized BarrierGuard API in terms of the parameterized BarrierGuard API with a Unit parameter.

However, I forgot that this now meant that you had to specify a Unit parameter in a couple of places whereas you didn't need to do that before. In particular, before the predicate BarrierGuard<...>::getAnIndirectBarrierNode/1 referred to a predicate with an indirection index column, but it now referred to a predicate with a Unit column 🤦 This was detected when Microsoft tried to upgrade to 2.24.2.

This PR fixes this by bringing the interface back to the old API. Sadly, this is yet another syntactic breakage, but:

  1. The API isn't very heavily used (it broke 1 query at Microsoft)
  2. The fix people need to apply is just to revert their 2.24.2 fix.

Copilot AI review requested due to automatic review settings February 24, 2026 12:45
@MathiasVP MathiasVP requested a review from a team as a code owner February 24, 2026 12:45
@github-actions github-actions bot added the C++ label Feb 24, 2026
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

Restores the pre-2.24.2 BarrierGuard / InstructionBarrierGuard non-parameterized API shape (no Unit column) while still delegating internally to the parameterized implementations, fixing a syntactic break introduced in #21185.

Changes:

  • Reintroduce non-parameterized getABarrierNode/0 and getAnIndirectBarrierNode/1 wrappers that hide the internal Unit parameter.
  • Add/adjust API-level documentation for the restored BarrierGuard wrapper and add wrapper methods for InstructionBarrierGuard.
Comments suppressed due to low confidence (1)

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll:2679

  • In this example the described blocked flow (x = source() to sink(x)) doesn’t match the code snippet, which assigns to *p and calls sink(*p). Updating the text to match the example (or adjusting the example to introduce x) would avoid confusion for users.
   * will block flow from `x = source()` to `sink(x)`.
   *

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

@MathiasVP
Copy link
Contributor Author

DCA was uneventful (as expected)

@MathiasVP MathiasVP merged commit 266130b into github:main Feb 24, 2026
22 checks passed
ropwareJB added a commit to microsoft/codeql that referenced this pull request Feb 24, 2026
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