The Wayback Machine - https://web.archive.org/web/20260214163334/https://github.com/github/codeql/pull/4186
Skip to content

Mergeback: 1.25 -> main#4186

Merged
asgerf merged 43 commits intomainfrom
rc/1.25
Sep 2, 2020
Merged

Mergeback: 1.25 -> main#4186
asgerf merged 43 commits intomainfrom
rc/1.25

Conversation

@jbj
Copy link
Contributor

@jbj jbj commented Sep 2, 2020

No description provided.

erik-krogh and others added 30 commits August 4, 2020 10:25
CodeQL docs: update CSS and master page template
…oreWrite` and move them into an internal module
Go 1.15 was just released, and there are [no changes](https://golang.org/doc/go1.15#language) to the language, so we might as well list it as supported.
Co-authored-by: Alistair Christie <54933897+hubwriter@users.noreply.github.com>
Docs: Add metadata option missing from reference table
For IR data flow I also added a `definitionByReferenceNodeFromArgument`
predicate to improve compatibility with AST data flow.
Co-authored-by: Aditya Sharad <6874315+adityasharad@users.noreply.github.com>
This predicate name was only used in IR data flow, not in AST data flow.
C++: Clarify the docs on DataFlow::Node::asExpr
Learning CodeQL docs: update links to match GitHub docs style
shati-patel and others added 13 commits August 14, 2020 12:03
QL language reference: update links to match GitHub docs style
The negation in this predicate did not get pulled into an
`#antijoin_rhs` predicate but got materialized as part of each
iteration, which meant that the temporary `ControlFlowNode` column did
not get projected away. The tuple counts looked like this on
kamailio/kamailio (iteration 20):

    5724      ~13%      {3} r9 = JOIN r8 WITH BasicBlocks::Cached::bb_successor_cached#ff@staged_ext AS R ON FIRST 2 OUTPUT r8.<2>, r8.<3>, r8.<1>
    5724      ~12%      {3} r10 = JOIN r8 WITH BasicBlocks::Cached::bb_successor_cached#ff@staged_ext AS R ON FIRST 2 OUTPUT r8.<3>, r8.<2>, r8.<1>
    124717061 ~11%      {4} r11 = JOIN r10 WITH project#FlowVar::FlowVar_internal::assignmentLikeOperation#ffff_10#join_rhs AS R ON FIRST 1 OUTPUT R.<1>, r10.<2>, r10.<1>, r10.<0>
    66        ~0%       {3} r12 = JOIN r11 WITH project#BasicBlocks::Cached::basic_block_member AS R ON FIRST 2 OUTPUT r11.<2>, r11.<3>, r11.<1>
    66                  {3} r13 = MATERIALIZE r12 AS antijoin_rhs
    5658      ~14%      {3} r14 = r9 AND NOT r13(r9.<0>, r9.<1>, r9.<2>)

After manually pulling out the join inside the negation, the time per
iteration drops from ~30 to <1s. The pipeline above is replaced with

    892394  ~0%      {4} r6 = r5 AND NOT FlowVar::FlowVar_internal::assignsToVar#fb AS R(r5.<3>, r5.<2>)
    892394  ~0%      {4} r7 = SCAN r6 OUTPUT r6.<1>, r6.<3>, r6.<0>, r6.<2>
    5658    ~11%     {3} r8 = JOIN r7 WITH BasicBlocks::Cached::bb_successor_cached#ff@staged_ext AS R ON FIRST 2 OUTPUT r7.<2>, r7.<1>, r7.<3>
This predicate was very slow on kamailio/kamailio:

    (696s) Tuple counts for FlowVar::FlowVar::definedPartiallyAt_dispred#ff:
    703569     ~3%     {3} r1 = SCAN FlowVar::FlowVar_internal::TBlockVar#fff AS I OUTPUT I.<1>, I.<0>, I.<2>
    7679540588 ~3%     {3} r2 = JOIN r1 WITH FlowVar::PartialDefinitions::PartialDefinition::partiallyDefines_dispred#ff_10#join_rhs AS R ON FIRST 1 OUTPUT R.<1>, r1.<1>, r1.<2>
    567217     ~2%     {2} r3 = JOIN r2 WITH project#FlowVar::PartialDefinitions::PartialDefinition#class#fff#2 AS R ON FIRST 2 OUTPUT r2.<2>, r2.<0>
                       return r3

After this change, the predicate takes no time at all:

    (22s) Tuple counts for FlowVar::FlowVar::definedPartiallyAt_dispred#ff:
    703569  ~3%     {3} r1 = SCAN FlowVar::FlowVar_internal::TBlockVar#fff AS I OUTPUT I.<1>, I.<0>, I.<2>
    567217  ~2%     {2} r2 = JOIN r1 WITH FlowVar::PartialDefinitions::PartialDefinition::partiallyDefinesVariableAt#fff_120#join_rhs AS R ON FIRST 2 OUTPUT r1.<2>, R.<2>
                    return r2

Looking at the code, it turned out that the predicates
`partiallyDefines` and `getSubBasicBlockStart` were almost always used
together and could therefore be merged into a single predicate to get
better join orderings. The predicate `partiallyDefinesThis` was never
used.
It turns out this predicate was used in a test, and that use can't be
replaced with the new `partiallyDefinesVariableAt` predicate since
`partiallyDefinesVariableAt` doesn't hold for a `PartialDefinition` that
defines something other than a variable.
C++: Fix two join orders in FlowVar.qll
@asgerf asgerf merged commit 2c0e9f0 into main Sep 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment