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

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy added inline comments.


================
Comment at: lib/IR/Verifier.cpp:3802
+static bool IsSingularStructTBAANode(const MDNode *MD,
+                                     SmallPtrSetImpl<const MDNode *> &Visited) {
+  if (MD->getNumOperands() != 3)
----------------
manmanren wrote:
> This seems somewhat heavy-weighted. I guess you are trying to differentiate a scalar type node from a struct type node with a single field, given that both can have 3 operands with the 2nd operand being a type and the 3rd being an offset. I don't think you can really differentiate the two cases. I feel like checking the offset being zero for the access type node of a tag should be enough.
> 
> I will not call them *singular" struct type node, since they are scalar type node.
> 
> createTBAAScalarTypeNode will generate such a scalar type node with 3 operands, createTBAANode (which is not used in clang any more) can generate a scalar type node with 2 operands.
Hi Manman,

I'd like to keep the recursive nature of this check and not just check
the offset only on the MDNode for the access tag.

IOW, I'd like to avoid invalid cases where the access tag has a full
blown struct type descriptor at offset 0, since in such (invalid)
cases `getMostGenericTBAA` will produce bad merged metadata that would
be hard to debug:

```
;; Invalid metadata that I want the verifier to reject

!root = !{!"C++ TBAA"}
!int = !{!"int", !root}
!float = !{!"float", !root}
!s_a = !{"s_a", !int, 5, !float, 20}
!s_b = !{"s_b", !int, 6, !float, 20}

!scalar_a = !{!"scalar-a", !s_a, 0}
!scalar_b = !{!"scalar-b", !s_b, 0}

!tag_a = !{!some_struct, !scalar_a, 20}
!tag_b = !{!some_struct, !scalar_b, 20}

!tag_c = !{!float, !float, 0}

!generic_tag = !{!int, !int, 0}
```

Both `!tag_a` and `!tag_b` should may-alias with `!tag_c`, but
`getMostGenericTBAA` will merge `!tag_a` and `!tag_b` into
`!generic_tag` which does not alias with `!tag_c`.

However, I'm happy to call replace "singular" with "scalar".



https://reviews.llvm.org/D26438