Page MenuHomePhabricator

[Verifier] Check for TBAA Access Tag presence
AbandonedPublic

Authored by lebedev.ri on Apr 10 2018, 8:34 AM.

Details

Summary

I don't really know if this is useful or not, i was just curious to try to make something like this.

There may be several use-cases:

  • Verify that front-end indeed did mark every instruction that could have such a TBAA Access Tag (TBAAVerifier::CouldHaveTBAAAccessTag()) with the tag.
  • Assuming that the input is ok (see previous point), verify that the passes don't loose these tags, don't.

The design is very simplistic right now:

  • One option to toggle the check (default to off)
  • One option on how to treat the missing access tag:
    • Treat any missing tag as a fatal error
    • Missing tag is never a error
    • If there is at least one instruction that is tagged, then a missing tag is an error.

The last option makes sense to be the default, e.g. because one can globally disable TBAA info
(-O0, -fno-strict-aliasing?), and then *nothing* is tagged. Since we can't know beforehand
if there is a TBAA metadata, i simply track whether we have seen any tagged instructions,
and decide whether this is a fatal error at the end.

An observation: while trying to test, i have *quickly* found this very simple test,

,
in which the loads/stores aren't being tagged (with roughly -O1 -fstrict-aliasing + verifier enabled).
I'm sure there are more.

Now, question[s]:

  • If TBAA is enabled, are there valid (i.e. not a bug) cases where these missing tags are genuinely ok/intentional? That would make this way less useful.
  • Does this have any actual use cases? (no is an ok answer, too)
  • Should i try to look into this further, look into missing tags?

Diff Detail

Repository
rL LLVM

Event Timeline

lebedev.ri created this revision.Apr 10 2018, 8:34 AM
lebedev.ri retitled this revision from [Verifier] Not really for review: check for TBAA Access Tag presence to [Verifier] Check for TBAA Access Tag presence.May 1 2018, 12:17 PM

Had a brief chat with @rjmccall about this, it seems this is conceptually fine, so let's turn this into an actual review.

kosarev added inline comments.May 1 2018, 12:56 PM
lib/IR/Verifier.cpp
5037–5038

We generate a significant amount of such instructions that are not supposed to be decorated. See for example D41562 and D39138. For my local experiments I was adding marks associated with load and store instructions that give a clue on their origin. If we can have such a mechanism in the mainline to make things like this patch more useful, then that would be great.

lebedev.ri planned changes to this revision.May 3 2018, 12:12 PM

Thank you @kosarev, i will dig deeper.

lib/IR/Verifier.cpp
5037–5038

We generate a significant amount of such instructions that are not supposed to be decorated.

Aha, interesting.

For my local experiments I was adding marks associated with load and store instructions that give a clue on their origin.

I did not look into details, but that sounds bad. It's kind-of a 'chicken and egg' problem.
We'd basically need a verifier to make sure *that* info is not lost :/

lebedev.ri abandoned this revision.Jun 21 2019, 8:49 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 21 2019, 8:49 AM
Herald added a subscriber: jfb. · View Herald Transcript