Skip to content

JS: fix cfg and dataflow for logical compound assignments#6459

Merged
codeql-ci merged 1 commit intogithub:mainfrom
erik-krogh:oreq
Aug 11, 2021
Merged

JS: fix cfg and dataflow for logical compound assignments#6459
codeql-ci merged 1 commit intogithub:mainfrom
erik-krogh:oreq

Conversation

@erik-krogh
Copy link
Copy Markdown
Contributor

@erik-krogh erik-krogh commented Aug 10, 2021

Fixes #6362

An evaluation looks fine.

The result difference gains TPs and removes FPs.


The change in CFGExtractor.java corrected an edge in the CFG.
For an assignment foo ||= bar there was an edge from bar to foo, which was incorrect.
It has been corrected to be an edge from bar to foo ||= bar.

And I've added data-flow edges from both the lhs and rhs to the assignment itself.
I'm somewhat unsure about the change in the data-flow graph.

@erik-krogh erik-krogh added WIP This is a work-in-progress, do not merge yet! Awaiting evaluation Do not merge yet, this PR is waiting for an evaluation to finish labels Aug 10, 2021
@github-actions github-actions Bot added the JS label Aug 10, 2021
@erik-krogh erik-krogh added no-change-note-required This PR does not need a change note and removed WIP This is a work-in-progress, do not merge yet! Awaiting evaluation Do not merge yet, this PR is waiting for an evaluation to finish labels Aug 10, 2021
@erik-krogh erik-krogh marked this pull request as ready for review August 10, 2021 18:21
@erik-krogh erik-krogh requested a review from a team as a code owner August 10, 2021 18:22
Copy link
Copy Markdown
Contributor

@asgerf asgerf left a comment

Choose a reason for hiding this comment

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

👍

I'm somewhat unsure about the change in the data-flow graph.

For &&= it will likely cause the same issue we're seeing with && but since we already have that problem with && it's not something that needs to be fixed in this PR.

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

Labels

JS 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.

[JavaScript] LGTM.com - false positive for logical OR assignment

3 participants