Skip to content

C#: Disallow magic in getAnAccess - #22631

Merged
MathiasVP merged 1 commit into
github:mainfrom
MathiasVP:no-magic-csharp
Sep 21, 2026
Merged

MathiasVP merged 1 commit into
github:mainfrom
MathiasVP:no-magic-csharp

Conversation

@MathiasVP

Copy link
Copy Markdown
Contributor

Microsoft had a case of bad magic in this predicate when we upgraded to CodeQL version 2.27.0. Here's a partial (awful) pipeline:

[2026-09-17 18:47:43] Evaluated non-recursive predicate m#Variable::Variable.getAnAccess/0#340bbb9f#fb@8b5f56i8 in 441814ms (evaluation was halted due to CancellationException).
Evaluated relational algebra for predicate m#Variable::Variable.getAnAccess/0#340bbb9f#fb@8b5f56i8 with tuple counts:
             7149985     ~3%    {1} r1 = SCAN `Expr::Expr.getEnclosingCallable/0#dispred#f6dee370` OUTPUT In.0
             7149985     ~3%    {1}    | STREAM DEDUP

        269552176500  ~1576%    {2} r2 = JOIN `DataFlowPublic::ExprNode.getExpr/0#dispred#ad3278f4_1#join_rhs` WITH @field CARTESIAN PRODUCT OUTPUT Rhs.0, Lhs.0
                   0     ~0%    {1}    | JOIN WITH Remote::AspNetQueryStringMember#dee4d488 ON FIRST 1 OUTPUT Lhs.1

             7149985     ~3%    {1} r3 = r1 UNION r2
                                return r3

This PR prevents magic in all the overrides of getAnAccess since I wouldn't expect this predicate to ever need magic in any override.

Copilot AI balanced review requested due to automatic review settings September 20, 2026 17:36
@MathiasVP
MathiasVP requested a review from a team as a code owner September 20, 2026 17:36
@MathiasVP MathiasVP added the C# label Sep 20, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The annotations consistently cover the relevant C# access predicate hierarchy without changing query semantics.

Review effort: Balanced
Findings: None

What changed in this PR

Prevents costly magic-set evaluation for C# access predicates.

Changes:

  • Adds pragma[nomagic] to base access predicates.
  • Applies it consistently to variable, field, property, indexer, and assignable-member overrides.
File Description
Variable.qll Disables magic for variable and field accesses.
Property.qll Disables magic for property and indexer accesses.
Member.qll Disables magic for member accesses.
Assignable.qll Disables magic for assignable access predicates.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@MathiasVP MathiasVP added the no-change-note-required This PR does not need a change note label Sep 20, 2026
@MathiasVP
MathiasVP merged commit 230dacd into github:main Sep 21, 2026
22 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C# 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.

3 participants