C++: Improve alias analysis for indirections #1736

dave-bartolomeo · 2019-08-14T01:09:03Z

This PR improves our alias analysis and SSA construction for access to memory via indirection, including fields of the indirection. Before, we could get precise flow through fields as long as we knew exactly which variable and which field was being accessed. Now, we can often tell that two accesses touch the same memory location, even if we don't know exactly which memory location that is.

Alias Analysis Changes

AliasAnalysis.qll is now parameterized base on a Configuration, which defines a type named Allocation. Allocation represents a contiguous region of memory that is disjoint from all other instances of Allocation. Two memory accesses that belong to different allocations can never overlap. When building unaliased SSA, Allocation represents an automatic (stack) variable, since that is all we care to model. When building aliased SSA, Allocation represents a ValueNumber for a VariableAddressInstruction, but in the future will also include InitializeParameterInstructions and possibly calls to malloc-like functions.

The entry points into AliasAnalysis.qll are now:

allocationEscapes(Configuration::Allocation allocation): Holds if the specified allocation's address escapes.
addressOperandBaseAndConstantOffset(AddressOperand addrOperand, Instruction base, int bitOffset): Holds if the specified AddressOperand points to an address equal to base plus a constant offset bitOffset. Two accesses with the same base address overlap if and only if their offsets overlap.
getAddressOperandAllocation(AddressOperand addrOperand): Gets the allocation into which addrOperand points, if known.

AliasedSSA.qll has been changed to track the allocation and base address for every indirect memory access. The overlap relationship now uses the following rules to determine overlap:

A def and use in different virtual variables never overlap.
A def and use that both have known but different allocations never overlap.
A def and use with the same base address overlap if and only if their offsets from that base address overlap. This ensures that *p and *p always exactly overlap, and p->x never overlaps p->y, even if we don't know exactly what p points to.

SimpleSSA.qll has been changed to track everything in terms of Allocations, but since an Allocation in SimpleSSA is just an IRAutomaticVariable, nothing really changed here.

Fixed handling of PointerOffsetInstruction in alias analysis to only propagate the pointer operand.

Test Changes

Added the first SSA sanity check, for operands with more than one associated MemoryLocation, and tests that run this check on our existing IR test sources.

Added a simple test for SSA for indirect parameter accesses.

Moved the points_to.ql test to its own directory, and made it use a new InlineExpectationsTest class. This class lets you define the results of your test in terms of (tag, value) pairs, each associated with a location. The base class looks for matching comments of the form $tag=value on the appropriate line, and reports an output row for each missing or unexpected result. The ".expected" file should always be an empty file. This allows us to modify the test sources, including inserting lines, without getting a bunch of diffs just due to line numbers changing.

Miscellaneous

Made duplicateOperand have the output of a problem query, including a link to the containing function.

Made IRVariable non-abstract, so that it is now possible to extend IRVariable to restrict rather than extend.

Fixed a bug where PrintSSA would print some properties on the wrong instruction.

Refactored the integer constant code a bit. Bit offsets are now printed in the format bytes:bits, where :bits is omitted if bits is zero. Thus, 21 bits is printed as 2:5, while 24 bits is printed as just 3.

dave-bartolomeo · 2019-08-14T01:09:39Z

This produces the expected results on our tests, but I still need to test it (including performance) on real snapshots.

jbj · 2019-08-14T12:57:11Z

This is going to be very hard to review. Is there any way it can be split up into multiple commits that are reasonable in size and make sense on their own?

dave-bartolomeo · 2019-08-14T19:36:13Z

@jbj As long as the individual commits don't have to actually work in isolation, I can definitely split it up.
I think the big chunks to review, in order, are:

AliasAnalysis.qll
SimpleSSA.qll+AliasConfiguration.qll (uses the new APIs from AliasAnalysis.qll)
AliasedSSA.qll+AliasConfiguration.qll (uses the new APIs from AliasAnalysis.qll, but does more with them)
The test changes
InlineExpectationsTest.qll (used by the points_to test, but easier to understand once you've seen the changes to that test).
Everything else (mostly simple changes)

I'll have it split in a couple hours.

Make `bitsToBytesAndBits` omit the leftover bits if zero.

Adds `InlineExpectationsTest.qll`, which makes it easy to create a QL test where the expectations are provided as comments in the source code being tested.

dave-bartolomeo · 2019-08-14T21:08:14Z

OK, I've curated all of the changes into separate commits. Many of those commits are pretty trivial. I've saved AliasedSSA.qll for last.

This uses the autoformatter from internal PR 34050. Some files that will change in github#1736 have been spared. ./build -j4 target/jars/qlformat find ql/cpp/ql -name "*.ql" -print0 | xargs -0 target/jars/qlformat --input find ql/cpp/ql -name "*.qll" -print0 | xargs -0 target/jars/qlformat --input (cd ql && git checkout 'cpp/ql/src/semmle/code/cpp/ir/implementation/**/*SSA*.qll') buildutils-internal/scripts/pr-checks/sync-identical-files.py --latest

Some files that will change in github#1736 have been spared. ./build -j4 target/jars/qlformat find ql/cpp/ql -name "*.ql" -print0 | xargs -0 target/jars/qlformat --input find ql/cpp/ql -name "*.qll" -print0 | xargs -0 target/jars/qlformat --input (cd ql && git checkout 'cpp/ql/src/semmle/code/cpp/ir/implementation/**/*SSA*.qll') buildutils-internal/scripts/pr-checks/sync-identical-files.py --latest

rdmarsh2 · 2019-08-22T23:21:33Z

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

+/**
+ * A memory allocation that can be tracked by the AliasedSSA alias analysis.
+ * For now, we track all variables accessed within the function, including both local variables
+ * and global variables. In the future, we will track indirect parameters as well.


What about modeled allocators or default new?

rdmarsh2 · 2019-08-22T23:33:16Z

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

-private Overlap getVariableMemoryLocationOverlap(VariableMemoryLocation def, VariableMemoryLocation use) {
-  overlappingIRVariableMemoryLocations(def, use) and
-  result = Interval::getOverlap(def.getStartBitOffset(), def.getEndBitOffset(), use.getStartBitOffset(), use.getEndBitOffset())
+private Overlap getIndirectMemoryLocationOverlap(IndirectMemoryLocation def, IndirectMemoryLocation use) {


This was written carefully to avoid a quadratic blowup on https://lgtm.com/projects/g/ALaDyn/piccante/latest/files/src/sobol.cpp - can you make sure it's still performant?

rdmarsh2 · 2019-08-22T23:40:18Z

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

+ */
+private string getBaseAddressString(ValueNumber baseAddress) {
+  // If it's the value of a parameter, just use the parameter name.
+  result = baseAddress.getAnInstruction().(InitializeParameterInstruction).getParameter().toString() or


Should we also special-case VariableAddressInstruction here?

jbj · 2020-04-29T12:48:32Z

I recently asked @dbartol to remind me what this PR does.

It's an analysis to connect definitions of *p with uses of *p, regardless of whether we have modelled the allocation behind it.
Most of the problem was already solved by modelling parameters as disjoint allocations.
An extra stage of value numbering is required, which has performance implications.
We decided to only revive this PR if we observe real issues.

dave-bartolomeo added the C++ label Aug 14, 2019

dave-bartolomeo requested review from jbj and rdmarsh2 August 14, 2019 01:09

dave-bartolomeo added 13 commits August 14, 2019 13:48

C++: Make duplicateOperand query report function name

8213284

C++: Make IRVariable non-abstract

d40e9f0

C++: Print memory accesses on the correct instruction in PrintSSA

448e4d4

C++: Add SSA sanity tests

9c1eb7e

C++: Refactor some integer constant code

2abcaed

Make `bitsToBytesAndBits` omit the leftover bits if zero.

C++: Update points_to test for new alias analysis

336b7d3

Adds `InlineExpectationsTest.qll`, which makes it easy to create a QL test where the expectations are provided as comments in the source code being tested.

C++: Fix escape tests for alias API changes

22b776d

C++: Add SSA sanity tests to IR tests

5b2baa4

C++: Add SSA test case for new alias analysis

4aab88e

C++: Fix dataflow test expectations for new alias analysis

5385f13

C++: Alias analysis changes to support precise indirections

d4ae9dd

C++: Update SimpleSSA to use the new alias analysis API

2bc3506

C++: Implement precise analysis of indirections in AliasedSSA

352e750

dave-bartolomeo force-pushed the dave/BetterAlias2 branch from a710609 to 352e750 Compare August 14, 2019 21:07

jbj mentioned this pull request Sep 12, 2019

C++: Autoformat IR SSA files #1928

Merged

rdmarsh2 reviewed Sep 13, 2019

View reviewed changes

dbartol mentioned this pull request Nov 22, 2019

C++/C#: Cleanup in preparation for indirect alias analysis #2421

Merged

adityasharad changed the base branch from master to main August 14, 2020 18:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

C++: Improve alias analysis for indirections #1736

C++: Improve alias analysis for indirections #1736

dave-bartolomeo commented Aug 14, 2019

dave-bartolomeo commented Aug 14, 2019

jbj commented Aug 14, 2019

dave-bartolomeo commented Aug 14, 2019

dave-bartolomeo commented Aug 14, 2019

rdmarsh2 Aug 22, 2019

rdmarsh2 Aug 22, 2019

rdmarsh2 Aug 22, 2019

jbj commented Apr 29, 2020

C++: Improve alias analysis for indirections #1736

Are you sure you want to change the base?

C++: Improve alias analysis for indirections #1736

Conversation

dave-bartolomeo commented Aug 14, 2019

Alias Analysis Changes

Test Changes

Miscellaneous

dave-bartolomeo commented Aug 14, 2019

jbj commented Aug 14, 2019

dave-bartolomeo commented Aug 14, 2019

dave-bartolomeo commented Aug 14, 2019

rdmarsh2 Aug 22, 2019

Choose a reason for hiding this comment

rdmarsh2 Aug 22, 2019

Choose a reason for hiding this comment

rdmarsh2 Aug 22, 2019

Choose a reason for hiding this comment

jbj commented Apr 29, 2020