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

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a subscriber: mibintc.
sammccall added a comment.
This revision is now accepted and ready to land.

LG because not crashing here is surely better than crashing, but I'm not sure whether the behaviour is actually right.
If we can't be sure, maybe leave a comment?



================
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));
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110315