lhames accepted this revision.
lhames added a comment.
Committed in r323163.
================
Comment at: Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2106
+// a vtable entry) from overloads (which require distinct entries).
+static bool isOverload(clang::CXXMethodDecl *m1, clang::CXXMethodDecl *m2) {
+ // FIXME: This should detect covariant return types, but currently doesn't.
----------------
clayborg wrote:
> I don't like things that can crash when asserts are off. I don't see why we
> wouldn't just check this, If someone does pass in a method from on AST and
> another from another AST, what will happen? Crash somewhere else? Why would
> we risk crashing or misbehaving here when it is so easy to check and avoid.
> I'll leave it at your discretion to do what you think is right though since I
> know clang does this all over.
This can not crash unless someone has called it out-of-contract (like passing a
nullptr into a routine that requires not-null). If user-input can cause a value
that would have been used as a non-null argument to be null it should be
guarded outside the method, not inside. I.e.:
static void foo(Foo *f) {
assert(f && "f must not be null");
//...
}
auto *f = lookup(readline());
if (!f)
return; // Can't call foo without a valid f so bail out
foo(f);
If you're passing something that can't be null, there's no need for the check:
Foo* bar(); // Never returns null
foo(bar()); // this is fine.
The case in this patch is like the latter: isOverload is called with two
CXXMethodDecls, where the second was found via a lookup on a CXXRecordDecl that
is on the same context as the first CXXMethodDecl. There's no intervening user
input that could mess with that, so we're safe.
Repository:
rL LLVM
https://reviews.llvm.org/D41997
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
