[PATCH] D152194: [NFC][CLANG] Fix nullptr dereference issue found by static analyzer tool
steakhal requested changes to this revision. steakhal added a comment. This revision now requires changes to proceed. I agree with @erichkeane. However, I can also see that the code could be improved. I don't understand why we have that variable hoisted from the guarded block in the first place. We could directly use `ReceiverType->castAs()` instead of `ReceiverObjectPtrType`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152194/new/ https://reviews.llvm.org/D152194 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D152194: [NFC][CLANG] Fix nullptr dereference issue found by static analyzer tool
erichkeane added inline comments.
Comment at: clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp:756
if (ReceiverType->isObjCIdType() || ReceiverType->isObjCClassType() ||
ASTCtxt.canAssignObjCInterfaces(ReceiverObjectPtrType, TrackedType)) {
const ObjCInterfaceDecl *InterfaceDecl = TrackedType->getInterfaceDecl();
erichkeane wrote:
> Manna wrote:
> > We are assigning: ReceiverObjectPtrType = nullptr return value from getAs.
> > ```
> > const auto *ReceiverObjectPtrType =
> > ReceiverType->getAs();
> > ```
> >
> > Then we are dereferencing nullptr ReceiverObjectPtrType when calling
> > canAssignObjCInterfaces()
> This isn't NFC, as `ReceiverObjectPtrType` is only used here. If the
> `MessageExpr` `ReceiverKind` is not `Instance` or `Class`, we never
> dereference this. So the declaration should be in this branch.
Oh, and ALSO, we don't dereference it if `ReceiverType` is `ObjCIdType` or
`ObhjCClassType`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D152194/new/
https://reviews.llvm.org/D152194
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D152194: [NFC][CLANG] Fix nullptr dereference issue found by static analyzer tool
erichkeane added inline comments.
Comment at: clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp:756
if (ReceiverType->isObjCIdType() || ReceiverType->isObjCClassType() ||
ASTCtxt.canAssignObjCInterfaces(ReceiverObjectPtrType, TrackedType)) {
const ObjCInterfaceDecl *InterfaceDecl = TrackedType->getInterfaceDecl();
Manna wrote:
> We are assigning: ReceiverObjectPtrType = nullptr return value from getAs.
> ```
> const auto *ReceiverObjectPtrType =
> ReceiverType->getAs();
> ```
>
> Then we are dereferencing nullptr ReceiverObjectPtrType when calling
> canAssignObjCInterfaces()
This isn't NFC, as `ReceiverObjectPtrType` is only used here. If the
`MessageExpr` `ReceiverKind` is not `Instance` or `Class`, we never dereference
this. So the declaration should be in this branch.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D152194/new/
https://reviews.llvm.org/D152194
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D152194: [NFC][CLANG] Fix nullptr dereference issue found by static analyzer tool
Manna added inline comments.
Comment at: clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp:756
if (ReceiverType->isObjCIdType() || ReceiverType->isObjCClassType() ||
ASTCtxt.canAssignObjCInterfaces(ReceiverObjectPtrType, TrackedType)) {
const ObjCInterfaceDecl *InterfaceDecl = TrackedType->getInterfaceDecl();
We are assigning: ReceiverObjectPtrType = nullptr return value from getAs.
```
const auto *ReceiverObjectPtrType =
ReceiverType->getAs();
```
Then we are dereferencing nullptr ReceiverObjectPtrType when calling
canAssignObjCInterfaces()
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D152194/new/
https://reviews.llvm.org/D152194
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D152194: [NFC][CLANG] Fix nullptr dereference issue found by static analyzer tool
Manna created this revision. Manna added a reviewer: erichkeane. Herald added subscribers: steakhal, manas, ASDenysPetrov, martong, dkrupp, donat.nagy, Szelethus, a.sidorin, baloghadamsoftware. Herald added a reviewer: NoQ. Herald added a project: All. Manna requested review of this revision. Herald added a project: clang. This patch uses castAs instead of getAs which will assert if the type doesn't match in findMethodDecl(clang::ObjCMessageExpr const *, clang::ObjCObjectPointerType const *, clang::ASTContext &). Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D152194 Files: clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp Index: clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp === --- clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp +++ clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp @@ -743,7 +743,7 @@ QualType ReceiverType = MessageExpr->getReceiverType(); const auto *ReceiverObjectPtrType = - ReceiverType->getAs(); + ReceiverType->castAs(); // Do this "devirtualization" on instance and class methods only. Trust the // static type on super and super class calls. Index: clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp === --- clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp +++ clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp @@ -743,7 +743,7 @@ QualType ReceiverType = MessageExpr->getReceiverType(); const auto *ReceiverObjectPtrType = - ReceiverType->getAs(); + ReceiverType->castAs(); // Do this "devirtualization" on instance and class methods only. Trust the // static type on super and super class calls. ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
