Skip to content

C#: Fix bad magic Element::fromSource in context of SelfAssignment.ql#7182

Merged
hvitved merged 1 commit intogithub:mainfrom
hvitved:csharp/self-assignment-bad-magic
Nov 22, 2021
Merged

C#: Fix bad magic Element::fromSource in context of SelfAssignment.ql#7182
hvitved merged 1 commit intogithub:mainfrom
hvitved:csharp/self-assignment-bad-magic

Conversation

@hvitved
Copy link
Copy Markdown
Contributor

@hvitved hvitved commented Nov 19, 2021

The optimizer decides to push in this magic context into Element::getLocation:

m#Element::Element::getLocation_dispred#bf(/* Element::Element */ cached unique entity this)
:-
  exists(/* Assignment::AssignExpr */ cached dontcare entity _,
         cached dontcare string _1 |
    rec #select#cpe#134#fff(_, this, _1)
  );
  ...

which makes StructuralComparisonConfig::candidate recursive, and perform extremely bad:

SelfAssignment.ql-14:StructuralComparison::StructuralComparisonConfiguration::candidate_dispred#fff ...................................................................................................................................................... 35m14s (4 evaluations with max 35m14s in StructuralComparison::StructuralComparisonConfiguration::candidate_dispred#fff/3@i6#ad086iwe)

@github-actions github-actions Bot added the C# label Nov 19, 2021
@hvitved hvitved marked this pull request as ready for review November 20, 2021 07:27
@hvitved hvitved requested a review from a team as a code owner November 20, 2021 07:27
@michaelnebel
Copy link
Copy Markdown
Contributor

michaelnebel commented Nov 22, 2021

So withtout the pragma[nomagic], what you see is what you get and the compiler won't rewrite the getLocation code?

@hvitved
Copy link
Copy Markdown
Contributor Author

hvitved commented Nov 22, 2021

So withtout the pragma[nomagic], what you see is what you get and the compiler won't rewrite the getLocation code?

With the pragma[nomagic], we prevent the compiler from applying the magic sets optimization on getLocation. Without the pragma, the optimizer applies a very bad context to getLocation, which makes the predicate StructuralComparisonConfig::candidate recursive.

@hvitved hvitved merged commit 39e3254 into github:main Nov 22, 2021
@hvitved hvitved deleted the csharp/self-assignment-bad-magic branch November 22, 2021 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@tamasvajk tamasvajk tamasvajk approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

C#

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@hvitved @michaelnebel @tamasvajk