[PATCH] An Aliasing Validator/Sanitizer

[Hal Finkel]

----- Original Message -----
> From: "Chandler Carruth" <chandlerc at gmail.com>
> To: reviews+D4446+public+a1977528632cb474 at reviews.llvm.org
> Cc: "Hal Finkel" <hfinkel at anl.gov>, "Nick Lewycky" <nlewycky at google.com>, "Chandler Carruth" <chandlerc at gmail.com>,
> "Andy Trick" <atrick at apple.com>, "Arnold Schwaighofer" <aschwaighofer at apple.com>, "Commit Messages and Patches for
> LLVM" <llvm-commits at cs.uiuc.edu>, "Alexey Samsonov" <samsonov at google.com>, "Kostya Serebryany" <kcc at google.com>
> Sent: Saturday, August 23, 2014 7:26:46 PM
> Subject: Re: [PATCH] An Aliasing Validator/Sanitizer
> 
> 
> 
> 
> 
> On Wed, Jul 9, 2014 at 4:55 PM, hfinkel at anl.gov < hfinkel at anl.gov >
> wrote:
> 
> 
> 
> As part of the conversation about the scoped-noalias metadata, our
> need for a validator / sanitizer for aliasing was heightened. Two
> use cases were discussed:
> 
> 1. To validate the user's use of 'restrict' on pointers.
> 
> 2. To validate LLVM's AA infrastructure.
> 
> This implementation is certainly not ready to be committed (although
> it is functional, and I've found AA bugs with it when self-hosting
> Clang with it enabled), but I'm posting it to start a conversation
> on what we want and how it should be implemented.
> 
> I originally wrote this in response to bugs appearing when I enabled
> the use of AA during code generation (and the current implementation
> is certainly biased toward that use case). There are two modes
> implemented:
> 
> 1. A mode where instrumentation is inserted to check NoAlias results
> on load/store and store/store pairs. Checking "all" such pairs in a
> function is impractical for large functions, so the current
> implementation checks only such pairs that occur in between likely
> scheduling barriers. Also, it inserts the checks late (after almost
> all optimizations) because my use case was focused on bugs that
> appeared from using AA during codegen. The checks could certainly be
> inserted earlier as well.
> Compiling with -mllvm -codegen-validate-aa enables this.
> 
> 2. A mode when uses of NoAlias results during instruction scheduling
> are recorded in a file. When compiling again later, this file can be
> read and only those specific pairs are instrumented to be checked.
> This has much lower overhead, but is also less sensitive (because
> instruction scheduling only uses AA when other methods fail to yield
> a definitive result and the result might be relevant to scheduling).
> Compiling with -mllvm -record-aa-sched-mi=SOME_DIRECTORY will cause
> the AA pairs used during instruction scheduling to be recorded and
> then compiling with -mllvm -codegen-validate-aa -mllvm
> -use-recorded-aa=SOME_DIRECTORY will cause the instrumentation to be
> inserted.
> 
> 
> 
> While your check strategy #2 seems very useful for tracking down
> miscompiles, it also seems really brittle and hard to use. I'd
> prefer to start with the simpler strategy in #1 and see if we need
> the complexity of recording AA-powered decisions and feeding them
> back into the optimizer.
> 
> 
> 
> 
> To focus more on user errors, we'd probably want the checks inserted
> earlier (prior to inlining), the checks to encode source-level
> locations (instead of or in addition to IR instructions), and to use
> the sanitizer runtimes to report errors (instead of manually
> building sprintf/write calls). Also, it currently has no test cases
> ;)
> I think these are the things I'd like to see addressed before it goes
> in (well, and some code cleanups, but whatever) -- an early and late
> mode, using debug information for source locations, and some runtime
> for error reporting.
> 
> 
> It feels like the UBSan runtimes likely are the best suited here,
> especially as you're not relying on shadow memory or anything of the
> sort. Since this impacts sanitizers in general and UBSan in
> particular, CC-ing Alexey who has been looking at UBSan recently and
> Kostya who may have more general ideas about instrumentation based
> analysis of these kinds of bugs.

Currently, all of the UBSan checks are inserted by Clang; are there any complications to doing this from the backend? [specifically, I wonder about the source-location data that the UBSan checks normally use].

> 
> 
> I'd also like to understand what the coverage of potential aliasing
> violations vs. overhead tradeoff currently is and whether there are
> any ways we can improve the coverage without *too* much runtime
> overhead (without using a feedback loop)... In particular, I'm
> hoping we could do something along the lines of checking the start
> and stop boundaries of loop accessed memory, or essentially doing
> what the vectorizer's runtime checks do but for correctness.

As you might imagine, this really ends up being the key issue. You can't check all in-scope pointer pairs, because this really blows up for large functions. So you need to limit the checks somehow, and doing this while still catching the interesting errors is key. The loop checking sounds easy to do for loops that SE understands for accesses to pointers that are linear recursions.

 -Hal

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory