Re: [Lldb-commits] [PATCH] Don't crash if a member declaration is incomplete
Ah, if clang-3.6 is on your path try -C clang-3.6 instead of /usr/bin/cc REPOSITORY rL LLVM http://reviews.llvm.org/D10509 EMAIL PREFERENCES http://reviews.llvm.org/settings/panel/emailpreferences/ ___ lldb-commits mailing list lldb-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] Don't crash if a member declaration is incomplete
Didn't help: kfischer@julia:~/llvm35/llvm-build$ dpkg --get-selections | grep clang clang install clang-3.6 install libclang-common-3.6-dev install libclang1-3.6:amd64 install kfischer@julia:~/llvm35/llvm-build$ dpkg --get-selections | grep libc++ libc++-dev:amd64 install libc++-dev:i386 install libc++-helpersinstall libc++1:amd64 install libc++1:i386 install Do I need to reconfigure something? REPOSITORY rL LLVM http://reviews.llvm.org/D10509 EMAIL PREFERENCES http://reviews.llvm.org/settings/panel/emailpreferences/ ___ lldb-commits mailing list lldb-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] Don't crash if a member declaration is incomplete
Hi @vharron, both my configurations were fresh clones of LLVM/Clang/LLDB, On Linux using CMake+Ninja, I got the following output from `ninja check-lldb`, https://gist.github.com/Keno/7a145dd774aead9db73d On OS X using CMake+Ninja, `ninja check` opens up a crash dialog for python, indicating a null pointer exception when importing the lldb module. I have not tested with XCode on OS X, though I can if you would like. REPOSITORY rL LLVM http://reviews.llvm.org/D10509 EMAIL PREFERENCES http://reviews.llvm.org/settings/panel/emailpreferences/ ___ lldb-commits mailing list lldb-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] Don't crash if a member declaration is incomplete
Hi Keno, The tests are generally passing cleanly on Linux and OSX for us. Can you post complete repro steps so we might be able to help troubleshoot? Please try building with CMake on Linux and Xcode on OSX to see if this improves for you. Vince On Jun 18, 2015 6:05 PM, "Keno Fischer" wrote: > @clayborg, would you mind trying out the tests and seeing if they can be > improved. I've tried getting the LLDB tests running, but on Linux the tests > just generally fail for me and on OS X, LLDB python segfaults as soon as it > tries to load LLDB, so I wasn't able to actually run them. Also, I don't > have any experience writing LLDB tests, so I'm sure they need to be > improved. > > > REPOSITORY > rL LLVM > > http://reviews.llvm.org/D10509 > > EMAIL PREFERENCES > http://reviews.llvm.org/settings/panel/emailpreferences/ > > > > ___ > lldb-commits mailing list > lldb-commits@cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits > ___ lldb-commits mailing list lldb-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] Don't crash if a member declaration is incomplete
@clayborg, would you mind trying out the tests and seeing if they can be improved. I've tried getting the LLDB tests running, but on Linux the tests just generally fail for me and on OS X, LLDB python segfaults as soon as it tries to load LLDB, so I wasn't able to actually run them. Also, I don't have any experience writing LLDB tests, so I'm sure they need to be improved. REPOSITORY rL LLVM http://reviews.llvm.org/D10509 EMAIL PREFERENCES http://reviews.llvm.org/settings/panel/emailpreferences/ ___ lldb-commits mailing list lldb-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] Don't crash if a member declaration is incomplete
Looks good. REPOSITORY rL LLVM http://reviews.llvm.org/D10509 EMAIL PREFERENCES http://reviews.llvm.org/settings/panel/emailpreferences/ ___ lldb-commits mailing list lldb-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] Don't crash if a member declaration is incomplete
Updated to use getAsTagDecl.
REPOSITORY
rL LLVM
http://reviews.llvm.org/D10509
Files:
source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
source/Symbol/ClangASTType.cpp
test/types/TestForwardTypes.py
test/types/forward_inheritance.cpp
test/types/forward_member.cpp
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2176,7 +2176,23 @@
}
}
}
-
+
+if (member_clang_type.GetCompleteType() == false)
+{
+GetObjectFile()->GetModule()->ReportError ("0x%8.8" PRIx64 " DW_TAG_member '%s' refers to type '%s' that is a forward declaration, not a complete definition."
+ "\nThis can happen due to missing images or compiler bugs.",
+ MakeUserID(die->GetOffset()),
+ name,
+ member_clang_type.GetTypeName().GetCString());
+
+// We have no choice other than to pretend that the base class
+// is complete. If we don't do this, clang will crash later when computing
+// the object layout.
+member_clang_type.StartTagDeclarationDefinition ();
+member_clang_type.CompleteTagDeclarationDefinition ();
+}
+
+
field_decl = class_clang_type.AddFieldToRecordType (name,
member_clang_type,
accessibility,
Index: source/Symbol/ClangASTType.cpp
===
--- source/Symbol/ClangASTType.cpp
+++ source/Symbol/ClangASTType.cpp
@@ -5851,15 +5851,11 @@
const clang::Type *t = qual_type.getTypePtr();
if (t)
{
-const clang::TagType *tag_type = llvm::dyn_cast(t);
-if (tag_type)
+clang::TagDecl *tag_decl = t->getAsTagDecl();
+if (tag_decl)
{
-clang::TagDecl *tag_decl = tag_type->getDecl();
-if (tag_decl)
-{
-tag_decl->startDefinition();
-return true;
-}
+tag_decl->startDefinition();
+return true;
}
const clang::ObjCObjectType *object_type = llvm::dyn_cast(t);
Index: test/types/TestForwardTypes.py
===
--- /dev/null
+++ test/types/TestForwardTypes.py
@@ -0,0 +1,44 @@
+"""
+Test that forward declarations to types in other compilation units do not crash
+the debugger.
+"""
+
+import AbstractBase
+import unittest2
+import lldb
+import sys
+from lldbtest import *
+
+class ForwardTypesTestCase(AbstractBase.GenericTester):
+mydir = AbstractBase.GenericTester.compute_mydir(__file__)
+
+def setUp(self):
+# Call super's setUp().
+AbstractBase.GenericTester.setUp(self)
+# disable "There is a running process, kill it and restart?" prompt
+self.runCmd("settings set auto-confirm true")
+self.addTearDownHook(lambda: self.runCmd("settings clear auto-confirm"))
+
+@dwarf_test
+def test_forward_member(self):
+"""Test that members with forwarded debug info don't crash the compiler"""
+self.source = 'forward_member.cpp'
+self.buildDwarf()
+target = self.dbg.createTarget("a.out")
+self.assertTrue(target, VALID_TARGET)
+self.runCmd("p (bar_t*)0")
+
+@dwarf_test
+def test_forward_inheritance(self):
+"""Test that bases with forwarded debug info don't crash the compiler"""
+self.source = 'forward_inheritance.cpp'
+self.buildDwarf()
+target = self.dbg.createTarget("a.out")
+self.assertTrue(target, VALID_TARGET)
+self.runCmd("p (bar*)0")
+
+if __name__ == '__main__':
+import atexit
+lldb.SBDebugger.Initialize()
+atexit.register(lambda: lldb.SBDebugger.Terminate())
+unittest2.main()
Index: test/types/forward_inheritance.cpp
===
--- /dev/nu
Re: [Lldb-commits] [PATCH] Don't crash if a member declaration is incomplete
REPOSITORY
rL LLVM
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2182
@@ +2181,3 @@
+{
+GetObjectFile()->GetModule()->ReportError
("0x%8.8" PRIx64 " DW_TAG_member '%s' refers to type '%s' that is a forward
declaration, not a complete definition."
+
"\nThis can happen due to missing images or compiler bugs.",
dblaikie wrote:
> Is this the same error text (modulo "member" rather than "inheritance") as
> the inheritance case that was already handled? Should the message go in one
> place rather than being duplicated?
It's a similar error message but it's in a different place (because of where
the bases vs members are constructed. I don't think it's worth moving the error
message until we have a way to actually search the rest of the images).
Comment at: source/Symbol/ClangASTType.cpp:5851
@@ -5850,1 +5850,3 @@
clang::QualType qual_type (GetQualType());
+clang::CXXRecordDecl *cxx_record_decl =
qual_type->getAsCXXRecordDecl();
+if (cxx_record_decl)
dblaikie wrote:
> loladiro wrote:
> > clayborg wrote:
> > > This patch is not needed. CXXRecordDecl is a TagType and it will be
> > > handled by the code below.
> > I thought so too, but it crashed without it. I now, realize that all that's
> > required is to use getAsTagDecl below rather than casting through the
> > TagType. That should handle this case as well. Will update.
> I'm guessing this (without context (both in the patch, and as I'm not an LLDB
> developer) I'm stabbing in the dark) is to address the forward decl as a base
> class produced by templates case? That wasn't handled, judging by the test
> case you added (or was that test case just for coverage, even though it was
> already covered?)?
This is required for both cases. The code was there, but it doesn't seem like
it ever actually worked because of this.
Comment at: source/Symbol/ClangASTType.cpp:5851-5857
@@ -5850,2 +5850,9 @@
clang::QualType qual_type (GetQualType());
+clang::CXXRecordDecl *cxx_record_decl =
qual_type->getAsCXXRecordDecl();
+if (cxx_record_decl)
+{
+cxx_record_decl->startDefinition();
+return true;
+}
+
const clang::Type *t = qual_type.getTypePtr();
clayborg wrote:
> This patch is not needed. CXXRecordDecl is a TagType and it will be handled
> by the code below.
I thought so too, but it crashed without it. I now, realize that all that's
required is to use getAsTagDecl below rather than casting through the TagType.
That should handle this case as well. Will update.
Comment at: test/types/TestForwardTypes.py:33
@@ +32,3 @@
+def test_forward_inheritance(self):
+"""Test that bases with forwarded debug info don't crash the
compiler"""
+self.source = 'forward_inheritance.cpp'
dblaikie wrote:
> Should these tests test for the specific error message/behavior rather than
> "do anything so long as it's not crashing"?
Yes, but I didn't know how to do that, so I figured I'd put it up and somebody
who know lldb will help out.
Comment at: test/types/forward_member.cpp:7
@@ +6,3 @@
+extern template struct foo;
+typedef foo fooint;
+
dblaikie wrote:
> are the typedefs (of foo and of bar) required here? I imagine not
> (similarly for the forward_inheritance example)
For some reason I had trouble getting it to crash without the typedefs, but
that was before I understood the issue fully, so I'll have another go at it.
http://reviews.llvm.org/D10509
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
___
lldb-commits mailing list
lldb-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] Don't crash if a member declaration is incomplete
REPOSITORY
rL LLVM
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2182
@@ +2181,3 @@
+{
+GetObjectFile()->GetModule()->ReportError
("0x%8.8" PRIx64 " DW_TAG_member '%s' refers to type '%s' that is a forward
declaration, not a complete definition."
+
"\nThis can happen due to missing images or compiler bugs.",
Is this the same error text (modulo "member" rather than "inheritance") as the
inheritance case that was already handled? Should the message go in one place
rather than being duplicated?
Comment at: source/Symbol/ClangASTType.cpp:5851
@@ -5850,1 +5850,3 @@
clang::QualType qual_type (GetQualType());
+clang::CXXRecordDecl *cxx_record_decl =
qual_type->getAsCXXRecordDecl();
+if (cxx_record_decl)
clayborg wrote:
> This patch is not needed. CXXRecordDecl is a TagType and it will be handled
> by the code below.
I'm guessing this (without context (both in the patch, and as I'm not an LLDB
developer) I'm stabbing in the dark) is to address the forward decl as a base
class produced by templates case? That wasn't handled, judging by the test case
you added (or was that test case just for coverage, even though it was already
covered?)?
Comment at: test/types/TestForwardTypes.py:33
@@ +32,3 @@
+def test_forward_inheritance(self):
+"""Test that bases with forwarded debug info don't crash the
compiler"""
+self.source = 'forward_inheritance.cpp'
Should these tests test for the specific error message/behavior rather than "do
anything so long as it's not crashing"?
Comment at: test/types/forward_inheritance.cpp:2
@@ +1,3 @@
+template
+class foo {
+ void func() { }
(I'd usually use struct rather than class when access doesn't matter - but...
it doesn't matter, so whichever suits)
Comment at: test/types/forward_inheritance.cpp:9
@@ +8,3 @@
+
+class bar : public fooint {
+};
you could drop the 'public' here, it doesn't matter (though, again, would
probably use struct rather than class, as well)
Comment at: test/types/forward_member.cpp:7
@@ +6,3 @@
+extern template struct foo;
+typedef foo fooint;
+
are the typedefs (of foo and of bar) required here? I imagine not
(similarly for the forward_inheritance example)
http://reviews.llvm.org/D10509
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
___
lldb-commits mailing list
lldb-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] Don't crash if a member declaration is incomplete
Changes to ClangASTType::StartTagDeclarationDefinition() should not be needed.
Please verify that the CXXRecordDecl is handled by the TagDecl:
const clang::Type *t = qual_type.getTypePtr();
if (t)
{
const clang::TagType *tag_type = llvm::dyn_cast(t);
if (tag_type)
{
clang::TagDecl *tag_decl = tag_type->getDecl();
if (tag_decl)
{
tag_decl->startDefinition();
return true;
}
}
REPOSITORY
rL LLVM
Comment at: source/Symbol/ClangASTType.cpp:5851-5857
@@ -5850,2 +5850,9 @@
clang::QualType qual_type (GetQualType());
+clang::CXXRecordDecl *cxx_record_decl =
qual_type->getAsCXXRecordDecl();
+if (cxx_record_decl)
+{
+cxx_record_decl->startDefinition();
+return true;
+}
+
const clang::Type *t = qual_type.getTypePtr();
This patch is not needed. CXXRecordDecl is a TagType and it will be handled by
the code below.
http://reviews.llvm.org/D10509
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
___
lldb-commits mailing list
lldb-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] Don't crash if a member declaration is incomplete
FYI if uploading diffs through the web interface it's useful to generate them with full context - e.g. `git diff -U9` or `svn diff --diff-cmd=diff -x -U99` REPOSITORY rL LLVM http://reviews.llvm.org/D10509 EMAIL PREFERENCES http://reviews.llvm.org/settings/panel/emailpreferences/ ___ lldb-commits mailing list lldb-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] Don't crash if a member declaration is incomplete
Hi clayborg,
This is progress toward PR19676/PR20781 and should have both cases error rather
than crashing the debugger
(The former PR already had a similar work around, but I believe the original
test case had the same issue as PR20781, so it never fully worked). Ideally, we
need to figure out how to search other images in the process for a definition
of this class at this point, instead of just pretending it's empty (which is
what this patch does), but I don't know enough about LLDB to make that happen.
Pointers appreciated.
REPOSITORY
rL LLVM
http://reviews.llvm.org/D10509
Files:
source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
source/Symbol/ClangASTType.cpp
test/types/TestForwardTypes.py
test/types/forward_inheritance.cpp
test/types/forward_member.cpp
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2176,7 +2176,23 @@
}
}
}
-
+
+if (member_clang_type.GetCompleteType() == false)
+{
+GetObjectFile()->GetModule()->ReportError ("0x%8.8" PRIx64 " DW_TAG_member '%s' refers to type '%s' that is a forward declaration, not a complete definition."
+ "\nThis can happen due to missing images or compiler bugs.",
+ MakeUserID(die->GetOffset()),
+ name,
+ member_clang_type.GetTypeName().GetCString());
+
+// We have no choice other than to pretend that the base class
+// is complete. If we don't do this, clang will crash later when computing
+// the object layout.
+member_clang_type.StartTagDeclarationDefinition ();
+member_clang_type.CompleteTagDeclarationDefinition ();
+}
+
+
field_decl = class_clang_type.AddFieldToRecordType (name,
member_clang_type,
accessibility,
Index: source/Symbol/ClangASTType.cpp
===
--- source/Symbol/ClangASTType.cpp
+++ source/Symbol/ClangASTType.cpp
@@ -5848,6 +5848,13 @@
if (IsValid())
{
clang::QualType qual_type (GetQualType());
+clang::CXXRecordDecl *cxx_record_decl = qual_type->getAsCXXRecordDecl();
+if (cxx_record_decl)
+{
+cxx_record_decl->startDefinition();
+return true;
+}
+
const clang::Type *t = qual_type.getTypePtr();
if (t)
{
Index: test/types/TestForwardTypes.py
===
--- /dev/null
+++ test/types/TestForwardTypes.py
@@ -0,0 +1,44 @@
+"""
+Test that forward declarations to types in other compilation units do not crash
+the debugger.
+"""
+
+import AbstractBase
+import unittest2
+import lldb
+import sys
+from lldbtest import *
+
+class ForwardTypesTestCase(AbstractBase.GenericTester):
+mydir = AbstractBase.GenericTester.compute_mydir(__file__)
+
+def setUp(self):
+# Call super's setUp().
+AbstractBase.GenericTester.setUp(self)
+# disable "There is a running process, kill it and restart?" prompt
+self.runCmd("settings set auto-confirm true")
+self.addTearDownHook(lambda: self.runCmd("settings clear auto-confirm"))
+
+@dwarf_test
+def test_forward_member(self):
+"""Test that members with forwarded debug info don't crash the compiler"""
+self.source = 'forward_member.cpp'
+self.buildDwarf()
+target = self.dbg.createTarget("a.out")
+self.assertTrue(target, VALID_TARGET)
+self.runCmd("p (bar_t*)0")
+
+@dwarf_test
+def test_forward_inheritance(self):
+"""Test that bases with forwarded debug info don't crash the compiler"""
+self.source = 'forward_inheritance.cpp'
+self.buildDwarf()
+target = self.dbg.createTarget("a.out")
+self.assertTrue(target, VALID_TARGET)
+self.runCmd("p (bar*)0")
+
+if __name__ == '__main__':
+import atexit
+lldb.SBDebugger.Initia
