[PATCH] D110315: [Sema] Fix a null pointer reference crash.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:14496
 
-  if (FSI->UsesFPIntrin && !FD->hasAttr<StrictFPAttr>())
+  if (FSI->UsesFPIntrin && FD && !FD->hasAttr<StrictFPAttr>())
     FD->addAttr(StrictFPAttr::CreateImplicit(Context));
----------------
sammccall wrote:
> hokein wrote:
> > I have a reproduce test case, and wait for the creduce to minimize it (will add it once creduce finishes)
> > 
> > I think the bug is obvious,  by reading the code on the line 14495, FD could be a nullptr.  
> Yes, the bug is clear.
> It's not obvious to me that the fix is right, because I don't know:
>  - when dcl can be null
>  - when/if dcl can be non-null but neither a function nor function template
>  - how the FP attr should apply in such cases
> 
> cc @mibintc who may know the answer to at least the 3rd question, and I guess your testcase will give an example of either 1 or 2.
> 
> I'm *fairly* sure that not applying the strictfpattr is better than crashing here though.
my understanding is that dcl is `null` if clang fails to to parse the function declarator in some way, but still can recover to parse the function body.
The testcase shows it happens for a broken function template. I think it is fine to not apply attr, since the ast node is broken and likely discarded afterwards

I'm going to land this patch now as this issue blocks one of our internal tools, @mibintc feel free to polish it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110315/new/

https://reviews.llvm.org/D110315