r359960 - Reduce amount of work ODR hashing does.
Richard Trieu via cfe-commits
cfe-commits at lists.llvm.org
Mon May 6 16:24:18 PDT 2019
There was no cycle for this crash. What happened is that an exponential
runtime is reduced to a linear runtime. Without this revision, ODR hashing
would have worked if the machine had enough memory and the user waited long
enough.
void foo(int a, int b) {}
When computing the ODR hash for function foo, it will visit the type int
twice, once per parameter. In general, re-visiting types shouldn't be a
problem, and in most cases, should be pretty fast.
class S {
void bar(S* s);
};
There's actually two ways to visit the Decl behind S, ODR::AddCXXRecordDecl
and ODR::AddDecl. When computing the ODR hash of S, ODR::AddCXXRecordDecl
is used for a deep dive into the AST of S. When reaching S another way,
(via FunctionDecl bar, parameter s, PointerType S*, RecordType S), then the
CXXRecordDecl gets processed through ODR::AddDecl, which only processes
enough information to identify S, but not any of its deeper details. This
allows self-reference without introducing cycles.
I think it would be possible to add some checks in debug mode to catch
cycles. I'm not sure it can detect redundant work as the function foo
example above shows that visiting the same types over multiple times is
expected.
*From: *David Blaikie <dblaikie at gmail.com>
*Date: *Sat, May 4, 2019 at 9:06 AM
*To: *Richard Trieu
*Cc: *cfe-commits
Does the ODR hashing have some sort of cycle breaking infrastructure -
> so that if the same type is seen more than once (eg: classes have
> members that have pointers back to the outer class type, etc) they
> don't cause indefinite cycles? Should that infrastructure have caught
> these cases & avoided the redundant work?
>
> I'm curious to understand better how these things work/overlap/or don't.
>
> On Fri, May 3, 2019 at 9:20 PM Richard Trieu via cfe-commits
> <cfe-commits at lists.llvm.org> wrote:
> >
> > Author: rtrieu
> > Date: Fri May 3 21:22:33 2019
> > New Revision: 359960
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=359960&view=rev
> > Log:
> > Reduce amount of work ODR hashing does.
> >
> > When a FunctionProtoType is in the original type in a DecayedType, the
> decayed
> > type is a PointerType which points back the original FunctionProtoType.
> The
> > visitor for ODRHashing will attempt to process both Type's, doing double
> work.
> > By chaining together multiple DecayedType's and FunctionProtoType's,
> this would
> > result in 2^N Type's visited only N DecayedType's and N
> FunctionProtoType's
> > exsit. Another bug where VisitDecayedType and VisitAdjustedType did
> > redundant work doubled the work at each level, giving 4^N Type's
> visited. This
> > patch removed the double work and detects when a FunctionProtoType
> decays to
> > itself to only check the Type once. This lowers the exponential runtime
> to
> > linear runtime. Fixes https://bugs.llvm.org/show_bug.cgi?id=41625
> >
> > Modified:
> > cfe/trunk/lib/AST/ODRHash.cpp
> > cfe/trunk/test/Modules/odr_hash.cpp
> >
> > Modified: cfe/trunk/lib/AST/ODRHash.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ODRHash.cpp?rev=359960&r1=359959&r2=359960&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/lib/AST/ODRHash.cpp (original)
> > +++ cfe/trunk/lib/AST/ODRHash.cpp Fri May 3 21:22:33 2019
> > @@ -703,14 +703,36 @@ public:
> > void VisitType(const Type *T) {}
> >
> > void VisitAdjustedType(const AdjustedType *T) {
> > - AddQualType(T->getOriginalType());
> > - AddQualType(T->getAdjustedType());
> > + QualType Original = T->getOriginalType();
> > + QualType Adjusted = T->getAdjustedType();
> > +
> > + // The original type and pointee type can be the same, as in the
> case of
> > + // function pointers decaying to themselves. Set a bool and only
> process
> > + // the type once, to prevent doubling the work.
> > + SplitQualType split = Adjusted.split();
> > + if (auto Pointer = dyn_cast<PointerType>(split.Ty)) {
> > + if (Pointer->getPointeeType() == Original) {
> > + Hash.AddBoolean(true);
> > + ID.AddInteger(split.Quals.getAsOpaqueValue());
> > + AddQualType(Original);
> > + VisitType(T);
> > + return;
> > + }
> > + }
> > +
> > + // The original type and pointee type are different, such as in the
> case
> > + // of a array decaying to an element pointer. Set a bool to false
> and
> > + // process both types.
> > + Hash.AddBoolean(false);
> > + AddQualType(Original);
> > + AddQualType(Adjusted);
> > +
> > VisitType(T);
> > }
> >
> > void VisitDecayedType(const DecayedType *T) {
> > - AddQualType(T->getDecayedType());
> > - AddQualType(T->getPointeeType());
> > + // getDecayedType and getPointeeType are derived from
> getAdjustedType
> > + // and don't need to be separately processed.
> > VisitAdjustedType(T);
> > }
> >
> >
> > Modified: cfe/trunk/test/Modules/odr_hash.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/odr_hash.cpp?rev=359960&r1=359959&r2=359960&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/test/Modules/odr_hash.cpp (original)
> > +++ cfe/trunk/test/Modules/odr_hash.cpp Fri May 3 21:22:33 2019
> > @@ -4587,6 +4587,43 @@ int num = bar();
> > #endif
> > }
> >
> > +namespace FunctionProtoTypeDecay {
> > +#if defined(FIRST)
> > +struct S1 {
> > + struct X {};
> > + using Y = X(X());
> > +};
> > +#elif defined(SECOND)
> > +struct S1 {
> > + struct X {};
> > + using Y = X(X(X()));
> > +};
> > +#else
> > +S1 s1;
> > +// expected-error at first.h:* {{'FunctionProtoTypeDecay::S1::Y' from
> module 'FirstModule' is not present in definition of
> 'FunctionProtoTypeDecay::S1' in module 'SecondModule'}}
> > +// expected-note at second.h:* {{declaration of 'Y' does not match}}
> > +#endif
> > +
> > +#if defined(FIRST)
> > +struct S2 {
> > + struct X {};
> > + using Y =
> > + X(X(X(X(X(X(X(X(X(X(X(X(X(X(X(X(
> > + X(X(X(X(X(X(X(X(X(X(X(X(X(X(X(X(
> > + X(X(X(X(X(X(X(X(X(X(X(X(X(X(X(X(
> > + X(X(X(X(X(X(X(X(X(X(X(X(X(X(X(X(
> > + ))))))))))))))))
> > + ))))))))))))))))
> > + ))))))))))))))))
> > + ))))))))))))))));
> > +};
> > +#elif defined(SECOND)
> > +#else
> > +S2 s2;
> > +#endif
> > +
> > +}
> > +
> > // Keep macros contained to one file.
> > #ifdef FIRST
> > #undef FIRST
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190506/580d54e7/attachment-0001.html>
More information about the cfe-commits
mailing list