Skip to content

Conversation

@aschackmull
Copy link
Contributor

@aschackmull aschackmull commented Dec 9, 2025

First commit as the title says - in particular, this allows BarrierGuards that work like assertions, i.e. they potentially throw instead of returning a boolean.
Second commit renames the module as the WithState naming felt a bit too constrained - it's not just (flow-)state, it's a general parameter that can be used for whatever.

Copilot AI review requested due to automatic review settings December 9, 2025 15:33
@aschackmull aschackmull added the no-change-note-required This PR does not need a change note label Dec 9, 2025
@aschackmull aschackmull requested review from a team as code owners December 9, 2025 15:33
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 generalizes the ValidationWrapper module in the Guards library to support GuardValue-based barrier guards and renames related components to better reflect their general parameter usage rather than being constrained to flow-state.

  • Changes the guardChecksSig signature from accepting boolean branch to accepting GuardValue gv
  • Renames StateSig to ParamSig, WithState to WithParam, and ValidationWrapperWithState to ParameterizedValidationWrapper
  • Updates Java and C++ implementations to adapt to the new GuardValue-based interface

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
shared/controlflow/codeql/controlflow/Guards.qll Generalizes validation wrapper signatures to use GuardValue instead of boolean, and renames state-related identifiers to more general parameter naming
java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll Adds adaptation layer to convert from boolean-based guard checks to GuardValue-based interface
cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaImpl.qll Updates module references and adds adaptation layer for GuardValue-based interface

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@aschackmull aschackmull force-pushed the guards/generalise-validationwrapper branch from af8f4e8 to 7d8e2d6 Compare December 10, 2025 08:33
Copy link
Contributor

@hvitved hvitved left a comment

Choose a reason for hiding this comment

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

LGTM, but could we have a test case?

@hvitved
Copy link
Contributor

hvitved commented Dec 10, 2025

Also, remember to run DCA.

@aschackmull aschackmull force-pushed the guards/generalise-validationwrapper branch from fd4a503 to eaa9686 Compare December 10, 2025 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C++ Java no-change-note-required This PR does not need a change note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants