[Lldb-commits] [lldb] r265124 - Change a recently added assert into lldbassert
Author: labath Date: Fri Apr 1 04:57:30 2016 New Revision: 265124 URL: http://llvm.org/viewvc/llvm-project?rev=265124&view=rev Log: Change a recently added assert into lldbassert Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp?rev=265124&r1=265123&r2=265124&view=diff == --- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp (original) +++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp Fri Apr 1 04:57:30 2016 @@ -12,6 +12,7 @@ #include "lldb/Core/Section.h" #include "lldb/Expression/DWARFExpression.h" #include "lldb/Symbol/ObjectFile.h" +#include "lldb/Utility/LLDBAssert.h" #include "DWARFCompileUnit.h" #include "DWARFDebugInfo.h" @@ -133,6 +134,6 @@ SymbolFileDWARFDwo::GetTypeSystemForLang DWARFDIE SymbolFileDWARFDwo::GetDIE(const DIERef &die_ref) { -assert(m_base_dwarf_cu->GetOffset() == die_ref.cu_offset); +lldbassert(m_base_dwarf_cu->GetOffset() == die_ref.cu_offset); return DebugInfo()->GetDIEForDIEOffset(die_ref.die_offset); } ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18646: Fix DWO breakage in r264909
tberghammer added a comment. This assert is NOT verifying the debug info itself. It is verifying that LLDB asked the right SymbolFileDWARFDwo to find a DIE based on a DIERef. Independently of the debug info (what can be garbage) passed in to LLDB this assert should never fire if LLDB has no major bug in the DWO handling logic. Considering that we have ~10k assert in LLVM, ~6k assert in clang and ~1k assert in LLDB you should compile your release binary without asserts if you want to avoid some crash what is caused by them. In this concrete case if we return an empty DWARFDIE from this function then the caller of the function will crash (with a SEGV) as it is not prepared to handle the case when no DWARFDIE can be found based on a DIERef as it should never happen if LLDB works correctly (independently of the debug info). I think the very high level question we are discussing is what should LLDB do when it notices that it has a bug in itself. In terms of a compiler displaying an "internal compiler error" and the exiting/crashing is definitely the good solution (to avoid miss compilation) but what is the expected behavior for an interactive application. If you want to be protected against every possible issue then we should remove all assert from LLDB, convince the LLVM and clang guys to do the same and protect every pointer deference with a null check just in case some of the previous function calls returned NULL. I don't think this is the approach we are wanting to go down as it will add a lot of overhead to LLDB and make a lot of bug silent what is very problematic in case of a debugger where displaying incorrect information is almost as bad as crashing the IDE. PS: Pavel changed the assert to an lldb_assert in the meantime. Repository: rL LLVM http://reviews.llvm.org/D18646 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D18689: Make FileSpec handling platform-independent
labath created this revision. labath added a reviewer: zturner. labath added a subscriber: lldb-commits. Even though FileSpec attempted to handle both kinds of path syntaxes (posix and windows) on both platforms, it relied on the llvm path library to do its work, whose behavior differed on different platforms. This led to subtle differences in FileSpec behavior between platforms. This replaces the pieces of the llvm library with our own implementations. The functions are simply copied from llvm, with #ifdefs replaced by runtime checks for ePathSyntaxWindows. http://reviews.llvm.org/D18689 Files: source/Host/common/FileSpec.cpp unittests/Host/FileSpecTest.cpp Index: unittests/Host/FileSpecTest.cpp === --- unittests/Host/FileSpecTest.cpp +++ unittests/Host/FileSpecTest.cpp @@ -22,18 +22,23 @@ FileSpec fs_windows("F:\\bar", false, FileSpec::ePathSyntaxWindows); EXPECT_STREQ("F:\\bar", fs_windows.GetCString()); -EXPECT_STREQ("F:", fs_windows.GetDirectory().GetCString()); +EXPECT_STREQ("F:\\", fs_windows.GetDirectory().GetCString()); EXPECT_STREQ("bar", fs_windows.GetFilename().GetCString()); FileSpec fs_posix_root("/", false, FileSpec::ePathSyntaxPosix); EXPECT_STREQ("/", fs_posix_root.GetCString()); EXPECT_EQ(nullptr, fs_posix_root.GetDirectory().GetCString()); EXPECT_STREQ("/", fs_posix_root.GetFilename().GetCString()); -FileSpec fs_windows_root("F:", false, FileSpec::ePathSyntaxWindows); -EXPECT_STREQ("F:", fs_windows_root.GetCString()); -EXPECT_EQ(nullptr, fs_windows_root.GetDirectory().GetCString()); -EXPECT_STREQ("F:", fs_windows_root.GetFilename().GetCString()); +FileSpec fs_windows_drive("F:", false, FileSpec::ePathSyntaxWindows); +EXPECT_STREQ("F:", fs_windows_drive.GetCString()); +EXPECT_EQ(nullptr, fs_windows_drive.GetDirectory().GetCString()); +EXPECT_STREQ("F:", fs_windows_drive.GetFilename().GetCString()); + +FileSpec fs_windows_root("F:\\", false, FileSpec::ePathSyntaxWindows); +EXPECT_STREQ("F:\\", fs_windows_root.GetCString()); +EXPECT_STREQ("F:", fs_windows_root.GetDirectory().GetCString()); +EXPECT_STREQ("\\", fs_windows_root.GetFilename().GetCString()); FileSpec fs_posix_long("/foo/bar/baz", false, FileSpec::ePathSyntaxPosix); EXPECT_STREQ("/foo/bar/baz", fs_posix_long.GetCString()); @@ -43,7 +48,7 @@ FileSpec fs_windows_long("F:\\bar\\baz", false, FileSpec::ePathSyntaxWindows); EXPECT_STREQ("F:\\bar\\baz", fs_windows_long.GetCString()); // We get "F:/bar" instead. -// EXPECT_STREQ("F:\\bar", fs_windows_long.GetDirectory().GetCString()); +EXPECT_STREQ("F:\\bar", fs_windows_long.GetDirectory().GetCString()); EXPECT_STREQ("baz", fs_windows_long.GetFilename().GetCString()); FileSpec fs_posix_trailing_slash("/foo/bar/", false, FileSpec::ePathSyntaxPosix); @@ -54,7 +59,7 @@ FileSpec fs_windows_trailing_slash("F:\\bar\\", false, FileSpec::ePathSyntaxWindows); EXPECT_STREQ("F:\\bar\\.", fs_windows_trailing_slash.GetCString()); // We get "F:/bar" instead. -// EXPECT_STREQ("F:\\bar", fs_windows_trailing_slash.GetDirectory().GetCString()); +EXPECT_STREQ("F:\\bar", fs_windows_trailing_slash.GetDirectory().GetCString()); EXPECT_STREQ(".", fs_windows_trailing_slash.GetFilename().GetCString()); } @@ -66,22 +71,22 @@ EXPECT_STREQ("/foo", fs_posix.GetDirectory().GetCString()); EXPECT_STREQ("bar", fs_posix.GetFilename().GetCString()); -FileSpec fs_windows("F:", false, FileSpec::ePathSyntaxWindows); -fs_windows.AppendPathComponent("bar"); -EXPECT_STREQ("F:\\bar", fs_windows.GetCString()); -EXPECT_STREQ("F:", fs_windows.GetDirectory().GetCString()); -EXPECT_STREQ("bar", fs_windows.GetFilename().GetCString()); +FileSpec fs_windows("F:\\bar", false, FileSpec::ePathSyntaxWindows); +fs_windows.AppendPathComponent("baz"); +EXPECT_STREQ("F:\\bar\\baz", fs_windows.GetCString()); +EXPECT_STREQ("F:\\bar", fs_windows.GetDirectory().GetCString()); +EXPECT_STREQ("baz", fs_windows.GetFilename().GetCString()); FileSpec fs_posix_root("/", false, FileSpec::ePathSyntaxPosix); fs_posix_root.AppendPathComponent("bar"); EXPECT_STREQ("/bar", fs_posix_root.GetCString()); EXPECT_STREQ("/", fs_posix_root.GetDirectory().GetCString()); EXPECT_STREQ("bar", fs_posix_root.GetFilename().GetCString()); -FileSpec fs_windows_root("F:", false, FileSpec::ePathSyntaxWindows); +FileSpec fs_windows_root("F:\\", false, FileSpec::ePathSyntaxWindows); fs_windows_root.AppendPathComponent("bar"); EXPECT_STREQ("F:\\bar", fs_windows_root.GetCString()); -EXPECT_STREQ("F:", fs_windows_root.GetDirectory().GetCString()); +EXPECT_STREQ("F:\\", fs_windows_root.GetDirectory().GetCString()); EXPECT_STREQ("bar", fs_windows_root.GetFilename().GetCString()); } Index: source/Host/common/FileSpec.cpp =
[Lldb-commits] [PATCH] D18692: Fix a cornercase in breakpoint reporting
labath created this revision. labath added a reviewer: clayborg. labath added a subscriber: lldb-commits. This resolves a similar problem as D16720 (which handled the case when we single-step onto a breakpoint), but this one deals with involutary stops: when we stop a thread (e.g. because another thread has hit a breakpont and we are doing a full stop), we can end up stopping it right before it executes a breakpoint instruction. In this case, the stop reason will be empty, but we will still step over the breakpoint when do the next resume, thereby missing a breakpoint hit. I have observed this happening in TestConcurrentEvents, but I have no idea how to reproduce this behavior more reliably. http://reviews.llvm.org/D18692 Files: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp === --- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -2089,6 +2089,23 @@ handled = true; } } +else +{ +addr_t pc = thread_sp->GetRegisterContext()->GetPC(); +lldb::BreakpointSiteSP bp_site_sp = + thread_sp->GetProcess()->GetBreakpointSiteList().FindByAddress(pc); + +// If the current pc is a breakpoint site then the StopInfo should be set to Breakpoint +// even though the remote stub did not set it as such. This can happen when +// the thread is involuntarily interrupted (e.g. due to stops on other +// threads) just as it is about to execute the breakpoint instruction. +if (bp_site_sp && bp_site_sp->ValidForThisThread(thread_sp.get())) +{ +thread_sp->SetStopInfo( + StopInfo::CreateStopReasonWithBreakpointSiteID(*thread_sp, bp_site_sp->GetID())); +handled = true; +} +} if (!handled && signo && did_exec == false) { Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp === --- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -2089,6 +2089,23 @@ handled = true; } } +else +{ +addr_t pc = thread_sp->GetRegisterContext()->GetPC(); +lldb::BreakpointSiteSP bp_site_sp = +thread_sp->GetProcess()->GetBreakpointSiteList().FindByAddress(pc); + +// If the current pc is a breakpoint site then the StopInfo should be set to Breakpoint +// even though the remote stub did not set it as such. This can happen when +// the thread is involuntarily interrupted (e.g. due to stops on other +// threads) just as it is about to execute the breakpoint instruction. +if (bp_site_sp && bp_site_sp->ValidForThisThread(thread_sp.get())) +{ +thread_sp->SetStopInfo( +StopInfo::CreateStopReasonWithBreakpointSiteID(*thread_sp, bp_site_sp->GetID())); +handled = true; +} +} if (!handled && signo && did_exec == false) { ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r265140 - Fix clean rule for a makefile
Author: labath Date: Fri Apr 1 07:59:37 2016 New Revision: 265140 URL: http://llvm.org/viewvc/llvm-project?rev=265140&view=rev Log: Fix clean rule for a makefile The test was failing on windows because the clean rule (which is executed even if the test is skipped) returned an error there. Modified: lldb/trunk/packages/Python/lldbsuite/test/lang/objc/objc-static-method-stripped/Makefile Modified: lldb/trunk/packages/Python/lldbsuite/test/lang/objc/objc-static-method-stripped/Makefile URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/objc/objc-static-method-stripped/Makefile?rev=265140&r1=265139&r2=265140&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/lang/objc/objc-static-method-stripped/Makefile (original) +++ lldb/trunk/packages/Python/lldbsuite/test/lang/objc/objc-static-method-stripped/Makefile Fri Apr 1 07:59:37 2016 @@ -11,6 +11,6 @@ a.out.stripped: a.out.dSYM clean:: rm -f a.out.stripped - rm -rf *.dSYM + rm -rf $(wildcard *.dSYM) include $(LEVEL)/Makefile.rules ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18692: Fix a cornercase in breakpoint reporting
labath updated this revision to Diff 52357. labath added a comment. I've realized that some foreign stubs may not send the "reason" field, so I limit the scope of this change to cases where "signal" is zero as well. http://reviews.llvm.org/D18692 Files: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp === --- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -2089,6 +2089,23 @@ handled = true; } } +else if (!signo) +{ +addr_t pc = thread_sp->GetRegisterContext()->GetPC(); +lldb::BreakpointSiteSP bp_site_sp = + thread_sp->GetProcess()->GetBreakpointSiteList().FindByAddress(pc); + +// If the current pc is a breakpoint site then the StopInfo should be set to Breakpoint +// even though the remote stub did not set it as such. This can happen when +// the thread is involuntarily interrupted (e.g. due to stops on other +// threads) just as it is about to execute the breakpoint instruction. +if (bp_site_sp && bp_site_sp->ValidForThisThread(thread_sp.get())) +{ +thread_sp->SetStopInfo( + StopInfo::CreateStopReasonWithBreakpointSiteID(*thread_sp, bp_site_sp->GetID())); +handled = true; +} +} if (!handled && signo && did_exec == false) { Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp === --- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -2089,6 +2089,23 @@ handled = true; } } +else if (!signo) +{ +addr_t pc = thread_sp->GetRegisterContext()->GetPC(); +lldb::BreakpointSiteSP bp_site_sp = +thread_sp->GetProcess()->GetBreakpointSiteList().FindByAddress(pc); + +// If the current pc is a breakpoint site then the StopInfo should be set to Breakpoint +// even though the remote stub did not set it as such. This can happen when +// the thread is involuntarily interrupted (e.g. due to stops on other +// threads) just as it is about to execute the breakpoint instruction. +if (bp_site_sp && bp_site_sp->ValidForThisThread(thread_sp.get())) +{ +thread_sp->SetStopInfo( +StopInfo::CreateStopReasonWithBreakpointSiteID(*thread_sp, bp_site_sp->GetID())); +handled = true; +} +} if (!handled && signo && did_exec == false) { ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18464: Implement `target modules dump headers`
amccarth added a comment. Ping. http://reviews.llvm.org/D18464 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D18697: Fix a bug in linux core file handling
labath created this revision. labath added reviewers: clayborg, zturner. labath added a subscriber: lldb-commits. There was a bug in linux core file handling, where if there was a running process with the same process id as the id in the core file, the core file debugging would fail, as we would pull some pieces of information (ProcessInfo structure) from the running process instead of the core file. I fix this by routing the ProcessInfo requests through the Process class and overriding it in ProcessElfCore to return correct data. A (slightly convoluted) test is included. http://reviews.llvm.org/D18697 Files: include/lldb/Target/Process.h packages/Python/lldbsuite/test/functionalities/postmortem/linux-core/TestLinuxCore.py source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp source/Plugins/Process/elf-core/ProcessElfCore.cpp source/Plugins/Process/elf-core/ProcessElfCore.h source/Target/Process.cpp Index: source/Target/Process.cpp === --- source/Target/Process.cpp +++ source/Target/Process.cpp @@ -3422,7 +3422,7 @@ else if (!process_arch.IsValid()) { ProcessInstanceInfo process_info; -platform_sp->GetProcessInfo (GetID(), process_info); +GetProcessInfo(process_info); const ArchSpec &process_arch = process_info.GetArchitecture(); if (process_arch.IsValid() && !GetTarget().GetArchitecture().IsExactMatch(process_arch)) { @@ -6481,6 +6481,12 @@ } } +bool +Process::GetProcessInfo(ProcessInstanceInfo &info) +{ +return GetTarget().GetPlatform()->GetProcessInfo(GetID(), info); +} + ThreadCollectionSP Process::GetHistoryThreads(lldb::addr_t addr) { Index: source/Plugins/Process/elf-core/ProcessElfCore.h === --- source/Plugins/Process/elf-core/ProcessElfCore.h +++ source/Plugins/Process/elf-core/ProcessElfCore.h @@ -111,6 +111,9 @@ const lldb::DataBufferSP GetAuxvData() override; +bool +GetProcessInfo(lldb_private::ProcessInstanceInfo &info) override; + protected: void Clear ( ); Index: source/Plugins/Process/elf-core/ProcessElfCore.cpp === --- source/Plugins/Process/elf-core/ProcessElfCore.cpp +++ source/Plugins/Process/elf-core/ProcessElfCore.cpp @@ -637,3 +637,12 @@ lldb::DataBufferSP buffer(new lldb_private::DataBufferHeap(start, len)); return buffer; } + +bool +ProcessElfCore::GetProcessInfo(ProcessInstanceInfo &info) +{ +info.Clear(); +info.SetProcessID(GetID()); +info.SetArchitecture(GetArchitecture()); +return true; +} Index: source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp === --- source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp +++ source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp @@ -656,7 +656,7 @@ const auto platform_sp = target.GetPlatform (); ProcessInstanceInfo process_info; -if (!platform_sp->GetProcessInfo (m_process->GetID (), process_info)) +if (!m_process->GetProcessInfo(process_info)) { if (log) log->Printf ("DynamicLoaderPOSIXDYLD::%s - failed to get process info for pid %" PRIu64, Index: packages/Python/lldbsuite/test/functionalities/postmortem/linux-core/TestLinuxCore.py === --- packages/Python/lldbsuite/test/functionalities/postmortem/linux-core/TestLinuxCore.py +++ packages/Python/lldbsuite/test/functionalities/postmortem/linux-core/TestLinuxCore.py @@ -4,29 +4,56 @@ from __future__ import print_function +import shutil +import struct + import lldb from lldbsuite.test.decorators import * from lldbsuite.test.lldbtest import * from lldbsuite.test import lldbutil class LinuxCoreTestCase(TestBase): +NO_DEBUG_INFO_TESTCASE = True mydir = TestBase.compute_mydir(__file__) +_i386_pid = 32306 +_x86_64_pid = 32259 + @skipIf(bugnumber="llvm.org/pr26947") -@no_debug_info_test def test_i386(self): """Test that lldb can read the process information from an i386 linux core file.""" -self.do_test("i386", 32306) +self.do_test("i386", self._i386_pid) -@no_debug_info_test def test_x86_64(self): """Test that lldb can read the process information from an x86_64 linux core file.""" -self.do_test("x86_64", 32259) +self.do_test("x86_64", self._x86_64_pid) + +def test_x86_64_same_pid(self): +"""Test that we read the information from the core correctly even if we have a running +process with the same PID around""" +try: +shutil.copyfile("x86_64.out", "x86_64-pid.out") +shutil.copyfile("x86_64.core", "x86_64-pid.core") +with open("x86
Re: [Lldb-commits] [PATCH] D18697: Fix a bug in linux core file handling
What if you are simultaneously debugging 2 processes with the same pid? Does this fail? On Fri, Apr 1, 2016 at 9:06 AM Pavel Labath wrote: > labath created this revision. > labath added reviewers: clayborg, zturner. > labath added a subscriber: lldb-commits. > > There was a bug in linux core file handling, where if there was a running > process with the same > process id as the id in the core file, the core file debugging would fail, > as we would pull some > pieces of information (ProcessInfo structure) from the running process > instead of the core file. > I fix this by routing the ProcessInfo requests through the Process class > and overriding it in > ProcessElfCore to return correct data. > > A (slightly convoluted) test is included. > > http://reviews.llvm.org/D18697 > > Files: > include/lldb/Target/Process.h > > packages/Python/lldbsuite/test/functionalities/postmortem/linux-core/TestLinuxCore.py > source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp > source/Plugins/Process/elf-core/ProcessElfCore.cpp > source/Plugins/Process/elf-core/ProcessElfCore.h > source/Target/Process.cpp > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18692: Fix a cornercase in breakpoint reporting
clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Looks fine. We should probably put the code that checks for a breakpoint at the thread PC into a function since it is done about 5 times in this function. http://reviews.llvm.org/D18692 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18464: Implement `target modules dump headers`
clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. I would limit this scope to dumping ObjectFiles since that seems to be what you want. So this command should probably change from: (lldb) target modules dump headers to: (lldb) target modules dump objfile Then CommandObjectTargetModulesDump would just get the ObjectFile from any modules in the arguments and call ObjectFile::Dump(...) instead of module->Dump() since module->Dump() dumps the module itself, and the object file and the symbol file. We already have a "target modules dump symfile", so "target modules dump objfile" makes more sense here. http://reviews.llvm.org/D18464 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18697: Fix a bug in linux core file handling
clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. See inlined comments. Comment at: source/Plugins/Process/elf-core/ProcessElfCore.cpp:644-647 @@ +643,6 @@ +{ +info.Clear(); +info.SetProcessID(GetID()); +info.SetArchitecture(GetArchitecture()); +return true; +} Can the ELF core really not figure out what its executable is? Shouldn't it be filling in info.m_executable by calling info.SetExecutableFile(...)? Comment at: source/Target/Process.cpp:6487 @@ +6486,3 @@ +{ +return GetTarget().GetPlatform()->GetProcessInfo(GetID(), info); +} We should probably be extra safe here and do: ``` PlatformSP platform_sp = GetTarget().GetPlatform(); if (platform_sp) return platform_sp->GetProcessInfo(GetID(), info); else return false; ``` http://reviews.llvm.org/D18697 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18464: Implement `target modules dump headers`
zturner added a subscriber: zturner. zturner added a comment. FWIW I believe we do actually want many of the PE headers, although I have to say I don't like the format of the output.It seems like we could break this up into smaller chunks like section headers, pe headers, binary headers, debug headers, and then allow the user to specify some combination of flags (or all) to display different levels of detail. But that could come as a followup. And also translate some of the hex codes to textual enum values like IMAGE_MACHINE_386 so it's easier to understand the output. In the future we might even want a way to add operating system specific stuff to the dump. Windows for example can have resource files embedded in them that show file version and things like that, and it's nice to get a unified view that shows some of this OS specific stuff interspersed in a nicely formatted manner. A lot of this type of information is useful for crash reporting and bucketing http://reviews.llvm.org/D18464 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18646: Fix DWO breakage in r264909
> On Apr 1, 2016, at 3:43 AM, Tamas Berghammer wrote: > > tberghammer added a comment. > > This assert is NOT verifying the debug info itself. It is verifying that LLDB > asked the right SymbolFileDWARFDwo to find a DIE based on a DIERef. > Independently of the debug info (what can be garbage) passed in to LLDB this > assert should never fire if LLDB has no major bug in the DWO handling logic. > > Considering that we have ~10k assert in LLVM, ~6k assert in clang and ~1k > assert in LLDB you should compile your release binary without asserts if you > want to avoid some crash what is caused by them. > > In this concrete case if we return an empty DWARFDIE from this function then > the caller of the function will crash (with a SEGV) as it is not prepared to > handle the case when no DWARFDIE can be found based on a DIERef as it should > never happen if LLDB works correctly (independently of the debug info). We do have places in the DWARF reader where we assert for nonsensical input, e.g. if you get to SymbolFileDwarf then the SymbolContext that you used to get there has to have a compile unit in it, otherwise you shouldn't find your way there. I don't know this particular section of code well enough to comment assertively here if this is or is not in this category. But one of the problems we have with clang, for instance, is that clang's assumption was that it was going to build up its data structures by parsing code, and it has defenses at the parse stage to reject input that would result in invalid types. So it seems eminently reasonable to assert if anything is wrong with for example the types it has already processed rather than do any work to try to back out. But lldb by necessity comes in below the layer of these defenses, since we transcode DWARF, not source code, into types, and so we come a cropper with asserts that weren't really fatal, but were reasonable for the normal mode of use in the compiler. By analogy in lldb, the rule for debug info, it is wise to assume there's going to be some junk in the debug info that you will need to sort through. Even if you have to scotch a whole compile unit's debug information because you can't make sense of it, that should not be a fatal error in the debugger. An assert that is provably only triggered by a programming error in that API's usage is sometimes acceptable. But unless you're really sure there's no way bad data could get you to the assert, then we should try to back out instead. > > I think the very high level question we are discussing is what should LLDB do > when it notices that it has a bug in itself. In terms of a compiler > displaying an "internal compiler error" and the exiting/crashing is > definitely the good solution (to avoid miss compilation) but what is the > expected behavior for an interactive application. If you want to be protected > against every possible issue then we should remove all assert from LLDB, > convince the LLVM and clang guys to do the same and protect every pointer > deference with a null check just in case some of the previous function calls > returned NULL. I don't think this is the approach we are wanting to go down > as it will add a lot of overhead to LLDB and make a lot of bug silent what is > very problematic in case of a debugger where displaying incorrect information > is almost as bad as crashing the IDE. Yea, we shouldn't silently swallow errors when we come across something that isn't what the code expected. Nor should we check every single pointer everywhere by reflex, that would be awkward as you say. But the design for lldb should be that we strenuously attempt to always return a "nope couldn't do that" error if at all possible. And callers should be prepared to handle that. The debugger has lots of independent parts. So if the expression parser has an error, or some type is mangled so we can't realize it, there are lots of other ways to look at data, and lots of other types to use which alternatives are rendered unavailable to the user if we crash on error. And a long debug session is pretty stateful, so just putting lldb out of process, letting it crash and then maybe auto-reattaching is only a partial solution at best. Jim > > PS: Pavel changed the assert to an lldb_assert in the meantime. > > > Repository: > rL LLVM > > http://reviews.llvm.org/D18646 > > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r265165 - Guard xunit result test class and test method name access to prevent testbot breakage
Author: tfiala Date: Fri Apr 1 12:59:51 2016 New Revision: 265165 URL: http://llvm.org/viewvc/llvm-project?rev=265165&view=rev Log: Guard xunit result test class and test method name access to prevent testbot breakage http://llvm.org/bugs/show_bug.cgi?id=27179 Modified: lldb/trunk/packages/Python/lldbsuite/test/xunit_formatter.py Modified: lldb/trunk/packages/Python/lldbsuite/test/xunit_formatter.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/xunit_formatter.py?rev=265165&r1=265164&r2=265165&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/xunit_formatter.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/xunit_formatter.py Fri Apr 1 12:59:51 2016 @@ -484,8 +484,8 @@ class XunitFormatter(ResultsFormatter): """ # Get elapsed time. -test_class = test_event["test_class"] -test_name = test_event["test_name"] +test_class = test_event.get("test_class", "") +test_name = test_event.get("test_name", "") event_time = test_event["event_time"] time_taken = self.elapsed_time_for_test( test_class, test_name, event_time) ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18692: Fix a cornercase in breakpoint reporting
jingham added a subscriber: jingham. jingham added a comment. The only worry I have about this is - thread A is stopped at a breakpoint, and then we stop on thread B instead, then we report a breakpoint, great! - The user steps thread B, which we do by suspending thread B and stepping A. - Then A stops, and we report ANOTHER hit on thread B. Your fix looks better than what was there previously. If it is easy to check "last stop reason was this breakpoint hit, and this thread's temporary resume state is suspended, then don't report the hit, that would be more accurate. http://reviews.llvm.org/D18692 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18464: Implement `target modules dump headers`
clayborg added a comment. In http://reviews.llvm.org/D18464#389612, @zturner wrote: > FWIW I believe we do actually want many of the PE headers, although I have to > say I don't like the format of the output.It seems like we could break > this up into smaller chunks like section headers, pe headers, binary headers, > debug headers, and then allow the user to specify some combination of flags > (or all) to display different levels of detail. But that could come as a > followup. And also translate some of the hex codes to textual enum values > like IMAGE_MACHINE_386 so it's easier to understand the output. > > In the future we might even want a way to add operating system specific stuff > to the dump. Windows for example can have resource files embedded in them > that show file version and things like that, and it's nice to get a unified > view that shows some of this OS specific stuff interspersed in a nicely > formatted manner. A lot of this type of information is useful for crash > reporting and bucketing I agreed. To go a step further, we could have ObjectFile::Dump() take an extra "Args &args" parameter where each object file can have its own dumping args. It could then allow the PECOFF dumping to have options that are similar to the program that actually dumps things on the windows platform, mach-o can have options for dumping load command and many other mach specific things, and ELF could have options to dump the program and sections headers, the raw ELF symbol table and much more. Then the new "objfile" command could chop up any remaining args and pass then down into ObjectFile::Dump(). http://reviews.llvm.org/D18464 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r265175 - mark TestCallWithTimeout.py XFAIL on macosx.
Author: tfiala Date: Fri Apr 1 13:42:45 2016 New Revision: 265175 URL: http://llvm.org/viewvc/llvm-project?rev=265175&view=rev Log: mark TestCallWithTimeout.py XFAIL on macosx. This test is failing on the CI but not locally for me. Needs investigation. tracked by: https://llvm.org/bugs/show_bug.cgi?id=27182 Modified: lldb/trunk/packages/Python/lldbsuite/test/expression_command/timeout/TestCallWithTimeout.py Modified: lldb/trunk/packages/Python/lldbsuite/test/expression_command/timeout/TestCallWithTimeout.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/expression_command/timeout/TestCallWithTimeout.py?rev=265175&r1=265174&r2=265175&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/expression_command/timeout/TestCallWithTimeout.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/expression_command/timeout/TestCallWithTimeout.py Fri Apr 1 13:42:45 2016 @@ -24,7 +24,7 @@ class ExprCommandWithTimeoutsTestCase(Te @expectedFlakeyFreeBSD("llvm.org/pr19605") -@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr21765") +@expectedFailureAll(oslist=["windows", "macosx"], bugnumber="llvm.org/pr21765") def test(self): """Test calling std::String member function.""" self.build() ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r265181 - Remove more of the code-running ObjC data formatter support
Author: enrico Date: Fri Apr 1 15:33:22 2016 New Revision: 265181 URL: http://llvm.org/viewvc/llvm-project?rev=265181&view=rev Log: Remove more of the code-running ObjC data formatter support Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjC.py lldb/trunk/source/Plugins/Language/ObjC/Cocoa.cpp lldb/trunk/source/Plugins/Language/ObjC/NSSet.cpp lldb/trunk/source/Plugins/Language/ObjC/ObjCLanguage.cpp Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjC.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjC.py?rev=265181&r1=265180&r2=265181&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjC.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjC.py Fri Apr 1 15:33:22 2016 @@ -192,7 +192,7 @@ class ObjCDataFormatterTestCase(TestBase def nsnumber_data_formatter_commands(self): # Now enable AppKit and check we are displaying Cocoa classes correctly -self.expect('frame variable num1 num2 num3 num4 num5 num6 num7 num8_Y num8_N num9', +self.expect('frame variable num1 num2 num3 num4 num5 num6 num7 num9', substrs = ['(NSNumber *) num1 = ',' (int)5', '(NSNumber *) num2 = ',' (float)3.1', '(NSNumber *) num3 = ',' (double)3.14', @@ -200,13 +200,8 @@ class ObjCDataFormatterTestCase(TestBase '(NSNumber *) num5 = ',' (char)65', '(NSNumber *) num6 = ',' (long)255', '(NSNumber *) num7 = ','200', -'(NSNumber *) num8_Y = ',' @"1"', -'(NSNumber *) num8_N = ',' @"0"', '(NSNumber *) num9 = ',' (short)-31616']) -self.expect('frame variable decimal_one', -substrs = ['(NSDecimalNumber *) decimal_one = 0x','1']) - self.expect('frame variable num_at1 num_at2 num_at3 num_at4', substrs = ['(NSNumber *) num_at1 = ',' (int)12', '(NSNumber *) num_at2 = ',' (int)-12', Modified: lldb/trunk/source/Plugins/Language/ObjC/Cocoa.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Language/ObjC/Cocoa.cpp?rev=265181&r1=265180&r2=265181&view=diff == --- lldb/trunk/source/Plugins/Language/ObjC/Cocoa.cpp (original) +++ lldb/trunk/source/Plugins/Language/ObjC/Cocoa.cpp Fri Apr 1 15:33:22 2016 @@ -78,9 +78,8 @@ lldb_private::formatters::NSBundleSummar return true; } } -// this is either an unknown subclass or an NSBundle that comes from [NSBundle mainBundle] -// which is encoded differently and needs to be handled by running code -return ExtractSummaryFromObjCExpression(valobj, "NSString*", "bundlePath", stream, options.GetLanguage()); + +return false; } bool @@ -124,7 +123,8 @@ lldb_private::formatters::NSTimeZoneSumm return true; } } -return ExtractSummaryFromObjCExpression(valobj, "NSString*", "name", stream, options.GetLanguage()); + +return false; } bool @@ -168,9 +168,8 @@ lldb_private::formatters::NSNotification return true; } } -// this is either an unknown subclass or an NSBundle that comes from [NSBundle mainBundle] -// which is encoded differently and needs to be handled by running code -return ExtractSummaryFromObjCExpression(valobj, "NSString*", "name", stream, options.GetLanguage()); + +return false; } bool @@ -204,22 +203,19 @@ lldb_private::formatters::NSMachPortSumm uint64_t port_number = 0; -do +if (!strcmp(class_name,"NSMachPort")) { -if (!strcmp(class_name,"NSMachPort")) +uint64_t offset = (ptr_size == 4 ? 12 : 20); +Error error; +port_number = process_sp->ReadUnsignedIntegerFromMemory(offset+valobj_addr, 4, 0, error); +if (error.Success()) { -uint64_t offset = (ptr_size == 4 ? 12 : 20); -Error error; -port_number = process_sp->ReadUnsignedIntegerFromMemory(offset+valobj_addr, 4, 0, error); -if (error.Success()) -break; +stream.Printf("mach port: %u",(uint32_t)(port_number & 0x)); +return true; } -if (!ExtractValueFromObjCExpression(valobj, "int", "machPort", port_number)) -return false; -} while (false); +} -stream.Printf("mach port: %u",(uint32_t)(por
[Lldb-commits] [lldb] r265188 - skip and xfail two std::list-related libcxx tests that fail on OS X with TOT libcxx
Author: tfiala Date: Fri Apr 1 16:36:58 2016 New Revision: 265188 URL: http://llvm.org/viewvc/llvm-project?rev=265188&view=rev Log: skip and xfail two std::list-related libcxx tests that fail on OS X with TOT libcxx Enrico has a bug on him to make this work across older libcxx list and newer libcxx list simultaneously. Needed in preparation of getting the OS X public CI to run the TSAN tests. tracked by: rdar://25499635 Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/list/TestDataFormatterLibcxxList.py lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/list/loop/TestDataFormatterLibcxxListLoop.py Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/list/TestDataFormatterLibcxxList.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/list/TestDataFormatterLibcxxList.py?rev=265188&r1=265187&r2=265188&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/list/TestDataFormatterLibcxxList.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/list/TestDataFormatterLibcxxList.py Fri Apr 1 16:36:58 2016 @@ -27,6 +27,7 @@ class LibcxxListDataFormatterTestCase(Te @skipIf(compiler="gcc") @skipIfWindows # libc++ not ported to Windows yet +@expectedFailureAll(oslist=["macosx"], bugnumber="rdar://25499635") def test_with_run_command(self): """Test that that file and class static variables display correctly.""" self.build() Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/list/loop/TestDataFormatterLibcxxListLoop.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/list/loop/TestDataFormatterLibcxxListLoop.py?rev=265188&r1=265187&r2=265188&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/list/loop/TestDataFormatterLibcxxListLoop.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/list/loop/TestDataFormatterLibcxxListLoop.py Fri Apr 1 16:36:58 2016 @@ -20,6 +20,7 @@ class LibcxxListDataFormatterTestCase(Te @skipIf(compiler="gcc") @skipIfWindows # libc++ not ported to Windows yet @add_test_categories(["pyapi"]) +@skipIfDarwin # rdar://25499635 def test_with_run_command(self): self.build() exe = os.path.join(os.getcwd(), "a.out") ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18689: Make FileSpec handling platform-independent
zturner added a comment. Ugh. The only better way I can think of to do this would be to go into LLVM and rip out all the preprocessor defines, and compile `_windows` and `_posix` versions of every function unconditionally, and then only use the preprocessor defines to do something like: #if defined(LLVM_ON_WIN32) #define parent_path parent_path_windows #else #define parent_path parent_path_posix #endif But... That's a lot of effort to go through. So while this code duplication isn't really that desirable, I guess it's ok given the amount of effort of the alternative. http://reviews.llvm.org/D18689 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18519: Allow building LLDB on Windows with MinGW 64/4.9.2 and later
zturner added a comment. A few minor points left. Most are just CMake indentation issues, so should be easy to fix. A few code changes though. Comment at: CMakeLists.txt:3-5 @@ -2,1 +2,5 @@ +if(MINGW_DEBUG) +# force debugging info into lldb sources +message("-- Building LLDB in Debug mode") +set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -g -O0") Can you try the /bigobj solution proposed by Pavel? which specific object file fails to link, is it an LLDB obj or a clang obj? Because as Pavel mentions clang has had this problem before, so maybe you just need to specify /bigobj for MinGW in the clang library and not in LLDB? Comment at: cmake/modules/LLDBConfig.cmake:225-235 @@ -224,3 +224,4 @@ +if (CMAKE_CXX_COMPILER_ID MATCHES "Clang") check_cxx_compiler_flag("-Wno-deprecated-register" CXX_SUPPORTS_NO_DEPRECATED_REGISTER) if (CXX_SUPPORTS_NO_DEPRECATED_REGISTER) @@ -232,3 +233,4 @@ if (CXX_SUPPORTS_NO_VLA_EXTENSION) set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-vla-extension") endif () +endif() All of this needs to be indented 2 spaces. CMake code is just like regular C++ code where everything inside of a conditional has to be indented. For CMake we use 2 spaces. Comment at: cmake/modules/LLDBConfig.cmake:225-236 @@ -224,3 +224,4 @@ +if (CMAKE_CXX_COMPILER_ID MATCHES "Clang") check_cxx_compiler_flag("-Wno-deprecated-register" CXX_SUPPORTS_NO_DEPRECATED_REGISTER) if (CXX_SUPPORTS_NO_DEPRECATED_REGISTER) @@ -232,4 +233,5 @@ if (CXX_SUPPORTS_NO_VLA_EXTENSION) set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-vla-extension") endif () +endif() Still indentation problems here. Comment at: cmake/modules/LLDBConfig.cmake:249-255 @@ -247,5 +249,8 @@ # Use the Unicode (UTF-16) APIs by default on Win32 if (CMAKE_SYSTEM_NAME MATCHES "Windows") + if(MINGW) +add_definitions( -D_UNICODE -DUNICODE -D_WIN32_WINNT=0x0600 -D_BSD_SOURCE) + else() add_definitions( /D _UNICODE /D UNICODE ) endif() Still indentation problems here. Comment at: source/API/CMakeLists.txt:74-85 @@ -73,6 +73,14 @@ # want every single library taking a dependency on the script interpreters. +if(MINGW) target_link_libraries(liblldb PRIVATE lldbPluginScriptInterpreterNone lldbPluginScriptInterpreterPython +Dbghelp # Needed for MiniDumpWriteDump ) +else() + target_link_libraries(liblldb PRIVATE +lldbPluginScriptInterpreterNone +lldbPluginScriptInterpreterPython +) +endif() How about this instead: target_link_libraries(liblldb PRIVATE lldbPluginScriptInterpreterNone lldbPluginScriptInterpreterPython) if(MINGW) target_link_libraries(liblldb PRIVATE Dbghelp) endif() Comment at: source/Plugins/Process/Windows/Live/DebuggerThread.cpp:380-383 @@ -379,5 +379,6 @@ { +DWORD dwPidToDetach = m_pid_to_detach; WINLOG_IFANY(WINDOWS_LOG_EVENT | WINDOWS_LOG_EXCEPTION | WINDOWS_LOG_PROCESS, "Breakpoint exception is cue to detach from process 0x%x", -m_pid_to_detach); +dwPidToDetach); ::DebugActiveProcessStop(m_pid_to_detach); Can this line be reverted? I'm not sure what the point of saving it into a temp variable is. Comment at: tools/argdumper/CMakeLists.txt:6-8 @@ +5,5 @@ +if(MINGW) +# link directly against the dll file +add_dependencies(lldb-argdumper liblldb) +target_link_libraries(lldb-argdumper -L"${CMAKE_BINARY_DIR}/bin" -llldb) +else() 2 space indent for CMake, not 4. Also the `target_link_libraries in the `else()` branch should be indented. As Pavel says, I'm not sure why this is needed, but I can't really argue against it because I don't know much about MinGW http://reviews.llvm.org/D18519 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r265196 - Fixed an issue where if we have DWARF in an executable that has multiple languages where these languages use different type systems, you can end up trying to find the a
Author: gclayton Date: Fri Apr 1 17:57:22 2016 New Revision: 265196 URL: http://llvm.org/viewvc/llvm-project?rev=265196&view=rev Log: Fixed an issue where if we have DWARF in an executable that has multiple languages where these languages use different type systems, you can end up trying to find the actualy definition for a forward declaration for a type, you will call: TypeSP SymbolFileDWARF::FindDefinitionTypeForDWARFDeclContext (const DWARFDeclContext &dwarf_decl_ctx); The problem was we might be looking for a type "Foo", and find one from another langauge. Then the DWARFASTParserClang would try to make an AST type using a CompilerType that might return an empty. This fix makes sure that when we create a DWARFDeclContext from a DWARFDIE that the DWARFDeclContext we set the language of the DIE. Then when we go to find matches for DWARFDeclContext, we end up with bunch of DIEs. We check each DWARFDIE that we found by asking it for its language and making sure the language is compatible with the type system that we want to use. This keeps us from using the wrong types to resolve forward declarations. Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDeclContext.h lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp?rev=265196&r1=265195&r2=265196&view=diff == --- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp (original) +++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp Fri Apr 1 17:57:22 2016 @@ -298,6 +298,7 @@ DWARFDIE::GetDWARFDeclContext (DWARFDecl { if (IsValid()) { +dwarf_decl_ctx.SetLanguage(GetLanguage()); m_die->GetDWARFDeclContext (GetDWARF(), GetCU(), dwarf_decl_ctx); } else Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDeclContext.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDeclContext.h?rev=265196&r1=265195&r2=265196&view=diff == --- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDeclContext.h (original) +++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDeclContext.h Fri Apr 1 17:57:22 2016 @@ -64,7 +64,8 @@ public: }; DWARFDeclContext () : -m_entries() +m_entries(), +m_language(lldb::eLanguageTypeUnknown) { } @@ -115,10 +116,23 @@ public: m_qualified_name.clear(); } +lldb::LanguageType +GetLanguage() const +{ +return m_language; +} + +void +SetLanguage(lldb::LanguageType language) +{ +m_language = language; +} + protected: typedef std::vector collection; collection m_entries; mutable std::string m_qualified_name; +lldb::LanguageType m_language; }; #endif // SymbolFileDWARF_DWARFDeclContext_h_ Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp?rev=265196&r1=265195&r2=265196&view=diff == --- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (original) +++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp Fri Apr 1 17:57:22 2016 @@ -3698,8 +3698,12 @@ SymbolFileDWARF::FindDefinitionTypeForDW } const size_t num_matches = die_offsets.size(); - - + +// Get the type system that we are looking to find a type for. We will use this +// to ensure any matches we find are in a language that this type system supports +const LanguageType language = dwarf_decl_ctx.GetLanguage(); +TypeSystem *type_system = (language == eLanguageTypeUnknown) ? nullptr : GetTypeSystemForLanguage(language); + if (num_matches) { for (size_t i=0; iSupportsLanguage(type_die.GetLanguage())) +continue; bool try_resolving_type = false; // Don't try and resolve the DIE we are looking for with the DIE itself! ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r265200 - Add some unit tests for ClangASTContext.
Author: zturner Date: Fri Apr 1 18:20:35 2016 New Revision: 265200 URL: http://llvm.org/viewvc/llvm-project?rev=265200&view=rev Log: Add some unit tests for ClangASTContext. In doing so, two bugs were uncovered (and fixed). The first bug is that ClangASTContext::RemoveFastQualifiers() was broken, and was not removing fast qualifiers (or doing anything else for that matter). The second bug is that UnifyAccessSpecifiers treated AS_None asymmetrically, which is probably an edge case, but seems like a bug nonetheless. Added: lldb/trunk/unittests/Symbol/ lldb/trunk/unittests/Symbol/CMakeLists.txt lldb/trunk/unittests/Symbol/TestClangASTContext.cpp Modified: lldb/trunk/include/lldb/Symbol/ClangASTContext.h lldb/trunk/source/Symbol/ClangASTContext.cpp lldb/trunk/source/Symbol/ClangUtil.cpp lldb/trunk/unittests/CMakeLists.txt Modified: lldb/trunk/include/lldb/Symbol/ClangASTContext.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/ClangASTContext.h?rev=265200&r1=265199&r2=265200&view=diff == --- lldb/trunk/include/lldb/Symbol/ClangASTContext.h (original) +++ lldb/trunk/include/lldb/Symbol/ClangASTContext.h Fri Apr 1 18:20:35 2016 @@ -1151,23 +1151,26 @@ public: clang::VarDecl * CreateVariableDeclaration (clang::DeclContext *decl_context, const char *name, clang::QualType type); -protected: +static lldb::opaque_compiler_type_t +GetOpaqueCompilerType(clang::ASTContext *ast, lldb::BasicType basic_type); + static clang::QualType -GetQualType (lldb::opaque_compiler_type_t type) +GetQualType(lldb::opaque_compiler_type_t type) { if (type) return clang::QualType::getFromOpaquePtr(type); return clang::QualType(); } - + static clang::QualType -GetCanonicalQualType (lldb::opaque_compiler_type_t type) +GetCanonicalQualType(lldb::opaque_compiler_type_t type) { if (type) return clang::QualType::getFromOpaquePtr(type).getCanonicalType(); return clang::QualType(); } +protected: //-- // Classes that inherit from ClangASTContext can see and modify these //-- Modified: lldb/trunk/source/Symbol/ClangASTContext.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/ClangASTContext.cpp?rev=265200&r1=265199&r2=265200&view=diff == --- lldb/trunk/source/Symbol/ClangASTContext.cpp (original) +++ lldb/trunk/source/Symbol/ClangASTContext.cpp Fri Apr 1 18:20:35 2016 @@ -911,113 +911,12 @@ ClangASTContext::GetBasicType (lldb::Bas CompilerType ClangASTContext::GetBasicType (ASTContext *ast, lldb::BasicType basic_type) { -if (ast) -{ -lldb::opaque_compiler_type_t clang_type = nullptr; - -switch (basic_type) -{ -case eBasicTypeInvalid: -case eBasicTypeOther: -break; -case eBasicTypeVoid: -clang_type = ast->VoidTy.getAsOpaquePtr(); -break; -case eBasicTypeChar: -clang_type = ast->CharTy.getAsOpaquePtr(); -break; -case eBasicTypeSignedChar: -clang_type = ast->SignedCharTy.getAsOpaquePtr(); -break; -case eBasicTypeUnsignedChar: -clang_type = ast->UnsignedCharTy.getAsOpaquePtr(); -break; -case eBasicTypeWChar: -clang_type = ast->getWCharType().getAsOpaquePtr(); -break; -case eBasicTypeSignedWChar: -clang_type = ast->getSignedWCharType().getAsOpaquePtr(); -break; -case eBasicTypeUnsignedWChar: -clang_type = ast->getUnsignedWCharType().getAsOpaquePtr(); -break; -case eBasicTypeChar16: -clang_type = ast->Char16Ty.getAsOpaquePtr(); -break; -case eBasicTypeChar32: -clang_type = ast->Char32Ty.getAsOpaquePtr(); -break; -case eBasicTypeShort: -clang_type = ast->ShortTy.getAsOpaquePtr(); -break; -case eBasicTypeUnsignedShort: -clang_type = ast->UnsignedShortTy.getAsOpaquePtr(); -break; -case eBasicTypeInt: -clang_type = ast->IntTy.getAsOpaquePtr(); -break; -case eBasicTypeUnsignedInt: -clang_type = ast->UnsignedIntTy.getAsOpaquePtr(); -break; -case eBasicTypeLong: -clang_type = ast->LongTy.getAsOpaquePtr(); -break; -case eBasicTypeUnsignedLong: -c