[PATCH] D49158: [clang-tidy] Fixing segfault when there's no IdentifierInfo
aaron.ballman added a comment.
In https://reviews.llvm.org/D49158#1172178, @juliehockett wrote:
> In https://reviews.llvm.org/D49158#1159882, @hokein wrote:
>
> > In https://reviews.llvm.org/D49158#1158327, @JonasToth wrote:
> >
> > > Is there a way to add a test, that would trigger the old segfault and
> > > show that it does not happen anymore with this fix?
> >
> >
> > +1, we should have a minimal test case for this fix,
> > https://bugs.llvm.org/show_bug.cgi?id=36150 provides a case, but we need to
> > reduce it (getting rid of the STD header).
>
>
> Does anyone know of a case where the base would not be a CXXRecordDecl that
> doesn't involve std::functional? (for background, I could only reproduce the
> error in the test case in https://bugs.llvm.org/show_bug.cgi?id=36150 on a
> Mac)
Perhaps something like:
template
struct S : Ty {
S(Ty Fn) : Ty(Fn) {}
};
void f() {
S s{[](){}};
}
Not certain if this is what you're looking for, but it is a trick used in
implementations sometimes.
https://reviews.llvm.org/D49158
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49158: [clang-tidy] Fixing segfault when there's no IdentifierInfo
juliehockett added a comment. In https://reviews.llvm.org/D49158#1159882, @hokein wrote: > In https://reviews.llvm.org/D49158#1158327, @JonasToth wrote: > > > Is there a way to add a test, that would trigger the old segfault and show > > that it does not happen anymore with this fix? > > > +1, we should have a minimal test case for this fix, > https://bugs.llvm.org/show_bug.cgi?id=36150 provides a case, but we need to > reduce it (getting rid of the STD header). Does anyone know of a case where the base would not be a CXXRecordDecl that doesn't involve std::functional? (for background, I could only reproduce the error in the test case in https://bugs.llvm.org/show_bug.cgi?id=36150 on a Mac) https://reviews.llvm.org/D49158 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49158: [clang-tidy] Fixing segfault when there's no IdentifierInfo
hokein added a comment. In https://reviews.llvm.org/D49158#1158327, @JonasToth wrote: > Is there a way to add a test, that would trigger the old segfault and show > that it does not happen anymore with this fix? +1, we should have a minimal test case for this fix, https://bugs.llvm.org/show_bug.cgi?id=36150 provides a case, but we need to reduce it (getting rid of the STD header). https://reviews.llvm.org/D49158 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49158: [clang-tidy] Fixing segfault when there's no IdentifierInfo
JonasToth added a comment. Is there a way to add a test, that would trigger the old segfault and show that it does not happen anymore with this fix? https://reviews.llvm.org/D49158 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49158: [clang-tidy] Fixing segfault when there's no IdentifierInfo
JonasToth added inline comments.
Comment at: clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:44
bool ) const {
- StringRef Name = Node->getIdentifier()->getName();
- llvm::StringMapConstIterator Pair = InterfaceMap.find(Name);
- if (Pair == InterfaceMap.end())
-return false;
- isInterface = Pair->second;
- return true;
+ if (!Node) return false;
+ if (const auto *Id = Node->getIdentifier()) {
Please move then `return false` onto the next line.
https://reviews.llvm.org/D49158
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49158: [clang-tidy] Fixing segfault when there's no IdentifierInfo
juliehockett created this revision.
juliehockett added reviewers: aaron.ballman, hokein, ilya-biryukov.
juliehockett added a project: clang-tools-extra.
Herald added a subscriber: xazax.hun.
Bug 36150 found a segfault on mac when a CXXRecordDecl has no IdentifierInfo,
this fixes it.
https://reviews.llvm.org/D49158
Files:
clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
Index: clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
===
--- clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -30,21 +30,27 @@
// previously.
void MultipleInheritanceCheck::addNodeToInterfaceMap(const CXXRecordDecl *Node,
bool isInterface) {
- StringRef Name = Node->getIdentifier()->getName();
- InterfaceMap.insert(std::make_pair(Name, isInterface));
+ if (const auto *Id = Node->getIdentifier()) {
+StringRef Name = Id->getName();
+InterfaceMap.insert(std::make_pair(Name, isInterface));
+ }
}
// Returns "true" if the boolean "isInterface" has been set to the
// interface status of the current Node. Return "false" if the
// interface status for the current node is not yet known.
bool MultipleInheritanceCheck::getInterfaceStatus(const CXXRecordDecl *Node,
bool ) const {
- StringRef Name = Node->getIdentifier()->getName();
- llvm::StringMapConstIterator Pair = InterfaceMap.find(Name);
- if (Pair == InterfaceMap.end())
-return false;
- isInterface = Pair->second;
- return true;
+ if (!Node) return false;
+ if (const auto *Id = Node->getIdentifier()) {
+StringRef Name = Id->getName();
+llvm::StringMapConstIterator Pair = InterfaceMap.find(Name);
+if (Pair == InterfaceMap.end())
+ return false;
+isInterface = Pair->second;
+return true;
+ }
+ return false;
}
bool MultipleInheritanceCheck::isCurrentClassInterface(
@@ -104,7 +110,7 @@
const auto *Base = cast(Ty->getDecl()->getDefinition());
if (!isInterface(Base)) NumConcrete++;
}
-
+
// Check virtual bases to see if there is more than one concrete
// non-virtual base.
for (const auto : D->vbases()) {
Index: clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
===
--- clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -30,21 +30,27 @@
// previously.
void MultipleInheritanceCheck::addNodeToInterfaceMap(const CXXRecordDecl *Node,
bool isInterface) {
- StringRef Name = Node->getIdentifier()->getName();
- InterfaceMap.insert(std::make_pair(Name, isInterface));
+ if (const auto *Id = Node->getIdentifier()) {
+StringRef Name = Id->getName();
+InterfaceMap.insert(std::make_pair(Name, isInterface));
+ }
}
// Returns "true" if the boolean "isInterface" has been set to the
// interface status of the current Node. Return "false" if the
// interface status for the current node is not yet known.
bool MultipleInheritanceCheck::getInterfaceStatus(const CXXRecordDecl *Node,
bool ) const {
- StringRef Name = Node->getIdentifier()->getName();
- llvm::StringMapConstIterator Pair = InterfaceMap.find(Name);
- if (Pair == InterfaceMap.end())
-return false;
- isInterface = Pair->second;
- return true;
+ if (!Node) return false;
+ if (const auto *Id = Node->getIdentifier()) {
+StringRef Name = Id->getName();
+llvm::StringMapConstIterator Pair = InterfaceMap.find(Name);
+if (Pair == InterfaceMap.end())
+ return false;
+isInterface = Pair->second;
+return true;
+ }
+ return false;
}
bool MultipleInheritanceCheck::isCurrentClassInterface(
@@ -104,7 +110,7 @@
const auto *Base = cast(Ty->getDecl()->getDefinition());
if (!isInterface(Base)) NumConcrete++;
}
-
+
// Check virtual bases to see if there is more than one concrete
// non-virtual base.
for (const auto : D->vbases()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
