date:20190318

[Lldb-commits] [PATCH] D59524: Improve error handling for Clang module imports

2019-03-18 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59524/new/

https://reviews.llvm.org/D59524



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

[Lldb-commits] [PATCH] D59524: Improve error handling for Clang module imports

2019-03-18 Thread Adrian Prantl via Phabricator via lldb-commits

aprantl created this revision.
aprantl added a reviewer: JDevlieghere.
Herald added a reviewer: serge-sans-paille.
Herald added a project: LLDB.
aprantl edited the summary of this revision.

Most notably this avoids nullptr dereferences when the import fails.

rdar://problem/48883558


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59524

Files:
  lldb/packages/Python/lldbsuite/test/lang/cpp/modules-import/Bar.h
  lldb/packages/Python/lldbsuite/test/lang/cpp/modules-import/Foo.h
  lldb/packages/Python/lldbsuite/test/lang/cpp/modules-import/Inputs/Bar.h
  lldb/packages/Python/lldbsuite/test/lang/cpp/modules-import/Inputs/Foo.h
  
lldb/packages/Python/lldbsuite/test/lang/cpp/modules-import/Inputs/module.modulemap
  lldb/packages/Python/lldbsuite/test/lang/cpp/modules-import/Makefile
  
lldb/packages/Python/lldbsuite/test/lang/cpp/modules-import/TestCXXModulesImport.py
  lldb/packages/Python/lldbsuite/test/lang/cpp/modules-import/module.modulemap
  lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp


Index: lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
@@ -229,15 +229,23 @@
 std::equal(sysroot_begin, sysroot_end, path_begin);
 // No need to inject search paths to modules in the sysroot.
 if (!is_system_module) {
+  auto error = [&]() {
+error_stream.Printf("error: No module map file in %s\n",
+module.search_path.AsCString());
+return false;
+  };
+
   bool is_system = true;
   bool is_framework = false;
   auto *dir =
   HS.getFileMgr().getDirectory(module.search_path.GetStringRef());
+  if (!dir)
+return error();
   auto *file = HS.lookupModuleMapFile(dir, is_framework);
+  if (!file)
+return error();
   if (!HS.loadModuleMapFile(file, is_system))
-error_stream.Printf("error: No module map file in %s\n",
-module.search_path.AsCString());
-  return false;
+return error();
 }
   }
   if (!HS.lookupModule(module.path.front().GetStringRef())) {
Index: 
lldb/packages/Python/lldbsuite/test/lang/cpp/modules-import/TestCXXModulesImport.py
===
--- 
lldb/packages/Python/lldbsuite/test/lang/cpp/modules-import/TestCXXModulesImport.py
+++ 
lldb/packages/Python/lldbsuite/test/lang/cpp/modules-import/TestCXXModulesImport.py
@@ -2,13 +2,9 @@
 
 from __future__ import print_function
 
-
-from distutils.version import StrictVersion
 import unittest2
-import os
-import time
 import lldb
-import platform
+import shutil
 
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
@@ -19,13 +15,32 @@
 
 mydir = TestBase.compute_mydir(__file__)
 
+def build(self):
+include = self.getBuildArtifact('include')
+lldbutil.mkdir_p(include)
+for f in ['Foo.h', 'Bar.h', 'module.modulemap']:
+shutil.copyfile(self.getSourcePath(os.path.join('Inputs', f)),
+os.path.join(include, f))
+super(CXXModulesImportTestCase, self).build()
+
 @skipUnlessDarwin
 @skipIf(macos_version=["<", "10.12"])
 def test_expr(self):
 self.build()
-exe = self.getBuildArtifact("a.out")
 target, process, thread, bkpt = lldbutil.run_to_source_breakpoint(
 self, 'break here', lldb.SBFileSpec('main.cpp'))
 
 self.expect("expr -l Objective-C++ -- @import Bar")
 self.expect("expr -- Bar()", substrs = ["success"])
+self.expect("expr -l Objective-C++ -- @import 
THIS_MODULE_DOES_NOT_EXIST",
+error=True)
+
+@skipUnlessDarwin
+@skipIf(macos_version=["<", "10.12"])
+def test_expr_failing_import(self):
+self.build()
+shutil.rmtree(self.getBuildArtifact('include'))
+target, process, thread, bkpt = lldbutil.run_to_source_breakpoint(
+self, 'break here', lldb.SBFileSpec('main.cpp'))
+
+self.expect("expr -l Objective-C++ -- @import Bar", error=True)
Index: lldb/packages/Python/lldbsuite/test/lang/cpp/modules-import/Makefile
===
--- lldb/packages/Python/lldbsuite/test/lang/cpp/modules-import/Makefile
+++ lldb/packages/Python/lldbsuite/test/lang/cpp/modules-import/Makefile
@@ -1,6 +1,5 @@
 LEVEL = ../../../make
 CXX_SOURCES := main.cpp
-
-CFLAGS += $(MANDATORY_MODULE_BUILD_CFLAGS)
+CFLAGS_EXTRAS = $(MANDATORY_MODULE_BUILD_CFLAGS) -I$(BUILDDIR)/include
 
 include $(LEVEL)/Makefile.rules


Index: lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
===
--- lldb/source/Plugi

[Lldb-commits] [lldb] r356416 - A target definition file that may work for

2019-03-18 Thread Jason Molenda via lldb-commits

Author: jmolenda
Date: Mon Mar 18 14:39:54 2019
New Revision: 356416

URL: http://llvm.org/viewvc/llvm-project?rev=356416&view=rev
Log:
A target definition file that may work for 
Aarch32 Cortex-M target processor debugging.

 

Added:
lldb/trunk/examples/python/armv7_cortex_m_target_defintion.py   (with props)

Added: lldb/trunk/examples/python/armv7_cortex_m_target_defintion.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/examples/python/armv7_cortex_m_target_defintion.py?rev=356416&view=auto
==
--- lldb/trunk/examples/python/armv7_cortex_m_target_defintion.py (added)
+++ lldb/trunk/examples/python/armv7_cortex_m_target_defintion.py Mon Mar 18 
14:39:54 2019
@@ -0,0 +1,140 @@
+#!/usr/bin/python
+#===-- armv7_cortex_m_target_definition.py.py --*- C++ -*-===//
+#
+# The LLVM Compiler Infrastructure
+#
+# Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+# See https://llvm.org/LICENSE.txt for license information.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+#
+#===--===//
+
+#--
+# DESCRIPTION
+#
+# This file can be used with the following setting:
+#   plugin.process.gdb-remote.target-definition-file
+# This setting should be used when you are trying to connect to a 
+# remote GDB server that doesn't support any of the register discovery
+# packets that LLDB normally uses. 
+#
+# Why is this necessary? LLDB doesn't require a new build of LLDB that
+# targets each new architecture you will debug with. Instead, all
+# architectures are supported and LLDB relies on extra GDB server 
+# packets to discover the target we are connecting to so that is can
+# show the right registers for each target. This allows the GDB server
+# to change and add new registers without requiring a new LLDB build
+# just so we can see new registers.
+#
+# This file implements the x86_64 registers for the darwin version of
+# GDB and allows you to connect to servers that use this register set. 
+# 
+# USAGE
+#
+# (lldb) settings set plugin.process.gdb-remote.target-definition-file 
/path/to/armv7_cortex_m_target_defintion.py
+# (lldb) gdb-remote other.baz.com:1234
+#
+# The target definition file will get used if and only if the 
+# qRegisterInfo packets are not supported when connecting to a remote
+# GDB server.
+#--
+
+from lldb import *
+
+# DWARF register numbers
+name_to_dwarf_regnum = {
+'r0'   : 0 , 
+'r1'   : 1 ,
+'r2'   : 2 ,
+'r3'   : 3 ,
+'r4'   : 4 ,
+'r5'   : 5 ,
+'r6'   : 6 ,
+'r7'   : 7 ,
+'r9'   : 8 ,
+'r10'  : 9 ,
+'r11'  : 10,
+'r12'  : 11,
+'sp'   : 12,
+'lr'   : 13,
+'pc'   : 14,
+'r15'  : 15,
+'xpsr' : 16,
+};
+
+name_to_generic_regnum = {
+'pc' : LLDB_REGNUM_GENERIC_PC,
+'sp' : LLDB_REGNUM_GENERIC_SP,
+'r7' : LLDB_REGNUM_GENERIC_FP,
+'lr' : LLDB_REGNUM_GENERIC_RA,
+'r0' : LLDB_REGNUM_GENERIC_ARG1,
+'r1' : LLDB_REGNUM_GENERIC_ARG2,
+'r2' : LLDB_REGNUM_GENERIC_ARG3,
+'r3' : LLDB_REGNUM_GENERIC_ARG4
+};
+
+
+def get_reg_num (reg_num_dict, reg_name):
+if reg_name in reg_num_dict:
+return reg_num_dict[reg_name]
+return LLDB_INVALID_REGNUM
+
+def get_reg_num (reg_num_dict, reg_name):
+if reg_name in reg_num_dict:
+return reg_num_dict[reg_name]
+return LLDB_INVALID_REGNUM
+
+armv7_register_infos = [
+{ 'name':'r0'   , 'set':0, 'bitsize':32 , 'encoding':eEncodingUint  , 
'format':eFormatAddressInfo, 'alt-name':'arg1' },
+{ 'name':'r1'   , 'set':0, 'bitsize':32 , 'encoding':eEncodingUint  , 
'format':eFormatAddressInfo, 'alt-name':'arg2' },
+{ 'name':'r2'   , 'set':0, 'bitsize':32 , 'encoding':eEncodingUint  , 
'format':eFormatAddressInfo, 'alt-name':'arg3' },
+{ 'name':'r3'   , 'set':0, 'bitsize':32 , 'encoding':eEncodingUint  , 
'format':eFormatAddressInfo, 'alt-name':'arg4' },
+{ 'name':'r4'   , 'set':0, 'bitsize':32 , 'encoding':eEncodingUint  , 
'format':eFormatAddressInfo   },
+{ 'name':'r5'   , 'set':0, 'bitsize':32 , 'encoding':eEncodingUint  , 
'format':eFormatAddressInfo   },
+{ 'name':'r6'   , 'set':0, 'bitsize':32 , 'encoding':eEncodingUint  , 
'format':eFormatAddressInfo   },
+{ 'name':'r7'   , 'set':0, 'bitsize':32 , 'encoding':eEncodingUint  , 
'format':eFormatAddressInfo, 'alt-name':'fp'  },
+{ 'name':'r8'   , 'set':0, 'bitsize':32 , 'encoding':eEncodingUint  , 
'format':eFormatAddressInfo   },
+{ 'name':'r9'   , 'set':0, 'bitsize':32 , 'encoding':eEncodingUint  , 
'format':eFormatAddressInfo   },
+{ 'name':'r10'  , 'set':0, 'bitsize':32 , 'encoding':eEncodingUint  , 
'format':eFormatAddressInfo   },
+{ 'name':'r11'  , 'set':0, 'bitsize':32 , 'encoding':eEncodingUint  , 
'format':eFormatAddressInfo

[Lldb-commits] [lldb] r356412 - [CMake] Set LLVM_DEFAULT_EXTERNAL_LIT in standalone build correctly on windows

2019-03-18 Thread Alex Langford via lldb-commits

Author: xiaobai
Date: Mon Mar 18 14:32:31 2019
New Revision: 356412

URL: http://llvm.org/viewvc/llvm-project?rev=356412&view=rev
Log:
[CMake] Set LLVM_DEFAULT_EXTERNAL_LIT in standalone build correctly on windows

LLVM installed llvm-lit with a .py suffix on windows. Let's match that
behavior here.

Modified:
lldb/trunk/cmake/modules/LLDBStandalone.cmake

Modified: lldb/trunk/cmake/modules/LLDBStandalone.cmake
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/LLDBStandalone.cmake?rev=356412&r1=356411&r2=356412&view=diff
==
--- lldb/trunk/cmake/modules/LLDBStandalone.cmake (original)
+++ lldb/trunk/cmake/modules/LLDBStandalone.cmake Mon Mar 18 14:32:31 2019
@@ -23,7 +23,12 @@ if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURR
   set(LLVM_MAIN_INCLUDE_DIR ${LLVM_MAIN_INCLUDE_DIR} CACHE PATH "Path to 
llvm/include")
   set(LLVM_LIBRARY_DIR ${LLVM_LIBRARY_DIR} CACHE PATH "Path to llvm/lib")
   set(LLVM_BINARY_DIR ${LLVM_BINARY_DIR} CACHE PATH "Path to LLVM build tree")
-  set(LLVM_DEFAULT_EXTERNAL_LIT ${LLVM_TOOLS_BINARY_DIR}/llvm-lit CACHE PATH 
"Path to llvm-lit")
+
+  set(lit_file_name "llvm-lit")
+  if(CMAKE_HOST_WIN32 AND NOT CYGWIN)
+set(lit_file_name "${lit_file_name}.py")
+  endif()
+  set(LLVM_DEFAULT_EXTERNAL_LIT "${LLVM_TOOLS_BINARY_DIR}/${lit_file_name}" 
CACHE PATH "Path to llvm-lit")
 
   if(CMAKE_CROSSCOMPILING)
 set(LLVM_NATIVE_BUILD "${LLDB_PATH_TO_LLVM_BUILD}/NATIVE")


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

[Lldb-commits] [PATCH] D59427: [WIP] [lldb] [API] Split SBRegistry into smaller files

2019-03-18 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

This looks what I had in mind, thanks Michał!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59427/new/

https://reviews.llvm.org/D59427



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

[Lldb-commits] [PATCH] D59498: [DWARF] Remove a couple of log statements

2019-03-18 Thread Greg Clayton via Phabricator via lldb-commits

clayborg accepted this revision.
clayborg added a comment.

I haven't used it in a long time. I can add it back temporarily if I ever need 
to.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59498/new/

https://reviews.llvm.org/D59498



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

[Lldb-commits] [PATCH] D59276: Delete dead code

2019-03-18 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

Given that this is not tested (I don't see any tests being removed) this LGTM.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59276/new/

https://reviews.llvm.org/D59276



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

[Lldb-commits] [PATCH] D59498: [DWARF] Remove a couple of log statements

2019-03-18 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere accepted this revision.
JDevlieghere added a subscriber: davide.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

In D59498#1433421 , @zturner wrote:

> In all cases, I think the question worth asking is not "could it be used for 
> X", but rather "how often is it actually used for X".  Otherwise, it's just 
> technical debt IMO.  There's a lot of things that are possible in theory, but 
> unless it exhibits value in practice, YAGNI.

I strongly agree with this. Unless this is something we want to know from a 
user, we can probably get this information by setting a breakpoint.

> This is bad advertising. It's like a butcher that sells vegan meat.  (Quote 
> @davide)

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59498/new/

https://reviews.llvm.org/D59498

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

[Lldb-commits] [lldb] r356401 - [API] Remove unneded LLDB_DISABLE_PYTHON markers.

2019-03-18 Thread Davide Italiano via lldb-commits

Author: davide
Date: Mon Mar 18 13:02:27 2019
New Revision: 356401

URL: http://llvm.org/viewvc/llvm-project?rev=356401&view=rev
Log:
[API] Remove unneded LLDB_DISABLE_PYTHON markers.

Modified:
lldb/trunk/include/lldb/API/SBDebugger.h
lldb/trunk/include/lldb/API/SBTypeCategory.h
lldb/trunk/include/lldb/API/SBTypeSynthetic.h
lldb/trunk/include/lldb/API/SBValue.h
lldb/trunk/source/API/SBDebugger.cpp
lldb/trunk/source/API/SBFrame.cpp
lldb/trunk/source/API/SBReproducer.cpp
lldb/trunk/source/API/SBTarget.cpp
lldb/trunk/source/API/SBTypeCategory.cpp
lldb/trunk/source/API/SBTypeSynthetic.cpp
lldb/trunk/source/API/SBValue.cpp

Modified: lldb/trunk/include/lldb/API/SBDebugger.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBDebugger.h?rev=356401&r1=356400&r2=356401&view=diff
==
--- lldb/trunk/include/lldb/API/SBDebugger.h (original)
+++ lldb/trunk/include/lldb/API/SBDebugger.h Mon Mar 18 13:02:27 2019
@@ -255,15 +255,11 @@ public:
 
   SBTypeFormat GetFormatForType(SBTypeNameSpecifier);
 
-#ifndef LLDB_DISABLE_PYTHON
   SBTypeSummary GetSummaryForType(SBTypeNameSpecifier);
-#endif
 
   SBTypeFilter GetFilterForType(SBTypeNameSpecifier);
 
-#ifndef LLDB_DISABLE_PYTHON
   SBTypeSynthetic GetSyntheticForType(SBTypeNameSpecifier);
-#endif
 
   void RunCommandInterpreter(bool auto_handle_events, bool spawn_thread);
 

Modified: lldb/trunk/include/lldb/API/SBTypeCategory.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBTypeCategory.h?rev=356401&r1=356400&r2=356401&view=diff
==
--- lldb/trunk/include/lldb/API/SBTypeCategory.h (original)
+++ lldb/trunk/include/lldb/API/SBTypeCategory.h Mon Mar 18 13:02:27 2019
@@ -47,9 +47,7 @@ public:
 
   uint32_t GetNumFilters();
 
-#ifndef LLDB_DISABLE_PYTHON
   uint32_t GetNumSynthetics();
-#endif
 
   SBTypeNameSpecifier GetTypeNameSpecifierForFilterAtIndex(uint32_t);
 
@@ -57,43 +55,29 @@ public:
 
   SBTypeNameSpecifier GetTypeNameSpecifierForSummaryAtIndex(uint32_t);
 
-#ifndef LLDB_DISABLE_PYTHON
   SBTypeNameSpecifier GetTypeNameSpecifierForSyntheticAtIndex(uint32_t);
-#endif
 
   SBTypeFilter GetFilterForType(SBTypeNameSpecifier);
 
   SBTypeFormat GetFormatForType(SBTypeNameSpecifier);
 
-#ifndef LLDB_DISABLE_PYTHON
   SBTypeSummary GetSummaryForType(SBTypeNameSpecifier);
-#endif
 
-#ifndef LLDB_DISABLE_PYTHON
   SBTypeSynthetic GetSyntheticForType(SBTypeNameSpecifier);
-#endif
 
-#ifndef LLDB_DISABLE_PYTHON
   SBTypeFilter GetFilterAtIndex(uint32_t);
-#endif
 
   SBTypeFormat GetFormatAtIndex(uint32_t);
 
-#ifndef LLDB_DISABLE_PYTHON
   SBTypeSummary GetSummaryAtIndex(uint32_t);
-#endif
 
-#ifndef LLDB_DISABLE_PYTHON
   SBTypeSynthetic GetSyntheticAtIndex(uint32_t);
-#endif
 
   bool AddTypeFormat(SBTypeNameSpecifier, SBTypeFormat);
 
   bool DeleteTypeFormat(SBTypeNameSpecifier);
 
-#ifndef LLDB_DISABLE_PYTHON
   bool AddTypeSummary(SBTypeNameSpecifier, SBTypeSummary);
-#endif
 
   bool DeleteTypeSummary(SBTypeNameSpecifier);
 
@@ -101,11 +85,9 @@ public:
 
   bool DeleteTypeFilter(SBTypeNameSpecifier);
 
-#ifndef LLDB_DISABLE_PYTHON
   bool AddTypeSynthetic(SBTypeNameSpecifier, SBTypeSynthetic);
 
   bool DeleteTypeSynthetic(SBTypeNameSpecifier);
-#endif
 
   lldb::SBTypeCategory &operator=(const lldb::SBTypeCategory &rhs);
 

Modified: lldb/trunk/include/lldb/API/SBTypeSynthetic.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBTypeSynthetic.h?rev=356401&r1=356400&r2=356401&view=diff
==
--- lldb/trunk/include/lldb/API/SBTypeSynthetic.h (original)
+++ lldb/trunk/include/lldb/API/SBTypeSynthetic.h Mon Mar 18 13:02:27 2019
@@ -12,8 +12,6 @@
 
 #include "lldb/API/SBDefines.h"
 
-#ifndef LLDB_DISABLE_PYTHON
-
 namespace lldb {
 
 class LLDB_API SBTypeSynthetic {
@@ -79,6 +77,4 @@ protected:
 
 } // namespace lldb
 
-#endif // LLDB_DISABLE_PYTHON
-
 #endif // LLDB_SBTypeSynthetic_h_

Modified: lldb/trunk/include/lldb/API/SBValue.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBValue.h?rev=356401&r1=356400&r2=356401&view=diff
==
--- lldb/trunk/include/lldb/API/SBValue.h (original)
+++ lldb/trunk/include/lldb/API/SBValue.h Mon Mar 18 13:02:27 2019
@@ -112,9 +112,7 @@ public:
 
   lldb::SBTypeFilter GetTypeFilter();
 
-#ifndef LLDB_DISABLE_PYTHON
   lldb::SBTypeSynthetic GetTypeSynthetic();
-#endif
 
   lldb::SBValue GetChildAtIndex(uint32_t idx);
 

Modified: lldb/trunk/source/API/SBDebugger.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SBDebugger.cpp?rev=356401&r1=356400&r2=356401&view=diff
==
--- lldb/trunk/source/API/SBDebugger.cpp (original)

[Lldb-commits] [PATCH] D59498: [DWARF] Remove a couple of log statements

2019-03-18 Thread Zachary Turner via Phabricator via lldb-commits

zturner marked 4 inline comments as done.
zturner added a comment.

In all cases, I think the question worth asking is not "could it be used for 
X", but rather "how often is it actually used for X".  Otherwise, it's just 
technical debt IMO.  There's a lot of things that are possible in theory, but 
unless it exhibits value in practice, YAGNI.




Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.cpp:121-144
-  Log *log(LogChannelDWARF::GetLogIfAll(DWARF_LOG_DEBUG_ARANGES));
-  size_t orig_arange_size = 0;
-  if (log) {
-orig_arange_size = m_aranges.GetSize();
-log->Printf("DWARFDebugAranges::Sort(minimize = %u) with %" PRIu64
-" entries",
-minimize, (uint64_t)orig_arange_size);

clayborg wrote:
> These two seem useful. They are only when logging .debug_aranges with 
> DWARF_LOG_DEBUG_ARANGES. What is left if we remove this?
What useful information does it tell you though?  That the function was called? 
 That's the part that doesn't seem useful.  It gives you some information about 
bytes saved and entries combined, which could arguably be useful, but are you 
aware of any situations where anyone actually looked at this log statement?  
One option is moving it up to a higher level if so, but I think it's worth 
seriously asking why you would want to know this, and if you did want to know 
it, why running LLDB in a debugger and/or adding some temporary logging to 
diagnose a specific problem wouldn't be sufficient.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp:93
-}
   }
 

clayborg wrote:
> All above logs seem useful. Sometimes we don't get a .debug_aranges section 
> which means we must parse the address ranges manually. When we are logging 
> .debug_aranges, it is handy to know where the info came from (.debug_aranges, 
> or manual index).
In that case, we can move the log up to a much higher level, and print one log 
all the way up in `SymbolFileDWARF` that says "aranges not present, using range 
info from manual index".  It doesn't need to be in the low level parsing code



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:168-176
-  Log *log(
-  LogChannelDWARF::GetLogIfAny(DWARF_LOG_DEBUG_INFO | DWARF_LOG_LOOKUPS));
-  if (log) {
-m_dwarf->GetObjectFile()->GetModule()->LogMessageVerboseBacktrace(
-log,
-"DWARFUnit::ExtractDIEsIfNeeded () for compile unit at "
-".debug_info[0x%8.8x]",

clayborg wrote:
> It is nice to know when we parse all DIEs for a compile unit. This helps us 
> to verify that our lazy DWARF parsing is working. When we have good 
> accelerator tables, like the Apple ones or DWARF 5 ones, then we will only 
> parse the DIEs in a compile unit when we need to. Seems like we need one when 
> it starts and one when it ends so we could see timestamp times...
This function already has this at the beginning:

```
static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
  static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
  Timer scoped_timer(func_cat, "%8.8x: DWARFUnit::ExtractDIEsIfNeeded()", 
m_offset);
```

if timing information is needed.  There are also other options, like running a 
profiler or function tracer.  For verifying that the lazy parsing is working, 
we should just have tests, not log statements that require someone to manually 
inspect.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:837-844
-Log *log(LogChannelDWARF::GetLogIfAll(DWARF_LOG_DEBUG_ARANGES));
-
-if (log) {
-  m_dwarf->GetObjectFile()->GetModule()->LogMessage(
-  log,
-  "DWARFUnit::GetFunctionAranges() for compile unit at "
-  ".debug_info[0x%8.8x]",

clayborg wrote:
> This seems useful as it tells us when we manually index a compile unit for 
> all function address ranges which can be expensive.
This information could be stored at a higher level, it doesn't have to be here.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59498/new/

https://reviews.llvm.org/D59498



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

[Lldb-commits] [PATCH] D59427: [WIP] [lldb] [API] Split SBRegistry into smaller files

2019-03-18 Thread Michał Górny via Phabricator via lldb-commits

mgorny updated this revision to Diff 191125.
mgorny added a comment.

Hmm, actually with that local `R` declaration I don't have to have an 
additional set of macros ;-).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59427/new/

https://reviews.llvm.org/D59427

Files:
  lldb/include/lldb/Utility/ReproducerInstrumentation.h
  lldb/source/API/SBAddress.cpp
  lldb/source/API/SBReproducer.cpp
  lldb/source/API/SBReproducerPrivate.h

Index: lldb/source/API/SBReproducerPrivate.h
===
--- lldb/source/API/SBReproducerPrivate.h
+++ lldb/source/API/SBReproducerPrivate.h
@@ -69,6 +69,8 @@
   return {};
 }
 
+template  void RegisterMethods(Registry &R);
+
 } // namespace repro
 } // namespace lldb_private
 
Index: lldb/source/API/SBReproducer.cpp
===
--- lldb/source/API/SBReproducer.cpp
+++ lldb/source/API/SBReproducer.cpp
@@ -42,6 +42,7 @@
 }
 
 SBRegistry::SBRegistry() {
+  Registry& R = *this;
 
   // Custom implementation.
   Register(&invoke(R);
   {
 LLDB_REGISTER_CONSTRUCTOR(SBAttachInfo, ());
 LLDB_REGISTER_CONSTRUCTOR(SBAttachInfo, (lldb::pid_t));
Index: lldb/source/API/SBAddress.cpp
===
--- lldb/source/API/SBAddress.cpp
+++ lldb/source/API/SBAddress.cpp
@@ -282,3 +282,41 @@
   }
   return LLDB_RECORD_RESULT(sb_line_entry);
 }
+
+namespace lldb_private {
+namespace repro {
+
+template <>
+void RegisterMethods(Registry &R) {
+  LLDB_REGISTER_CONSTRUCTOR(SBAddress, ());
+  LLDB_REGISTER_CONSTRUCTOR(SBAddress, (const lldb::SBAddress &));
+  LLDB_REGISTER_CONSTRUCTOR(SBAddress, (lldb::SBSection, lldb::addr_t));
+  LLDB_REGISTER_CONSTRUCTOR(SBAddress, (lldb::addr_t, lldb::SBTarget &));
+  LLDB_REGISTER_METHOD(const lldb::SBAddress &,
+   SBAddress, operator=,(const lldb::SBAddress &));
+  LLDB_REGISTER_METHOD_CONST(bool, SBAddress, IsValid, ());
+  LLDB_REGISTER_METHOD_CONST(bool, SBAddress, operator bool, ());
+  LLDB_REGISTER_METHOD(void, SBAddress, Clear, ());
+  LLDB_REGISTER_METHOD(void, SBAddress, SetAddress,
+   (lldb::SBSection, lldb::addr_t));
+  LLDB_REGISTER_METHOD_CONST(lldb::addr_t, SBAddress, GetFileAddress, ());
+  LLDB_REGISTER_METHOD_CONST(lldb::addr_t, SBAddress, GetLoadAddress,
+ (const lldb::SBTarget &));
+  LLDB_REGISTER_METHOD(void, SBAddress, SetLoadAddress,
+   (lldb::addr_t, lldb::SBTarget &));
+  LLDB_REGISTER_METHOD(bool, SBAddress, OffsetAddress, (lldb::addr_t));
+  LLDB_REGISTER_METHOD(lldb::SBSection, SBAddress, GetSection, ());
+  LLDB_REGISTER_METHOD(lldb::addr_t, SBAddress, GetOffset, ());
+  LLDB_REGISTER_METHOD(bool, SBAddress, GetDescription, (lldb::SBStream &));
+  LLDB_REGISTER_METHOD(lldb::SBModule, SBAddress, GetModule, ());
+  LLDB_REGISTER_METHOD(lldb::SBSymbolContext, SBAddress, GetSymbolContext,
+   (uint3_t));
+  LLDB_REGISTER_METHOD(lldb::SBCompileUnit, SBAddress, GetCompileUnit, ());
+  LLDB_REGISTER_METHOD(lldb::SBFunction, SBAddress, GetFunction, ());
+  LLDB_REGISTER_METHOD(lldb::SBBlock, SBAddress, GetBlock, ());
+  LLDB_REGISTER_METHOD(lldb::SBSymbol, SBAddress, GetSymbol, ());
+  LLDB_REGISTER_METHOD(lldb::SBLineEntry, SBAddress, GetLineEntry, ());
+}
+
+}
+}
Index: lldb/include/lldb/Utility/ReproducerInstrumentation.h
===
--- lldb/include/lldb/Utility/ReproducerInstrumentation.h
+++ lldb/include/lldb/Utility/ReproducerInstrumentation.h
@@ -68,18 +68,18 @@
 // #define LLDB_REPRO_INSTR_TRACE
 
 #define LLDB_REGISTER_CONSTRUCTOR(Class, Signature)\
-  Register(&construct::doit, "", #Class,   \
+  R.Register(&construct::doit, "", #Class, \
   #Class, #Signature)
 #define LLDB_REGISTER_METHOD(Result, Class, Method, Signature) \
-  Register(\
+  R.Register(  \
   &invoke::method<(&Class::Method)>::doit, \
   #Result, #Class, #Method, #Signature)
 #define LLDB_REGISTER_METHOD_CONST(Result, Class, Method, Signature)   \
-  Register(&invoke::method_const<(&Class::Method)>::doit, \
#Result, #Class, #Method, #Signature)
 #define LLDB_REGISTER_STATIC_METHOD(Result, Class, Method, Signature)  \
-  Register(static_cast(&Class::Method), \
+  R.Register(static_cast(&Class::Method), \
  #Result, #Class, #Method, #Signature)
 
 #define LLDB_RECORD_CONSTRUCTOR(Class, Signature, ...) \
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D59427: [WIP] [lldb] [API] Split SBRegistry into smaller files

2019-03-18 Thread Michał Górny via Phabricator via lldb-commits

mgorny updated this revision to Diff 191124.
mgorny added a comment.

Does this look right? I've added temporary `*2` versions of the macros to make 
it build without having to port everything. If it looks fine, I'll go with 
other files.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59427/new/

https://reviews.llvm.org/D59427

Files:
  lldb/include/lldb/Utility/ReproducerInstrumentation.h
  lldb/source/API/SBAddress.cpp
  lldb/source/API/SBReproducer.cpp
  lldb/source/API/SBReproducerPrivate.h

Index: lldb/source/API/SBReproducerPrivate.h
===
--- lldb/source/API/SBReproducerPrivate.h
+++ lldb/source/API/SBReproducerPrivate.h
@@ -69,6 +69,8 @@
   return {};
 }
 
+template  void RegisterMethods(Registry &R);
+
 } // namespace repro
 } // namespace lldb_private
 
Index: lldb/source/API/SBReproducer.cpp
===
--- lldb/source/API/SBReproducer.cpp
+++ lldb/source/API/SBReproducer.cpp
@@ -42,6 +42,7 @@
 }
 
 SBRegistry::SBRegistry() {
+  Registry& R = *this;
 
   // Custom implementation.
   Register(&invoke(R);
   {
 LLDB_REGISTER_CONSTRUCTOR(SBAttachInfo, ());
 LLDB_REGISTER_CONSTRUCTOR(SBAttachInfo, (lldb::pid_t));
Index: lldb/source/API/SBAddress.cpp
===
--- lldb/source/API/SBAddress.cpp
+++ lldb/source/API/SBAddress.cpp
@@ -282,3 +282,41 @@
   }
   return LLDB_RECORD_RESULT(sb_line_entry);
 }
+
+namespace lldb_private {
+namespace repro {
+
+template <>
+void RegisterMethods(Registry &R) {
+  LLDB_REGISTER_CONSTRUCTOR2(SBAddress, ());
+  LLDB_REGISTER_CONSTRUCTOR2(SBAddress, (const lldb::SBAddress &));
+  LLDB_REGISTER_CONSTRUCTOR2(SBAddress, (lldb::SBSection, lldb::addr_t));
+  LLDB_REGISTER_CONSTRUCTOR2(SBAddress, (lldb::addr_t, lldb::SBTarget &));
+  LLDB_REGISTER_METHOD2(const lldb::SBAddress &,
+   SBAddress, operator=,(const lldb::SBAddress &));
+  LLDB_REGISTER_METHOD_CONST2(bool, SBAddress, IsValid, ());
+  LLDB_REGISTER_METHOD_CONST2(bool, SBAddress, operator bool, ());
+  LLDB_REGISTER_METHOD2(void, SBAddress, Clear, ());
+  LLDB_REGISTER_METHOD2(void, SBAddress, SetAddress,
+   (lldb::SBSection, lldb::addr_t));
+  LLDB_REGISTER_METHOD_CONST2(lldb::addr_t, SBAddress, GetFileAddress, ());
+  LLDB_REGISTER_METHOD_CONST2(lldb::addr_t, SBAddress, GetLoadAddress,
+ (const lldb::SBTarget &));
+  LLDB_REGISTER_METHOD2(void, SBAddress, SetLoadAddress,
+   (lldb::addr_t, lldb::SBTarget &));
+  LLDB_REGISTER_METHOD2(bool, SBAddress, OffsetAddress, (lldb::addr_t));
+  LLDB_REGISTER_METHOD2(lldb::SBSection, SBAddress, GetSection, ());
+  LLDB_REGISTER_METHOD2(lldb::addr_t, SBAddress, GetOffset, ());
+  LLDB_REGISTER_METHOD2(bool, SBAddress, GetDescription, (lldb::SBStream &));
+  LLDB_REGISTER_METHOD2(lldb::SBModule, SBAddress, GetModule, ());
+  LLDB_REGISTER_METHOD2(lldb::SBSymbolContext, SBAddress, GetSymbolContext,
+   (uint32_t));
+  LLDB_REGISTER_METHOD2(lldb::SBCompileUnit, SBAddress, GetCompileUnit, ());
+  LLDB_REGISTER_METHOD2(lldb::SBFunction, SBAddress, GetFunction, ());
+  LLDB_REGISTER_METHOD2(lldb::SBBlock, SBAddress, GetBlock, ());
+  LLDB_REGISTER_METHOD2(lldb::SBSymbol, SBAddress, GetSymbol, ());
+  LLDB_REGISTER_METHOD2(lldb::SBLineEntry, SBAddress, GetLineEntry, ());
+}
+
+}
+}
Index: lldb/include/lldb/Utility/ReproducerInstrumentation.h
===
--- lldb/include/lldb/Utility/ReproducerInstrumentation.h
+++ lldb/include/lldb/Utility/ReproducerInstrumentation.h
@@ -82,6 +82,22 @@
   Register(static_cast(&Class::Method), \
  #Result, #Class, #Method, #Signature)
 
+// temporary transition macros
+#define LLDB_REGISTER_CONSTRUCTOR2(Class, Signature)\
+  R.Register(&construct::doit, "", #Class, \
+  #Class, #Signature)
+#define LLDB_REGISTER_METHOD2(Result, Class, Method, Signature) \
+  R.Register(  \
+  &invoke::method<(&Class::Method)>::doit, \
+  #Result, #Class, #Method, #Signature)
+#define LLDB_REGISTER_METHOD_CONST2(Result, Class, Method, Signature)   \
+  R.Register(&invoke::method_const<(&Class::Method)>::doit, \
+   #Result, #Class, #Method, #Signature)
+#define LLDB_REGISTER_STATIC_METHOD2(Result, Class, Method, Signature)  \
+  R.Register(static_cast(&Class::Method), \
+ #Result, #Class, #Method, #Signature)
+
 #define LLDB_RECORD_CONSTRUCTOR(Class, Signature, ...) \
   LLDB_LOG(GetLogIfAllCategoriesSet(LIBLLDB_LOG_API), "{0} ({1})", \
LLVM_PRETTY_FUNCTION, log_args(__VA_ARGS__));   \
__

[Lldb-commits] [PATCH] D59498: [DWARF] Remove a couple of log statements

2019-03-18 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

Most of these logs seem useful. See inline comments.




Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.cpp:121-144
-  Log *log(LogChannelDWARF::GetLogIfAll(DWARF_LOG_DEBUG_ARANGES));
-  size_t orig_arange_size = 0;
-  if (log) {
-orig_arange_size = m_aranges.GetSize();
-log->Printf("DWARFDebugAranges::Sort(minimize = %u) with %" PRIu64
-" entries",
-minimize, (uint64_t)orig_arange_size);

These two seem useful. They are only when logging .debug_aranges with 
DWARF_LOG_DEBUG_ARANGES. What is left if we remove this?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp:93
-}
   }
 

All above logs seem useful. Sometimes we don't get a .debug_aranges section 
which means we must parse the address ranges manually. When we are logging 
.debug_aranges, it is handy to know where the info came from (.debug_aranges, 
or manual index).



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:168-176
-  Log *log(
-  LogChannelDWARF::GetLogIfAny(DWARF_LOG_DEBUG_INFO | DWARF_LOG_LOOKUPS));
-  if (log) {
-m_dwarf->GetObjectFile()->GetModule()->LogMessageVerboseBacktrace(
-log,
-"DWARFUnit::ExtractDIEsIfNeeded () for compile unit at "
-".debug_info[0x%8.8x]",

It is nice to know when we parse all DIEs for a compile unit. This helps us to 
verify that our lazy DWARF parsing is working. When we have good accelerator 
tables, like the Apple ones or DWARF 5 ones, then we will only parse the DIEs 
in a compile unit when we need to. Seems like we need one when it starts and 
one when it ends so we could see timestamp times...



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:837-844
-Log *log(LogChannelDWARF::GetLogIfAll(DWARF_LOG_DEBUG_ARANGES));
-
-if (log) {
-  m_dwarf->GetObjectFile()->GetModule()->LogMessage(
-  log,
-  "DWARFUnit::GetFunctionAranges() for compile unit at "
-  ".debug_info[0x%8.8x]",

This seems useful as it tells us when we manually index a compile unit for all 
function address ranges which can be expensive.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59498/new/

https://reviews.llvm.org/D59498



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

[Lldb-commits] [PATCH] D59498: [DWARF] Remove a couple of log statements

2019-03-18 Thread Zachary Turner via Phabricator via lldb-commits

zturner created this revision.
zturner added reviewers: JDevlieghere, aprantl.
Herald added a subscriber: jdoerfert.

I don't find these specific log statements particularly high value, as they 
appear more just like simple function tracing calls (i.e. they log low level 
implementation details, not high level program flow) so I'm removing them from 
the low level DWARF parsing libraries.

I'm prepared to be convinced that we actually should do something other than 
outright delete them, but I did look at each one specifically as well as the 
surrounding context before deciding that they were probably ok to delete.

My reasoning for deleting these is that they are all on the success codepath 
and have a very limited number of call-sites (in most cases actually just 1), 
so in most cases a previous log statement would necessarily be followed by the 
deleted log statement, so therefore it doesn't actually add much (if any) 
diagnostic value.


https://reviews.llvm.org/D59498

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp

Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -165,15 +165,6 @@
 
   DWARFDebugInfoEntry die;
   // Keep a flat array of the DIE for binary lookup by DIE offset
-  Log *log(
-  LogChannelDWARF::GetLogIfAny(DWARF_LOG_DEBUG_INFO | DWARF_LOG_LOOKUPS));
-  if (log) {
-m_dwarf->GetObjectFile()->GetModule()->LogMessageVerboseBacktrace(
-log,
-"DWARFUnit::ExtractDIEsIfNeeded () for compile unit at "
-".debug_info[0x%8.8x]",
-GetOffset());
-  }
 
   uint32_t depth = 0;
   // We are in our compile unit, parse starting at the offset we were told to
@@ -834,15 +825,6 @@
 const DWARFDebugAranges &DWARFUnit::GetFunctionAranges() {
   if (m_func_aranges_up == NULL) {
 m_func_aranges_up.reset(new DWARFDebugAranges());
-Log *log(LogChannelDWARF::GetLogIfAll(DWARF_LOG_DEBUG_ARANGES));
-
-if (log) {
-  m_dwarf->GetObjectFile()->GetModule()->LogMessage(
-  log,
-  "DWARFUnit::GetFunctionAranges() for compile unit at "
-  ".debug_info[0x%8.8x]",
-  GetOffset());
-}
 const DWARFDebugInfoEntry *die = DIEPtr();
 if (die)
   die->BuildFunctionAddressRangeTable(m_dwarf, this,
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
@@ -22,7 +22,6 @@
 #include "DWARFDebugInfo.h"
 #include "DWARFDebugInfoEntry.h"
 #include "DWARFFormValue.h"
-#include "LogChannelDWARF.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -48,17 +47,10 @@
 
   assert(m_dwarf2Data);
 
-  Log *log(LogChannelDWARF::GetLogIfAll(DWARF_LOG_DEBUG_ARANGES));
-
   m_cu_aranges_up = llvm::make_unique();
   const DWARFDataExtractor &debug_aranges_data =
   m_dwarf2Data->get_debug_aranges_data();
   if (debug_aranges_data.GetByteSize() > 0) {
-if (log)
-  log->Printf(
-  "DWARFDebugInfo::GetCompileUnitAranges() for \"%s\" from "
-  ".debug_aranges",
-  m_dwarf2Data->GetObjectFile()->GetFileSpec().GetPath().c_str());
 llvm::Error error = m_cu_aranges_up->extract(debug_aranges_data);
 if (error)
   return std::move(error);
@@ -74,22 +66,13 @@
 
   // Manually build arange data for everything that wasn't in the
   // .debug_aranges table.
-  bool printed = false;
   const size_t num_compile_units = GetNumCompileUnits();
   for (size_t idx = 0; idx < num_compile_units; ++idx) {
 DWARFUnit *cu = GetCompileUnitAtIndex(idx);
 
 dw_offset_t offset = cu->GetOffset();
-if (cus_with_data.find(offset) == cus_with_data.end()) {
-  if (log) {
-if (!printed)
-  log->Printf(
-  "DWARFDebugInfo::GetCompileUnitAranges() for \"%s\" by parsing",
-  m_dwarf2Data->GetObjectFile()->GetFileSpec().GetPath().c_str());
-printed = true;
-  }
+if (cus_with_data.find(offset) == cus_with_data.end())
   cu->BuildAddressRangeTable(m_dwarf2Data, m_cu_aranges_up.get());
-}
   }
 
   const bool minimize = true;
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.cpp
@@ -13,13 +13,11 @@
 
 #include 
 
-#include "lldb/Utility/Log.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/Timer.h"
 
 #include "DWARFUnit.h"
 #include "DWARFDebugInfo.h"
-#include "LogChannelDWARF.h"
 #include "SymbolFileDWARF.h"
 
 using namespace lldb;
@@

[Lldb-commits] [PATCH] D59430: Update DwarfDebugInfoEntry to use llvm::Error and llvm::Expected

2019-03-18 Thread Zachary Turner via Phabricator via lldb-commits

zturner added a comment.

ping


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59430/new/

https://reviews.llvm.org/D59430



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

[Lldb-commits] [PATCH] D59276: Delete dead code

2019-03-18 Thread Zachary Turner via Phabricator via lldb-commits

zturner added a comment.

ping


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59276/new/

https://reviews.llvm.org/D59276



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

[Lldb-commits] [lldb] r356379 - Skip TestVSCode_setFunctionBreakpoints on linux

2019-03-18 Thread Pavel Labath via lldb-commits

Author: labath
Date: Mon Mar 18 09:04:53 2019
New Revision: 356379

URL: http://llvm.org/viewvc/llvm-project?rev=356379&view=rev
Log:
Skip TestVSCode_setFunctionBreakpoints on linux

Test hangs under heavy load.

Modified:

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/breakpoint/TestVSCode_setFunctionBreakpoints.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/breakpoint/TestVSCode_setFunctionBreakpoints.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/breakpoint/TestVSCode_setFunctionBreakpoints.py?rev=356379&r1=356378&r2=356379&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/breakpoint/TestVSCode_setFunctionBreakpoints.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/breakpoint/TestVSCode_setFunctionBreakpoints.py
 Mon Mar 18 09:04:53 2019
@@ -114,6 +114,7 @@ class TestVSCode_setFunctionBreakpoints(
 
 @skipIfWindows
 @skipIfDarwin # Skip this test for now until we can figure out why tings 
aren't working on build bots
+@skipIfLinux # test hangs on linux under heavy load
 @no_debug_info_test
 def test_functionality(self):
 '''Tests hitting breakpoints and the functionality of a single


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

[Lldb-commits] [lldb] r356378 - Fix some "variable 'foo' set but not used" warnings

2019-03-18 Thread Pavel Labath via lldb-commits

Author: labath
Date: Mon Mar 18 09:04:46 2019
New Revision: 356378

URL: http://llvm.org/viewvc/llvm-project?rev=356378&view=rev
Log:
Fix some "variable 'foo' set but not used" warnings

gcc-8 diagnoses these.

Modified:
lldb/trunk/source/API/SBFrame.cpp
lldb/trunk/source/API/SBTarget.cpp

Modified: lldb/trunk/source/API/SBFrame.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SBFrame.cpp?rev=356378&r1=356377&r2=356378&view=diff
==
--- lldb/trunk/source/API/SBFrame.cpp (original)
+++ lldb/trunk/source/API/SBFrame.cpp Mon Mar 18 09:04:46 2019
@@ -1084,7 +1084,6 @@ lldb::SBValue SBFrame::EvaluateExpressio
   Log *expr_log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
 #endif
 
-  ExpressionResults exe_results = eExpressionSetupError;
   SBValue expr_result;
 
   if (expr == nullptr || expr[0] == '\0') {
@@ -1117,8 +1116,7 @@ lldb::SBValue SBFrame::EvaluateExpressio
   frame_description.GetData());
 }
 
-exe_results = target->EvaluateExpression(expr, frame, expr_value_sp,
- options.ref());
+target->EvaluateExpression(expr, frame, expr_value_sp, options.ref());
 expr_result.SetSP(expr_value_sp, options.GetFetchDynamicValue());
   }
 }

Modified: lldb/trunk/source/API/SBTarget.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SBTarget.cpp?rev=356378&r1=356377&r2=356378&view=diff
==
--- lldb/trunk/source/API/SBTarget.cpp (original)
+++ lldb/trunk/source/API/SBTarget.cpp Mon Mar 18 09:04:46 2019
@@ -2356,7 +2356,6 @@ lldb::SBValue SBTarget::EvaluateExpressi
   Log *expr_log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
 #endif
   SBValue expr_result;
-  ExpressionResults exe_results = eExpressionSetupError;
   ValueObjectSP expr_value_sp;
   TargetSP target_sp(GetSP());
   StackFrame *frame = NULL;
@@ -2383,8 +2382,7 @@ lldb::SBValue SBTarget::EvaluateExpressi
   expr, options.GetFetchDynamicValue(),
   frame_description.GetString().str().c_str());
 #endif
-  exe_results =
-  target->EvaluateExpression(expr, frame, expr_value_sp, 
options.ref());
+  target->EvaluateExpression(expr, frame, expr_value_sp, options.ref());
 
   expr_result.SetSP(expr_value_sp, options.GetFetchDynamicValue());
 }


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

[Lldb-commits] [PATCH] D59495: Fix an out-of-bounds error in RegisterContextDarwin_arm64

2019-03-18 Thread Pavel Labath via Phabricator via lldb-commits

labath created this revision.
labath added a reviewer: jasonmolenda.
Herald added subscribers: kristof.beyls, javed.absar.

gcc diagnoses this as "array subscript 63 is above array bounds of
'RegisterContextDarwin_arm64::VReg [32]'".

The correct fix seems to be subtracting the fpu register base index, but
I have no way of verifying that this actually works.


https://reviews.llvm.org/D59495

Files:
  source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp


Index: source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp
===
--- source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp
+++ source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp
@@ -429,7 +429,7 @@
   case fpu_v29:
   case fpu_v30:
   case fpu_v31:
-value.SetBytes(fpu.v[reg].bytes.buffer, reg_info->byte_size,
+value.SetBytes(fpu.v[reg - fpu_v0].bytes.buffer, reg_info->byte_size,
endian::InlHostByteOrder());
 break;
 


Index: source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp
===
--- source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp
+++ source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp
@@ -429,7 +429,7 @@
   case fpu_v29:
   case fpu_v30:
   case fpu_v31:
-value.SetBytes(fpu.v[reg].bytes.buffer, reg_info->byte_size,
+value.SetBytes(fpu.v[reg - fpu_v0].bytes.buffer, reg_info->byte_size,
endian::InlHostByteOrder());
 break;
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r356377 - Fix libstdc++ data formatters for python3

2019-03-18 Thread Pavel Labath via lldb-commits

Author: labath
Date: Mon Mar 18 08:42:08 2019
New Revision: 356377

URL: http://llvm.org/viewvc/llvm-project?rev=356377&view=rev
Log:
Fix libstdc++ data formatters for python3

Use floor-division for consistentcy across python versions. This fixes a
couple of libstdc++ data formatter tests.

Modified:
lldb/trunk/examples/synthetic/gnu_libstdcpp.py

Modified: lldb/trunk/examples/synthetic/gnu_libstdcpp.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/examples/synthetic/gnu_libstdcpp.py?rev=356377&r1=356376&r2=356377&view=diff
==
--- lldb/trunk/examples/synthetic/gnu_libstdcpp.py (original)
+++ lldb/trunk/examples/synthetic/gnu_libstdcpp.py Mon Mar 18 08:42:08 2019
@@ -1,3 +1,4 @@
+from __future__ import division
 import re
 import lldb.formatters.Logger
 
@@ -195,7 +196,7 @@ class StdVectorSynthProvider:
 if (num_children % self.data_size) != 0:
 return 0
 else:
-num_children = num_children / self.data_size
+num_children = num_children // self.data_size
 return num_children
 except:
 return 0
@@ -257,7 +258,7 @@ class StdVectorSynthProvider:
 return None
 element_type = self.start_p.GetType().GetPointeeType()
 element_bits = 8 * element_type.GetByteSize()
-element_offset = (index / element_bits) * \
+element_offset = (index // element_bits) * \
 element_type.GetByteSize()
 bit_offset = index % element_bits
 element = self.start_p.CreateChildAtOffset(


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

[Lldb-commits] [lldb] r356370 - Fix TestCommandScriptImmediateOutput for python3

2019-03-18 Thread Pavel Labath via lldb-commits

Author: labath
Date: Mon Mar 18 07:13:12 2019
New Revision: 356370

URL: http://llvm.org/viewvc/llvm-project?rev=356370&view=rev
Log:
Fix TestCommandScriptImmediateOutput for python3

s/iteritems/items

Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/command_script_immediate_output/TestCommandScriptImmediateOutput.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/command_script_immediate_output/TestCommandScriptImmediateOutput.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/command_script_immediate_output/TestCommandScriptImmediateOutput.py?rev=356370&r1=356369&r2=356370&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/command_script_immediate_output/TestCommandScriptImmediateOutput.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/command_script_immediate_output/TestCommandScriptImmediateOutput.py
 Mon Mar 18 07:13:12 2019
@@ -66,7 +66,7 @@ class CommandScriptImmediateOutputTestCa
 starter_string = 'Starter Garbage\n'
 write_string = 'writing to file with mode: '
 
-for path, mode in test_files.iteritems():
+for path, mode in test_files.items():
 with open(path, 'w+') as init:
 init.write(starter_string)
 
@@ -78,7 +78,7 @@ class CommandScriptImmediateOutputTestCa
 self.sendline(
 'command script add -f custom_command.write_file mywrite',
 patterns=[prompt])
-for path, mode in test_files.iteritems():
+for path, mode in test_files.items():
 command = 'mywrite "' + path + '" ' + mode
 
 self.sendline(command, patterns=[prompt])
@@ -87,7 +87,7 @@ class CommandScriptImmediateOutputTestCa
 
 self.quit(gracefully=False)
 
-for path, mode in test_files.iteritems():
+for path, mode in test_files.items():
 with open(path, 'r') as result:
 if mode in ['r', 'a', 'a+']:
 self.assertEquals(result.readline(), starter_string)


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

[Lldb-commits] [PATCH] D59482: [ObjectYAML] Add basic minidump generation support

2019-03-18 Thread Pavel Labath via Phabricator via lldb-commits

labath updated this revision to Diff 191079.
labath marked 11 inline comments as done.
labath added a comment.

Updating to address review comments (thank you for the super quick turnaround).


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59482/new/

https://reviews.llvm.org/D59482

Files:
  include/llvm/ObjectYAML/MinidumpYAML.h
  include/llvm/ObjectYAML/ObjectYAML.h
  lib/ObjectYAML/CMakeLists.txt
  lib/ObjectYAML/MinidumpYAML.cpp
  lib/ObjectYAML/ObjectYAML.cpp
  tools/yaml2obj/CMakeLists.txt
  tools/yaml2obj/yaml2minidump.cpp
  tools/yaml2obj/yaml2obj.cpp
  tools/yaml2obj/yaml2obj.h
  unittests/ObjectYAML/CMakeLists.txt
  unittests/ObjectYAML/MinidumpYAMLTest.cpp

Index: unittests/ObjectYAML/MinidumpYAMLTest.cpp
===
--- /dev/null
+++ unittests/ObjectYAML/MinidumpYAMLTest.cpp
@@ -0,0 +1,183 @@
+//===- MinidumpYAMLTest.cpp - Tests for Minidump<->YAML code --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "llvm/ObjectYAML/MinidumpYAML.h"
+#include "llvm/Object/Minidump.h"
+#include "llvm/ObjectYAML/ObjectYAML.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace llvm::minidump;
+
+static Expected>
+toBinary(StringRef Yaml) {
+  SmallString<0> Binary;
+  raw_svector_ostream OS(Binary);
+  if (Error E = MinidumpYAML::writeAsBinary(Yaml, OS))
+return std::move(E);
+
+  return object::MinidumpFile::create(MemoryBufferRef(Binary, "Binary"));
+}
+
+TEST(MinidumpYAML, Basic) {
+  auto ExpectedFile = toBinary(R"(
+--- !minidump
+Streams:
+  - Type:SystemInfo
+Processor Arch:  ARM64
+Platform ID: Linux
+CSD Version RVA: 0x01020304
+CPU:
+  CPUID:   0x05060708
+  - Type:LinuxMaps
+Text: |
+  400d9000-400db000 r-xp  b3:04 227/system/bin/app_process
+  400db000-400dc000 r--p 1000 b3:04 227/system/bin/app_process
+
+  - Type:LinuxAuxv
+Content: DEADBEEFBAADF00D)");
+  ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+  object::MinidumpFile &File = **ExpectedFile;
+
+  ASSERT_EQ(3u, File.streams().size());
+
+  EXPECT_EQ(StreamType::SystemInfo, File.streams()[0].Type);
+  auto ExpectedSysInfo = File.getSystemInfo();
+  ASSERT_THAT_EXPECTED(ExpectedSysInfo, Succeeded());
+  const SystemInfo &SysInfo = *ExpectedSysInfo;
+  EXPECT_EQ(ProcessorArchitecture::ARM64, SysInfo.ProcessorArch);
+  EXPECT_EQ(OSPlatform::Linux, SysInfo.PlatformId);
+  EXPECT_EQ(0x01020304u, SysInfo.CSDVersionRVA);
+  EXPECT_EQ(0x05060708u, SysInfo.CPU.Arm.CPUID);
+
+  EXPECT_EQ(StreamType::LinuxMaps, File.streams()[1].Type);
+  EXPECT_EQ("400d9000-400db000 r-xp  b3:04 227"
+"/system/bin/app_process\n"
+"400db000-400dc000 r--p 1000 b3:04 227"
+"/system/bin/app_process\n",
+toStringRef(*File.getRawStream(StreamType::LinuxMaps)));
+
+  EXPECT_EQ(StreamType::LinuxAuxv, File.streams()[2].Type);
+  EXPECT_EQ((ArrayRef{0xDE, 0xAD, 0xBE, 0xEF, 0xBA, 0xAD, 0xF0, 0x0D}),
+File.getRawStream(StreamType::LinuxAuxv));
+}
+
+TEST(MinidumpYAML, X86SystemInfo) {
+  // Vendor ID too short.
+  auto ExpectedFile = toBinary(R"(
+--- !minidump
+Streams:
+  - Type:SystemInfo
+Processor Arch:  X86
+Platform ID: Linux
+CPU:
+  Vendor ID:   LLVMLLVMLLV
+  Version Info:0x01020304
+  Feature Info:0x05060708
+  AMD Extended Features: 0x09000102)");
+  ASSERT_THAT_EXPECTED(ExpectedFile, Failed());
+
+  // Vendor ID too long.
+  ExpectedFile = toBinary(R"(
+--- !minidump
+Streams:
+  - Type:SystemInfo
+Processor Arch:  X86
+Platform ID: Linux
+CPU:
+  Vendor ID:   LLVMLLVMLLVML
+  Version Info:0x01020304
+  Feature Info:0x05060708
+  AMD Extended Features: 0x09000102)");
+  ASSERT_THAT_EXPECTED(ExpectedFile, Failed());
+
+  ExpectedFile = toBinary(R"(
+--- !minidump
+Streams:
+  - Type:SystemInfo
+Processor Arch:  X86
+Platform ID: Linux
+CPU:
+  Vendor ID:   LLVMLLVMLLVM
+  Version Info:0x01020304
+  Feature Info:0x05060708
+  AMD Extended Features: 0x09000102)");
+  ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+  object::MinidumpFile &File = **ExpectedFile;
+
+  ASSERT_EQ(1u, File.streams().size());
+
+  auto ExpectedSysInfo = File.getSystemInfo();
+  ASSERT_THAT_EXPECTED(ExpectedSysInfo, Succeeded());
+  const SystemInfo &SysInfo = *ExpectedSysInfo;
+  EXPECT_EQ(ProcessorArchitecture::X86, SysInfo.ProcessorArch);
+  EXPECT_EQ(OSPlatform::Linux, SysInfo.PlatformId);

[Lldb-commits] [PATCH] D59482: [ObjectYAML] Add basic minidump generation support

2019-03-18 Thread Pavel Labath via Phabricator via lldb-commits

labath marked 11 inline comments as done.
labath added inline comments.



Comment at: lib/ObjectYAML/MinidumpYAML.cpp:181
+  FeaturesRef.writeAsBinary(FeaturesStream);
+  std::fill(Features.begin(), Features.end(), 0);
+  std::memcpy(Features.begin(), FeaturesStorage.begin(),

jhenderson wrote:
> Is this definitely necessary? What's the type of Info.ProcessorFeatures? 
> Similar comments for Vendor ID below.
No, it's not necessary, it's just that I was too lazy to implement proper error 
checking for this (and this field isn't that important -- I don't believe we 
actually use it for anything right now, but I have to print it in some way). 
I've now implemented proper checking for this and the VendorID fields 
(including tests).



Comment at: lib/ObjectYAML/MinidumpYAML.cpp:306
+Error MinidumpYAML::writeAsBinary(StringRef Yaml, raw_ostream &OS) {
+  yaml::Input yin(Yaml);
+  Object Obj;

jhenderson wrote:
> I don't think `yin` matches our (current) variable naming guidelines. It 
> should be `Yin` at least, though it wasn't immediately clear what was meant 
> even then at first to me, so perhaps a different name might be better (simply 
> `Input` would work for me).
There's definitely plenty of places that use `yin`, with the yaml library 
itself being the most prominent user. I think you could argue that it fits the 
naming guidelines due to the "emulates c++ stl interface" (i.e `cin`) 
exception, but I don't think the name really matters that much here, so I've 
just renamed it to `Input`.



Comment at: tools/yaml2obj/yaml2obj.cpp:61
+  if (Doc.Minidump) {
+MinidumpYAML::writeAsBinary(*Doc.Minidump, Out);
+return 0;

jhenderson wrote:
> I feel like it would make more sense for this to match the other versions 
> above, i.e. I think this should be at least `return yaml2minidump(...);`
Yeah, I had the same feeling, but then I also didn't like creating a 
`yaml2minidump.cpp` just to put a two-line function in it (and putting this 
function into the ObjectYAML library also didn't feel right).

The root cause here is that I've chose to fully library-ize the minidump code, 
which doesn't fit in with how the other object files work (though there is a 
kind of a precedent for this in DWARFYAML). My reason for doing that was that I 
think it would be very useful to have this functionality accessible to lldb 
unit tests (and I've also gotten interest from a downstream user who wanted to 
use this functionality to synthesize minidumps in their unit tests).

Long story short, I've now put the created yaml2minidump.cpp and put the 
yaml2minidump in it.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59482/new/

https://reviews.llvm.org/D59482



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

[Lldb-commits] [PATCH] D59485: [ASTImporter] Allow adding a shim to the ASTImporter

2019-03-18 Thread Balázs Kéri via Phabricator via lldb-commits

balazske added a comment.

The replace of `Import` calls may work if the internal state of the 
`ASTImporter` is updated correctly from the 'shim' object (especially if not 
every Decl is handled by the shim). For this to work some of the state of 
`ASTImporter` that was intended to be internal must be made public, for example 
map of imported decls and lookup table. The shim may corrupt the `ASTImporter` 
state for example when the lookup table is not updated correctly. (Probably for 
the LLDB special use cases this is not a problem?)


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59485/new/

https://reviews.llvm.org/D59485



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

[Lldb-commits] [PATCH] D59485: [ASTImporter] Allow adding a shim to the ASTImporter

2019-03-18 Thread Balázs Kéri via Phabricator via lldb-commits

balazske added a comment.

I do not know if "Shim" is the correct name for this, maybe "Strategy" is 
better?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59485/new/

https://reviews.llvm.org/D59485



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

[Lldb-commits] [PATCH] D59485: [ASTImporter] Allow adding a shim to the ASTImporter

2019-03-18 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor created this revision.
teemperor added reviewers: martong, balazske, a.sidorin, shafik.
Herald added subscribers: cfe-commits, rnkovacs.
Herald added a project: clang.

We are currently implementing support in LLDB that reconstructs the STL 
templates from
the target program in the expression evaluator. This reconstruction happens 
during the
import process from our debug info AST into the expression evaluation AST[1], 
which means
we need a way to intercept the ASTImporter import process in some controlled 
fashion.

This patch adds a shim to the ASTImporter that allows us to intercept any 
Import calls an
ASTImporter may make (either externally or while importing internally). We then 
provide
a shim in LLDB that does do our template reconstruction.

The shim is on purpose not reusing some ASTImporter functionalities like the 
map of already
imported decls because we want to keep it flexible. If a Shim wants to use this 
functionality
it needs to update/check the map itself.

[1] The reason for this is that we can't reliable import templates with the 
ASTImporter, so
actually reconstructing the template in the debug info AST and then importing 
it doesn't work.
It would also be much slower.


Repository:
  rC Clang

https://reviews.llvm.org/D59485

Files:
  clang/include/clang/AST/ASTImporter.h
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp

Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -316,11 +316,13 @@
 std::unique_ptr Unit;
 TranslationUnitDecl *TUDecl = nullptr;
 std::unique_ptr Importer;
-TU(StringRef Code, StringRef FileName, ArgVector Args)
+ImportShim *Shim;
+TU(StringRef Code, StringRef FileName, ArgVector Args,
+   ImportShim *Shim = nullptr)
 : Code(Code), FileName(FileName),
   Unit(tooling::buildASTFromCodeWithArgs(this->Code, Args,
  this->FileName)),
-  TUDecl(Unit->getASTContext().getTranslationUnitDecl()) {
+  TUDecl(Unit->getASTContext().getTranslationUnitDecl()), Shim(Shim) {
   Unit->enableSourceFileDiagnostics();
 }
 
@@ -331,6 +333,7 @@
 new ASTImporter(ToAST->getASTContext(), ToAST->getFileManager(),
 Unit->getASTContext(), Unit->getFileManager(),
 false, &LookupTable));
+Importer->setShim(Shim);
   }
   assert(&ToAST->getASTContext() == &Importer->getToContext());
   createVirtualFileIfNeeded(ToAST, FileName, Code);
@@ -401,11 +404,12 @@
   // Must not be called more than once within the same test.
   std::tuple
   getImportedDecl(StringRef FromSrcCode, Language FromLang, StringRef ToSrcCode,
-  Language ToLang, StringRef Identifier = DeclToImportID) {
+  Language ToLang, StringRef Identifier = DeclToImportID,
+  ImportShim *Shim = nullptr) {
 ArgVector FromArgs = getArgVectorForLanguage(FromLang),
   ToArgs = getArgVectorForLanguage(ToLang);
 
-FromTUs.emplace_back(FromSrcCode, InputFileName, FromArgs);
+FromTUs.emplace_back(FromSrcCode, InputFileName, FromArgs, Shim);
 TU &FromTU = FromTUs.back();
 
 assert(!ToAST);
@@ -455,6 +459,12 @@
 return ToAST->getASTContext().getTranslationUnitDecl();
   }
 
+  ASTImporter &getImporter(Decl *From, Language ToLang) {
+lazyInitToAST(ToLang, "", OutputFileName);
+TU *FromTU = findFromTU(From);
+return *FromTU->Importer;
+  }
+
   // Import the given Decl into the ToCtx.
   // May be called several times in a given test.
   // The different instances of the param From may have different ASTContext.
@@ -544,6 +554,81 @@
   EXPECT_THAT(RedeclsD1, ::testing::ContainerEq(RedeclsD2));
 }
 
+namespace {
+class RedirectShim : public ImportShim {
+  llvm::Optional Import(ASTImporter &Importer, Decl *FromD) override {
+auto *ND = dyn_cast(FromD);
+if (!ND)
+  return {};
+if (ND->getName() != "shouldNotBeImported")
+  return {};
+for (Decl *D : Importer.getToContext().getTranslationUnitDecl()->decls())
+  if (auto ND = dyn_cast(D))
+if (ND->getName() == "realDecl")
+  return ND;
+return {};
+  }
+};
+} // namespace
+
+struct ShimTestCase : ASTImporterOptionSpecificTestBase {};
+
+// Test that the shim can intercept an import call.
+TEST_P(ShimTestCase, InterceptImport) {
+  RedirectShim Shim;
+  Decl *From, *To;
+  std::tie(From, To) = getImportedDecl("class shouldNotBeImported {};",
+   Lang_CXX, "class realDecl {};", Lang_CXX,
+   "shouldNotBeImported", &Shim);
+  auto *Imported = cast(To);
+  EXPECT_EQ(Imported->getQualifiedNameAsString(), "realDecl");
+
+  // Make sure the Shim prevented the importing of the decl.
+  auto *ToTU = Imported->ge

[Lldb-commits] [lldb] r356362 - Fix "type qualifiers ignored on cast result type" warnings

2019-03-18 Thread Pavel Labath via lldb-commits

Author: labath
Date: Mon Mar 18 03:50:46 2019
New Revision: 356362

URL: http://llvm.org/viewvc/llvm-project?rev=356362&view=rev
Log:
Fix "type qualifiers ignored on cast result type" warnings

These warnings start to get emitted with gcc-8.

Modified:
lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp

Modified: lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp?rev=356362&r1=356361&r2=356362&view=diff
==
--- lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp (original)
+++ lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp Mon Mar 18 
03:50:46 2019
@@ -162,7 +162,7 @@ UUID MinidumpParser::GetModuleUUID(const
 return UUID();
 
   const CvSignature cv_signature =
-  static_cast(static_cast(*signature));
+  static_cast(static_cast(*signature));
 
   if (cv_signature == CvSignature::Pdb70) {
 // PDB70 record
@@ -263,9 +263,8 @@ ArchSpec MinidumpParser::GetArchitecture
   llvm::Triple triple;
   triple.setVendor(llvm::Triple::VendorType::UnknownVendor);
 
-  const MinidumpCPUArchitecture arch =
-  static_cast(
-  static_cast(system_info->processor_arch));
+  const MinidumpCPUArchitecture arch = static_cast(
+  static_cast(system_info->processor_arch));
 
   switch (arch) {
   case MinidumpCPUArchitecture::X86:
@@ -285,8 +284,8 @@ ArchSpec MinidumpParser::GetArchitecture
 break;
   }
 
-  const MinidumpOSPlatform os = static_cast(
-  static_cast(system_info->platform_id));
+  const MinidumpOSPlatform os = static_cast(
+  static_cast(system_info->platform_id));
 
   // TODO add all of the OSes that Minidump/breakpad distinguishes?
   switch (os) {

Modified: lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp?rev=356362&r1=356361&r2=356362&view=diff
==
--- lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp (original)
+++ lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp Mon Mar 18 
03:50:46 2019
@@ -19,11 +19,10 @@ const MinidumpHeader *MinidumpHeader::Pa
   Status error = consumeObject(data, header);
 
   const MinidumpHeaderConstants signature =
-  static_cast(
-  static_cast(header->signature));
-  const MinidumpHeaderConstants version =
-  static_cast(
-  static_cast(header->version) & 0x);
+  static_cast(
+  static_cast(header->signature));
+  const MinidumpHeaderConstants version = static_cast(
+  static_cast(header->version) & 0x);
   // the high 16 bits of the version field are implementation specific
 
   if (error.Fail() || signature != MinidumpHeaderConstants::Signature ||
@@ -115,8 +114,7 @@ const MinidumpMiscInfo *MinidumpMiscInfo
 }
 
 llvm::Optional MinidumpMiscInfo::GetPid() const {
-  uint32_t pid_flag =
-  static_cast(MinidumpMiscInfoFlags::ProcessID);
+  uint32_t pid_flag = static_cast(MinidumpMiscInfoFlags::ProcessID);
   if (flags1 & pid_flag)
 return llvm::Optional(process_id);
 


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

[Lldb-commits] [PATCH] D59482: [ObjectYAML] Add basic minidump generation support

2019-03-18 Thread James Henderson via Phabricator via lldb-commits

jhenderson added a comment.

I've not reviewed the unit test yet, but the bulk of the body looks fine, apart 
from some minor details.




Comment at: include/llvm/ObjectYAML/MinidumpYAML.h:27
+struct Stream {
+  enum StreamKind {
+HexKind,

I think it is worth making this an `enum class`.



Comment at: lib/ObjectYAML/MinidumpYAML.cpp:55
+Callback(OS);
+  assert(OS.tell() == BeginOffset + NextOffset);
+  (void)BeginOffset;

Perhaps worth a quick message:

`assert(OS.tell() == BeginOffset + NextOffset && "some message here");`



Comment at: lib/ObjectYAML/MinidumpYAML.cpp:70
+template 
+static inline void mapAs(yaml::IO &IO, const char *Key, EndianType &Val) {
+  MapType Mapped = static_cast(Val);

Maybe worth calling this `mapRequiredAs`, to line up with the `mapRequired` 
name.



Comment at: lib/ObjectYAML/MinidumpYAML.cpp:96
+template 
+static inline void mapHex(yaml::IO &IO, const char *Key, EndianType &Val) {
+  mapAs::type>(IO, Key, Val);

Similar to above, perhaps `mapRequiredHex`?



Comment at: lib/ObjectYAML/MinidumpYAML.cpp:100
+
+/// Perform an optionam yaml-mapping of an endian-aware type as an
+/// appropriately-sized hex value.

`optionam`?



Comment at: lib/ObjectYAML/MinidumpYAML.cpp:181
+  FeaturesRef.writeAsBinary(FeaturesStream);
+  std::fill(Features.begin(), Features.end(), 0);
+  std::memcpy(Features.begin(), FeaturesStorage.begin(),

Is this definitely necessary? What's the type of Info.ProcessorFeatures? 
Similar comments for Vendor ID below.



Comment at: lib/ObjectYAML/MinidumpYAML.cpp:183
+  std::memcpy(Features.begin(), FeaturesStorage.begin(),
+  std::min(FeaturesStorage.size(), Features.size()));
+}

If there is too much data to fit, should this emit an error, rather than 
silently truncate? Same below with Vendor ID.



Comment at: lib/ObjectYAML/MinidumpYAML.cpp:306
+Error MinidumpYAML::writeAsBinary(StringRef Yaml, raw_ostream &OS) {
+  yaml::Input yin(Yaml);
+  Object Obj;

I don't think `yin` matches our (current) variable naming guidelines. It should 
be `Yin` at least, though it wasn't immediately clear what was meant even then 
at first to me, so perhaps a different name might be better (simply `Input` 
would work for me).



Comment at: tools/yaml2obj/yaml2obj.cpp:61
+  if (Doc.Minidump) {
+MinidumpYAML::writeAsBinary(*Doc.Minidump, Out);
+return 0;

I feel like it would make more sense for this to match the other versions 
above, i.e. I think this should be at least `return yaml2minidump(...);`


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59482/new/

https://reviews.llvm.org/D59482



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

[Lldb-commits] [PATCH] D58347: Reinitialize UnwindTable when the SymbolFile changes

2019-03-18 Thread Pavel Labath via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL356361: Reinitialize UnwindTable when the SymbolFile changes 
(authored by labath, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58347/new/

https://reviews.llvm.org/D58347

Files:
  lldb/trunk/include/lldb/Core/Module.h
  lldb/trunk/lit/SymbolFile/Inputs/target-symbols-add-unwind.c
  lldb/trunk/lit/SymbolFile/target-symbols-add-unwind.test
  lldb/trunk/source/Core/Module.cpp
  lldb/trunk/source/Symbol/UnwindTable.cpp

Index: lldb/trunk/source/Symbol/UnwindTable.cpp
===
--- lldb/trunk/source/Symbol/UnwindTable.cpp
+++ lldb/trunk/source/Symbol/UnwindTable.cpp
@@ -46,7 +46,7 @@
   if (!object_file)
 return;
 
-  SectionList *sl = object_file->GetSectionList();
+  SectionList *sl = m_module.GetSectionList();
   if (!sl)
 return;
 
Index: lldb/trunk/source/Core/Module.cpp
===
--- lldb/trunk/source/Core/Module.cpp
+++ lldb/trunk/source/Core/Module.cpp
@@ -1300,6 +1300,12 @@
 sym_vendor->SectionFileAddressesChanged();
 }
 
+UnwindTable &Module::GetUnwindTable() {
+  if (!m_unwind_table)
+m_unwind_table.emplace(*this);
+  return *m_unwind_table;
+}
+
 SectionList *Module::GetUnifiedSectionList() {
   if (!m_sections_up)
 m_sections_up = llvm::make_unique();
@@ -1446,6 +1452,10 @@
 // one
 obj_file->ClearSymtab();
 
+// Clear the unwind table too, as that may also be affected by the
+// symbol file information.
+m_unwind_table.reset();
+
 // The symbol file might be a directory bundle ("/tmp/a.out.dSYM")
 // instead of a full path to the symbol file within the bundle
 // ("/tmp/a.out.dSYM/Contents/Resources/DWARF/a.out"). So we need to
Index: lldb/trunk/lit/SymbolFile/target-symbols-add-unwind.test
===
--- lldb/trunk/lit/SymbolFile/target-symbols-add-unwind.test
+++ lldb/trunk/lit/SymbolFile/target-symbols-add-unwind.test
@@ -0,0 +1,26 @@
+# TODO: When it's possible to run "image show-unwind" without a running
+# process, we can remove the unsupported line below, and hard-code an ELF
+# triple in the test.
+# UNSUPPORTED: system-windows, system-darwin
+
+# RUN: cd %T
+# RUN: %clang %S/Inputs/target-symbols-add-unwind.c -nostdlib -g \
+# RUN:   -fno-unwind-tables -o target-symbols-add-unwind.debug
+# RUN: llvm-objcopy --strip-debug target-symbols-add-unwind.debug \
+# RUN:   target-symbols-add-unwind.stripped
+# RUN: %lldb target-symbols-add-unwind.stripped -s %s -o quit | FileCheck %s
+
+process launch --stop-at-entry
+image show-unwind -n _start
+# CHECK-LABEL: image show-unwind -n _start
+# CHECK-NOT: debug_frame UnwindPlan:
+
+target symbols add -s target-symbols-add-unwind.stripped target-symbols-add-unwind.debug
+# CHECK-LABEL: target symbols add
+# CHECK: symbol file {{.*}} has been added to {{.*}}
+
+image show-unwind -n _start
+# CHECK-LABEL: image show-unwind -n _start
+# CHECK: debug_frame UnwindPlan:
+# CHECK-NEXT: This UnwindPlan originally sourced from DWARF CFI
+# CHECK-NEXT: This UnwindPlan is sourced from the compiler: yes.
Index: lldb/trunk/lit/SymbolFile/Inputs/target-symbols-add-unwind.c
===
--- lldb/trunk/lit/SymbolFile/Inputs/target-symbols-add-unwind.c
+++ lldb/trunk/lit/SymbolFile/Inputs/target-symbols-add-unwind.c
@@ -0,0 +1 @@
+void _start() {}
Index: lldb/trunk/include/lldb/Core/Module.h
===
--- lldb/trunk/include/lldb/Core/Module.h
+++ lldb/trunk/include/lldb/Core/Module.h
@@ -706,7 +706,7 @@
   /// Returns the unwind table for this module. If this object has no
   /// associated object file, an empty UnwindTable is returned.
   //--
-  UnwindTable &GetUnwindTable() { return m_unwind_table; }
+  UnwindTable &GetUnwindTable();
 
   llvm::VersionTuple GetVersion();
 
@@ -1105,8 +1105,9 @@
   lldb::ObjectFileSP m_objfile_sp; ///< A shared pointer to the object file
///parser for this module as it may or may
///not be shared with the SymbolFile
-  UnwindTable m_unwind_table{*this}; ///< Table of FuncUnwinders objects created
- /// for this Module's functions
+  llvm::Optional m_unwind_table; ///< Table of FuncUnwinders
+  /// objects created for this
+  /// Module's functions
   lldb::SymbolVendorUP
   m_symfile_up; ///< A pointer to the symbol vendor for this module.
   std::vector
__

[Lldb-commits] [lldb] r356361 - Reinitialize UnwindTable when the SymbolFile changes

2019-03-18 Thread Pavel Labath via lldb-commits

Author: labath
Date: Mon Mar 18 03:45:02 2019
New Revision: 356361

URL: http://llvm.org/viewvc/llvm-project?rev=356361&view=rev
Log:
Reinitialize UnwindTable when the SymbolFile changes

Summary:
This is a preparatory step to enable adding of unwind plans by symbol
file plugins.

Although at the surface it seems that currently symbol files have
nothing to do with unwinding, this isn't entirely correct even now. The
mere act of adding a symbol file can have the effect of making more
sections (typically .debug_frame) available to the unwinding machinery,
so that it can have more unwind strategies to choose from.

Up until now, we've had a bug, which went largely unnoticed, where
unwind info in the manually added symbols files (target symbols add) was
being ignored during unwinding. Reinitializing the UnwindTable fixes
that bug too.

Reviewers: clayborg, jasonmolenda, alexshap

Subscribers: jdoerfert, lldb-commits

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

Added:
lldb/trunk/lit/SymbolFile/Inputs/
lldb/trunk/lit/SymbolFile/Inputs/target-symbols-add-unwind.c
lldb/trunk/lit/SymbolFile/target-symbols-add-unwind.test
Modified:
lldb/trunk/include/lldb/Core/Module.h
lldb/trunk/source/Core/Module.cpp
lldb/trunk/source/Symbol/UnwindTable.cpp

Modified: lldb/trunk/include/lldb/Core/Module.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Module.h?rev=356361&r1=356360&r2=356361&view=diff
==
--- lldb/trunk/include/lldb/Core/Module.h (original)
+++ lldb/trunk/include/lldb/Core/Module.h Mon Mar 18 03:45:02 2019
@@ -706,7 +706,7 @@ public:
   /// Returns the unwind table for this module. If this object has no
   /// associated object file, an empty UnwindTable is returned.
   //--
-  UnwindTable &GetUnwindTable() { return m_unwind_table; }
+  UnwindTable &GetUnwindTable();
 
   llvm::VersionTuple GetVersion();
 
@@ -1105,8 +1105,9 @@ protected:
   lldb::ObjectFileSP m_objfile_sp; ///< A shared pointer to the object file
///parser for this module as it may or may
///not be shared with the SymbolFile
-  UnwindTable m_unwind_table{*this}; ///< Table of FuncUnwinders objects 
created
- /// for this Module's functions
+  llvm::Optional m_unwind_table; ///< Table of FuncUnwinders
+  /// objects created for this
+  /// Module's functions
   lldb::SymbolVendorUP
   m_symfile_up; ///< A pointer to the symbol vendor for this module.
   std::vector

Added: lldb/trunk/lit/SymbolFile/Inputs/target-symbols-add-unwind.c
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/Inputs/target-symbols-add-unwind.c?rev=356361&view=auto
==
--- lldb/trunk/lit/SymbolFile/Inputs/target-symbols-add-unwind.c (added)
+++ lldb/trunk/lit/SymbolFile/Inputs/target-symbols-add-unwind.c Mon Mar 18 
03:45:02 2019
@@ -0,0 +1 @@
+void _start() {}

Added: lldb/trunk/lit/SymbolFile/target-symbols-add-unwind.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/target-symbols-add-unwind.test?rev=356361&view=auto
==
--- lldb/trunk/lit/SymbolFile/target-symbols-add-unwind.test (added)
+++ lldb/trunk/lit/SymbolFile/target-symbols-add-unwind.test Mon Mar 18 
03:45:02 2019
@@ -0,0 +1,26 @@
+# TODO: When it's possible to run "image show-unwind" without a running
+# process, we can remove the unsupported line below, and hard-code an ELF
+# triple in the test.
+# UNSUPPORTED: system-windows, system-darwin
+
+# RUN: cd %T
+# RUN: %clang %S/Inputs/target-symbols-add-unwind.c -nostdlib -g \
+# RUN:   -fno-unwind-tables -o target-symbols-add-unwind.debug
+# RUN: llvm-objcopy --strip-debug target-symbols-add-unwind.debug \
+# RUN:   target-symbols-add-unwind.stripped
+# RUN: %lldb target-symbols-add-unwind.stripped -s %s -o quit | FileCheck %s
+
+process launch --stop-at-entry
+image show-unwind -n _start
+# CHECK-LABEL: image show-unwind -n _start
+# CHECK-NOT: debug_frame UnwindPlan:
+
+target symbols add -s target-symbols-add-unwind.stripped 
target-symbols-add-unwind.debug
+# CHECK-LABEL: target symbols add
+# CHECK: symbol file {{.*}} has been added to {{.*}}
+
+image show-unwind -n _start
+# CHECK-LABEL: image show-unwind -n _start
+# CHECK: debug_frame UnwindPlan:
+# CHECK-NEXT: This UnwindPlan originally sourced from DWARF CFI
+# CHECK-NEXT: This UnwindPlan is sourced from the compiler: yes.

Modified: lldb/trunk/source/Core/Module.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Module.cpp?rev=356361&r1=356360&r2=356361&view=diff

[Lldb-commits] [PATCH] D59482: [ObjectYAML] Add basic minidump generation support

2019-03-18 Thread Pavel Labath via Phabricator via lldb-commits

labath created this revision.
labath added reviewers: jhenderson, zturner, clayborg, aprantl.
Herald added a subscriber: mgorny.
Herald added a project: LLVM.

This patch adds the ability to read a yaml form of a minidump file and
write it out as binary. Apart from the minidump header and the stream
directory, only three basic stream kinds are supported:

- Text: This kind is used for streams which contain textual data. This is 
typically the contents of a /proc file on linix (e.g. /proc/PID/maps). In this 
case, we just put the raw stream contents into the yaml.
- SystemInfo: This stream contains various bits of information about the host 
system in binary form. We expose the data in a structured form.
- Hex: This kind is used as a fallback when we don't have any special knowledge 
about the stream. In this case, we just print the stream contents in hex.

For this code to be really useful, more stream kinds will need to be
added (particularly for things like lists of memory regions and loaded
modules). However, these can be added incrementally.


Repository:
  rL LLVM

https://reviews.llvm.org/D59482

Files:
  include/llvm/ObjectYAML/MinidumpYAML.h
  include/llvm/ObjectYAML/ObjectYAML.h
  lib/ObjectYAML/CMakeLists.txt
  lib/ObjectYAML/MinidumpYAML.cpp
  lib/ObjectYAML/ObjectYAML.cpp
  tools/yaml2obj/yaml2obj.cpp
  tools/yaml2obj/yaml2obj.h
  unittests/ObjectYAML/CMakeLists.txt
  unittests/ObjectYAML/MinidumpYAMLTest.cpp

Index: unittests/ObjectYAML/MinidumpYAMLTest.cpp
===
--- /dev/null
+++ unittests/ObjectYAML/MinidumpYAMLTest.cpp
@@ -0,0 +1,121 @@
+//===- MinidumpYAMLTest.cpp - Tests for Minidump<->YAML code --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "llvm/ObjectYAML/MinidumpYAML.h"
+#include "llvm/Object/Minidump.h"
+#include "llvm/ObjectYAML/ObjectYAML.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace llvm::minidump;
+
+static Expected>
+toBinary(StringRef Yaml) {
+  SmallString<0> Binary;
+  raw_svector_ostream OS(Binary);
+  if (Error E = MinidumpYAML::writeAsBinary(Yaml, OS))
+  return std::move(E);
+
+  return object::MinidumpFile::create(MemoryBufferRef(Binary, "Binary"));
+}
+
+TEST(MinidumpYAML, Basic) {
+  auto ExpectedFile = toBinary(R"(
+--- !minidump
+Streams:
+  - Type:SystemInfo
+Processor Arch:  ARM64
+Platform ID: Linux
+CSD Version RVA: 0x01020304
+CPU:
+  CPUID:   0x05060708
+  - Type:LinuxMaps
+Text: |
+  400d9000-400db000 r-xp  b3:04 227/system/bin/app_process
+  400db000-400dc000 r--p 1000 b3:04 227/system/bin/app_process
+
+  - Type:LinuxAuxv
+Content: DEADBEEFBAADF00D)");
+  ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+  object::MinidumpFile &File = **ExpectedFile;
+
+  ASSERT_EQ(3u, File.streams().size());
+
+  EXPECT_EQ(StreamType::SystemInfo, File.streams()[0].Type);
+  auto ExpectedSysInfo = File.getSystemInfo();
+  ASSERT_THAT_EXPECTED(ExpectedSysInfo, Succeeded());
+  const SystemInfo &SysInfo = *ExpectedSysInfo;
+  EXPECT_EQ(ProcessorArchitecture::ARM64, SysInfo.ProcessorArch);
+  EXPECT_EQ(OSPlatform::Linux, SysInfo.PlatformId);
+  EXPECT_EQ(0x01020304u, SysInfo.CSDVersionRVA);
+  EXPECT_EQ(0x05060708u, SysInfo.CPU.Arm.CPUID);
+
+  EXPECT_EQ(StreamType::LinuxMaps, File.streams()[1].Type);
+  EXPECT_EQ("400d9000-400db000 r-xp  b3:04 227"
+"/system/bin/app_process\n"
+"400db000-400dc000 r--p 1000 b3:04 227"
+"/system/bin/app_process\n",
+toStringRef(*File.getRawStream(StreamType::LinuxMaps)));
+
+  EXPECT_EQ(StreamType::LinuxAuxv, File.streams()[2].Type);
+  EXPECT_EQ((ArrayRef{0xDE, 0xAD, 0xBE, 0xEF, 0xBA, 0xAD, 0xF0, 0x0D}),
+File.getRawStream(StreamType::LinuxAuxv));
+}
+
+TEST(MinidumpYAML, X86SystemInfo) {
+  auto ExpectedFile = toBinary(R"(
+--- !minidump
+Streams:
+  - Type:SystemInfo
+Processor Arch:  X86
+Platform ID: Linux
+CPU:
+  Vendor ID:   LLVMLLVMLLVM
+  Version Info:0x01020304
+  Feature Info:0x05060708
+  AMD Extended Features: 0x09000102)");
+  ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+  object::MinidumpFile &File = **ExpectedFile;
+
+  ASSERT_EQ(1u, File.streams().size());
+
+  auto ExpectedSysInfo = File.getSystemInfo();
+  ASSERT_THAT_EXPECTED(ExpectedSysInfo, Succeeded());
+  const SystemInfo &SysInfo = *ExpectedSysInfo;
+  EXPECT_EQ(ProcessorArchitecture::X86, SysInfo.ProcessorArch);
+  EXPECT_EQ(OSPlatform::Linux, SysInfo.PlatformId);
+  EXPECT_EQ("LLVMLLVMLLVM

[Lldb-commits] [PATCH] D59433: Fix UUID decoding from minidump files.

2019-03-18 Thread Pavel Labath via Phabricator via lldb-commits

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Looks good to me. I have some comments inline and below, but none of them is 
really substantial.

In D59433#1431602 , @clayborg wrote:

> In D59433#1431488 , @zturner wrote:
>
> > "MinidumpNew" is a little bit vague.  What's "new" about it?  Is there a 
> > way we could come up with a better name?
>
>
> This was an existing file that I just added new tests to, and it seemed to be 
> where most of the minidump tests were.

The name is a relict from when we were implementing our own minidump reader. In 
that context, the existing implementation which relied on the windows APIs was 
the "old" thing, and the one which read the files directly was "new". But this 
distinction doesn't really make sense anymore, so maybe we could start getting 
rid of it by putting the new tests into a fresh file (`TestMinidumpUUIDs.py` ?).

> 
> 
>> As an aside, since there's no interactivity in these tests, they seem like a 
>> good candidate for lit/Minidump, with 1 file for each test.  Something like:
>> 
>>   ; zero-uuid-modules.test
>>   ; RUN: lldb -core %S/Inputs/linux-arm-zero-uuids.dmp -S %p | FileCheck %s
>>   
>>   target modules list
>>   
>>   ; CHECK: [  0] -----   
>>  /tmp/a
>>   ; CHECK: [  1] -----   
>>  /tmp/b
>> 
>> 
>> Let's see what Pavel thinks though.

I think turning these into lit tests would be reasonable, but I also don't have 
a problem with them staying like they are.

Comment at: 
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py:533
+self.assertEqual(2, len(modules))
+self.verify_module(modules[0], "/tmp/a", None)
+self.verify_module(modules[1], "/tmp/b", None)

Am I right in thinking that if I have an object file called `/tmp/a` on my 
system, then lldb will pick it up (and it's build-id), causing this test to 
fail ? If that's the case then it would be better to use some path which is 
less likely to exist.

Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:175-183
+
+// Many times UUIDs are all zeroes. This can cause more than one module
+// to claim it has a valid UUID of all zeroes and causes the files to all
+// merge into the first module that claims this valid zero UUID.
+bool all_zeroes = true;
+for (size_t i = 0; all_zeroes && i < sizeof(pdb70_uuid->Uuid); ++i)
+  all_zeroes = pdb70_uuid->Uuid[i] == 0;

Instead of all of this, I believe you should be able to just replace the 
`UUID::fromData` with `UUID::fromOptionalData` in the calls below.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59433/new/

https://reviews.llvm.org/D59433

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

[Lldb-commits] [PATCH] D59427: [WIP] [lldb] [API] Split SBRegistry into smaller files

2019-03-18 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

I like the fact that we're moving the register methods into the respective 
class files. Among other things, this should make it easier for the 
instrumentation tool to insert register calls as well. However, I am not fond 
of introducing a public SB API call for something that should really be a 
private matter. One way to fix that would be to make the `Initialize` function 
private and make the SBReproducer class a friend, but perhaps it would be even 
better to not put this function into the public headers at all.

One way to achieve that would be to forward-declare some template function in 
SBReproducerPrivate.h (or maybe even a completely new header). Something like 
this ought to do the trick:

  namespace lldb_private { namespace repro {
  template void RegisterMethods(Registry &R);
  }}

Then each cpp file could specialize this function to do what it needs to do:

  namespace lldb_private { namespace repro {
  template<> void RegisterMethods(Registry &R) {
  LLDB_REGISTER_FOO(...);
  ...
  }
  }}

The reproducer initialization would become just a collection of 
`RegisterMethod()` calls, and no public API would be affected.

BTW: I believe the compiler errors are down to the fact that the REGISTER_FOO 
macros assume they are called from the context of `Registry` class. If we go 
down this path, then this will no longer be true, and the macros will need to 
be adjusted slightly (e.g. to take the registry as an extra argument or 
something). It technically is possible for SBFoo.cpp to implement a method 
belonging to the Registry class (which would avoid needing to modify the 
macro), but this would probably be just confusing.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59427/new/

https://reviews.llvm.org/D59427



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

[Lldb-commits] [PATCH] D59524: Improve error handling for Clang module imports

[Lldb-commits] [PATCH] D59524: Improve error handling for Clang module imports

[Lldb-commits] [lldb] r356416 - A target definition file that may work for

[Lldb-commits] [lldb] r356412 - [CMake] Set LLVM_DEFAULT_EXTERNAL_LIT in standalone build correctly on windows

[Lldb-commits] [PATCH] D59427: [WIP] [lldb] [API] Split SBRegistry into smaller files

[Lldb-commits] [PATCH] D59498: [DWARF] Remove a couple of log statements

[Lldb-commits] [PATCH] D59276: Delete dead code

[Lldb-commits] [PATCH] D59498: [DWARF] Remove a couple of log statements

[Lldb-commits] [lldb] r356401 - [API] Remove unneded LLDB_DISABLE_PYTHON markers.

[Lldb-commits] [PATCH] D59498: [DWARF] Remove a couple of log statements

[Lldb-commits] [PATCH] D59427: [WIP] [lldb] [API] Split SBRegistry into smaller files

[Lldb-commits] [PATCH] D59427: [WIP] [lldb] [API] Split SBRegistry into smaller files

[Lldb-commits] [PATCH] D59498: [DWARF] Remove a couple of log statements

[Lldb-commits] [PATCH] D59498: [DWARF] Remove a couple of log statements

[Lldb-commits] [PATCH] D59430: Update DwarfDebugInfoEntry to use llvm::Error and llvm::Expected

[Lldb-commits] [PATCH] D59276: Delete dead code

[Lldb-commits] [lldb] r356379 - Skip TestVSCode_setFunctionBreakpoints on linux

[Lldb-commits] [lldb] r356378 - Fix some "variable 'foo' set but not used" warnings

[Lldb-commits] [PATCH] D59495: Fix an out-of-bounds error in RegisterContextDarwin_arm64

[Lldb-commits] [lldb] r356377 - Fix libstdc++ data formatters for python3

[Lldb-commits] [lldb] r356370 - Fix TestCommandScriptImmediateOutput for python3

[Lldb-commits] [PATCH] D59482: [ObjectYAML] Add basic minidump generation support

[Lldb-commits] [PATCH] D59482: [ObjectYAML] Add basic minidump generation support

[Lldb-commits] [PATCH] D59485: [ASTImporter] Allow adding a shim to the ASTImporter

[Lldb-commits] [PATCH] D59485: [ASTImporter] Allow adding a shim to the ASTImporter

[Lldb-commits] [PATCH] D59485: [ASTImporter] Allow adding a shim to the ASTImporter

[Lldb-commits] [lldb] r356362 - Fix "type qualifiers ignored on cast result type" warnings

[Lldb-commits] [PATCH] D59482: [ObjectYAML] Add basic minidump generation support

[Lldb-commits] [PATCH] D58347: Reinitialize UnwindTable when the SymbolFile changes

[Lldb-commits] [lldb] r356361 - Reinitialize UnwindTable when the SymbolFile changes

[Lldb-commits] [PATCH] D59482: [ObjectYAML] Add basic minidump generation support

[Lldb-commits] [PATCH] D59433: Fix UUID decoding from minidump files.

[Lldb-commits] [PATCH] D59427: [WIP] [lldb] [API] Split SBRegistry into smaller files

33 matches

Site Navigation

Mail list logo

Footer information