[Lldb-commits] [PATCH] D45700: Improve LLDB's handling of non-local minidumps

2018-04-18 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

LGTM


Repository:
  rL LLVM

https://reviews.llvm.org/D45700



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] r330302 - Improve LLDB's handling of non-local minidumps

2018-04-18 Thread Leonard Mosescu via lldb-commits
Author: lemo
Date: Wed Apr 18 16:10:46 2018
New Revision: 330302

URL: http://llvm.org/viewvc/llvm-project?rev=330302=rev
Log:
Improve LLDB's handling of non-local minidumps

Normally, LLDB is creating a high-fidelity representation of a live
process, including a list of modules and sections, with the 
associated memory address ranges. In order to build the module and
section map LLDB tries to locate the local module image (object file)
and will parse it.

This does not work for postmortem debugging scenarios where the crash
dump (minidump in this case) was captured on a different machine.

Fortunately the minidump format encodes enough information about
each module's memory range to allow us to create placeholder modules.
This enables most LLDB functionality involving address-to-module
translations.

Also, we may want to completly disable the search for matching
local object files if we load minidumps unless we can prove that the
local image matches the one from the crash origin.
(not part of this change, see: llvm.org/pr35193)

Example: Identify the module from a stack frame PC:

Before:
  thread #1, stop reason = Exception 0xc005 encountered at address 0x164d14
frame #0: 0x00164d14
frame #1: 0x00167c79
frame #2: 0x00167e6d
frame #3: 0x7510336a
frame #4: 0x77759882
frame #5: 0x77759855

After:
  thread #1, stop reason = Exception 0xc005 encountered at address 0x164d14
frame #0: 0x00164d14 C:\Users\amccarth\Documents\Visual Studio 
2013\Projects\fizzbuzz\Debug\fizzbuzz.exe
frame #1: 0x00167c79 C:\Users\amccarth\Documents\Visual Studio 
2013\Projects\fizzbuzz\Debug\fizzbuzz.exe
frame #2: 0x00167e6d C:\Users\amccarth\Documents\Visual Studio 
2013\Projects\fizzbuzz\Debug\fizzbuzz.exe
frame #3: 0x7510336a C:\Windows\SysWOW64\kernel32.dll
frame #4: 0x77759882 C:\Windows\SysWOW64\ntdll.dll
frame #5: 0x77759855 C:\Windows\SysWOW64\ntdll.dll

Example: target modules list

Before:
error: the target has no associated executable images

After:
[ 0] C:\Windows\System32\MSVCP120D.dll 
[ 1] C:\Windows\SysWOW64\kernel32.dll 
[ 2] C:\Users\amccarth\Documents\Visual Studio 
2013\Projects\fizzbuzz\Debug\fizzbuzz.exe 
[ 3] C:\Windows\System32\MSVCR120D.dll 
[ 4] C:\Windows\SysWOW64\KERNELBASE.dll 
[ 5] C:\Windows\SysWOW64\ntdll.dll

NOTE: the minidump format also includes the debug info GUID, so we can
fill-in the module UUID from it, but this part was excluded from this change
to keep the changes simple (the LLDB UUID is hardcoded to be either 16 or
20 bytes, while the CodeView GUIDs are normally 24 bytes)

Differential Revision: https://reviews.llvm.org/D45700


Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump/TestMiniDump.py
lldb/trunk/source/Commands/CommandObjectTarget.cpp
lldb/trunk/source/Core/Module.cpp
lldb/trunk/source/Core/Section.cpp
lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp
lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.h

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py?rev=330302=330301=330302=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
 Wed Apr 18 16:10:46 2018
@@ -70,6 +70,17 @@ class MiniDumpNewTestCase(TestBase):
 self.assertEqual(self.process.GetProcessID(), self._linux_x86_64_pid)
 self.check_state()
 
+def test_modules_in_mini_dump(self):
+"""Test that lldb can read the list of modules from the minidump."""
+# target create -c linux-x86_64.dmp
+self.dbg.CreateTarget(None)
+self.target = self.dbg.GetSelectedTarget()
+self.process = self.target.LoadCore("linux-x86_64.dmp")
+self.assertTrue(self.process, PROCESS_IS_VALID)
+self.assertEqual(self.target.GetNumModules(), 9)
+for module in self.target.modules:
+self.assertTrue(module.IsValid())
+
 def test_thread_info_in_minidump(self):
 """Test that lldb can read the thread information from the Minidump."""
 # target create -c linux-x86_64.dmp
@@ -100,6 +111,7 @@ class MiniDumpNewTestCase(TestBase):
 self.assertEqual(thread.GetNumFrames(), 2)
 frame = thread.GetFrameAtIndex(0)
 self.assertTrue(frame.IsValid())
+self.assertTrue(frame.GetModule().IsValid())
 pc = frame.GetPC()
 eip = frame.FindRegister("pc")
 self.assertTrue(eip.IsValid())

Modified: 

[Lldb-commits] [PATCH] D45700: Improve LLDB's handling of non-local minidumps

2018-04-18 Thread Adrian McCarthy via Phabricator via lldb-commits
amccarth accepted this revision.
amccarth added a comment.

I actually preferred the factory solution.

What I intended to express is that the modification of the target doesn't seems 
like it should be inside the PlaceholderModule class, so whether you use a 
factory or not doesn't really address that aspect of the coupling.  In my head, 
modification of the target should be done by the client that instantiates the 
PlaceholderModule (whether it does that via constructor or factory).

But this isn't a new problem, so I'm happy to consider the coupling problem 
outside the scope of this patch.


https://reviews.llvm.org/D45700



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D45700: Improve LLDB's handling of non-local minidumps

2018-04-18 Thread Leonard Mosescu via lldb-commits
Greg/Pavel, does the latest revision look good to you? Thanks!

On Wed, Apr 18, 2018 at 10:31 AM, Leonard Mosescu via Phabricator <
revi...@reviews.llvm.org> wrote:

> lemo marked an inline comment as done.
> lemo added a comment.
>
> In https://reviews.llvm.org/D45700#1070491, @amccarth wrote:
>
> > LGTM, but consider highlighting the side effect to `target` when the
> factory makes a Placeholder module.
>
>
> Good observation: taking a step back, the factory introduces too much
> coupling, especially if we want to extend this placeholder module approach
> to other uses. Because of this, I reverted back to the standalone
> PlaceholderModule::CreateImageSection() approach. Thanks Adrian!
>
>
>
> 
> Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:70
> +  // Creates a synthetic module section covering the whole module image
> +  void CreateImageSection(const MinidumpModule *module, Target& target) {
> +const ConstString section_name(".module_image");
> 
> amccarth wrote:
> > I didn't notice before that target is a non-const ref.  Maybe the
> comment should explain why target is modified (and/or maybe in
> PlaceholderModule::Create).
> Updated the function comment. This is similar to how other places set the
> section load address (ex. ObjectFileELF::SetLoadAddress)
>
>
> https://reviews.llvm.org/D45700
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D45700: Improve LLDB's handling of non-local minidumps

2018-04-18 Thread Leonard Mosescu via Phabricator via lldb-commits
lemo added subscribers: amccarth, clayborg, labath, zturner.
lemo marked an inline comment as done.
lemo added a comment.

Greg/Pavel, does the latest revision look good to you? Thanks!


https://reviews.llvm.org/D45700



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D45628: [LLDB] Support compressed debug info sections (.zdebug*)

2018-04-18 Thread Erik Welander via Phabricator via lldb-commits
alur marked 3 inline comments as done.
alur added a comment.

Thank you, I do need someone to push this on my behalf.


https://reviews.llvm.org/D45628



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D45628: [LLDB] Support compressed debug info sections (.zdebug*)

2018-04-18 Thread Erik Welander via Phabricator via lldb-commits
alur updated this revision to Diff 142962.

https://reviews.llvm.org/D45628

Files:
  packages/Python/lldbsuite/test/linux/compressed-debug-info/Makefile
  
packages/Python/lldbsuite/test/linux/compressed-debug-info/TestCompressedDebugInfo.py
  packages/Python/lldbsuite/test/linux/compressed-debug-info/a.c
  source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -1775,13 +1775,22 @@
 return eSectionTypeOther;
   }
 
+  llvm::StringRef mapped_name;
+  if (section_name.startswith(".zdebug")) {
+// .zdebug* are compressed equivalents of .debug* sections, and should map
+// to the same corresponding type.
+mapped_name = section_name.drop_front(2);
+  } else {
+mapped_name = section_name.drop_front(1);
+  }
+
   // MISSING? .gnu_debugdata - "mini debuginfo / MiniDebugInfo" section,
   // http://sourceware.org/gdb/onlinedocs/gdb/MiniDebugInfo.html
   // MISSING? .debug-index -
   // http://src.chromium.org/viewvc/chrome/trunk/src/build/gdb-add-index?pathrev=144644
   // MISSING? .debug_types - Type descriptions from DWARF 4? See
   // http://gcc.gnu.org/wiki/DwarfSeparateTypeInfo
-  return llvm::StringSwitch(section_name.drop_front(1))
+  return llvm::StringSwitch(mapped_name)
   .Case("text", eSectionTypeCode)
   .Case("data", eSectionTypeData)
   .Case("bss", eSectionTypeZeroFill)
@@ -2823,6 +2832,7 @@
 void ObjectFileELF::RelocateSection(lldb_private::Section *section)
 {
   static const llvm::StringRef debug_prefix(".debug");
+  static const llvm::StringRef zdebug_prefix(".zdebug");
 
   // Set relocated bit so we stop getting called, regardless of
   // whether we actually relocate.
@@ -2838,7 +2848,8 @@
 return;
 
   // We don't relocate non-debug sections at the moment
-  if (section_name.startswith(debug_prefix))
+  if (section_name.startswith(debug_prefix) ||
+  section_name.startswith(zdebug_prefix))
 return;
 
   // Relocation section names to look for
@@ -,7 +3344,8 @@
 return section->GetObjectFile()->ReadSectionData(section, section_offset,
  dst, dst_len);
 
-  if (!section->Test(SHF_COMPRESSED))
+  if (!llvm::object::Decompressor::isCompressedELFSection(
+  section->Get(), section->GetName().GetStringRef()))
 return ObjectFile::ReadSectionData(section, section_offset, dst, dst_len);
 
   // For compressed sections we need to read to full data to be able to
@@ -3352,7 +3364,8 @@
   Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_MODULES);
 
   size_t result = ObjectFile::ReadSectionData(section, section_data);
-  if (result == 0 || !section->Test(SHF_COMPRESSED))
+  if (result == 0 || !llvm::object::Decompressor::isCompressedELFSection(
+ section->Get(), section->GetName().GetStringRef()))
 return result;
 
   auto Decompressor = llvm::object::Decompressor::create(
Index: packages/Python/lldbsuite/test/linux/compressed-debug-info/a.c
===
--- /dev/null
+++ packages/Python/lldbsuite/test/linux/compressed-debug-info/a.c
@@ -0,0 +1,4 @@
+int main() {
+  int z = 2;
+  return z;
+}
Index: packages/Python/lldbsuite/test/linux/compressed-debug-info/TestCompressedDebugInfo.py
===
--- /dev/null
+++ packages/Python/lldbsuite/test/linux/compressed-debug-info/TestCompressedDebugInfo.py
@@ -0,0 +1,47 @@
+""" Tests that compressed debug info sections are used. """
+import os
+import lldb
+import sys
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestCompressedDebugInfo(TestBase):
+  mydir = TestBase.compute_mydir(__file__)
+
+  def setUp(self):
+TestBase.setUp(self)
+
+  @no_debug_info_test  # Prevent the genaration of the dwarf version of this test
+  @skipUnlessPlatform(["linux"])
+  def test_compressed_debug_info(self):
+"""Tests that the 'frame variable' works with compressed debug info."""
+
+self.build()
+process = lldbutil.run_to_source_breakpoint(
+self, "main", lldb.SBFileSpec("a.c"), exe_name="compressed.out")[1]
+
+# The process should be stopped at a breakpoint, and the z variable should
+# be in the top frame.
+self.assertTrue(process.GetState() == lldb.eStateStopped,
+STOPPED_DUE_TO_BREAKPOINT)
+frame = process.GetThreadAtIndex(0).GetFrameAtIndex(0)
+self.assertTrue(frame.FindVariable("z").IsValid(), "z is not valid")
+
+  @no_debug_info_test  # Prevent the genaration of the dwarf version of this test
+  @skipUnlessPlatform(["linux"])
+  def test_compressed_debug_info_gnu(self):
+"""Tests that the 'frame variable' works with gnu-style compressed debug 

[Lldb-commits] [PATCH] D45700: Improve LLDB's handling of non-local minidumps

2018-04-18 Thread Leonard Mosescu via Phabricator via lldb-commits
lemo marked an inline comment as done.
lemo added a comment.

In https://reviews.llvm.org/D45700#1070491, @amccarth wrote:

> LGTM, but consider highlighting the side effect to `target` when the factory 
> makes a Placeholder module.


Good observation: taking a step back, the factory introduces too much coupling, 
especially if we want to extend this placeholder module approach to other uses. 
Because of this, I reverted back to the standalone 
PlaceholderModule::CreateImageSection() approach. Thanks Adrian!




Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:70
+  // Creates a synthetic module section covering the whole module image
+  void CreateImageSection(const MinidumpModule *module, Target& target) {
+const ConstString section_name(".module_image");

amccarth wrote:
> I didn't notice before that target is a non-const ref.  Maybe the comment 
> should explain why target is modified (and/or maybe in 
> PlaceholderModule::Create).
Updated the function comment. This is similar to how other places set the 
section load address (ex. ObjectFileELF::SetLoadAddress)


https://reviews.llvm.org/D45700



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D45700: Improve LLDB's handling of non-local minidumps

2018-04-18 Thread Leonard Mosescu via Phabricator via lldb-commits
lemo updated this revision to Diff 142960.

https://reviews.llvm.org/D45700

Files:
  
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
  
packages/Python/lldbsuite/test/functionalities/postmortem/minidump/TestMiniDump.py
  source/Commands/CommandObjectTarget.cpp
  source/Core/Module.cpp
  source/Core/Section.cpp
  source/Plugins/Process/minidump/ProcessMinidump.cpp
  source/Plugins/Process/minidump/ProcessMinidump.h

Index: source/Plugins/Process/minidump/ProcessMinidump.h
===
--- source/Plugins/Process/minidump/ProcessMinidump.h
+++ source/Plugins/Process/minidump/ProcessMinidump.h
@@ -61,6 +61,8 @@
 
   uint32_t GetPluginVersion() override;
 
+  SystemRuntime *GetSystemRuntime() override { return nullptr; }
+
   Status DoDestroy() override;
 
   void RefreshStateAfterStop() override;
Index: source/Plugins/Process/minidump/ProcessMinidump.cpp
===
--- source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -19,6 +19,7 @@
 #include "lldb/Core/State.h"
 #include "lldb/Target/DynamicLoader.h"
 #include "lldb/Target/MemoryRegionInfo.h"
+#include "lldb/Target/SectionLoadList.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/UnixSignals.h"
 #include "lldb/Utility/DataBufferLLVM.h"
@@ -31,9 +32,53 @@
 // C includes
 // C++ includes
 
+using namespace lldb;
 using namespace lldb_private;
 using namespace minidump;
 
+//--
+/// A placeholder module used for minidumps, where the original
+/// object files may not be available (so we can't parse the object
+/// files to extract the set of sections/segments)
+///
+/// This placeholder module has a single synthetic section (.module_image)
+/// which represents the module memory range covering the whole module.
+//--
+class PlaceholderModule : public Module {
+public:
+  PlaceholderModule(const FileSpec _spec, const ArchSpec ) :
+Module(file_spec, arch) {}
+
+  // Creates a synthetic module section covering the whole module image
+  // (and sets the section load address as well)
+  void CreateImageSection(const MinidumpModule *module, Target& target) {
+const ConstString section_name(".module_image");
+lldb::SectionSP section_sp(new Section(
+shared_from_this(), // Module to which this section belongs.
+nullptr,// ObjectFile
+0,  // Section ID.
+section_name,   // Section name.
+eSectionTypeContainer,  // Section type.
+module->base_of_image,  // VM address.
+module->size_of_image,  // VM size in bytes of this section.
+0,  // Offset of this section in the file.
+module->size_of_image,  // Size of the section as found in the file.
+12, // Alignment of the section (log2)
+0,  // Flags for this section.
+1));// Number of host bytes per target byte
+section_sp->SetPermissions(ePermissionsExecutable | ePermissionsReadable);
+GetSectionList()->AddSection(section_sp);
+target.GetSectionLoadList().SetSectionLoadAddress(
+section_sp, module->base_of_image);
+  }
+
+  ObjectFile *GetObjectFile() override { return nullptr; }
+
+  SectionList *GetSectionList() override {
+return Module::GetUnifiedSectionList();
+  }
+};
+
 ConstString ProcessMinidump::GetPluginNameStatic() {
   static ConstString g_name("minidump");
   return g_name;
@@ -281,7 +326,18 @@
 Status error;
 lldb::ModuleSP module_sp = GetTarget().GetSharedModule(module_spec, );
 if (!module_sp || error.Fail()) {
-  continue;
+  // We failed to locate a matching local object file. Fortunately,
+  // the minidump format encodes enough information about each module's
+  // memory range to allow us to create placeholder modules.
+  //
+  // This enables most LLDB functionality involving address-to-module
+  // translations (ex. identifing the module for a stack frame PC) and
+  // modules/sections commands (ex. target modules list, ...)
+  auto placeholder_module =
+  std::make_shared(file_spec, GetArchitecture());
+  placeholder_module->CreateImageSection(module, GetTarget());
+  module_sp = placeholder_module;
+  GetTarget().GetImages().Append(module_sp);
 }
 
 if (log) {
Index: source/Core/Section.cpp
===
--- source/Core/Section.cpp
+++ source/Core/Section.cpp
@@ -326,10 +326,11 @@
 // The top most section prints the module basename
 const char *name = NULL;
 ModuleSP module_sp(GetModule());
-const FileSpec _spec = m_obj_file->GetFileSpec();
 
-if 

[Lldb-commits] [PATCH] D45333: WIP: [LIT] Have lit run the lldb test suite

2018-04-18 Thread Jonas Devlieghere via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL330275: [LIT] Have lit run the lldb test suite (authored by 
JDevlieghere, committed by ).
Herald added a subscriber: jkorous.

Changed prior to commit:
  https://reviews.llvm.org/D45333?vs=142028=142958#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D45333

Files:
  lldb/trunk/lit/Suite/lit.cfg
  lldb/trunk/lit/Suite/lit.site.cfg.in
  lldb/trunk/lit/Suite/lldbtest.py
  lldb/trunk/test/CMakeLists.txt

Index: lldb/trunk/lit/Suite/lit.cfg
===
--- lldb/trunk/lit/Suite/lit.cfg
+++ lldb/trunk/lit/Suite/lit.cfg
@@ -0,0 +1,29 @@
+# -*- Python -*-
+
+# Configuration file for the 'lit' test runner.
+
+import os
+
+import lit.formats
+
+# name: The name of this test suite.
+config.name = 'lldb-Suite'
+
+# suffixes: A list of file extensions to treat as test files.
+config.suffixes = ['.py']
+
+# test_source_root: The root path where tests are located.
+# test_exec_root: The root path where tests should be run.
+config.test_source_root = os.path.join(config.lldb_src_root, 'packages','Python','lldbsuite','test')
+config.test_exec_root = config.test_source_root
+
+# Build dotest command.
+dotest_cmd = [config.dotest_path, '-q']
+dotest_cmd.extend(config.dotest_args_str.split(';'))
+
+# Load LLDB test format.
+sys.path.append(os.path.join(config.lldb_src_root, "lit", "Suite"))
+import lldbtest
+
+# testFormat: The test format to use to interpret tests.
+config.test_format = lldbtest.LLDBTest(dotest_cmd)
Index: lldb/trunk/lit/Suite/lldbtest.py
===
--- lldb/trunk/lit/Suite/lldbtest.py
+++ lldb/trunk/lit/Suite/lldbtest.py
@@ -0,0 +1,64 @@
+from __future__ import absolute_import
+import os
+
+import subprocess
+import sys
+
+import lit.Test
+import lit.TestRunner
+import lit.util
+from lit.formats.base import TestFormat
+
+
+class LLDBTest(TestFormat):
+def __init__(self, dotest_cmd):
+self.dotest_cmd = dotest_cmd
+
+def getTestsInDirectory(self, testSuite, path_in_suite, litConfig,
+localConfig):
+source_path = testSuite.getSourcePath(path_in_suite)
+for filename in os.listdir(source_path):
+# Ignore dot files and excluded tests.
+if (filename.startswith('.') or filename in localConfig.excludes):
+continue
+
+# Ignore files that don't start with 'Test'.
+if not filename.startswith('Test'):
+continue
+
+filepath = os.path.join(source_path, filename)
+if not os.path.isdir(filepath):
+base, ext = os.path.splitext(filename)
+if ext in localConfig.suffixes:
+yield lit.Test.Test(testSuite, path_in_suite +
+(filename, ), localConfig)
+
+def execute(self, test, litConfig):
+if litConfig.noExecute:
+return lit.Test.PASS, ''
+
+if test.config.unsupported:
+return (lit.Test.UNSUPPORTED, 'Test is unsupported')
+
+testPath, testFile = os.path.split(test.getSourcePath())
+cmd = self.dotest_cmd + [testPath, '-p', testFile]
+
+try:
+out, err, exitCode = lit.util.executeCommand(
+cmd,
+env=test.config.environment,
+timeout=litConfig.maxIndividualTestTime)
+except lit.util.ExecuteCommandTimeoutException:
+return (lit.Test.TIMEOUT, 'Reached timeout of {} seconds'.format(
+litConfig.maxIndividualTestTime))
+
+if exitCode:
+return lit.Test.FAIL, out + err
+
+passing_test_line = 'RESULT: PASSED'
+if passing_test_line not in out and passing_test_line not in err:
+msg = ('Unable to find %r in dotest output:\n\n%s%s' %
+   (passing_test_line, out, err))
+return lit.Test.UNRESOLVED, msg
+
+return lit.Test.PASS, ''
Index: lldb/trunk/lit/Suite/lit.site.cfg.in
===
--- lldb/trunk/lit/Suite/lit.site.cfg.in
+++ lldb/trunk/lit/Suite/lit.site.cfg.in
@@ -0,0 +1,28 @@
+@LIT_SITE_CFG_IN_HEADER@
+
+config.test_exec_root = "@LLVM_BINARY_DIR@"
+config.llvm_src_root = "@LLVM_SOURCE_DIR@"
+config.llvm_obj_root = "@LLVM_BINARY_DIR@"
+config.llvm_tools_dir = "@LLVM_TOOLS_DIR@"
+config.llvm_libs_dir = "@LLVM_LIBS_DIR@"
+config.llvm_build_mode = "@LLVM_BUILD_MODE@"
+config.lit_tools_dir = "@LLVM_LIT_TOOLS_DIR@"
+config.lldb_obj_root = "@LLDB_BINARY_DIR@"
+config.lldb_src_root = "@LLDB_SOURCE_DIR@"
+config.target_triple = "@TARGET_TRIPLE@"
+config.python_executable = "@PYTHON_EXECUTABLE@"
+config.dotest_path = "@LLDB_SOURCE_DIR@/test/dotest.py"
+config.dotest_args_str = "@LLDB_DOTEST_ARGS@"
+
+# Support substitution of the tools and libs dirs with user parameters. This is
+# used when we 

[Lldb-commits] [lldb] r330275 - [LIT] Have lit run the lldb test suite

2018-04-18 Thread Jonas Devlieghere via lldb-commits
Author: jdevlieghere
Date: Wed Apr 18 10:08:49 2018
New Revision: 330275

URL: http://llvm.org/viewvc/llvm-project?rev=330275=rev
Log:
[LIT] Have lit run the lldb test suite

This is the first in what will hopefully become a series of patches to
replace the driver logic in dotest.py with LIT. The motivation for this
change is that there's no point in maintaining two driver
implementations. Since all of the LLVM projects are using lit, this is
the obvious choice.

Obviously the goal is maintain full compatibility with the functionality
offered by dotest. As such we won't be removing anything until that
point has been reached.

This patch is the initial attempt (referred to as v1) to run the lldb
test suite with lit. To do so we introduced a custom LLDB test format
that invokes dotest.py with a single test file.

Differential revision: https://reviews.llvm.org/D45333

Added:
lldb/trunk/lit/Suite/
lldb/trunk/lit/Suite/lit.cfg
lldb/trunk/lit/Suite/lit.site.cfg.in
lldb/trunk/lit/Suite/lldbtest.py
Modified:
lldb/trunk/test/CMakeLists.txt

Added: lldb/trunk/lit/Suite/lit.cfg
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Suite/lit.cfg?rev=330275=auto
==
--- lldb/trunk/lit/Suite/lit.cfg (added)
+++ lldb/trunk/lit/Suite/lit.cfg Wed Apr 18 10:08:49 2018
@@ -0,0 +1,29 @@
+# -*- Python -*-
+
+# Configuration file for the 'lit' test runner.
+
+import os
+
+import lit.formats
+
+# name: The name of this test suite.
+config.name = 'lldb-Suite'
+
+# suffixes: A list of file extensions to treat as test files.
+config.suffixes = ['.py']
+
+# test_source_root: The root path where tests are located.
+# test_exec_root: The root path where tests should be run.
+config.test_source_root = os.path.join(config.lldb_src_root, 
'packages','Python','lldbsuite','test')
+config.test_exec_root = config.test_source_root
+
+# Build dotest command.
+dotest_cmd = [config.dotest_path, '-q']
+dotest_cmd.extend(config.dotest_args_str.split(';'))
+
+# Load LLDB test format.
+sys.path.append(os.path.join(config.lldb_src_root, "lit", "Suite"))
+import lldbtest
+
+# testFormat: The test format to use to interpret tests.
+config.test_format = lldbtest.LLDBTest(dotest_cmd)

Added: lldb/trunk/lit/Suite/lit.site.cfg.in
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Suite/lit.site.cfg.in?rev=330275=auto
==
--- lldb/trunk/lit/Suite/lit.site.cfg.in (added)
+++ lldb/trunk/lit/Suite/lit.site.cfg.in Wed Apr 18 10:08:49 2018
@@ -0,0 +1,28 @@
+@LIT_SITE_CFG_IN_HEADER@
+
+config.test_exec_root = "@LLVM_BINARY_DIR@"
+config.llvm_src_root = "@LLVM_SOURCE_DIR@"
+config.llvm_obj_root = "@LLVM_BINARY_DIR@"
+config.llvm_tools_dir = "@LLVM_TOOLS_DIR@"
+config.llvm_libs_dir = "@LLVM_LIBS_DIR@"
+config.llvm_build_mode = "@LLVM_BUILD_MODE@"
+config.lit_tools_dir = "@LLVM_LIT_TOOLS_DIR@"
+config.lldb_obj_root = "@LLDB_BINARY_DIR@"
+config.lldb_src_root = "@LLDB_SOURCE_DIR@"
+config.target_triple = "@TARGET_TRIPLE@"
+config.python_executable = "@PYTHON_EXECUTABLE@"
+config.dotest_path = "@LLDB_SOURCE_DIR@/test/dotest.py"
+config.dotest_args_str = "@LLDB_DOTEST_ARGS@"
+
+# Support substitution of the tools and libs dirs with user parameters. This is
+# used when we can't determine the tool dir at configuration time.
+try:
+config.llvm_tools_dir = config.llvm_tools_dir % lit_config.params
+config.llvm_libs_dir = config.llvm_libs_dir % lit_config.params
+config.llvm_build_mode = config.llvm_build_mode % lit_config.params
+except KeyError as e:
+key, = e.args
+lit_config.fatal("unable to find %r parameter, use '--param=%s=VALUE'" % 
(key,key))
+
+# Let the main config do the real work.
+lit_config.load_config(config, "@LLDB_SOURCE_DIR@/lit/Suite/lit.cfg")

Added: lldb/trunk/lit/Suite/lldbtest.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Suite/lldbtest.py?rev=330275=auto
==
--- lldb/trunk/lit/Suite/lldbtest.py (added)
+++ lldb/trunk/lit/Suite/lldbtest.py Wed Apr 18 10:08:49 2018
@@ -0,0 +1,64 @@
+from __future__ import absolute_import
+import os
+
+import subprocess
+import sys
+
+import lit.Test
+import lit.TestRunner
+import lit.util
+from lit.formats.base import TestFormat
+
+
+class LLDBTest(TestFormat):
+def __init__(self, dotest_cmd):
+self.dotest_cmd = dotest_cmd
+
+def getTestsInDirectory(self, testSuite, path_in_suite, litConfig,
+localConfig):
+source_path = testSuite.getSourcePath(path_in_suite)
+for filename in os.listdir(source_path):
+# Ignore dot files and excluded tests.
+if (filename.startswith('.') or filename in localConfig.excludes):
+continue
+
+# Ignore files that don't start with 'Test'.
+if not filename.startswith('Test'):
+   

[Lldb-commits] [PATCH] D45628: [LLDB] Support compressed debug info sections (.zdebug*)

2018-04-18 Thread Davide Italiano via Phabricator via lldb-commits
davide requested changes to this revision.
davide added a comment.
This revision now requires changes to proceed.

Some minor comments, almost ready to go. Do you have commit access or you need 
somebody to push this on your behalf?




Comment at: 
packages/Python/lldbsuite/test/linux/compressed-debug-info/TestCompressedDebugInfo.py:22-40
+exe = self.getBuildArtifact("compressed.out")
+
+self.target = self.dbg.CreateTarget(exe)
+self.assertTrue(self.target, VALID_TARGET)
+
+main_bp = self.target.BreakpointCreateByName("main", "compressed.out")
+self.assertTrue(main_bp, VALID_BREAKPOINT)

This test is too much boilerplate. We moved to a new, more concise style. You 
might consider using
```
(target, process, thread, main_breakpoint) = 
lldbutil.run_to_source_breakpoint(self, 
"break here", src_file_spec, exe_name = exe)
```

instead.



Comment at: 
packages/Python/lldbsuite/test/linux/compressed-debug-info/TestCompressedDebugInfo.py:47-64
+self.build()
+exe = self.getBuildArtifact("compressed.gnu.out")
+
+self.target = self.dbg.CreateTarget(exe)
+self.assertTrue(self.target, VALID_TARGET)
+
+main_bp = self.target.BreakpointCreateByName("main", 
"compressed.gnu.out")

ditto.



Comment at: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:1778-1781
+  llvm::StringRef mapped_name;
+  if (section_name.startswith(".zdebug")) {
+mapped_name = section_name.drop_front(2);
+  } else {

Can you add a comment to explain why are you dropping the first two character 
here? 



https://reviews.llvm.org/D45628



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D45628: [LLDB] Support compressed debug info sections (.zdebug*)

2018-04-18 Thread Greg Clayton via Phabricator via lldb-commits
clayborg accepted this revision.
clayborg added a comment.

Very nice!


https://reviews.llvm.org/D45628



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D45573: Report more precise error message when attach fails

2018-04-18 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
labath marked an inline comment as done.
Closed by commit rL330247: Report more precise error message when attach fails 
(authored by labath, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D45573?vs=142194=142907#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D45573

Files:
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
  lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp


Index: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
===
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
@@ -126,6 +126,8 @@
 return self.qfThreadInfo()
 if packet == "qC":
 return self.qC()
+if packet == "QEnableErrorStrings":
+return self.QEnableErrorStrings()
 if packet == "?":
 return self.haltReason()
 if packet[0] == "H":
@@ -137,6 +139,9 @@
 if data is not None:
 return self._qXferResponse(data, has_more)
 return ""
+if packet.startswith("vAttach;"):
+pid = packet.partition(';')[2]
+return self.vAttach(int(pid, 16))
 if packet[0] == "Z":
 return self.setBreakpoint(packet)
 return self.other(packet)
@@ -177,6 +182,9 @@
 def qC(self):
 return "QC0"
 
+def QEnableErrorStrings(self):
+return "OK"
+
 def haltReason(self):
 # SIGINT is 2, return type is 2 digit hex string
 return "S02"
@@ -187,6 +195,9 @@
 def _qXferResponse(self, data, has_more):
 return "%s%s" % ("m" if has_more else "l", escape_binary(data))
 
+def vAttach(self, pid):
+raise self.UnexpectedPacketException()
+
 def selectThread(self, op, thread_id):
 return "OK"
 
Index: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py
===
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py
@@ -11,3 +11,28 @@
 target = self.createTarget("a.yaml")
 process = self.connect(target)
 self.assertPacketLogContains(["qProcessInfo", "qfThreadInfo"])
+
+def test_attach_fail(self):
+error_msg = "mock-error-msg"
+
+class MyResponder(MockGDBServerResponder):
+# Pretend we don't have any process during the initial queries.
+def qC(self):
+return "E42"
+
+def qfThreadInfo(self):
+return "OK" # No threads.
+
+# Then, when we are asked to attach, error out.
+def vAttach(self, pid):
+return "E42;" + error_msg.encode("hex")
+
+self.server.responder = MyResponder()
+
+target = self.dbg.CreateTarget("")
+process = self.connect(target)
+lldbutil.expect_state_changes(self, self.dbg.GetListener(), process, 
[lldb.eStateConnected])
+
+error = lldb.SBError()
+target.AttachToProcessWithID(lldb.SBListener(), 47, error)
+self.assertEquals(error_msg, error.GetCString())
Index: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -3804,11 +3804,9 @@
 response.GetError() == 0x87) {
   process->SetExitStatus(-1, "cannot attach to process due to "
  "System Integrity Protection");
-}
-// E01 code from vAttach means that the attach failed
-if (::strstr(continue_cstr, "vAttach") != NULL &&
-response.GetError() == 0x1) {
-  process->SetExitStatus(-1, "unable to attach");
+} else if (::strstr(continue_cstr, "vAttach") != NULL &&
+   response.GetStatus().Fail()) {
+  process->SetExitStatus(-1, response.GetStatus().AsCString());
 } else {
   process->SetExitStatus(-1, "lost connection");
 }


Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py

[Lldb-commits] [lldb] r330247 - Report more precise error message when attach fails

2018-04-18 Thread Pavel Labath via lldb-commits
Author: labath
Date: Wed Apr 18 04:56:21 2018
New Revision: 330247

URL: http://llvm.org/viewvc/llvm-project?rev=330247=rev
Log:
Report more precise error message when attach fails

Summary:
If the remote stub sends a specific error message instead of just a E??
code, we can use this to display a more informative error message
instead of just the generic "unable to attach" message.

I write a test for this using the SB API.
On the console this will show up like:
(lldb) process attach ...
error: attach failed: 

if the stub supports error messages, or:
error: attach failed: Error ??

if it doesn't.

Reviewers: jingham, JDevlieghere

Subscribers: lldb-commits

Differential Revision: https://reviews.llvm.org/D45573

Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py?rev=330247=330246=330247=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py
 Wed Apr 18 04:56:21 2018
@@ -11,3 +11,28 @@ class TestGDBRemoteClient(GDBRemoteTestB
 target = self.createTarget("a.yaml")
 process = self.connect(target)
 self.assertPacketLogContains(["qProcessInfo", "qfThreadInfo"])
+
+def test_attach_fail(self):
+error_msg = "mock-error-msg"
+
+class MyResponder(MockGDBServerResponder):
+# Pretend we don't have any process during the initial queries.
+def qC(self):
+return "E42"
+
+def qfThreadInfo(self):
+return "OK" # No threads.
+
+# Then, when we are asked to attach, error out.
+def vAttach(self, pid):
+return "E42;" + error_msg.encode("hex")
+
+self.server.responder = MyResponder()
+
+target = self.dbg.CreateTarget("")
+process = self.connect(target)
+lldbutil.expect_state_changes(self, self.dbg.GetListener(), process, 
[lldb.eStateConnected])
+
+error = lldb.SBError()
+target.AttachToProcessWithID(lldb.SBListener(), 47, error)
+self.assertEquals(error_msg, error.GetCString())

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py?rev=330247=330246=330247=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
 Wed Apr 18 04:56:21 2018
@@ -126,6 +126,8 @@ class MockGDBServerResponder:
 return self.qfThreadInfo()
 if packet == "qC":
 return self.qC()
+if packet == "QEnableErrorStrings":
+return self.QEnableErrorStrings()
 if packet == "?":
 return self.haltReason()
 if packet[0] == "H":
@@ -137,6 +139,9 @@ class MockGDBServerResponder:
 if data is not None:
 return self._qXferResponse(data, has_more)
 return ""
+if packet.startswith("vAttach;"):
+pid = packet.partition(';')[2]
+return self.vAttach(int(pid, 16))
 if packet[0] == "Z":
 return self.setBreakpoint(packet)
 return self.other(packet)
@@ -177,6 +182,9 @@ class MockGDBServerResponder:
 def qC(self):
 return "QC0"
 
+def QEnableErrorStrings(self):
+return "OK"
+
 def haltReason(self):
 # SIGINT is 2, return type is 2 digit hex string
 return "S02"
@@ -187,6 +195,9 @@ class MockGDBServerResponder:
 def _qXferResponse(self, data, has_more):
 return "%s%s" % ("m" if has_more else "l", escape_binary(data))
 
+def vAttach(self, pid):
+raise self.UnexpectedPacketException()
+
 def selectThread(self, op, thread_id):
 return "OK"
 

Modified: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp?rev=330247=330246=330247=diff
==
--- 

[Lldb-commits] [PATCH] D45703: Use a utility function to speed up "process load image" for POSIX Platforms

2018-04-18 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: source/Plugins/Platform/POSIX/PlatformPOSIX.cpp:1166
+  // The dlopen succeeded!
+  if (token != 0x0)
+return process->AddImageToken(token);

jingham wrote:
> labath wrote:
> > Use LLDB_INVALID_IMAGE_TOKEN here?
> That wouldn't be right.  LLDB_INVALID_IMAGE_TOKEN is the invalid token in 
> lldb's namespace for image loading tokens, which has no relationship to any 
> given platform's invalid token specifier.  In fact, LLDB_INVALID_IMAGE_TOKEN 
> is UINT32_MAX, so it is actually different from the POSIX invalid image token.
I see. Thanks for the explanation.



Comment at: source/Target/Process.cpp:6251
+UtilityFunction *Process::GetLoadImageUtilityFunction(Platform *platform) {
+  if (platform != GetTarget().GetPlatform().get())
+return nullptr;

jingham wrote:
> labath wrote:
> > jingham wrote:
> > > labath wrote:
> > > > Will this ever be true? I would not expect one to be able to change the 
> > > > platform of a process mid-session.
> > > Reading through the code I could not convince myself that it could NOT 
> > > happen.  Target::SetPlatform is a function anybody can call at any time.  
> > > I agree that it would be odd to do so, and we might think about 
> > > prohibiting it (Target::SetPlatform could return an error, and forbid 
> > > changing the Platform, if the Target has a live process.)
> > > 
> > > If everybody agrees that that is a good idea, I can do that and remove 
> > > this check.
> > I see three call in the codebase to SetPlatform. All of them seem to be in 
> > the initialization code, though some of them seem to happen after the 
> > process is launched (DynamicLoaderDarwinKernel constructor).
> > 
> > So it may not be possible to completely forbid setting the platform this 
> > way, but I think we can at least ban changing the platform once it has 
> > already been set in a pretty hard way (lldb_assert or even assert). I think 
> > a lot of things would currently break if someone started changing the 
> > platforms of a running process all of a sudden.
> Not sure.  It seems reasonable to make a target, run it against one platform, 
> then switch the platform and run it again against the new platform.  I'm not 
> sure I'm comfortable saying that a target can only ever use one platform.
Yeah, you're right. I suppose that makes sense. In my mind a target was 
associated with a given platform from the moment it got created (that is how I 
always do things: `platform select`, `platform connect`, and only then `target 
create`). I never tried what would happen if I reversed that. So I guess the 
platform can change during the lifetime of a Target, but I hope that is not the 
case for the lifetime of a Process.


Repository:
  rL LLVM

https://reviews.llvm.org/D45703



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits