FunctionDecl::getBody() returning nullptr

[Richard Smith via cfe-commits]

It looks like this would only happen for a late-parsed template that the
analysis code is checking before it is parsed. Should we really be running
these checks at all in that case?

Also, it looks like this code doesn't actually want the body at all, and
just wants to get the location of the definition. Asking for the body here
does not seem like the right approach, because it'll cause the external AST
source (if there is one) to fault the body in.

The right fix is probably something like:

  const FunctionDecl *Def;
  SourceLocation SL = (D->isDefined(Def) ? Def : D)->getLocation();

On Wed, Aug 19, 2015 at 1:44 PM, Aaron Ballman <aaron at aaronballman.com>
wrote:

> The attached patch resolves the null pointer crash I am seeing.
>
> ~Aaron
>
> On Wed, Aug 19, 2015 at 1:24 PM, Aaron Ballman <aaron at aaronballman.com>
> wrote:
> > On Wed, Aug 19, 2015 at 11:33 AM, Aaron Ballman <aaron at aaronballman.com>
> wrote:
> >> When I run the following test code through clang-tidy -checks=*, I get
> >> a crash from some relatively strange behavior with FunctionDecl.
> >>
> >> template <class T> struct remove_reference      {typedef T type;};
> >> template <class T> struct remove_reference<T&>  {typedef T type;};
> >> template <class T> struct remove_reference<T&&> {typedef T type;};
> >>
> >> template <typename T>
> >> typename remove_reference<T>::type&& move(T&& arg) {
> >>   return static_cast<typename remove_reference<T>::type&&>(arg);
> >> }
> >>
> >> AnalysisConsumer::getModeForDecl() is called, and it has code that
> >> does: D->hasBody() ? D->getBody()->stuff : stuff;
> >>
> >> What's strange is that hasBody() returns true, but getBody() returns
> >> nullptr. I don't think that this should be possible. I'm wondering
> >> whether it's purposeful that hasBody() does not check
> >> Definition->Body?
> >
> > Looking into this a bit further, the issue is that hasBody() checks
> > for Definition->Body *or* Definition->IsLateTemplateParsed when
> > deciding to return true. In my case, Body is null, but
> > IsLateTemplateParsed is true, so hasBody() returns true. However,
> > getBody() doesn't have any special logic for late template parsed
> > function bodies.
> >
> >> Also, the FunctionDecl in question is for move(), which does have a
> >> body, so I do not understand why the definition would claim there is
> >> no body.
> >>
> >> Ideas, or should I file a bug report?
> >
> > From my further look, I wonder if the correct solution to this is to
> > simply call getBody() and handle a nullptr return instead of calling
> > hasBody() first. It basically makes the check to ignore late parsed
> > template bodies an implicit one.
> >
> > ~Aaron
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150819/c83409c0/attachment.html>