[PATCH] D26438: [Verifier] Add verification for TBAA metadata

[Sanjoy Das via llvm-commits]

sanjoy created this revision.
sanjoy added reviewers: manmanren, dexonsmith, mehdi_amini, chandlerc, reames.
sanjoy added a subscriber: llvm-commits.
Herald added a subscriber: mcrosier.

*This is a work in progress (and lacks unit tests).  However, I think
some early feedback here on the general direction of the patch will be
helpful.*

This change adds some verification in the IR verifier around struct path
TBAA metadata.

Other than some basic sanity checks (e.g. we get constant integers where
we expect integers), this checks:

- That by the time an struct access tuple `(base-type, offset)` is "reduced" to a scalar base type, the offset is `0`.  For instance, in C++ you can't start from, say `("struct-a", 16)`, and end up with `("int", 4)` -- by the time the base type is `"int"`, the offset better be zero.  In particular, a variant of this invariant is needed for `llvm::getMostGenericTBAA` to be correct.
- That there are no cycles in a struct path.
- That struct type nodes have their offsets listed in an ascending order.
- That when generating the struct access path, you eventually reach the access type listed in the tbaa tag node.

One specific thing I want an opinion on from someone more familiar with
metadata: the verifier does not allow `null` metadata operands.  The
entire metadata DAG starting from the `!tbaa` access tag should be
formed of `MDString`, `MDNode` and `ConstantAsMetadata`.  I don't think
any pass should *not* be spuriously dropping any of these operands, but
let me know if you disagree.

There is one place where there is an ambiguity in the specification:
given a base type `!{!"whatever", !N, i64 1}` there is no way to tell if
this is a struct containing `!N` at offset `i64 1` or a scalar immutable
TBAA node with `!N` as the parent.  Right now I interpret this as the
former (as the implementation has been doing for 3 years).  I will fix
the specification in a forthcoming documentation patch.


https://reviews.llvm.org/D26438

Files:
  lib/IR/Verifier.cpp
  test/Analysis/BasicAA/full-store-partial-alias.ll
  test/Analysis/CFLAliasAnalysis/Steensgaard/full-store-partial-alias.ll
  test/Analysis/TypeBasedAliasAnalysis/aliastest.ll
  test/Analysis/TypeBasedAliasAnalysis/cyclic.ll
  test/Analysis/TypeBasedAliasAnalysis/dse.ll
  test/Analysis/TypeBasedAliasAnalysis/dynamic-indices.ll
  test/Analysis/TypeBasedAliasAnalysis/intrinsics.ll
  test/Analysis/TypeBasedAliasAnalysis/licm.ll
  test/Analysis/TypeBasedAliasAnalysis/memcpyopt.ll
  test/CodeGen/ARM/2011-05-04-MultipleLandingPadSuccs.ll
  test/Instrumentation/AddressSanitizer/X86/bug_11395.ll
  test/Instrumentation/ThreadSanitizer/read_from_global.ll
  test/Instrumentation/ThreadSanitizer/vptr_read.ll
  test/Instrumentation/ThreadSanitizer/vptr_update.ll
  test/Transforms/GVN/PRE/preserve-tbaa.ll
  test/Transforms/GVN/tbaa.ll
  test/Transforms/JumpThreading/thread-loads.ll
  test/Transforms/LICM/2011-04-06-PromoteResultOfPromotion.ll
  test/Transforms/SLPVectorizer/X86/crash_scheduling.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D26438.77299.patch
Type: text/x-patch
Size: 21766 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161109/3ebadf9c/attachment.bin>