[Lldb-commits] [PATCH] D50864: Add libc++ data formatter for std::function

2018-08-20 Thread Jim Ingham via Phabricator via lldb-commits 
jingham added a comment.

For the most part this is fine.  There are two bits to work on:

1. I think you could do all of this work on a static std::function object from 
the data section even if you don't have a process.  It might be worth seeing 
whether you can do that.  It looks like you can make static std::function 
objects...
2. I think this code would be a little easier to understand if you first stated 
clearly the kinds of std::function variants that you've discovered at the 
beginning of the provider.  Then when you get to the section of code that 
handles each of the cases, start the code with: "This covers the case...".  Be 
kind to two years in the future you when you will have forgotten how all this 
stuff works...




Comment at: source/Plugins/Language/CPlusPlus/LibCxx.cpp:58
+
+  if (process != nullptr) {
+Status status;

vsk wrote:
> Please use an early exit here, and everywhere else in the function where it's 
> possible to do so. If the process has really disappeared, there's no need to 
> print anything. In general there's no need to present an incomplete result 
> from this formatter.
Data formatters can get called in the absence of a process.  For simple C 
structs, for instance, they might be used to look at an initialized static 
object in a DATA section.  For instance, I could do something like:
 
#include 
auto g_func = [](int x, int y) {return x + y; };
int
main(int argc, char ** argv)
{
  int ret = g_func(argc, 100);
  return ret;
}

and I should be able to see g_func using "target var" before running the 
program.  I think the only place you care about the process here is whether you 
should resolve a load address or not.  Target->ReadMemory will read from a 
running process if that target has one (but preferring data from the underlying 
files if that's faster.)  So I think this formatter should be able to do its 
job with or without a process.

But I agree with Vedant that in cases where you can't get useful information 
(for instance if you can't get the vtable address), then you should return 
false immediately and not try to present a summary.  That way instead of 
showing an incomplete summary, lldb will show the raw printing of the type, 
which is more useful than a place-holder summary string.

You already do that for the case where you can't read the process memory.  
Using early return when you discover an error will make it easier to ensure you 
are doing that right every time you get an error.



Comment at: source/Plugins/Language/CPlusPlus/LibCxx.cpp:80
+  Symbol *symbol;
+
+  if (target.GetSectionLoadList().ResolveLoadAddress(

vsk wrote:
> Instead of attempting to parse symbol names, which is inherently brittle and 
> complicated, why not get the base address of the callable and look up it's 
> location via the symbol table? You'd lose some precision in being able to 
> disambiguate lambdas vs. regular functions, but that's a great tradeoff to 
> make, because the code would be simpler and more reliable.
I'm not sure I agree.  There were some cases where the only way Shafik could 
find the target was looking at the demangled name.  There's nothing here that's 
so complex that it isn't worth doing, and we can test the cases to ensure that 
it keeps working.  The alternative is leaving folks with no idea what their 
std::function holds and no way of knowing why this works sometimes but not 
others.



Comment at: source/Plugins/Language/CPlusPlus/LibCxx.cpp:129
+   R"(::operator\(\)\(.*\))";
+  ;
+

This ";" is not doing anything, is it?



Comment at: source/Plugins/Language/CPlusPlus/LibCxx.cpp:136-140
+// Given:
+//
+//main::$_1::__invoke(...)
+//
+// We want to slice off __invoke(...) and append operator()()

In the first bit of code, you put the explaining comment before the code that 
handles the condition.  This time it's in the middle of the code.  Maybe move 
this comment before "if (symbol_name.contains(...)"



Comment at: source/Plugins/Language/CPlusPlus/LibCxx.cpp:202-204
+  stream.Printf(" __f_ = %llu", member__f_pointer_value);
+
+  return true;

I don't think this is valuable.  You are just reporting the value of __f_ here? 
If that's all you can do, I think it's better to return false indicating that 
you really couldn't make sense of this object, and showing the raw value.


https://reviews.llvm.org/D50864



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

[Lldb-commits] [PATCH] D50997: Automatically set path to sanitizer runtime when running tests on macOS.

2018-08-20 Thread Phabricator via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB340218: Set path to sanitizer runtime when running tests 
through LIT on macOS. (authored by adrian, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D50997?vs=161566=161575#toc

Repository:
  rLLDB LLDB

https://reviews.llvm.org/D50997

Files:
  lit/Suite/lit.cfg
  lit/Suite/lit.site.cfg.in


Index: lit/Suite/lit.site.cfg.in
===
--- lit/Suite/lit.site.cfg.in
+++ lit/Suite/lit.site.cfg.in
@@ -9,6 +9,10 @@
 config.lit_tools_dir = "@LLVM_LIT_TOOLS_DIR@"
 config.lldb_obj_root = "@LLDB_BINARY_DIR@"
 config.lldb_src_root = "@LLDB_SOURCE_DIR@"
+config.cmake_cxx_compiler = "@CMAKE_CXX_COMPILER@"
+config.host_os = "@HOST_OS@"
+config.host_triple = "@LLVM_HOST_TRIPLE@"
+config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.python_executable = "@PYTHON_EXECUTABLE@"
 config.dotest_path = "@LLDB_SOURCE_DIR@/test/dotest.py"
Index: lit/Suite/lit.cfg
===
--- lit/Suite/lit.cfg
+++ lit/Suite/lit.cfg
@@ -19,6 +19,19 @@
'Python', 'lldbsuite', 'test')
 config.test_exec_root = config.test_source_root
 
+# macOS flags needed for LLDB built with address sanitizer.
+if 'Address' in config.llvm_use_sanitizer and \
+   'Darwin' in config.host_os and \
+   'x86' in config.host_triple:
+  import subprocess
+  resource_dir = subprocess.check_output(
+config.cmake_cxx_compiler +' -print-resource-dir', shell=True).strip()
+  runtime = os.path.join(resource_dir, 'lib', 'darwin',
+ 'libclang_rt.asan_osx_dynamic.dylib')
+  config.environment['ASAN_OPTIONS'] = \
+'detect_stack_use_after_return=1:container_overflow=0'
+  config.environment['DYLD_INSERT_LIBRARIES'] = runtime
+
 # Build dotest command.
 dotest_cmd = [config.dotest_path, '-q']
 dotest_cmd.extend(config.dotest_args_str.split(';'))


Index: lit/Suite/lit.site.cfg.in
===
--- lit/Suite/lit.site.cfg.in
+++ lit/Suite/lit.site.cfg.in
@@ -9,6 +9,10 @@
 config.lit_tools_dir = "@LLVM_LIT_TOOLS_DIR@"
 config.lldb_obj_root = "@LLDB_BINARY_DIR@"
 config.lldb_src_root = "@LLDB_SOURCE_DIR@"
+config.cmake_cxx_compiler = "@CMAKE_CXX_COMPILER@"
+config.host_os = "@HOST_OS@"
+config.host_triple = "@LLVM_HOST_TRIPLE@"
+config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.python_executable = "@PYTHON_EXECUTABLE@"
 config.dotest_path = "@LLDB_SOURCE_DIR@/test/dotest.py"
Index: lit/Suite/lit.cfg
===
--- lit/Suite/lit.cfg
+++ lit/Suite/lit.cfg
@@ -19,6 +19,19 @@
'Python', 'lldbsuite', 'test')
 config.test_exec_root = config.test_source_root
 
+# macOS flags needed for LLDB built with address sanitizer.
+if 'Address' in config.llvm_use_sanitizer and \
+   'Darwin' in config.host_os and \
+   'x86' in config.host_triple:
+  import subprocess
+  resource_dir = subprocess.check_output(
+config.cmake_cxx_compiler +' -print-resource-dir', shell=True).strip()
+  runtime = os.path.join(resource_dir, 'lib', 'darwin',
+ 'libclang_rt.asan_osx_dynamic.dylib')
+  config.environment['ASAN_OPTIONS'] = \
+'detect_stack_use_after_return=1:container_overflow=0'
+  config.environment['DYLD_INSERT_LIBRARIES'] = runtime
+
 # Build dotest command.
 dotest_cmd = [config.dotest_path, '-q']
 dotest_cmd.extend(config.dotest_args_str.split(';'))
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r340218 - Set path to sanitizer runtime when running tests through LIT on macOS.

2018-08-20 Thread Adrian Prantl via lldb-commits 
Author: adrian
Date: Mon Aug 20 15:00:31 2018
New Revision: 340218

URL: http://llvm.org/viewvc/llvm-project?rev=340218=rev
Log:
Set path to sanitizer runtime when running tests through LIT on macOS.

rdar://problem/42984739

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

Modified:
lldb/trunk/lit/Suite/lit.cfg
lldb/trunk/lit/Suite/lit.site.cfg.in

Modified: lldb/trunk/lit/Suite/lit.cfg
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Suite/lit.cfg?rev=340218=340217=340218=diff
==
--- lldb/trunk/lit/Suite/lit.cfg (original)
+++ lldb/trunk/lit/Suite/lit.cfg Mon Aug 20 15:00:31 2018
@@ -19,6 +19,19 @@ config.test_source_root = os.path.join(c
'Python', 'lldbsuite', 'test')
 config.test_exec_root = config.test_source_root
 
+# macOS flags needed for LLDB built with address sanitizer.
+if 'Address' in config.llvm_use_sanitizer and \
+   'Darwin' in config.host_os and \
+   'x86' in config.host_triple:
+  import subprocess
+  resource_dir = subprocess.check_output(
+config.cmake_cxx_compiler +' -print-resource-dir', shell=True).strip()
+  runtime = os.path.join(resource_dir, 'lib', 'darwin',
+ 'libclang_rt.asan_osx_dynamic.dylib')
+  config.environment['ASAN_OPTIONS'] = \
+'detect_stack_use_after_return=1:container_overflow=0'
+  config.environment['DYLD_INSERT_LIBRARIES'] = runtime
+
 # Build dotest command.
 dotest_cmd = [config.dotest_path, '-q']
 dotest_cmd.extend(config.dotest_args_str.split(';'))

Modified: lldb/trunk/lit/Suite/lit.site.cfg.in
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Suite/lit.site.cfg.in?rev=340218=340217=340218=diff
==
--- lldb/trunk/lit/Suite/lit.site.cfg.in (original)
+++ lldb/trunk/lit/Suite/lit.site.cfg.in Mon Aug 20 15:00:31 2018
@@ -9,6 +9,10 @@ config.llvm_build_mode = "@LLVM_BUILD_MO
 config.lit_tools_dir = "@LLVM_LIT_TOOLS_DIR@"
 config.lldb_obj_root = "@LLDB_BINARY_DIR@"
 config.lldb_src_root = "@LLDB_SOURCE_DIR@"
+config.cmake_cxx_compiler = "@CMAKE_CXX_COMPILER@"
+config.host_os = "@HOST_OS@"
+config.host_triple = "@LLVM_HOST_TRIPLE@"
+config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.python_executable = "@PYTHON_EXECUTABLE@"
 config.dotest_path = "@LLDB_SOURCE_DIR@/test/dotest.py"


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

[Lldb-commits] [lldb] r340219 - Reflow comments

2018-08-20 Thread Adrian Prantl via lldb-commits 
Author: adrian
Date: Mon Aug 20 15:00:32 2018
New Revision: 340219

URL: http://llvm.org/viewvc/llvm-project?rev=340219=rev
Log:
Reflow comments

Modified:
lldb/trunk/lit/Suite/lldbtest.py

Modified: lldb/trunk/lit/Suite/lldbtest.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Suite/lldbtest.py?rev=340219=340218=340219=diff
==
--- lldb/trunk/lit/Suite/lldbtest.py (original)
+++ lldb/trunk/lit/Suite/lldbtest.py Mon Aug 20 15:00:32 2018
@@ -44,9 +44,9 @@ class LLDBTest(TestFormat):
 return (lit.Test.UNSUPPORTED, 'Test is unsupported')
 
 testPath, testFile = os.path.split(test.getSourcePath())
-# On Windows, the system does not always correctly interpret shebang 
lines.
-# To make sure we can execute the tests, add python exe as the first 
parameter
-# of the command.
+# On Windows, the system does not always correctly interpret
+# shebang lines.  To make sure we can execute the tests, add
+# python exe as the first parameter of the command.
 cmd = [sys.executable] + self.dotest_cmd + [testPath, '-p', testFile]
 
 try:


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

[Lldb-commits] [PATCH] D50997: Automatically set path to sanitizer runtime when running tests on macOS.

2018-08-20 Thread Adrian Prantl via Phabricator via lldb-commits 
aprantl added a comment.

Thanks, that was quite helpful!


https://reviews.llvm.org/D50997



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

[Lldb-commits] [PATCH] D50997: Automatically set path to sanitizer runtime when running tests on macOS.

2018-08-20 Thread Vedant Kumar via Phabricator via lldb-commits 
vsk accepted this revision.
vsk added a comment.
This revision is now accepted and ready to land.

(LGTM with the second comment addressed.)


https://reviews.llvm.org/D50997



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

[Lldb-commits] [PATCH] D50997: Automatically set path to sanitizer runtime when running tests on macOS.

2018-08-20 Thread Vedant Kumar via Phabricator via lldb-commits 
vsk added a comment.

Thanks so much for doing this!




Comment at: lit/Suite/lit.cfg:28
+  resource_dir = subprocess.check_output(config.cmake_cxx_compiler +
+ ' -print-resource-dir', shell=True)
+  runtime = os.path.join(resource_dir[:-1],

This might be easier to read as either `import commands; 
commands.getoutput(...)` or `check_output(..., shell=True).strip()`. Then it's 
possible to drop the [:-1], which is nice, because it's not clear that that 
strips a trailing newline.



Comment at: lit/Suite/lit.cfg:31
+ 'lib/darwin/libclang_rt.asan_osx_dynamic.dylib')
+  config.environment['ASAN_OPTIONS'] =  'detect_stack_use-after_return=1'
+  config.environment['DYLD_INSERT_LIBRARIES'] = runtime

The dash should be an underscore, and lldb hits false positives when the 
container overflow check is enabled. How about: 
`ASAN_OPTIONS=detect_stack_use_after_return=1:container_overflow=0`?


https://reviews.llvm.org/D50997



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

[Lldb-commits] [PATCH] D50997: Automatically set path to sanitizer runtime when running tests on macOS.

2018-08-20 Thread Adrian Prantl via Phabricator via lldb-commits 
aprantl created this revision.
aprantl added a reviewer: vsk.

Automatically set path to sanitizer runtime when running tests on macOS.

rdar://problem/42984739


https://reviews.llvm.org/D50997

Files:
  lit/Suite/lit.cfg
  lit/Suite/lit.site.cfg.in


Index: lit/Suite/lit.site.cfg.in
===
--- lit/Suite/lit.site.cfg.in
+++ lit/Suite/lit.site.cfg.in
@@ -9,6 +9,10 @@
 config.lit_tools_dir = "@LLVM_LIT_TOOLS_DIR@"
 config.lldb_obj_root = "@LLDB_BINARY_DIR@"
 config.lldb_src_root = "@LLDB_SOURCE_DIR@"
+config.cmake_cxx_compiler = "@CMAKE_CXX_COMPILER@"
+config.host_os = "@HOST_OS@"
+config.host_triple = "@LLVM_HOST_TRIPLE@"
+config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.python_executable = "@PYTHON_EXECUTABLE@"
 config.dotest_path = "@LLDB_SOURCE_DIR@/test/dotest.py"
Index: lit/Suite/lit.cfg
===
--- lit/Suite/lit.cfg
+++ lit/Suite/lit.cfg
@@ -19,6 +19,18 @@
'Python', 'lldbsuite', 'test')
 config.test_exec_root = config.test_source_root
 
+# macOS flags needed for LLDB built with address sanitizer.
+if 'Address' in config.llvm_use_sanitizer and \
+   'Darwin' in config.host_os and \
+   'x86' in config.host_triple:
+  import subprocess
+  resource_dir = subprocess.check_output(config.cmake_cxx_compiler +
+ ' -print-resource-dir', shell=True)
+  runtime = os.path.join(resource_dir[:-1],
+ 'lib/darwin/libclang_rt.asan_osx_dynamic.dylib')
+  config.environment['ASAN_OPTIONS'] =  'detect_stack_use-after_return=1'
+  config.environment['DYLD_INSERT_LIBRARIES'] = runtime
+
 # Build dotest command.
 dotest_cmd = [config.dotest_path, '-q']
 dotest_cmd.extend(config.dotest_args_str.split(';'))


Index: lit/Suite/lit.site.cfg.in
===
--- lit/Suite/lit.site.cfg.in
+++ lit/Suite/lit.site.cfg.in
@@ -9,6 +9,10 @@
 config.lit_tools_dir = "@LLVM_LIT_TOOLS_DIR@"
 config.lldb_obj_root = "@LLDB_BINARY_DIR@"
 config.lldb_src_root = "@LLDB_SOURCE_DIR@"
+config.cmake_cxx_compiler = "@CMAKE_CXX_COMPILER@"
+config.host_os = "@HOST_OS@"
+config.host_triple = "@LLVM_HOST_TRIPLE@"
+config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.python_executable = "@PYTHON_EXECUTABLE@"
 config.dotest_path = "@LLDB_SOURCE_DIR@/test/dotest.py"
Index: lit/Suite/lit.cfg
===
--- lit/Suite/lit.cfg
+++ lit/Suite/lit.cfg
@@ -19,6 +19,18 @@
'Python', 'lldbsuite', 'test')
 config.test_exec_root = config.test_source_root
 
+# macOS flags needed for LLDB built with address sanitizer.
+if 'Address' in config.llvm_use_sanitizer and \
+   'Darwin' in config.host_os and \
+   'x86' in config.host_triple:
+  import subprocess
+  resource_dir = subprocess.check_output(config.cmake_cxx_compiler +
+ ' -print-resource-dir', shell=True)
+  runtime = os.path.join(resource_dir[:-1],
+ 'lib/darwin/libclang_rt.asan_osx_dynamic.dylib')
+  config.environment['ASAN_OPTIONS'] =  'detect_stack_use-after_return=1'
+  config.environment['DYLD_INSERT_LIBRARIES'] = runtime
+
 # Build dotest command.
 dotest_cmd = [config.dotest_path, '-q']
 dotest_cmd.extend(config.dotest_args_str.split(';'))
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

2018-08-20 Thread Eugene Birukov via Phabricator via lldb-commits 
EugeneBi updated this revision to Diff 161520.
EugeneBi added a comment.

Mark added the test. Please let us know if this is OK.


https://reviews.llvm.org/D49685

Files:
  
packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/TestLinuxCore.py
  source/Target/Platform.cpp

Index: source/Target/Platform.cpp
===
--- source/Target/Platform.cpp
+++ source/Target/Platform.cpp
@@ -228,17 +228,36 @@
 module_spec, module_sp, module_search_paths_ptr, old_module_sp_ptr,
 did_create_ptr, false);
 
+  // Module resolver lambda.
+  auto resolver = [&](const ModuleSpec ) {
+Status error(eErrorTypeGeneric);
+ModuleSpec resolved_spec;
+// Check if we have sysroot set.
+if (m_sdk_sysroot) {
+  // Prepend sysroot to module spec.
+  resolved_spec = spec;
+  resolved_spec.GetFileSpec().PrependPathComponent(
+m_sdk_sysroot.GetStringRef());
+  // Try to get shared module with resolved spec.
+  error = ModuleList::GetSharedModule( 
+resolved_spec, module_sp, module_search_paths_ptr, old_module_sp_ptr, 
+did_create_ptr, false);
+}
+// If we don't have sysroot or it didn't work then 
+// try original module spec.
+if (!error.Success()) {
+  resolved_spec = spec;
+  error = ModuleList::GetSharedModule(
+resolved_spec, module_sp, module_search_paths_ptr, old_module_sp_ptr, 
+did_create_ptr, false);
+}
+if (error.Success() && module_sp)
+  module_sp->SetPlatformFileSpec(resolved_spec.GetFileSpec());
+return error;
+  };
+
   return GetRemoteSharedModule(module_spec, process, module_sp,
-   [&](const ModuleSpec ) {
- Status error = ModuleList::GetSharedModule(
- spec, module_sp, module_search_paths_ptr,
- old_module_sp_ptr, did_create_ptr, false);
- if (error.Success() && module_sp)
-   module_sp->SetPlatformFileSpec(
-   spec.GetFileSpec());
- return error;
-   },
-   did_create_ptr);
+   resolver, did_create_ptr);
 }
 
 bool Platform::GetModuleSpec(const FileSpec _file_spec,
Index: packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -6,6 +6,7 @@
 
 import shutil
 import struct
+import os
 
 import lldb
 from lldbsuite.test.decorators import *
@@ -203,6 +204,33 @@
 for regname, value in values.iteritems():
 self.expect("register read {}".format(regname), substrs=["{} = {}".format(regname, value)])
 
+@expectedFailureAll(bugnumber="llvm.org/pr37371", hostoslist=["windows"])
+@skipIf(triple='^mips')
+@skipIfLLVMTargetMissing("X86")
+def test_i386_sysroot(self):
+"""Test that lldb can find the exe for an i386 linux core file using the sysroot."""
+
+# Copy linux-i386.out to tmp_sysroot/home/labath/test/a.out (since it was compiled as
+# /home/labath/test/a.out)
+tmp_sysroot = "/tmp/lldb_i386_mock_sysroot"
+executable = tmp_sysroot + "/home/labath/test/a.out"
+lldbutil.mkdir_p(os.path.dirname(executable))
+shutil.copyfile("linux-i386.out", executable)
+
+# Set sysroot and load core
+self.runCmd("platform select remote-linux --sysroot '%s'" % tmp_sysroot)
+target = self.dbg.CreateTarget(None)
+self.assertTrue(target, VALID_TARGET)
+process = target.LoadCore("linux-i386.core")
+
+# Check that we found a.out from the sysroot
+self.check_all(process, self._i386_pid, self._i386_regions, "a.out")
+
+self.dbg.DeleteTarget(target)
+
+# Clean up tmp_sysroot
+shutil.rmtree(tmp_sysroot)
+
 def check_memory_regions(self, process, region_count):
 region_list = process.GetMemoryRegions()
 self.assertEqual(region_list.GetSize(), region_count)
@@ -299,15 +327,7 @@
 self.dbg.SetOutputFileHandle(None, False)
 self.dbg.SetErrorFileHandle(None, False)
 
-def do_test(self, filename, pid, region_count, thread_name):
-target = self.dbg.CreateTarget(filename + ".out")
-process = target.LoadCore(filename + ".core")
-self.assertTrue(process, PROCESS_IS_VALID)
-self.assertEqual(process.GetNumThreads(), 1)
-self.assertEqual(process.GetProcessID(), pid)
-
-self.check_state(process)
-
+def check_stack(self, process, pid, thread_name):
 thread =

[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

2018-08-20 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg added a comment.

You can always run a.out through obj2yaml and check that in. There is test 
suite support for using a yaml file that converts it to a binary and debugs it

  functionalities/gdb_remote_client/gdbclientutils.py
  429:def createTarget(self, yaml_path):
  431:Create a target by auto-generating the object based on the given 
yaml
  437:yaml_base, ext = os.path.splitext(yaml_path)
  438:obj_path = self.getBuildArtifact(yaml_base)
  439:self.yaml2obj(yaml_path, obj_path)


https://reviews.llvm.org/D49685



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

[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

2018-08-20 Thread Eugene Birukov via Phabricator via lldb-commits 
EugeneBi added a comment.

In https://reviews.llvm.org/D49685#1201351, @magardne wrote:

> @labath Eugene asked me to help add a unit test for this. I have the updated 
> diff, but I can't seem to attach it to this code review -- it must be because 
> I'm not the original author? I'll attach the diff to this comment and maybe 
> you can try to update the review. Otherwise we can wait until Eugene gets 
> back on Monday.
>
> The diff makes the following changes:
>
> - Add an executable: 
> test/testcases/functionalities/postmortem/elf-core/mock_sysroot/home/labath/test/a.out
> - This is a copy of linux-i386.out (which is why it's in 
> mock_sysroot/home/labath...). Let me know if this is ok, or if I should try 
> to create a new binary/core
> - Implement the test you suggested: set platform and sysroot, open the core, 
> check threads/stacks are valid
>
>   I verified that the test fails without the changes to Platform.cpp and 
> passes after the changes.
>
>   F6945416: sysroot_cr.diff 


Mark, thanks a lot for doing this. I have one request though: let's not add new 
binaries to the repo. Instead, the test should create a temporary directory 
somewhere (in /tmp, I guess) and copy necessary files there. After the test is 
done, wipe it out.


https://reviews.llvm.org/D49685



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

[Lldb-commits] [PATCH] D50677: Remove manual byte counting from Opcode::Dump

2018-08-20 Thread Raphael Isemann via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL340179: Remove manual byte counting from Opcode::Dump 
(authored by teemperor, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D50677?vs=160484=161492#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D50677

Files:
  lldb/trunk/source/Core/Opcode.cpp


Index: lldb/trunk/source/Core/Opcode.cpp
===
--- lldb/trunk/source/Core/Opcode.cpp
+++ lldb/trunk/source/Core/Opcode.cpp
@@ -23,40 +23,41 @@
 using namespace lldb_private;
 
 int Opcode::Dump(Stream *s, uint32_t min_byte_width) {
-  int bytes_written = 0;
+  const uint32_t previous_bytes = s->GetWrittenBytes();
   switch (m_type) {
   case Opcode::eTypeInvalid:
-bytes_written = s->PutCString("");
+s->PutCString("");
 break;
   case Opcode::eType8:
-bytes_written = s->Printf("0x%2.2x", m_data.inst8);
+s->Printf("0x%2.2x", m_data.inst8);
 break;
   case Opcode::eType16:
-bytes_written = s->Printf("0x%4.4x", m_data.inst16);
+s->Printf("0x%4.4x", m_data.inst16);
 break;
   case Opcode::eType16_2:
   case Opcode::eType32:
-bytes_written = s->Printf("0x%8.8x", m_data.inst32);
+s->Printf("0x%8.8x", m_data.inst32);
 break;
 
   case Opcode::eType64:
-bytes_written = s->Printf("0x%16.16" PRIx64, m_data.inst64);
+s->Printf("0x%16.16" PRIx64, m_data.inst64);
 break;
 
   case Opcode::eTypeBytes:
 for (uint32_t i = 0; i < m_data.inst.length; ++i) {
   if (i > 0)
-bytes_written += s->PutChar(' ');
-  bytes_written += s->Printf("%2.2x", m_data.inst.bytes[i]);
+s->PutChar(' ');
+  s->Printf("%2.2x", m_data.inst.bytes[i]);
 }
 break;
   }
 
-  // Add spaces to make sure bytes dispay comes out even in case opcodes aren't
-  // all the same size
-  if (static_cast(bytes_written) < min_byte_width)
-bytes_written = s->Printf("%*s", min_byte_width - bytes_written, "");
-  return bytes_written;
+  uint32_t bytes_written_so_far = s->GetWrittenBytes() - previous_bytes;
+  // Add spaces to make sure bytes display comes out even in case opcodes 
aren't
+  // all the same size.
+  if (bytes_written_so_far < min_byte_width)
+s->Printf("%*s", min_byte_width - bytes_written_so_far, "");
+  return s->GetWrittenBytes() - previous_bytes;
 }
 
 lldb::ByteOrder Opcode::GetDataByteOrder() const {


Index: lldb/trunk/source/Core/Opcode.cpp
===
--- lldb/trunk/source/Core/Opcode.cpp
+++ lldb/trunk/source/Core/Opcode.cpp
@@ -23,40 +23,41 @@
 using namespace lldb_private;
 
 int Opcode::Dump(Stream *s, uint32_t min_byte_width) {
-  int bytes_written = 0;
+  const uint32_t previous_bytes = s->GetWrittenBytes();
   switch (m_type) {
   case Opcode::eTypeInvalid:
-bytes_written = s->PutCString("");
+s->PutCString("");
 break;
   case Opcode::eType8:
-bytes_written = s->Printf("0x%2.2x", m_data.inst8);
+s->Printf("0x%2.2x", m_data.inst8);
 break;
   case Opcode::eType16:
-bytes_written = s->Printf("0x%4.4x", m_data.inst16);
+s->Printf("0x%4.4x", m_data.inst16);
 break;
   case Opcode::eType16_2:
   case Opcode::eType32:
-bytes_written = s->Printf("0x%8.8x", m_data.inst32);
+s->Printf("0x%8.8x", m_data.inst32);
 break;
 
   case Opcode::eType64:
-bytes_written = s->Printf("0x%16.16" PRIx64, m_data.inst64);
+s->Printf("0x%16.16" PRIx64, m_data.inst64);
 break;
 
   case Opcode::eTypeBytes:
 for (uint32_t i = 0; i < m_data.inst.length; ++i) {
   if (i > 0)
-bytes_written += s->PutChar(' ');
-  bytes_written += s->Printf("%2.2x", m_data.inst.bytes[i]);
+s->PutChar(' ');
+  s->Printf("%2.2x", m_data.inst.bytes[i]);
 }
 break;
   }
 
-  // Add spaces to make sure bytes dispay comes out even in case opcodes aren't
-  // all the same size
-  if (static_cast(bytes_written) < min_byte_width)
-bytes_written = s->Printf("%*s", min_byte_width - bytes_written, "");
-  return bytes_written;
+  uint32_t bytes_written_so_far = s->GetWrittenBytes() - previous_bytes;
+  // Add spaces to make sure bytes display comes out even in case opcodes aren't
+  // all the same size.
+  if (bytes_written_so_far < min_byte_width)
+s->Printf("%*s", min_byte_width - bytes_written_so_far, "");
+  return s->GetWrittenBytes() - previous_bytes;
 }
 
 lldb::ByteOrder Opcode::GetDataByteOrder() const {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r340179 - Remove manual byte counting from Opcode::Dump

2018-08-20 Thread Raphael Isemann via lldb-commits 
Author: teemperor
Date: Mon Aug 20 08:51:14 2018
New Revision: 340179

URL: http://llvm.org/viewvc/llvm-project?rev=340179=rev
Log:
Remove manual byte counting from Opcode::Dump

Summary:
Stream now has byte-counting functionality, so let's use this instead of manual 
byte
counting.

Reviewers: clayborg, davide

Reviewed By: davide

Subscribers: davide, lldb-commits

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

Modified:
lldb/trunk/source/Core/Opcode.cpp

Modified: lldb/trunk/source/Core/Opcode.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Opcode.cpp?rev=340179=340178=340179=diff
==
--- lldb/trunk/source/Core/Opcode.cpp (original)
+++ lldb/trunk/source/Core/Opcode.cpp Mon Aug 20 08:51:14 2018
@@ -23,40 +23,41 @@ using namespace lldb;
 using namespace lldb_private;
 
 int Opcode::Dump(Stream *s, uint32_t min_byte_width) {
-  int bytes_written = 0;
+  const uint32_t previous_bytes = s->GetWrittenBytes();
   switch (m_type) {
   case Opcode::eTypeInvalid:
-bytes_written = s->PutCString("");
+s->PutCString("");
 break;
   case Opcode::eType8:
-bytes_written = s->Printf("0x%2.2x", m_data.inst8);
+s->Printf("0x%2.2x", m_data.inst8);
 break;
   case Opcode::eType16:
-bytes_written = s->Printf("0x%4.4x", m_data.inst16);
+s->Printf("0x%4.4x", m_data.inst16);
 break;
   case Opcode::eType16_2:
   case Opcode::eType32:
-bytes_written = s->Printf("0x%8.8x", m_data.inst32);
+s->Printf("0x%8.8x", m_data.inst32);
 break;
 
   case Opcode::eType64:
-bytes_written = s->Printf("0x%16.16" PRIx64, m_data.inst64);
+s->Printf("0x%16.16" PRIx64, m_data.inst64);
 break;
 
   case Opcode::eTypeBytes:
 for (uint32_t i = 0; i < m_data.inst.length; ++i) {
   if (i > 0)
-bytes_written += s->PutChar(' ');
-  bytes_written += s->Printf("%2.2x", m_data.inst.bytes[i]);
+s->PutChar(' ');
+  s->Printf("%2.2x", m_data.inst.bytes[i]);
 }
 break;
   }
 
-  // Add spaces to make sure bytes dispay comes out even in case opcodes aren't
-  // all the same size
-  if (static_cast(bytes_written) < min_byte_width)
-bytes_written = s->Printf("%*s", min_byte_width - bytes_written, "");
-  return bytes_written;
+  uint32_t bytes_written_so_far = s->GetWrittenBytes() - previous_bytes;
+  // Add spaces to make sure bytes display comes out even in case opcodes 
aren't
+  // all the same size.
+  if (bytes_written_so_far < min_byte_width)
+s->Printf("%*s", min_byte_width - bytes_written_so_far, "");
+  return s->GetWrittenBytes() - previous_bytes;
 }
 
 lldb::ByteOrder Opcode::GetDataByteOrder() const {


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

[Lldb-commits] [lldb] r340168 - Fix lit.cfg for python3: can only concatenate str (not "bytes") to str

2018-08-20 Thread Stefan Granitz via lldb-commits 
Author: stefan.graenitz
Date: Mon Aug 20 05:37:54 2018
New Revision: 340168

URL: http://llvm.org/viewvc/llvm-project?rev=340168=rev
Log:
Fix lit.cfg for python3: can only concatenate str (not "bytes") to str

Modified:
lldb/trunk/lit/lit.cfg

Modified: lldb/trunk/lit/lit.cfg
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/lit.cfg?rev=340168=340167=340168=diff
==
--- lldb/trunk/lit/lit.cfg (original)
+++ lldb/trunk/lit/lit.cfg Mon Aug 20 05:37:54 2018
@@ -78,7 +78,7 @@ if platform.system() in ['Darwin']:
 except OSError:
 res = -1
 if res == 0 and out:
-sdk_path = out
+sdk_path = lit.util.to_string(out)
 lit_config.note('using SDKROOT: %r' % sdk_path)
 config.cc += " -isysroot %s" % sdk_path
 config.cxx += " -isysroot %s" % sdk_path


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

Re: [Lldb-commits] [lldb] r339974 - Add a relocation for R_AARCH64_ABS32 in ObjectFileELF

2018-08-20 Thread Davide Italiano via lldb-commits 
Oh, yeah, indeed (and probably crafting a binary with yaml2obj or
llvm-mc which triggers this reloc should be trivial [you might
consider taking a look at the tests in lld/ELF as probably have tons
of similar examples).
On Mon, Aug 20, 2018 at 12:34 PM Pavel Labath via lldb-commits
 wrote:
>
> On 20/08/18 08:17, Davide Italiano via lldb-commits wrote:
> > On Fri, Aug 17, 2018 at 2:36 AM Stephane Sezer via lldb-commits
> >  wrote:
> >>
> >> Author: sas
> >> Date: Thu Aug 16 17:35:47 2018
> >> New Revision: 339974
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=339974=rev
> >> Log:
> >> Add a relocation for R_AARCH64_ABS32 in ObjectFileELF
> >>
> >> Summary:
> >> .rela.debug_info relocations are being done via
> >> ObjectFileELF::ApplyRelocations for aarch64. Currently, the switch case
> >> that iterates over the relocation type is only implemented for a few
> >> different types and `assert(false)`es over the rest.
> >>
> >> Implement the relocation for R_AARCH64_ABS32 in ApplyRelocations
> >>
> >> Reviewers: sas, xiaobai, javed.absar, espindola
> >>
> >> Reviewed By: sas
> >>
> >> Subscribers: emaste, arichardson, kristof.beyls
> >>
> >> Differential Revision: https://reviews.llvm.org/D50369
> >>
> >> Change by Nathan Lanza 
> >>
> >
> > This change should be unit testable, can you please give it a shot?
> > Alternatively, you can probably use `lldb-test`, but maybe that
> > requires some infrastructure to be written that's not there yet and I
> > think it's not fair to ask you to write it.
> >
> > --
> > Davide
> > ___
> > lldb-commits mailing list
> > lldb-commits@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
> >
>
> lldb-test has the ability to dump the contents of object file sections.
> The applied relocations should be reflected there.
>
> pl
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] r339974 - Add a relocation for R_AARCH64_ABS32 in ObjectFileELF

2018-08-20 Thread Pavel Labath via lldb-commits 
On 20/08/18 08:17, Davide Italiano via lldb-commits wrote:
> On Fri, Aug 17, 2018 at 2:36 AM Stephane Sezer via lldb-commits
>  wrote:
>>
>> Author: sas
>> Date: Thu Aug 16 17:35:47 2018
>> New Revision: 339974
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=339974=rev
>> Log:
>> Add a relocation for R_AARCH64_ABS32 in ObjectFileELF
>>
>> Summary:
>> .rela.debug_info relocations are being done via
>> ObjectFileELF::ApplyRelocations for aarch64. Currently, the switch case
>> that iterates over the relocation type is only implemented for a few
>> different types and `assert(false)`es over the rest.
>>
>> Implement the relocation for R_AARCH64_ABS32 in ApplyRelocations
>>
>> Reviewers: sas, xiaobai, javed.absar, espindola
>>
>> Reviewed By: sas
>>
>> Subscribers: emaste, arichardson, kristof.beyls
>>
>> Differential Revision: https://reviews.llvm.org/D50369
>>
>> Change by Nathan Lanza 
>>
> 
> This change should be unit testable, can you please give it a shot?
> Alternatively, you can probably use `lldb-test`, but maybe that
> requires some infrastructure to be written that's not there yet and I
> think it's not fair to ask you to write it.
> 
> --
> Davide
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
> 

lldb-test has the ability to dump the contents of object file sections.
The applied relocations should be reflected there.

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

Re: [Lldb-commits] [lldb] r339974 - Add a relocation for R_AARCH64_ABS32 in ObjectFileELF

2018-08-20 Thread Davide Italiano via lldb-commits 
On Fri, Aug 17, 2018 at 2:36 AM Stephane Sezer via lldb-commits
 wrote:
>
> Author: sas
> Date: Thu Aug 16 17:35:47 2018
> New Revision: 339974
>
> URL: http://llvm.org/viewvc/llvm-project?rev=339974=rev
> Log:
> Add a relocation for R_AARCH64_ABS32 in ObjectFileELF
>
> Summary:
> .rela.debug_info relocations are being done via
> ObjectFileELF::ApplyRelocations for aarch64. Currently, the switch case
> that iterates over the relocation type is only implemented for a few
> different types and `assert(false)`es over the rest.
>
> Implement the relocation for R_AARCH64_ABS32 in ApplyRelocations
>
> Reviewers: sas, xiaobai, javed.absar, espindola
>
> Reviewed By: sas
>
> Subscribers: emaste, arichardson, kristof.beyls
>
> Differential Revision: https://reviews.llvm.org/D50369
>
> Change by Nathan Lanza 
>

This change should be unit testable, can you please give it a shot?
Alternatively, you can probably use `lldb-test`, but maybe that
requires some infrastructure to be written that's not there yet and I
think it's not fair to ask you to write it.

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

[Lldb-commits] [PATCH] D50677: Remove manual byte counting from Opcode::Dump

2018-08-20 Thread Davide Italiano via Phabricator via lldb-commits 
davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D50677



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

[Lldb-commits] [lldb] r340151 - [NFC] Minor update to comment

2018-08-20 Thread Aleksandr Urakov via lldb-commits 
Author: aleksandr.urakov
Date: Sun Aug 19 22:59:27 2018
New Revision: 340151

URL: http://llvm.org/viewvc/llvm-project?rev=340151=rev
Log:
[NFC] Minor update to comment

Update comment after rLLDB339994

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

Modified:
lldb/trunk/source/Symbol/ClangASTContext.cpp

Modified: lldb/trunk/source/Symbol/ClangASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/ClangASTContext.cpp?rev=340151=340150=340151=diff
==
--- lldb/trunk/source/Symbol/ClangASTContext.cpp (original)
+++ lldb/trunk/source/Symbol/ClangASTContext.cpp Sun Aug 19 22:59:27 2018
@@ -6738,7 +6738,7 @@ CompilerType ClangASTContext::GetChildCo
   }
 }
 
-// Setting this to UINT32_MAX to make sure we don't compute it
+// Setting this to INT32_MAX to make sure we don't compute it
 // twice...
 bit_offset = INT32_MAX;
 


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