C++: Interprocedural escape/alias analysis in IR construction #792

rdmarsh2 · 2019-01-18T01:40:14Z

This PR isn't quite finished; I'm going to need to add support for this parameters. I'm opening it early to see test results for IR-based libraries and to get discussion started.

jbj · 2019-01-25T07:39:50Z

As I mentioned in the meeting yesterday, let's remember to add sound handling of virtual dispatch.

rdmarsh2 · 2019-02-04T21:41:52Z

@jbj @dave-bartolomeo I've added sound handling of virtual dispatch; is there anything else this PR is waiting on?

jbj · 2019-02-05T09:57:38Z

I didn't know this was ready for review. Can you please describe what problem is being solved, what your solution is, and what problems remain? Ideally this should be as code or commit comments, but PR comments will also do.

As always, I'd like to see performance numbers on Wireshark and other large databases. You can find an up-to-date Wireshark snapshot in https://drive.google.com/drive/folders/0B5VJBNgK-GjvREQ4Y2ZicmM2VEk. Before evaluating performance, please rebase on master, which now contains a handful of performance fixes that makes the IR feasible on large snapshots.

rdmarsh2 · 2019-02-19T19:30:07Z

Briefly, this improves our escape analysis for variable addresses such that x can be put into the SSA graph of f in the following code:

int * add(int *in1, int *in2, int *out) {
  *out = *in1 + *in2;
  return out;
}

int f(int x) {
  add(&x, &x, &x)
  return x;
}

It may also result in improvements to alias analysis in cases where the return value of a function is always a particular argument, but this is less common.

rdmarsh2 · 2019-02-27T21:19:50Z

Benchmarked it today; it looks like about a 10% slowdown in IR construction (2526 seconds vs 2798 seconds on a Wireshark snapshot). I'll fix the conflicts and rebase

jbj

As I talked about in the meeting, I think performance of this can be good if we avoid recursion through forall.

jbj · 2019-02-28T13:07:47Z

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasAnalysis.qll

    )
  )
 }
-


jbj · 2019-02-28T13:07:53Z

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasAnalysis.qll

 * escapes outside the domain of the analysis.
 */
-predicate operandEscapes(Operand operand) {
+ predicate operandEscapes(Operand operand) {


rdmarsh2 · 2019-03-01T18:10:18Z

I've done a big performance refactor; it looks like this PR is now a slowdown of about 2.5% on Wireshark.

rdmarsh2 · 2019-03-01T20:35:44Z

The test failures look like it's now considering pointers passed as parameters to be non-escaping in the callee by default. @dave-bartolomeo and I have discussed this as something we might want to do anyways, but I don't think we've done the work to show that it actually improves results. @jbj what do you think?

rdmarsh2 · 2019-03-01T23:51:35Z

I misunderstood the test change; it's actually due to adding models, and we're still assuming the addresses of parameters are escaped.

jbj · 2019-03-02T11:15:38Z

I've opened rdmarsh2#1 (a PR against this PR) to fix the last performance problem. When that's merged, performance LGTM.

Then there's just the test failures left to fix before we can merge this.

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasAnalysis.qll

cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/AliasAnalysis.qll

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasAnalysis.qll

cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/AliasAnalysis.qll

cpp/ql/test/library-tests/ir/escape/points_to.ql

jbj

A few review comments and a failing test.

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasAnalysis.qll

The `automaticVariableAddressEscapes` predicate got join-ordered badly in its `unaliased_ssa` version. These are the tuple counts on Wireshark, where one pipeline step is seen to have 716 million tuples: ``` [2019-03-02 11:29:41] (42s) Starting to evaluate predicate AliasAnalysis::automaticVariableAddressEscapes#2#f [2019-03-02 11:30:06] (67s) Tuple counts: 353419 ~0% {1} r1 = JOIN project#Instruction::VariableAddressInstruction#class#2#ff WITH AliasAnalysis::resultEscapesNonReturn#2#f ON project#Instruction::VariableAddressInstruction#class#2#ff.<0>=AliasAnalysis::resultEscapesNonReturn#2#f.<0> OUTPUT FIELDS {AliasAnalysis::resultEscapesNonReturn#2#f.<0>} 353419 ~0% {2} r2 = JOIN r1 WITH IRConstruction::Cached::getInstructionEnclosingFunctionIR#ff@staged_ext ON r1.<0>=IRConstruction::Cached::getInstructionEnclosingFunctionIR#ff@staged_ext.<0> OUTPUT FIELDS {IRConstruction::Cached::getInstructionEnclosingFunctionIR#ff@staged_ext.<1>,r1.<0>} 353419 ~0% {2} r3 = JOIN r2 WITH FunctionIR::FunctionIR::getFunction_dispred#3#ff ON r2.<0>=FunctionIR::FunctionIR::getFunction_dispred#3#ff.<0> OUTPUT FIELDS {FunctionIR::FunctionIR::getFunction_dispred#3#ff.<1>,r2.<1>} 716040298 ~0% {2} r4 = JOIN r3 WITH IRVariable::IRVariable#class#3#ff_10#join_rhs ON r3.<0>=IRVariable::IRVariable#class#3#ff_10#join_rhs.<0> OUTPUT FIELDS {IRVariable::IRVariable#class#3#ff_10#join_rhs.<1>,r3.<1>} 4480139 ~0% {2} r5 = JOIN r4 WITH IRVariable::IRAutomaticVariable#class#3#ff ON r4.<0>=IRVariable::IRAutomaticVariable#class#3#ff.<0> OUTPUT FIELDS {r4.<1>,r4.<0>} 66760 ~91% {1} r6 = JOIN r5 WITH Instruction::VariableInstruction::getVariable_dispred#2#ff ON r5.<0>=Instruction::VariableInstruction::getVariable_dispred#2#ff.<0> AND r5.<1>=Instruction::VariableInstruction::getVariable_dispred#2#ff.<1> OUTPUT FIELDS {r5.<1>} return r6 [2019-03-02 11:30:06] (67s) >>> Relation AliasAnalysis::automaticVariableAddressEscapes#2#f: 35531 rows using 0 MB ``` The predicate contained a cyclic join, which is always hard to optimize. I couldn't see a reason to join the `FunctionIR`, so I removed that part. The predicate is now fast, and there are no changes in the tests.

jbj

LGTM

rdmarsh2 added C++ WIP This is a work-in-progress, do not merge yet! labels Jan 18, 2019

rdmarsh2 requested a review from dave-bartolomeo January 18, 2019 01:40

rdmarsh2 requested a review from a team as a code owner January 18, 2019 01:40

rdmarsh2 removed the WIP This is a work-in-progress, do not merge yet! label Feb 1, 2019

rdmarsh2 force-pushed the rdmarsh/cpp/escape-analysis branch from 79fa09f to 8dc8d1c Compare February 27, 2019 21:22

jbj added this to the 1.20 milestone Feb 28, 2019

jbj reviewed Feb 28, 2019

View reviewed changes

rdmarsh2 changed the base branch from master to rc/1.20 March 1, 2019 17:54

rdmarsh2 requested a review from jbj March 1, 2019 18:08

jbj reviewed Mar 4, 2019

View reviewed changes

rdmarsh2 force-pushed the rdmarsh/cpp/escape-analysis branch from 39bfce8 to 6ca9ace Compare March 5, 2019 19:06

jbj reviewed Mar 6, 2019

View reviewed changes

rdmarsh2 force-pushed the rdmarsh/cpp/escape-analysis branch from 507c949 to 6aa8944 Compare March 7, 2019 21:08

Robert Marsh added 7 commits March 7, 2019 13:14

C++: test changes for interproc escape analysis

bd39698

C++: add new interprocedural escape analysis

466e110

C++: escape analysis for this parameters

6089172

C++: test for virtual functions in escape analysis

2c94a88

C++: assume arguments to virtual functions escape

c70bd28

C++: refactor alias analysis for performance

726f38c

C++: fix unused variable warning

6f76c13

Robert Marsh and others added 10 commits March 7, 2019 13:14

C++: refactor escape analysis for performance

09321ee

C++: fix escape test failures

a72cd23

C++: remove duplicate logic

878502f

C++: points-to for argument-returning calls

97c11a5

C++: add phi node support to escape analysis

7e30ce0

C++: remove VariableAddress from points_to test

17ad124

C++: respond to PR comments

ef836c3

C++: accept dataflow test change

8a2a467

C++: fix whitespace

07bc9ca

rdmarsh2 force-pushed the rdmarsh/cpp/escape-analysis branch from 92600e9 to 07bc9ca Compare March 7, 2019 21:15

jbj approved these changes Mar 8, 2019

View reviewed changes

jbj merged commit db104ed into github:rc/1.20 Mar 8, 2019

C++: Interprocedural escape/alias analysis in IR construction #792

C++: Interprocedural escape/alias analysis in IR construction #792

Uh oh!

Conversation

rdmarsh2 commented Jan 18, 2019

Uh oh!

jbj commented Jan 25, 2019

Uh oh!

rdmarsh2 commented Feb 4, 2019

Uh oh!

jbj commented Feb 5, 2019

Uh oh!

rdmarsh2 commented Feb 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rdmarsh2 commented Feb 27, 2019

Uh oh!

jbj left a comment

Choose a reason for hiding this comment

Uh oh!

jbj Feb 28, 2019

Choose a reason for hiding this comment

Uh oh!

jbj Feb 28, 2019

Choose a reason for hiding this comment

Uh oh!

rdmarsh2 commented Mar 1, 2019

Uh oh!

rdmarsh2 commented Mar 1, 2019

Uh oh!

rdmarsh2 commented Mar 1, 2019

Uh oh!

jbj commented Mar 2, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jbj left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jbj left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rdmarsh2 commented Feb 19, 2019 •

edited

Loading