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

[Manman Ren via llvm-commits]

manmanren added a comment.

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

> 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.


Can you give an example where clang generates a struct path tag node where the 2nd field is not a scalar type node?

  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.

I think we only check the immutable bit on the tag node (not the type nodes).

Manman


https://reviews.llvm.org/D26438