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

[Sanjoy Das via llvm-commits]

sanjoy added a comment.

Hi Manman,

In https://reviews.llvm.org/D26438#590834, @sanjoy wrote:

> > I may have misunderstood your point, but for struct-path TBAA, the last integer for a type node is the offset. Only for old scalar TBAA, the last optional integer specifies whether it is immutable. There is no ambiguity in struct-path TBAA.
>
> Thanks, that clears it up for me.


There is this bit of language in the TBAA implementation:

  // The second field is the access type node, it
  // must be a scalar type node.

but in the TBAA metadata generated by clang it seems like we have access types like `!{!"int", !N, i64 0}` which look like struct nodes.  I can think of three ways to resolve this:

1. State that access types are not necessarily scalar type nodes.  However, this is not consistent with how `getMostGenericTBAA` walks up the parent types of the access type node.
2. State that `!{!"int", !N, i64 0}` is a scalar type node.
3. State that access types can be either scalar type nodes or a struct type node with one member at offset 0.

I'm not sure what the cleanest option is though I'm slightly leaning towards the second one.

A second related point to what you said before about the immutable bit is that today we'll upgrade

  define i32 @f(i32* %p) {
    %val = load i32, i32* %p, !tbaa !0
    ret i32 %val
  }
  
  !0 = !{!"const int", !1, i64 1}
  !1 = !{!"const char", !2, i64 1}
  !2 = !{!"char", !3}
  !3 = !{!"root"}

to

  define i32 @f(i32* %p) {
    %val = load i32, i32* %p, !tbaa !0
    ret i32 %val
  }
  
  !0 = !{!1, !1, i64 0, i64 1}
  !1 = !{!"const int", !2}
  !2 = !{!"const char", !3, i64 1}
  !3 = !{!"char", !4}
  !4 = !{!"root"}

This is a problem since in the old scalar tbaa metadata `!{!"char", !2, i64 1}` meant "immutable `const char`" but after the upgrade it means `struct const_char` with a `char` at offset 1.  IOW, given what you said, I don't think the auto-upgrade path is correct.


https://reviews.llvm.org/D26438