[Lldb-commits] [PATCH] D74398: [lldb-server] jThreadsInfo returns stack memory

2020-02-12 Thread Jaroslav Sevcik via Phabricator via lldb-commits
jarin added a comment.

Thank you for the feedback! I am a bit busy with other things ATM, but I should 
be able to get back to this next week.

I should give credit where it's due - Pavel pointed me to the debug-server code 
and I took the idea from there. We do not always have the frame pointer list, 
so I took the hybrid approach of copying the top stack section from each 
thread, too.

As for some preliminary numbers, with 8ms RTT we see thread list update time in 
our UI improve from 800ms to 250ms in an Unreal engine sample. I will get more 
numbers (packet sizes) when I hack on this again next week.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74398



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


[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

2020-02-12 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. Thanks for doing this.


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

https://reviews.llvm.org/D74217



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


[Lldb-commits] [PATCH] D74478: [lldb] Let TypeSystemClang::GetDisplayTypeName remove anonymous and inline namespaces.

2020-02-12 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik added a comment.

I can see how stripping `__1` would be nice but I seeing `(anonymous 
namespace)` may be useful to know especially b/c it effects visibility and 
linkage. It would be nicer if could make this optional and default it off but 
be able to turn it back on.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D74478



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


[Lldb-commits] [PATCH] D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging

2020-02-12 Thread Paolo Severini via Phabricator via lldb-commits
paolosev updated this revision to Diff 244300.
paolosev added a comment.

Fix patch after rebasing:

- Use macros to initialize plugins in SystemInitializer.
- Move tests from 
/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/ to 
/test/API/functionalities/gdb_remote_client/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72751

Files:
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/DynamicLoader/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.cpp
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.h
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
  lldb/test/API/functionalities/gdb_remote_client/TestWasm.py
  lldb/test/API/functionalities/gdb_remote_client/test_sym.yaml
  
lldb/test/API/functionalities/gdb_remote_client/test_wasm_embedded_debug_sections.yaml
  
lldb/test/API/functionalities/gdb_remote_client/test_wasm_external_debug_sections.yaml
  lldb/test/Shell/ObjectFile/wasm/basic.yaml
  lldb/test/Shell/ObjectFile/wasm/embedded-debug-sections.yaml
  lldb/test/Shell/ObjectFile/wasm/stripped-debug-sections.yaml
  lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml
  lldb/tools/lldb-test/SystemInitializerTest.cpp

Index: lldb/tools/lldb-test/SystemInitializerTest.cpp
===
--- lldb/tools/lldb-test/SystemInitializerTest.cpp
+++ lldb/tools/lldb-test/SystemInitializerTest.cpp
@@ -99,6 +99,7 @@
 LLDB_PLUGIN_DECLARE(DynamicLoaderMacOS)
 LLDB_PLUGIN_DECLARE(DynamicLoaderPOSIXDYLD)
 LLDB_PLUGIN_DECLARE(DynamicLoaderStatic)
+LLDB_PLUGIN_DECLARE(DynamicLoaderWasmDYLD)
 LLDB_PLUGIN_DECLARE(DynamicLoaderWindowsDYLD)
 
 using namespace lldb_private;
@@ -236,6 +237,7 @@
   LLDB_PLUGIN_INITIALIZE(DynamicLoaderMacOSXDYLD);
   LLDB_PLUGIN_INITIALIZE(DynamicLoaderMacOS);
   LLDB_PLUGIN_INITIALIZE(DynamicLoaderPOSIXDYLD);
+  LLDB_PLUGIN_INITIALIZE(DynamicLoaderWasmDYLD); // Before DynamicLoaderStatic.
   LLDB_PLUGIN_INITIALIZE(DynamicLoaderStatic);
   LLDB_PLUGIN_INITIALIZE(DynamicLoaderWindowsDYLD);
 
@@ -324,6 +326,7 @@
   LLDB_PLUGIN_TERMINATE(DynamicLoaderMacOSXDYLD);
   LLDB_PLUGIN_TERMINATE(DynamicLoaderMacOS);
   LLDB_PLUGIN_TERMINATE(DynamicLoaderPOSIXDYLD);
+  LLDB_PLUGIN_TERMINATE(DynamicLoaderWasmDYLD);
   LLDB_PLUGIN_TERMINATE(DynamicLoaderStatic);
   LLDB_PLUGIN_TERMINATE(DynamicLoaderWindowsDYLD);
 
Index: lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml
===
--- lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml
+++ lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml
@@ -13,11 +13,11 @@
 # CHECK: Plugin name: wasm
 # CHECK: Architecture: wasm32-unknown-unknown-wasm
 # CHECK: UUID: 
-# CHECK: Executable: true
+# CHECK: Executable: false
 # CHECK: Stripped: true
-# CHECK: Type: executable
+# CHECK: Type: shared library
 # CHECK: Strata: user
-# CHECK: Base VM address: 0xa
+# CHECK: Base VM address: 0x0
 
 # CHECK: Name: code
 # CHECK: Type: code
Index: lldb/test/Shell/ObjectFile/wasm/stripped-debug-sections.yaml
===
--- lldb/test/Shell/ObjectFile/wasm/stripped-debug-sections.yaml
+++ lldb/test/Shell/ObjectFile/wasm/stripped-debug-sections.yaml
@@ -4,9 +4,9 @@
 # CHECK: Plugin name: wasm
 # CHECK: Architecture: wasm32-unknown-unknown-wasm
 # CHECK: UUID: 
-# CHECK: Executable: true
-# CHECK: Stripped: true
-# CHECK: Type: executable
+# CHECK: Executable: false
+# CHECK: Stripped: false
+# CHECK: Type: shared library
 # CHECK: Strata: user
 # CHECK: Base VM address: 0x0
 
Index: lldb/test/Shell/ObjectFile/wasm/embedded-debug-sections.yaml
===
--- lldb/test/Shell/ObjectFile/wasm/embedded-debug-sections.yaml
+++ lldb/test/Shell/ObjectFile/wasm/embedded-debug-sections.yaml
@@ -4,11 +4,11 @@
 # CHECK: Plugin name: wasm
 # CHECK: Architecture: wasm32-unknown-unknown-wasm
 # CHECK: UUID: 
-# CHECK: Executable: true
-# CHECK: Stripped: true
-# CHECK: Type: executable
+# CHECK: Executable: false
+# CHECK: Stripped: false
+# CHECK: Type: shared library
 # CHECK: Strata: user
-# CHECK: Base VM address: 0xa
+# CHECK: Base VM address: 0x0
 
 # CHECK: Name: code
 # CHECK: Type: code
Index: lldb/test/Shell/ObjectFile/wasm/basic.yaml
===
--- lldb/test/Shell/ObjectFile/wasm/basic.yaml
+++ lldb/test/Shell/ObjectFile/wasm/basic.yaml
@@ -4,11 +4,11 @@
 # CHECK: Plugin name: wasm
 # CHECK: Architecture: wasm32-unknown-unknown-wasm
 # CHECK: UUID: 
-# CHECK: Executable: true
-# CHECK: Stripped: true
-# CHECK: Type: executable
+# CHECK: Executable: false
+# CHECK: Stripped: false
+# CHECK: 

[Lldb-commits] [PATCH] D73067: [lldb/CMake] Auto-generate the Initialize and Terminate calls for plugin

2020-02-12 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 244299.
JDevlieghere added a comment.
Herald added a subscriber: emaste.

Complete rewrite based on the external forward declarations.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D73067

Files:
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/ABI/ARC/ABISysV_arc.cpp
  lldb/source/Plugins/ABI/ARM/ABIMacOSX_arm.cpp
  lldb/source/Plugins/ABI/Hexagon/ABISysV_hexagon.cpp
  lldb/source/Plugins/ABI/SystemZ/ABISysV_s390x.cpp
  lldb/source/Plugins/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/Hexagon-DYLD/DynamicLoaderHexagonDYLD.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  lldb/source/Plugins/ExpressionParser/Clang/CMakeLists.txt
  lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
  lldb/source/Plugins/Language/ObjC/CMakeLists.txt
  lldb/source/Plugins/LanguageRuntime/CPlusPlus/CMakeLists.txt
  lldb/source/Plugins/LanguageRuntime/ObjC/CMakeLists.txt
  lldb/source/Plugins/ObjectFile/JIT/ObjectFileJIT.cpp
  lldb/source/Plugins/Platform/POSIX/CMakeLists.txt
  lldb/source/Plugins/Plugins.def.in
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/tools/lldb-test/CMakeLists.txt
  lldb/tools/lldb-test/SystemInitializerTest.cpp
  lldb/unittests/Disassembler/CMakeLists.txt
  lldb/unittests/UnwindAssembly/ARM64/CMakeLists.txt
  lldb/unittests/UnwindAssembly/PPC64/CMakeLists.txt

Index: lldb/unittests/UnwindAssembly/PPC64/CMakeLists.txt
===
--- lldb/unittests/UnwindAssembly/PPC64/CMakeLists.txt
+++ lldb/unittests/UnwindAssembly/PPC64/CMakeLists.txt
@@ -5,7 +5,7 @@
 lldbSymbol
 lldbTarget
 lldbPluginUnwindAssemblyInstEmulation
-lldbPluginDisassemblerLLVM
+lldbPluginDisassemblerLLVMC
 lldbPluginInstructionPPC64
 lldbPluginProcessUtility
   LINK_COMPONENTS
Index: lldb/unittests/UnwindAssembly/ARM64/CMakeLists.txt
===
--- lldb/unittests/UnwindAssembly/ARM64/CMakeLists.txt
+++ lldb/unittests/UnwindAssembly/ARM64/CMakeLists.txt
@@ -5,7 +5,7 @@
 lldbSymbol
 lldbTarget
 lldbPluginUnwindAssemblyInstEmulation
-lldbPluginDisassemblerLLVM
+lldbPluginDisassemblerLLVMC
 lldbPluginInstructionARM64
 lldbPluginProcessUtility
   LINK_COMPONENTS
Index: lldb/unittests/Disassembler/CMakeLists.txt
===
--- lldb/unittests/Disassembler/CMakeLists.txt
+++ lldb/unittests/Disassembler/CMakeLists.txt
@@ -6,7 +6,7 @@
   lldbCore
   lldbSymbol
   lldbTarget
-  lldbPluginDisassemblerLLVM
+  lldbPluginDisassemblerLLVMC
   lldbPluginProcessUtility
 LINK_COMPONENTS
   Support
Index: lldb/tools/lldb-test/SystemInitializerTest.cpp
===
--- lldb/tools/lldb-test/SystemInitializerTest.cpp
+++ lldb/tools/lldb-test/SystemInitializerTest.cpp
@@ -17,88 +17,9 @@
 
 #include 
 
-LLDB_PLUGIN_DECLARE(ABIMacOSX_arm64)
-LLDB_PLUGIN_DECLARE(ABISysV_arm64)
-LLDB_PLUGIN_DECLARE(ABIMacOSX_arm)
-LLDB_PLUGIN_DECLARE(ABISysV_arm)
-LLDB_PLUGIN_DECLARE(ABISysV_arc)
-LLDB_PLUGIN_DECLARE(ABISysV_hexagon)
-LLDB_PLUGIN_DECLARE(ABISysV_mips)
-LLDB_PLUGIN_DECLARE(ABISysV_mips64)
-LLDB_PLUGIN_DECLARE(ABISysV_ppc)
-LLDB_PLUGIN_DECLARE(ABISysV_ppc64)
-LLDB_PLUGIN_DECLARE(ABISysV_s390x)
-LLDB_PLUGIN_DECLARE(ABIMacOSX_i386)
-LLDB_PLUGIN_DECLARE(ABISysV_i386)
-LLDB_PLUGIN_DECLARE(ABISysV_x86_64)
-LLDB_PLUGIN_DECLARE(ABIWindows_x86_64)
-LLDB_PLUGIN_DECLARE(ObjectFileBreakpad)
-LLDB_PLUGIN_DECLARE(ObjectFileELF)
-LLDB_PLUGIN_DECLARE(ObjectFileMachO)
-LLDB_PLUGIN_DECLARE(ObjectFilePECOFF)
-LLDB_PLUGIN_DECLARE(ObjectFileWasm)
-LLDB_PLUGIN_DECLARE(ObjectContainerBSDArchive)
-LLDB_PLUGIN_DECLARE(ObjectContainerUniversalMachO)
-LLDB_PLUGIN_DECLARE(ScriptInterpreterNone)
-LLDB_PLUGIN_DECLARE(PlatformFreeBSD)
-LLDB_PLUGIN_DECLARE(PlatformLinux)
-LLDB_PLUGIN_DECLARE(PlatformNetBSD)
-LLDB_PLUGIN_DECLARE(PlatformOpenBSD)
-LLDB_PLUGIN_DECLARE(PlatformWindows)
-LLDB_PLUGIN_DECLARE(PlatformAndroid)
-LLDB_PLUGIN_DECLARE(PlatformMacOSX)
-LLDB_PLUGIN_DECLARE(TypeSystemClang)
-LLDB_PLUGIN_DECLARE(ArchitectureArm)
-LLDB_PLUGIN_DECLARE(ArchitectureMips)
-LLDB_PLUGIN_DECLARE(ArchitecturePPC64)
-LLDB_PLUGIN_DECLARE(DisassemblerLLVMC)
-LLDB_PLUGIN_DECLARE(JITLoaderGDB)
-LLDB_PLUGIN_DECLARE(ProcessElfCore)
-LLDB_PLUGIN_DECLARE(ProcessMachCore)
-LLDB_PLUGIN_DECLARE(ProcessMinidump)
-LLDB_PLUGIN_DECLARE(MemoryHistoryASan)
-LLDB_PLUGIN_DECLARE(InstrumentationRuntimeASan)
-LLDB_PLUGIN_DECLARE(InstrumentationRuntimeTSan)
-LLDB_PLUGIN_DECLARE(InstrumentationRuntimeUBSan)
-LLDB_PLUGIN_DECLARE(InstrumentationRuntimeMainThreadChecker)
-LLDB_PLUGIN_DECLARE(SymbolVendorELF)
-LLDB_PLUGIN_DECLARE(SymbolFileBreakpad)
-LLDB_PLUGIN_DECLARE(SymbolFileDWARF)

[Lldb-commits] [PATCH] D74398: [lldb-server] jThreadsInfo returns stack memory

2020-02-12 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda added a comment.

Nice improvement.  debugserver does a similar thing in our jThreadsInfo 
implementation as well as our ? packet response (T05, etc) where we include the 
current & caller stack frame, assuming the frame pointer and pc are saved to 
the stack,

1581545891.235902071 < 627> read packet: 
$T05thread:d4414d;threads:d4414d;thread-pcs:7fff7149ffd0;00:;01:28f9bfeffe7f;02:0200;
 [...] 
memory:0x7ffeefbfe4a0=d0e4bfeffe7ff20d2971ff7f;memory:0x7ffeefbfe4d0=f0e4bfeffe7f66cf2971ff7f;#00

This gives the thread plans enough information that we can eliminate extra 
reads while instruction stepping.  We try to do a full stack walk using the 
same "follow the $fp chain" algorithm for the jThreadsInfo packet for all 
threads.

It's interesting to send up the entire stack frame contents for frame 0.  
There's a good chance we'll need to fetch some/all of this for the selected 
frame (the frame the user is looking at), but it's probably not helping 
anything for the other threads.

As Pavel was suggesting, I'd get the size of the inferior's pointers from lldb, 
and I would read the bytes for the saved frame pointer into a buffer, wrap it 
in a DataExtractor, and let it do the correct byte swapping.

We'll still have an assumption that the stack moves downward, and that the 
saved pc and frame pointer are at the start of every stack frame.  It's fine 
for x86/arm, but we probably have support for an architecture where this isn't 
true by this point, I wouldn't be surprised.  We'll stop on the first memory 
read, so if we get back a bogus spilled-fp value, we will stop scanning, so I 
wouldn't expect it to cause problems, but those targets may not benefit from 
this.  (debugserver only works on x86/arm, so it didn't have to figure this 
out.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74398



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


[Lldb-commits] [PATCH] D73665: Stop emitting a breakpoint for each location in a breakpoint when responding to breakpoint commands.

2020-02-12 Thread Greg Clayton via Phabricator via lldb-commits
clayborg updated this revision to Diff 244275.
clayborg added a comment.

Added a test to cover breakpoint events. The test has an executable and a 
shared library and we set breakpoints in the main executable and in the shared 
library. When we run, we stop at the entry point to the program and then the 
breakpoints are asked to be set. The breakpoint in the main executable should 
be resolved, and the one in the shared library should not be resolved since the 
shared library hasn't been loaded yet. We also set a breakpoint via the 
"preRunCommands" in the "launch" packet. This ensures we set a breakpoint using 
a LLDB command for which we expect to not get any breakpoint events for since 
the breakpoint wasn't set via VS Code DAP packets. After we stop, at the entry 
point we verify no breakpoint events have been received, and then we run to one 
of the breakpoints that was set via the VS Code DAP packets and check that we 
received only one event for the breakpoint set via DAP packets and not for the 
LLDB command created breakpoint.

Fixed lldbvscode_testcase.py set_source_breakpoints and 
set_function_breakpoints to return breakpoint IDs and not breakpoint ID + 
location ID. Also fixed verify_breakpoint_hit so that it actally checks that 
breakpoints were being set by now using an assertTrue call. Prior to this, this 
function would return true or false, but no one was looking at the result. 
Looking around the code it seems this should assert and cause the test to fail.

Modified vscode.py to store any breakpoint events in the object so tests can 
access these events in the test python code for verification.

Modified the "description" field of "stopped" events when we stop at 
breakpoints to contain "breakpoint %u" where %u is the breakpoint ID. This 
allows lldbvscode_testcase.verify_breakpoint_hit() to actually work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73665

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/test/API/tools/lldb-vscode/breakpoint-events/Makefile
  
lldb/test/API/tools/lldb-vscode/breakpoint-events/TestVSCode_breakpointEvents.py
  lldb/test/API/tools/lldb-vscode/breakpoint-events/foo.cpp
  lldb/test/API/tools/lldb-vscode/breakpoint-events/foo.h
  lldb/test/API/tools/lldb-vscode/breakpoint-events/main.cpp
  lldb/tools/lldb-vscode/BreakpointBase.cpp
  lldb/tools/lldb-vscode/BreakpointBase.h
  lldb/tools/lldb-vscode/ExceptionBreakpoint.cpp
  lldb/tools/lldb-vscode/FunctionBreakpoint.cpp
  lldb/tools/lldb-vscode/JSONUtils.cpp
  lldb/tools/lldb-vscode/JSONUtils.h
  lldb/tools/lldb-vscode/LLDBUtils.cpp
  lldb/tools/lldb-vscode/LLDBUtils.h
  lldb/tools/lldb-vscode/SourceBreakpoint.cpp
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -379,27 +379,20 @@
 if (event_mask & lldb::SBTarget::eBroadcastBitBreakpointChanged) {
   auto event_type =
   lldb::SBBreakpoint::GetBreakpointEventTypeFromEvent(event);
-  const auto num_locs =
-  lldb::SBBreakpoint::GetNumBreakpointLocationsFromEvent(event);
   auto bp = lldb::SBBreakpoint::GetBreakpointFromEvent(event);
-  bool added = event_type & lldb::eBreakpointEventTypeLocationsAdded;
-  bool removed =
-  event_type & lldb::eBreakpointEventTypeLocationsRemoved;
-  if (added || removed) {
-for (size_t i = 0; i < num_locs; ++i) {
-  auto bp_loc =
-  lldb::SBBreakpoint::GetBreakpointLocationAtIndexFromEvent(
-  event, i);
-  auto bp_event = CreateEventObject("breakpoint");
-  llvm::json::Object body;
-  body.try_emplace("breakpoint", CreateBreakpoint(bp_loc));
-  if (added)
-body.try_emplace("reason", "new");
-  else
-body.try_emplace("reason", "removed");
-  bp_event.try_emplace("body", std::move(body));
-  g_vsc.SendJSON(llvm::json::Value(std::move(bp_event)));
-}
+  // If the breakpoint was originated from the IDE, it will have the
+  // BreakpointBase::GetBreakpointLabel() label attached. Regardless
+  // of wether the locations were added or removed, the breakpoint
+  // ins't going away, so we the reason is always "changed".
+  if ((event_type & lldb::eBreakpointEventTypeLocationsAdded ||
+   event_type & lldb::eBreakpointEventTypeLocationsRemoved) &&
+  bp.MatchesName(BreakpointBase::GetBreakpointLabel())) {
+auto bp_event = CreateEventObject("breakpoint");
+llvm::json::Object body;
+ 

[Lldb-commits] [lldb] 6e30fd0 - [lldb/Plugins] Move DynamicLoaderMacOS into DynamicLoaderMacOSXDYLD (NFCI)

2020-02-12 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2020-02-12T13:44:20-08:00
New Revision: 6e30fd05c92dff39cc196fadaa0489b907b29be8

URL: 
https://github.com/llvm/llvm-project/commit/6e30fd05c92dff39cc196fadaa0489b907b29be8
DIFF: 
https://github.com/llvm/llvm-project/commit/6e30fd05c92dff39cc196fadaa0489b907b29be8.diff

LOG: [lldb/Plugins] Move DynamicLoaderMacOS into DynamicLoaderMacOSXDYLD (NFCI)

Move the logic for initialization and termination for DynamicLoaderMacOS
into DynamicLoaderMacOSXDYLD so that there's one initializer for the
DynamicLoaderMacOSXDYLD plugin.

Added: 


Modified: 
lldb/source/API/SystemInitializerFull.cpp
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
lldb/tools/lldb-test/SystemInitializerTest.cpp

Removed: 




diff  --git a/lldb/source/API/SystemInitializerFull.cpp 
b/lldb/source/API/SystemInitializerFull.cpp
index bf8ace9d9280..888a7579f84d 100644
--- a/lldb/source/API/SystemInitializerFull.cpp
+++ b/lldb/source/API/SystemInitializerFull.cpp
@@ -110,7 +110,6 @@ LLDB_PLUGIN_DECLARE(StructuredDataDarwinLog)
 LLDB_PLUGIN_DECLARE(PlatformRemoteGDBServer)
 LLDB_PLUGIN_DECLARE(ProcessGDBRemote)
 LLDB_PLUGIN_DECLARE(DynamicLoaderMacOSXDYLD)
-LLDB_PLUGIN_DECLARE(DynamicLoaderMacOS)
 LLDB_PLUGIN_DECLARE(DynamicLoaderPOSIXDYLD)
 LLDB_PLUGIN_DECLARE(DynamicLoaderStatic)
 LLDB_PLUGIN_DECLARE(DynamicLoaderWindowsDYLD)
@@ -259,7 +258,6 @@ llvm::Error SystemInitializerFull::Initialize() {
 
   LLDB_PLUGIN_INITIALIZE(ProcessGDBRemote);
   LLDB_PLUGIN_INITIALIZE(DynamicLoaderMacOSXDYLD);
-  LLDB_PLUGIN_INITIALIZE(DynamicLoaderMacOS);
   LLDB_PLUGIN_INITIALIZE(DynamicLoaderPOSIXDYLD);
   LLDB_PLUGIN_INITIALIZE(DynamicLoaderStatic);
   LLDB_PLUGIN_INITIALIZE(DynamicLoaderWindowsDYLD);
@@ -347,7 +345,6 @@ void SystemInitializerFull::Terminate() {
   LLDB_PLUGIN_TERMINATE(StructuredDataDarwinLog);
 
   LLDB_PLUGIN_TERMINATE(DynamicLoaderMacOSXDYLD);
-  LLDB_PLUGIN_TERMINATE(DynamicLoaderMacOS);
   LLDB_PLUGIN_TERMINATE(DynamicLoaderPOSIXDYLD);
   LLDB_PLUGIN_TERMINATE(DynamicLoaderStatic);
   LLDB_PLUGIN_TERMINATE(DynamicLoaderWindowsDYLD);

diff  --git 
a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp 
b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
index 2ebac49f31aa..7bc14061ffe0 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
@@ -28,8 +28,6 @@
 using namespace lldb;
 using namespace lldb_private;
 
-LLDB_PLUGIN(DynamicLoaderMacOS)
-
 // Create an instance of this class. This function is filled into the plugin
 // info class that gets handed out by the plugin factory and allows the lldb to
 // instantiate an instance of this class.

diff  --git 
a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp 
b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
index 53f1bd09b6c1..9cb6d1fcb612 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
@@ -6,6 +6,11 @@
 //
 
//===--===//
 
+#include "DynamicLoaderMacOSXDYLD.h"
+#include "DynamicLoaderDarwin.h"
+#include "DynamicLoaderMacOS.h"
+#include "Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h"
+#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "lldb/Breakpoint/StoppointCallbackContext.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/Module.h"
@@ -25,12 +30,6 @@
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/State.h"
 
-#include "DynamicLoaderDarwin.h"
-#include "DynamicLoaderMacOSXDYLD.h"
-
-#include "Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h"
-#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
-
 //#define ENABLE_DEBUG_PRINTF // COMMENT THIS LINE OUT PRIOR TO CHECKIN
 #ifdef ENABLE_DEBUG_PRINTF
 #include 
@@ -1123,9 +1122,11 @@ bool DynamicLoaderMacOSXDYLD::GetSharedCacheInformation(
 void DynamicLoaderMacOSXDYLD::Initialize() {
   PluginManager::RegisterPlugin(GetPluginNameStatic(),
 GetPluginDescriptionStatic(), CreateInstance);
+  DynamicLoaderMacOS::Initialize();
 }
 
 void DynamicLoaderMacOSXDYLD::Terminate() {
+  DynamicLoaderMacOS::Terminate();
   PluginManager::UnregisterPlugin(CreateInstance);
 }
 

diff  --git a/lldb/tools/lldb-test/SystemInitializerTest.cpp 
b/lldb/tools/lldb-test/SystemInitializerTest.cpp
index 6835bda6ebdb..3e82b8cbf1b0 100644
--- a/lldb/tools/lldb-test/SystemInitializerTest.cpp
+++ b/lldb/tools/lldb-test/SystemInitializerTest.cpp
@@ -96,7 +96,6 @@ LLDB_PLUGIN_DECLARE(StructuredDataDarwinLog)
 LLDB_PLUGIN_DECLARE(PlatformRemoteGDBServer)
 LLDB_PLUGIN_DECLARE(ProcessGDBRemote)
 

[Lldb-commits] [lldb] 654086c - [lldb/Plugins] Move SymbolFileDWARFDebugMap into SymbolFileDWARF (NFCI)

2020-02-12 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2020-02-12T11:30:17-08:00
New Revision: 654086cbf548c8571affcb24f4a4a1cf54988756

URL: 
https://github.com/llvm/llvm-project/commit/654086cbf548c8571affcb24f4a4a1cf54988756
DIFF: 
https://github.com/llvm/llvm-project/commit/654086cbf548c8571affcb24f4a4a1cf54988756.diff

LOG: [lldb/Plugins] Move SymbolFileDWARFDebugMap into SymbolFileDWARF (NFCI)

Move the logic for initialization and termination for
SymbolFileDWARFDebugMap into SymbolFileDWARF so that there's one
initializer for the SymbolFileDWARF plugin.

Added: 


Modified: 
lldb/source/API/SystemInitializerFull.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
lldb/tools/lldb-test/SystemInitializerTest.cpp

Removed: 




diff  --git a/lldb/source/API/SystemInitializerFull.cpp 
b/lldb/source/API/SystemInitializerFull.cpp
index 4290d9ef6f2e..bf8ace9d9280 100644
--- a/lldb/source/API/SystemInitializerFull.cpp
+++ b/lldb/source/API/SystemInitializerFull.cpp
@@ -88,7 +88,6 @@ LLDB_PLUGIN_DECLARE(EmulateInstructionARM64)
 LLDB_PLUGIN_DECLARE(EmulateInstructionMIPS)
 LLDB_PLUGIN_DECLARE(EmulateInstructionMIPS64)
 LLDB_PLUGIN_DECLARE(EmulateInstructionPPC64)
-LLDB_PLUGIN_DECLARE(SymbolFileDWARFDebugMap)
 LLDB_PLUGIN_DECLARE(ItaniumABILanguageRuntime)
 LLDB_PLUGIN_DECLARE(AppleObjCRuntime)
 LLDB_PLUGIN_DECLARE(SystemRuntimeMacOSX)
@@ -230,7 +229,6 @@ llvm::Error SystemInitializerFull::Initialize() {
   LLDB_PLUGIN_INITIALIZE(EmulateInstructionMIPS64);
   LLDB_PLUGIN_INITIALIZE(EmulateInstructionPPC64);
 
-  LLDB_PLUGIN_INITIALIZE(SymbolFileDWARFDebugMap);
   LLDB_PLUGIN_INITIALIZE(ItaniumABILanguageRuntime);
   LLDB_PLUGIN_INITIALIZE(AppleObjCRuntime);
   LLDB_PLUGIN_INITIALIZE(SystemRuntimeMacOSX);
@@ -324,7 +322,6 @@ void SystemInitializerFull::Terminate() {
   LLDB_PLUGIN_TERMINATE(EmulateInstructionMIPS64);
   LLDB_PLUGIN_TERMINATE(EmulateInstructionPPC64);
 
-  LLDB_PLUGIN_TERMINATE(SymbolFileDWARFDebugMap);
   LLDB_PLUGIN_TERMINATE(ItaniumABILanguageRuntime);
   LLDB_PLUGIN_TERMINATE(AppleObjCRuntime);
   LLDB_PLUGIN_TERMINATE(SystemRuntimeMacOSX);

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 98cd34936522..39484f0a198b 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -226,6 +226,7 @@ void SymbolFileDWARF::Initialize() {
   PluginManager::RegisterPlugin(GetPluginNameStatic(),
 GetPluginDescriptionStatic(), CreateInstance,
 DebuggerInitialize);
+  SymbolFileDWARFDebugMap::Initialize();
 }
 
 void SymbolFileDWARF::DebuggerInitialize(Debugger ) {
@@ -240,6 +241,7 @@ void SymbolFileDWARF::DebuggerInitialize(Debugger 
) {
 }
 
 void SymbolFileDWARF::Terminate() {
+  SymbolFileDWARFDebugMap::Terminate();
   PluginManager::UnregisterPlugin(CreateInstance);
   LogChannelDWARF::Terminate();
 }

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
index 218aadc82169..01e50efdcb5f 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
@@ -39,8 +39,6 @@
 using namespace lldb;
 using namespace lldb_private;
 
-LLDB_PLUGIN(SymbolFileDWARFDebugMap)
-
 char SymbolFileDWARFDebugMap::ID;
 
 // Subclass lldb_private::Module so we can intercept the

diff  --git a/lldb/tools/lldb-test/SystemInitializerTest.cpp 
b/lldb/tools/lldb-test/SystemInitializerTest.cpp
index 46ef4d11566c..6835bda6ebdb 100644
--- a/lldb/tools/lldb-test/SystemInitializerTest.cpp
+++ b/lldb/tools/lldb-test/SystemInitializerTest.cpp
@@ -74,7 +74,6 @@ LLDB_PLUGIN_DECLARE(EmulateInstructionARM64)
 LLDB_PLUGIN_DECLARE(EmulateInstructionMIPS)
 LLDB_PLUGIN_DECLARE(EmulateInstructionMIPS64)
 LLDB_PLUGIN_DECLARE(EmulateInstructionPPC64)
-LLDB_PLUGIN_DECLARE(SymbolFileDWARFDebugMap)
 LLDB_PLUGIN_DECLARE(ItaniumABILanguageRuntime)
 LLDB_PLUGIN_DECLARE(AppleObjCRuntime)
 LLDB_PLUGIN_DECLARE(SystemRuntimeMacOSX)
@@ -205,7 +204,6 @@ llvm::Error SystemInitializerTest::Initialize() {
   LLDB_PLUGIN_INITIALIZE(EmulateInstructionMIPS64);
   LLDB_PLUGIN_INITIALIZE(EmulateInstructionPPC64);
 
-  LLDB_PLUGIN_INITIALIZE(SymbolFileDWARFDebugMap);
   LLDB_PLUGIN_INITIALIZE(ItaniumABILanguageRuntime);
   LLDB_PLUGIN_INITIALIZE(AppleObjCRuntime);
   LLDB_PLUGIN_INITIALIZE(SystemRuntimeMacOSX);
@@ -299,7 +297,6 @@ void SystemInitializerTest::Terminate() {
   LLDB_PLUGIN_TERMINATE(EmulateInstructionMIPS64);
   LLDB_PLUGIN_TERMINATE(EmulateInstructionPPC64);
 
-  LLDB_PLUGIN_TERMINATE(SymbolFileDWARFDebugMap);
   LLDB_PLUGIN_TERMINATE(ItaniumABILanguageRuntime);
   

[Lldb-commits] [PATCH] D73860: [lldb/StringPrinter] Avoid reading garbage in uninitialized strings

2020-02-12 Thread Vedant Kumar via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7aabad131288: [lldb/StringPrinter] Avoid reading garbage in 
uninitialized strings (authored by vsk).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D73860?vs=242661=244233#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73860

Files:
  lldb/source/DataFormatters/StringPrinter.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
@@ -1,4 +1,58 @@
 #include 
+#include 
+
+// For more information about libc++'s std::string ABI, see:
+//
+//   https://joellaity.com/2020/01/31/string.html
+
+// A corrupt string which hits the SSO code path, but has an invalid size.
+static struct {
+  // Set the size of this short-mode string to 116. Note that in short mode,
+  // the size is encoded as `size << 1`.
+  unsigned char size = 232;
+
+  // 23 garbage bytes for the inline string payload.
+  char inline_buf[23] = {0};
+} garbage_string_short_mode;
+
+// A corrupt libcxx string in long mode with a payload that contains a utf8
+// sequence that's inherently too long.
+static unsigned char garbage_utf8_payload1[] = {
+  250 // This means that we expect a 5-byte sequence, this is invalid.
+};
+static struct {
+  uint64_t cap = 5;
+  uint64_t size = 4;
+  unsigned char *data = _utf8_payload1[0];
+} garbage_string_long_mode1;
+
+// A corrupt libcxx string in long mode with a payload that contains a utf8
+// sequence that's too long to fit in the buffer.
+static unsigned char garbage_utf8_payload2[] = {
+  240 // This means that we expect a 4-byte sequence, but the buffer is too
+  // small for this.
+};
+static struct {
+  uint64_t cap = 3;
+  uint64_t size = 2;
+  unsigned char *data = _utf8_payload1[0];
+} garbage_string_long_mode2;
+
+// A corrupt libcxx string which has an invalid size (i.e. a size greater than
+// the capacity of the string).
+static struct {
+  uint64_t cap = 5;
+  uint64_t size = 7;
+  const char *data = "foo";
+} garbage_string_long_mode3;
+
+// A corrupt libcxx string in long mode with a payload that would trigger a
+// buffer overflow.
+static struct {
+  uint64_t cap = 5;
+  uint64_t size = 2;
+  uint64_t data = 0xfffeULL;
+} garbage_string_long_mode4;
 
 int main()
 {
@@ -17,6 +71,23 @@
 std::u32string u32_string(U"");
 std::u32string u32_empty(U"");
 std::basic_string uchar(5, 'a');
+
+#if _LIBCPP_ABI_VERSION == 1
+std::string garbage1, garbage2, garbage3, garbage4, garbage5;
+if (sizeof(std::string) == sizeof(garbage_string_short_mode))
+  memcpy((void *), _string_short_mode, sizeof(std::string));
+if (sizeof(std::string) == sizeof(garbage_string_long_mode1))
+  memcpy((void *), _string_long_mode1, sizeof(std::string));
+if (sizeof(std::string) == sizeof(garbage_string_long_mode2))
+  memcpy((void *), _string_long_mode2, sizeof(std::string));
+if (sizeof(std::string) == sizeof(garbage_string_long_mode3))
+  memcpy((void *), _string_long_mode3, sizeof(std::string));
+if (sizeof(std::string) == sizeof(garbage_string_long_mode4))
+  memcpy((void *), _string_long_mode4, sizeof(std::string));
+#else
+#error "Test potentially needs to be updated for a new std::string ABI."
+#endif
+
 S.assign(L"!"); // Set break point at this line.
 return 0;
 }
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
@@ -51,6 +51,8 @@
 "settings set target.max-children-count 256",
 check=False)
 
+is_64_bit = self.process().GetAddressByteSize() == 8
+
 # Execute the cleanup function during test case tear down.
 self.addTearDownHook(cleanup)
 
@@ -114,3 +116,10 @@
 '(%s::basic_string, '
 '%s::allocator >) uchar = "a"'%(ns,ns,ns),
 ])
+
+if is_64_bit:
+self.expect("frame variable garbage1", substrs=['garbage1 = Summary Unavailable'])
+self.expect("frame variable garbage2", substrs=['garbage2 = 

[Lldb-commits] [PATCH] D73808: [lldb/TypeSystemClang] Supply trivial TypeSourceInfo to NonTypeTemplateParmDecl::Create

2020-02-12 Thread Vedant Kumar via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd6e47a405a34: [lldb/TypeSystemClang] Supply trivial 
TypeSourceInfo to NonTypeTemplateParmDecl… (authored by vsk).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D73808?vs=241819=244232#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73808

Files:
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/test/API/lang/cpp/class-template-parameter-pack/main.cpp


Index: lldb/test/API/lang/cpp/class-template-parameter-pack/main.cpp
===
--- lldb/test/API/lang/cpp/class-template-parameter-pack/main.cpp
+++ lldb/test/API/lang/cpp/class-template-parameter-pack/main.cpp
@@ -34,6 +34,11 @@
   bool isIntBool() { return true; }
 };
 
+template struct array {
+  int Arr[Size];
+  array() {}
+};
+
 int main (int argc, char const *argv[])
 {
 C myC;
@@ -53,12 +58,15 @@
 D myLesserD;
 myD.member = 64;
 (void)D().isIntBool();
-(void)D().isIntBool();
-return myD.member != 64;   //% self.expect("expression -- myD", 
DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["64"])
+(void)D().isIntBool(); //% self.expect("expression -- myD", 
DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["64"])
 //% self.expect("expression -- 
myLesserD.isIntBool()", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["false"])
 //% self.expect("expression -- 
myD.isIntBool()", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["true"])
 
 // See comment above.
 //#% self.expect("expression -- D().isIntBool()", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["false"])
 //#% self.expect("expression -- D().isIntBool()", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["true"])
+
+array<3> myArray; //% self.expect("expression -- myArray", 
DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["Arr"])
+
+return 1;
 }
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -1277,11 +1277,12 @@
 if (name && name[0])
   identifier_info = (name);
 if (IsValueParam(template_param_infos.args[i])) {
+  QualType template_param_type =
+  template_param_infos.args[i].getIntegralType();
   template_param_decls.push_back(NonTypeTemplateParmDecl::Create(
   ast, decl_context, SourceLocation(), SourceLocation(), depth, i,
-  identifier_info, template_param_infos.args[i].getIntegralType(),
-  parameter_pack, nullptr));
-
+  identifier_info, template_param_type, parameter_pack,
+  ast.getTrivialTypeSourceInfo(template_param_type)));
 } else {
   template_param_decls.push_back(TemplateTypeParmDecl::Create(
   ast, decl_context, SourceLocation(), SourceLocation(), depth, i,


Index: lldb/test/API/lang/cpp/class-template-parameter-pack/main.cpp
===
--- lldb/test/API/lang/cpp/class-template-parameter-pack/main.cpp
+++ lldb/test/API/lang/cpp/class-template-parameter-pack/main.cpp
@@ -34,6 +34,11 @@
   bool isIntBool() { return true; }
 };
 
+template struct array {
+  int Arr[Size];
+  array() {}
+};
+
 int main (int argc, char const *argv[])
 {
 C myC;
@@ -53,12 +58,15 @@
 D myLesserD;
 myD.member = 64;
 (void)D().isIntBool();
-(void)D().isIntBool();
-return myD.member != 64;	//% self.expect("expression -- myD", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["64"])
+(void)D().isIntBool(); //% self.expect("expression -- myD", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["64"])
 //% self.expect("expression -- myLesserD.isIntBool()", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["false"])
 //% self.expect("expression -- myD.isIntBool()", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["true"])
 
 // See comment above.
 //#% self.expect("expression -- D().isIntBool()", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["false"])
 //#% self.expect("expression -- D().isIntBool()", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["true"])
+
+array<3> myArray; //% self.expect("expression -- myArray", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["Arr"])
+
+return 1;
 }
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -1277,11 +1277,12 @@
 

[Lldb-commits] [PATCH] D74018: [lldb/LibCxx] Have ExtractLibcxxStringInfo return an Optional result, NFC

2020-02-12 Thread Vedant Kumar via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG90a94c02fb24: [lldb/LibCxx] Have ExtractLibcxxStringInfo 
return an Optional result, NFC (authored by vsk).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D74018?vs=242496=244234#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74018

Files:
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp

Index: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -468,22 +468,20 @@
   eLibcxxStringLayoutModeInvalid = 0x
 };
 
-// this function abstracts away the layout and mode details of a libc++ string
-// and returns the address of the data and the size ready for callers to
-// consume
-static bool ExtractLibcxxStringInfo(ValueObject ,
-ValueObjectSP _sp,
-uint64_t ) {
+/// Determine the size in bytes of \p valobj (a libc++ std::string object) and
+/// extract its data payload. Return the size + payload pair.
+static llvm::Optional>
+ExtractLibcxxStringInfo(ValueObject ) {
   ValueObjectSP D(valobj.GetChildAtIndexPath({0, 0, 0, 0}));
   if (!D)
-return false;
+return {};
 
   ValueObjectSP layout_decider(
 D->GetChildAtIndexPath(llvm::ArrayRef({0, 0})));
 
   // this child should exist
   if (!layout_decider)
-return false;
+return {};
 
   ConstString g_data_name("__data_");
   ConstString g_size_name("__size_");
@@ -497,13 +495,13 @@
   if (layout == eLibcxxStringLayoutModeDSC) {
 ValueObjectSP size_mode(D->GetChildAtIndexPath({1, 1, 0}));
 if (!size_mode)
-  return false;
+  return {};
 
 if (size_mode->GetName() != g_size_name) {
   // we are hitting the padding structure, move along
   size_mode = D->GetChildAtIndexPath({1, 1, 1});
   if (!size_mode)
-return false;
+return {};
 }
 
 size_mode_value = (size_mode->GetValueAsUnsigned(0));
@@ -511,7 +509,7 @@
   } else {
 ValueObjectSP size_mode(D->GetChildAtIndexPath({1, 0, 0}));
 if (!size_mode)
-  return false;
+  return {};
 
 size_mode_value = (size_mode->GetValueAsUnsigned(0));
 short_mode = ((size_mode_value & 1) == 0);
@@ -520,12 +518,12 @@
   if (short_mode) {
 ValueObjectSP s(D->GetChildAtIndex(1, true));
 if (!s)
-  return false;
-location_sp = s->GetChildAtIndex(
+  return {};
+ValueObjectSP location_sp = s->GetChildAtIndex(
 (layout == eLibcxxStringLayoutModeDSC) ? 0 : 1, true);
-size = (layout == eLibcxxStringLayoutModeDSC)
-   ? size_mode_value
-   : ((size_mode_value >> 1) % 256);
+const uint64_t size = (layout == eLibcxxStringLayoutModeDSC)
+  ? size_mode_value
+  : ((size_mode_value >> 1) % 256);
 
 // When the small-string optimization takes place, the data must fit in the
 // inline string buffer (23 bytes on x86_64/Darwin). If it doesn't, it's
@@ -534,39 +532,44 @@
 const llvm::Optional max_bytes =
 location_sp->GetCompilerType().GetByteSize(
 exe_ctx.GetBestExecutionContextScope());
-if (!max_bytes || size > *max_bytes)
-  return false;
+if (!max_bytes || size > *max_bytes || !location_sp)
+  return {};
 
-return (location_sp.get() != nullptr);
-  } else {
-ValueObjectSP l(D->GetChildAtIndex(0, true));
-if (!l)
-  return false;
-// we can use the layout_decider object as the data pointer
-location_sp = (layout == eLibcxxStringLayoutModeDSC)
-  ? layout_decider
-  : l->GetChildAtIndex(2, true);
-ValueObjectSP size_vo(l->GetChildAtIndex(1, true));
-const unsigned capacity_index =
-(layout == eLibcxxStringLayoutModeDSC) ? 2 : 0;
-ValueObjectSP capacity_vo(l->GetChildAtIndex(capacity_index, true));
-if (!size_vo || !location_sp || !capacity_vo)
-  return false;
-size = size_vo->GetValueAsUnsigned(LLDB_INVALID_OFFSET);
-const uint64_t cap = capacity_vo->GetValueAsUnsigned(LLDB_INVALID_OFFSET);
-if (size == LLDB_INVALID_OFFSET || cap == LLDB_INVALID_OFFSET || cap < size)
-  return false;
-return true;
+return std::make_pair(size, location_sp);
   }
+
+  ValueObjectSP l(D->GetChildAtIndex(0, true));
+  if (!l)
+return {};
+  // we can use the layout_decider object as the data pointer
+  ValueObjectSP location_sp = (layout == eLibcxxStringLayoutModeDSC)
+  ? layout_decider
+  : l->GetChildAtIndex(2, true);
+  ValueObjectSP size_vo(l->GetChildAtIndex(1, true));
+  const unsigned capacity_index =
+  (layout == eLibcxxStringLayoutModeDSC) ? 2 : 

[Lldb-commits] [lldb] 7aabad1 - [lldb/StringPrinter] Avoid reading garbage in uninitialized strings

2020-02-12 Thread Vedant Kumar via lldb-commits

Author: Vedant Kumar
Date: 2020-02-12T11:24:03-08:00
New Revision: 7aabad131288a598c90903ae7d96383105ae3fc2

URL: 
https://github.com/llvm/llvm-project/commit/7aabad131288a598c90903ae7d96383105ae3fc2
DIFF: 
https://github.com/llvm/llvm-project/commit/7aabad131288a598c90903ae7d96383105ae3fc2.diff

LOG: [lldb/StringPrinter] Avoid reading garbage in uninitialized strings

This patch fixes a few related out-of-bounds read bugs in the
string data formatters. These issues have to do with mishandling of un-
initialized strings. These manifest as ASan exceptions when debugging a
clang binary.

The first issue was that the std::string formatter treated strings in
"short mode" with length greater than the size of the inline buffer as
valid.

The second issue was that the StringPrinter facility did not check that
a full utf8 codepoint sequence can be read from the buffer (i.e. there
are some missing range checks). I took the opportunity here to delete
some untested code that was meant to deal with invalid input and replace
it with fail-on-invalid logic ([1][2][3]). This means we'll give up on
formatting an invalid string instead of guessing our way through it.

The third issue is that StringPrinter did not check that a utf8 sequence
could actually be fully read from the string payload. This one is especially
tricky as we may overflow the buffer pointer while reading the sequence.

I also noticed that the std::string formatter would spew the raw version of
the underlying ValueObject when garbage is detected. I've changed this to
just print "Summary Unavailable" instead, as we do elsewhere.

I've added regression tests for these issues to
test/functionalities/data-formatter/data-formatter-stl/libcxx/string.

[1]
http://lab.llvm.org:8080/coverage/coverage-reports/coverage/Users/buildslave/jenkins/workspace/coverage/llvm-project/lldb/source/DataFormatters/StringPrinter.cpp.html#L136
[2]
http://lab.llvm.org:8080/coverage/coverage-reports/coverage/Users/buildslave/jenkins/workspace/coverage/llvm-project/lldb/source/DataFormatters/StringPrinter.cpp.html#L163
[3]
http://lab.llvm.org:8080/coverage/coverage-reports/coverage/Users/buildslave/jenkins/workspace/coverage/llvm-project/lldb/source/DataFormatters/StringPrinter.cpp.html#L357

rdar://59080026

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

Added: 


Modified: 
lldb/source/DataFormatters/StringPrinter.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp

Removed: 




diff  --git a/lldb/source/DataFormatters/StringPrinter.cpp 
b/lldb/source/DataFormatters/StringPrinter.cpp
index 1e05c7106c60..92dd71d17b8c 100644
--- a/lldb/source/DataFormatters/StringPrinter.cpp
+++ b/lldb/source/DataFormatters/StringPrinter.cpp
@@ -131,14 +131,15 @@ 
GetPrintableImpl(uint8_t *buffer,
  uint8_t *) {
   StringPrinter::StringPrinterBufferPointer retval{nullptr};
 
-  unsigned utf8_encoded_len = llvm::getNumBytesForUTF8(*buffer);
+  const unsigned utf8_encoded_len = llvm::getNumBytesForUTF8(*buffer);
 
-  if (1u + std::distance(buffer, buffer_end) < utf8_encoded_len) {
-// I don't have enough bytes - print whatever I have left
-retval = {buffer, static_cast(1 + buffer_end - buffer)};
-next = buffer_end + 1;
+  // If the utf8 encoded length is invalid, or if there aren't enough bytes to
+  // print, this is some kind of corrupted string.
+  if (utf8_encoded_len == 0 || utf8_encoded_len > 4)
+return retval;
+  if ((buffer_end - buffer) < utf8_encoded_len)
+// There's no room in the buffer for the utf8 sequence.
 return retval;
-  }
 
   char32_t codepoint = 0;
   switch (utf8_encoded_len) {
@@ -160,12 +161,6 @@ 
GetPrintableImpl(uint8_t *buffer,
 (unsigned char)*buffer, (unsigned char)*(buffer + 1),
 (unsigned char)*(buffer + 2), (unsigned char)*(buffer + 3));
 break;
-  default:
-// this is probably some bogus non-character thing just print it as-is and
-// hope to sync up again soon
-retval = {buffer, 1};
-next = buffer + 1;
-return retval;
   }
 
   if (codepoint) {
@@ -215,9 +210,7 @@ 
GetPrintableImpl(uint8_t *buffer,
 return retval;
   }
 
-  // this should not happen - but just in case.. try to resync at some point
-  retval = {buffer, 1};
-  next = buffer + 1;
+  // We couldn't figure out how to print this string.
   return retval;
 }
 
@@ -227,7 +220,7 @@ 
GetPrintableImpl(uint8_t *buffer,
 static StringPrinter::StringPrinterBufferPointer
 GetPrintable(StringPrinter::StringElementType type, uint8_t *buffer,
  uint8_t *buffer_end, uint8_t *) {
-  if (!buffer)
+  if (!buffer || buffer >= buffer_end)
 return {nullptr};
 
   switch (type) 

[Lldb-commits] [lldb] 90a94c0 - [lldb/LibCxx] Have ExtractLibcxxStringInfo return an Optional result, NFC

2020-02-12 Thread Vedant Kumar via lldb-commits

Author: Vedant Kumar
Date: 2020-02-12T11:24:03-08:00
New Revision: 90a94c02fb243f0b49ee9933b0e8145147f84e32

URL: 
https://github.com/llvm/llvm-project/commit/90a94c02fb243f0b49ee9933b0e8145147f84e32
DIFF: 
https://github.com/llvm/llvm-project/commit/90a94c02fb243f0b49ee9933b0e8145147f84e32.diff

LOG: [lldb/LibCxx] Have ExtractLibcxxStringInfo return an Optional result, NFC

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

Added: 


Modified: 
lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
index e9e4db97ebbe..7152ff407f29 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -468,22 +468,20 @@ enum LibcxxStringLayoutMode {
   eLibcxxStringLayoutModeInvalid = 0x
 };
 
-// this function abstracts away the layout and mode details of a libc++ string
-// and returns the address of the data and the size ready for callers to
-// consume
-static bool ExtractLibcxxStringInfo(ValueObject ,
-ValueObjectSP _sp,
-uint64_t ) {
+/// Determine the size in bytes of \p valobj (a libc++ std::string object) and
+/// extract its data payload. Return the size + payload pair.
+static llvm::Optional>
+ExtractLibcxxStringInfo(ValueObject ) {
   ValueObjectSP D(valobj.GetChildAtIndexPath({0, 0, 0, 0}));
   if (!D)
-return false;
+return {};
 
   ValueObjectSP layout_decider(
 D->GetChildAtIndexPath(llvm::ArrayRef({0, 0})));
 
   // this child should exist
   if (!layout_decider)
-return false;
+return {};
 
   ConstString g_data_name("__data_");
   ConstString g_size_name("__size_");
@@ -497,13 +495,13 @@ static bool ExtractLibcxxStringInfo(ValueObject ,
   if (layout == eLibcxxStringLayoutModeDSC) {
 ValueObjectSP size_mode(D->GetChildAtIndexPath({1, 1, 0}));
 if (!size_mode)
-  return false;
+  return {};
 
 if (size_mode->GetName() != g_size_name) {
   // we are hitting the padding structure, move along
   size_mode = D->GetChildAtIndexPath({1, 1, 1});
   if (!size_mode)
-return false;
+return {};
 }
 
 size_mode_value = (size_mode->GetValueAsUnsigned(0));
@@ -511,7 +509,7 @@ static bool ExtractLibcxxStringInfo(ValueObject ,
   } else {
 ValueObjectSP size_mode(D->GetChildAtIndexPath({1, 0, 0}));
 if (!size_mode)
-  return false;
+  return {};
 
 size_mode_value = (size_mode->GetValueAsUnsigned(0));
 short_mode = ((size_mode_value & 1) == 0);
@@ -520,12 +518,12 @@ static bool ExtractLibcxxStringInfo(ValueObject ,
   if (short_mode) {
 ValueObjectSP s(D->GetChildAtIndex(1, true));
 if (!s)
-  return false;
-location_sp = s->GetChildAtIndex(
+  return {};
+ValueObjectSP location_sp = s->GetChildAtIndex(
 (layout == eLibcxxStringLayoutModeDSC) ? 0 : 1, true);
-size = (layout == eLibcxxStringLayoutModeDSC)
-   ? size_mode_value
-   : ((size_mode_value >> 1) % 256);
+const uint64_t size = (layout == eLibcxxStringLayoutModeDSC)
+  ? size_mode_value
+  : ((size_mode_value >> 1) % 256);
 
 // When the small-string optimization takes place, the data must fit in the
 // inline string buffer (23 bytes on x86_64/Darwin). If it doesn't, it's
@@ -534,39 +532,44 @@ static bool ExtractLibcxxStringInfo(ValueObject ,
 const llvm::Optional max_bytes =
 location_sp->GetCompilerType().GetByteSize(
 exe_ctx.GetBestExecutionContextScope());
-if (!max_bytes || size > *max_bytes)
-  return false;
+if (!max_bytes || size > *max_bytes || !location_sp)
+  return {};
 
-return (location_sp.get() != nullptr);
-  } else {
-ValueObjectSP l(D->GetChildAtIndex(0, true));
-if (!l)
-  return false;
-// we can use the layout_decider object as the data pointer
-location_sp = (layout == eLibcxxStringLayoutModeDSC)
-  ? layout_decider
-  : l->GetChildAtIndex(2, true);
-ValueObjectSP size_vo(l->GetChildAtIndex(1, true));
-const unsigned capacity_index =
-(layout == eLibcxxStringLayoutModeDSC) ? 2 : 0;
-ValueObjectSP capacity_vo(l->GetChildAtIndex(capacity_index, true));
-if (!size_vo || !location_sp || !capacity_vo)
-  return false;
-size = size_vo->GetValueAsUnsigned(LLDB_INVALID_OFFSET);
-const uint64_t cap = capacity_vo->GetValueAsUnsigned(LLDB_INVALID_OFFSET);
-if (size == LLDB_INVALID_OFFSET || cap == LLDB_INVALID_OFFSET || cap < 
size)
-  return false;
-return true;
+return std::make_pair(size, location_sp);
   }
+
+  ValueObjectSP l(D->GetChildAtIndex(0, true));
+  if (!l)
+return {};
+  // we can use the 

[Lldb-commits] [lldb] d6e47a4 - [lldb/TypeSystemClang] Supply trivial TypeSourceInfo to NonTypeTemplateParmDecl::Create

2020-02-12 Thread Vedant Kumar via lldb-commits

Author: Vedant Kumar
Date: 2020-02-12T11:24:02-08:00
New Revision: d6e47a405a3481c9c40ebe1c339349c01c504bd6

URL: 
https://github.com/llvm/llvm-project/commit/d6e47a405a3481c9c40ebe1c339349c01c504bd6
DIFF: 
https://github.com/llvm/llvm-project/commit/d6e47a405a3481c9c40ebe1c339349c01c504bd6.diff

LOG: [lldb/TypeSystemClang] Supply trivial TypeSourceInfo to 
NonTypeTemplateParmDecl::Create

This fixes a UBSan error seen while debugging clang:

Member call on null pointer of type 'clang::TypeSourceInfo'

rdar://58783517

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

Added: 


Modified: 
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
lldb/test/API/lang/cpp/class-template-parameter-pack/main.cpp

Removed: 




diff  --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 3d9a80dd3f73..cbe0301fe162 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -1277,11 +1277,12 @@ static TemplateParameterList 
*CreateTemplateParameterList(
 if (name && name[0])
   identifier_info = (name);
 if (IsValueParam(template_param_infos.args[i])) {
+  QualType template_param_type =
+  template_param_infos.args[i].getIntegralType();
   template_param_decls.push_back(NonTypeTemplateParmDecl::Create(
   ast, decl_context, SourceLocation(), SourceLocation(), depth, i,
-  identifier_info, template_param_infos.args[i].getIntegralType(),
-  parameter_pack, nullptr));
-
+  identifier_info, template_param_type, parameter_pack,
+  ast.getTrivialTypeSourceInfo(template_param_type)));
 } else {
   template_param_decls.push_back(TemplateTypeParmDecl::Create(
   ast, decl_context, SourceLocation(), SourceLocation(), depth, i,

diff  --git a/lldb/test/API/lang/cpp/class-template-parameter-pack/main.cpp 
b/lldb/test/API/lang/cpp/class-template-parameter-pack/main.cpp
index 9278d01f8c57..836a8301658a 100644
--- a/lldb/test/API/lang/cpp/class-template-parameter-pack/main.cpp
+++ b/lldb/test/API/lang/cpp/class-template-parameter-pack/main.cpp
@@ -34,6 +34,11 @@ template <> struct D : D {
   bool isIntBool() { return true; }
 };
 
+template struct array {
+  int Arr[Size];
+  array() {}
+};
+
 int main (int argc, char const *argv[])
 {
 C myC;
@@ -53,12 +58,15 @@ int main (int argc, char const *argv[])
 D myLesserD;
 myD.member = 64;
 (void)D().isIntBool();
-(void)D().isIntBool();
-return myD.member != 64;   //% self.expect("expression -- myD", 
DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["64"])
+(void)D().isIntBool(); //% self.expect("expression -- myD", 
DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["64"])
 //% self.expect("expression -- 
myLesserD.isIntBool()", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["false"])
 //% self.expect("expression -- 
myD.isIntBool()", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["true"])
 
 // See comment above.
 //#% self.expect("expression -- D().isIntBool()", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["false"])
 //#% self.expect("expression -- D().isIntBool()", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["true"])
+
+array<3> myArray; //% self.expect("expression -- myArray", 
DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["Arr"])
+
+return 1;
 }



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


[Lldb-commits] [PATCH] D73860: [lldb/StringPrinter] Avoid reading garbage in uninitialized strings

2020-02-12 Thread Vedant Kumar via Phabricator via lldb-commits
vsk marked an inline comment as done.
vsk added a comment.

Thanks everyone for the reviews!




Comment at: 
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp:29
+if (sizeof(std::string) == sizeof(garbage_string_sso))
+  memcpy((void *), _string_sso, sizeof(std::string));
+if (sizeof(std::string) == sizeof(garbage_string_long))

vsk wrote:
> teemperor wrote:
> > teemperor wrote:
> > > shafik wrote:
> > > > vsk wrote:
> > > > > shafik wrote:
> > > > > > While I get what you are doing here, we know he structure of libc++ 
> > > > > > SSO implementation and we are manually building a corrupt one, this 
> > > > > > is fragile to changes in the implementation. 
> > > > > > 
> > > > > > I don't have an immediate suggestion for an alternative approach 
> > > > > > but if we stick with this we should stick a big comment explaining 
> > > > > > this, perhaps laying out the assumptions of the internal layout we 
> > > > > > are assuming and maybe some sanity checks maybe using `offsetof` to 
> > > > > > verify fields exist and are where we expect them to be.
> > > > > I don't see how this is fragile. The structure of libc++'s SSO 
> > > > > implementation is ABI, and is unlikely to change (esp. not in a way 
> > > > > that turns either one of the garbage strings into a valid string). 
> > > > > I've left comments explaining what's wrong with both of the garbage 
> > > > > strings, but can leave a pointer to 
> > > > > https://joellaity.com/2020/01/31/string.html for more info?
> > > > Sure, that note would be fine.
> > > Can you instead do a `#if _LIBCPP_ABI_VERSION == 1` and have the #else as 
> > > an #error that this test needs updating. We don't support any other 
> > > libc++ ABI beside 1 in LLDB but if we ever do then this should not 
> > > silently pass.
> > Actually the #if *and* a static_assert comparing the size would be best 
> > IMHO.
> Sure, but the size check is not primarily about the ABI. The garbage examples 
> presuppose 64-bit pointer & size types, which is not true on some watches.
I'm not sure how to write a static assert that isn't a little brittle. Maybe 
`static_assert(sizeof(void *) != 8 || sizeof(std::string) == 24, "unknown 
std::string layout")`?


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

https://reviews.llvm.org/D73860



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


[Lldb-commits] [PATCH] D74018: [lldb/LibCxx] Have ExtractLibcxxStringInfo return an Optional result, NFC

2020-02-12 Thread Vedant Kumar via Phabricator via lldb-commits
vsk marked 2 inline comments as done.
vsk added inline comments.



Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:507
 
 size_mode_value = (size_mode->GetValueAsUnsigned(0));
 short_mode = ((size_mode_value & 0x80) == 0);

JDevlieghere wrote:
> Would it make sense to split the computation of `size_mode_value` and 
> `short_mode` into a helper?
My 2c is that having this inline seems a little more readable, but I'm not 
strongly opposed.



Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:667
   options.SetStream();
-
-  if (prefix_token.empty())
-options.SetPrefixToken(nullptr);
-  else
-options.SetPrefixToken(prefix_token);
-
+  options.SetPrefixToken(prefix_token.empty() ? nullptr : 
prefix_token.c_str());
   options.SetQuote('"');

This causes some kind of use-after-free, I've backed this change out.


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

https://reviews.llvm.org/D74018



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


[Lldb-commits] [PATCH] D74252: Fix+re-enable Assert StackFrame Recognizer on Linux

2020-02-12 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added inline comments.



Comment at: lldb/source/Target/AssertFrameRecognizer.cpp:152
+ConstString func_name = sym_ctx.GetFunctionName();
+if (func_name == ConstString(function_name) ||
+alternate_function_name.empty() ||

teemperor wrote:
> JDevlieghere wrote:
> > I believe someone added an overload for comparing ConstStrings with 
> > StringRefs. We shouldn't have to pay the price of creating one here just 
> > for comparison. Same below.
> I really don't want a == overload for StringRef in `ConstString`. The *only* 
> reason for using ConstString is using less memory for duplicate strings and 
> being fast to compare against other ConstStrings. So if we add an overload 
> for comparing against StringRef then code like this will look really innocent 
> even though it essentially goes against the idea of ConstString. Either both 
> string lists are using ConstString or neither does (which I would prefer). 
> But having a list of normal strings and comparing it against ConstStrings 
> usually means that the design is kinda flawed. Then we have both normal 
> string comparisons but also the whole overhead of ConstString.
> 
> Looking at this patch we already construct at one point a ConstString from 
> the function name at one point, so that might as well be a ConstString in the 
> first place. If we had an implicit comparison than the conversion here would 
> look really innocent and no one would notice.
Hmm, you're totally right, I must be confusing this with something else then. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74252



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


[Lldb-commits] [PATCH] D73860: [lldb/StringPrinter] Avoid reading garbage in uninitialized strings

2020-02-12 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.

In D73860#1872746 , @vsk wrote:

> Ping, @teemperor do you have any concerns about this one?


Didn't see Jonas comment when he accepted, sorry. LGTM.




Comment at: 
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp:29
+if (sizeof(std::string) == sizeof(garbage_string_sso))
+  memcpy((void *), _string_sso, sizeof(std::string));
+if (sizeof(std::string) == sizeof(garbage_string_long))

teemperor wrote:
> shafik wrote:
> > vsk wrote:
> > > shafik wrote:
> > > > While I get what you are doing here, we know he structure of libc++ SSO 
> > > > implementation and we are manually building a corrupt one, this is 
> > > > fragile to changes in the implementation. 
> > > > 
> > > > I don't have an immediate suggestion for an alternative approach but if 
> > > > we stick with this we should stick a big comment explaining this, 
> > > > perhaps laying out the assumptions of the internal layout we are 
> > > > assuming and maybe some sanity checks maybe using `offsetof` to verify 
> > > > fields exist and are where we expect them to be.
> > > I don't see how this is fragile. The structure of libc++'s SSO 
> > > implementation is ABI, and is unlikely to change (esp. not in a way that 
> > > turns either one of the garbage strings into a valid string). I've left 
> > > comments explaining what's wrong with both of the garbage strings, but 
> > > can leave a pointer to https://joellaity.com/2020/01/31/string.html for 
> > > more info?
> > Sure, that note would be fine.
> Can you instead do a `#if _LIBCPP_ABI_VERSION == 1` and have the #else as an 
> #error that this test needs updating. We don't support any other libc++ ABI 
> beside 1 in LLDB but if we ever do then this should not silently pass.
Actually the #if *and* a static_assert comparing the size would be best IMHO.


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

https://reviews.llvm.org/D73860



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


[Lldb-commits] [PATCH] D73860: [lldb/StringPrinter] Avoid reading garbage in uninitialized strings

2020-02-12 Thread Vedant Kumar via Phabricator via lldb-commits
vsk added a comment.

Ping, @teemperor do you have any concerns about this one?


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

https://reviews.llvm.org/D73860



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


[Lldb-commits] [PATCH] D74136: [LLDB] WIP: Optionally follow DW_AT_decl_file when setting breakpoint

2020-02-12 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

In D74136#1871751 , @labath wrote:

> In D74136#1870029 , @jingham wrote:
>
> > In D74136#1869622 , @kwk wrote:
> >
> > > @labath @jingham to summarize from what I read here and what I chatted 
> > > about with @labath , the following  is a possible way to go for now, 
> > > right?
> > >
> > > 1. We're not going to introduce my flag.
> > > 2. You're both not perfectly happy with the way things are documented at 
> > > the moment and dislike some of the implementation as in in LLDB but 
> > > chaning it should not be part of this patch.
> > > 3. @jingham wouldn't want to introduce `--compile-unit` as a flag that 
> > > @labath proposed.
> > > 4. You want `breakpoint set --file` to search everything, that is to say 
> > > compilation units and files referenced in `DW_AT_decl_file`.
> > >
> > >   If you can confirm that this is correct, then I can refactor this patch 
> > > to remove the switch and change the default behavior for `breakpoint set 
> > > --file`. Especially point 4. is important I guess.
> >
> >
> > There's a point (5) which is that the search in 4 should be conditioned on 
> > the setting of the "target.inline-breakpoint-strategy".  That way if people 
> > have big projects but don't ever #include source files, and don't build 
> > with LTO, they can turn off these extra searches, which might end up being 
> > costly and to no purpose for them.
>
>
> I think we're on the same page, but I'll just try to rephrase this in my own 
> words.
>
> Basically, I'm hoping that we could get the `--file+--function` combo to 
> behave the same way `--file+--line`. I.e., that the --file argument should 
> specify the file that the function is defined in (i.e. the DW_AT_decl_file 
> attribute of the function), and not the compile unit (DW_AT_name of the 
> compile unit containing the function). The way the 
> `inline-breakpoint-strategy` setting comes into play is that if the setting 
> value is "headers" and the user specifies a `.cpp` file, then we will assume 
> that the .cpp file also matches the name of one of the compile units, and we 
> will not attempt to search compile units with different names (which is the 
> same thing we do for line breakpoints, I believe).


That is my understanding as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74136



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


[Lldb-commits] [PATCH] D74136: [LLDB] WIP: Optionally follow DW_AT_decl_file when setting breakpoint

2020-02-12 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

This way the only bit of the matrix of "restricting function name breakpoints 
by containing source file" that we don't support is "set a breakpoint on this 
function, but ONLY when it is inlined into another CU, NOT when it is in its 
defining CU."  That seems the least useful of the options, and I'm happy to 
trade off not adding another option to the already overstuffed "break set" list 
of options.  And remember, if you actually needed this it would be 
straightforward to write a scripted breakpoint resolver to do that job.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74136



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


[Lldb-commits] [PATCH] D74475: [lldb] Replace assertTrue(a == b, "msg") with assertEquals(a, b, "msg") in the test suite

2020-02-12 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

LGTM. I don't know how complex your script is, but could you include it in the 
commit message or the script directory so that we can run this again in the 
future?


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

https://reviews.llvm.org/D74475



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


[Lldb-commits] [PATCH] D74451: [lldb/Plugins] Have one initializer per ABI plugin

2020-02-12 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

In D74451#1871641 , @labath wrote:

> Yeah, I expected something like this would be needed. This is fine, but I'd 
> call the files `ABIX86.h` instead of just `X86.h`, to match the names of the 
> classes and everything...


Yup, I've updated the diff


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

https://reviews.llvm.org/D74451



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


[Lldb-commits] [PATCH] D74451: [lldb/Plugins] Have one initializer per ABI plugin

2020-02-12 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 244205.

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

https://reviews.llvm.org/D74451

Files:
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp
  lldb/source/Plugins/ABI/AArch64/ABIAArch64.h
  lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
  lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
  lldb/source/Plugins/ABI/AArch64/CMakeLists.txt
  lldb/source/Plugins/ABI/ARM/ABIARM.cpp
  lldb/source/Plugins/ABI/ARM/ABIARM.h
  lldb/source/Plugins/ABI/ARM/CMakeLists.txt
  lldb/source/Plugins/ABI/Mips/ABIMips.cpp
  lldb/source/Plugins/ABI/Mips/ABIMips.h
  lldb/source/Plugins/ABI/Mips/CMakeLists.txt
  lldb/source/Plugins/ABI/PowerPC/ABIPowerPC.cpp
  lldb/source/Plugins/ABI/PowerPC/ABIPowerPC.h
  lldb/source/Plugins/ABI/PowerPC/CMakeLists.txt
  lldb/source/Plugins/ABI/X86/ABIX86.cpp
  lldb/source/Plugins/ABI/X86/ABIX86.h
  lldb/source/Plugins/ABI/X86/CMakeLists.txt

Index: lldb/source/Plugins/ABI/X86/CMakeLists.txt
===
--- lldb/source/Plugins/ABI/X86/CMakeLists.txt
+++ lldb/source/Plugins/ABI/X86/CMakeLists.txt
@@ -1,4 +1,5 @@
 add_lldb_library(lldbPluginABIX86 PLUGIN
+  ABIX86.cpp
   ABIMacOSX_i386.cpp
   ABISysV_i386.cpp
   ABISysV_x86_64.cpp
Index: lldb/source/Plugins/ABI/X86/ABIX86.h
===
--- /dev/null
+++ lldb/source/Plugins/ABI/X86/ABIX86.h
@@ -0,0 +1,17 @@
+//===-- X86.h ---*- C++ -*-===//
+//
+// 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
+//
+//===--===//
+
+#ifndef liblldb_ABIX86_h_
+#define liblldb_ABIX86_h_
+
+class ABIX86 {
+public:
+  static void Initialize();
+  static void Terminate();
+};
+#endif
Index: lldb/source/Plugins/ABI/X86/ABIX86.cpp
===
--- /dev/null
+++ lldb/source/Plugins/ABI/X86/ABIX86.cpp
@@ -0,0 +1,30 @@
+//===-- X86.h -===//
+//
+// 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 "ABIX86.h"
+#include "ABIMacOSX_i386.h"
+#include "ABISysV_i386.h"
+#include "ABISysV_x86_64.h"
+#include "ABIWindows_x86_64.h"
+#include "lldb/Core/PluginManager.h"
+
+LLDB_PLUGIN(ABIX86)
+
+void ABIX86::Initialize() {
+  ABIMacOSX_i386::Initialize();
+  ABISysV_i386::Initialize();
+  ABISysV_x86_64::Initialize();
+  ABIWindows_x86_64::Initialize();
+}
+
+void ABIX86::Terminate() {
+  ABIMacOSX_i386::Terminate();
+  ABISysV_i386::Terminate();
+  ABISysV_x86_64::Terminate();
+  ABIWindows_x86_64::Terminate();
+}
Index: lldb/source/Plugins/ABI/PowerPC/CMakeLists.txt
===
--- lldb/source/Plugins/ABI/PowerPC/CMakeLists.txt
+++ lldb/source/Plugins/ABI/PowerPC/CMakeLists.txt
@@ -1,4 +1,5 @@
 add_lldb_library(lldbPluginABIPowerPC PLUGIN
+  ABIPowerPC.cpp
   ABISysV_ppc.cpp
   ABISysV_ppc64.cpp
 
Index: lldb/source/Plugins/ABI/PowerPC/ABIPowerPC.h
===
--- /dev/null
+++ lldb/source/Plugins/ABI/PowerPC/ABIPowerPC.h
@@ -0,0 +1,17 @@
+//===-- PowerPC.h ---*- C++ -*-===//
+//
+// 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
+//
+//===--===//
+
+#ifndef liblldb_ABIPowerPC_h_
+#define liblldb_ABIPowerPC_h_
+
+class ABIPowerPC {
+public:
+  static void Initialize();
+  static void Terminate();
+};
+#endif
Index: lldb/source/Plugins/ABI/PowerPC/ABIPowerPC.cpp
===
--- /dev/null
+++ lldb/source/Plugins/ABI/PowerPC/ABIPowerPC.cpp
@@ -0,0 +1,24 @@
+//===-- PowerPC.h -===//
+//
+// 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 "ABIPowerPC.h"
+#include "ABISysV_ppc.h"
+#include "ABISysV_ppc64.h"
+#include "lldb/Core/PluginManager.h"
+
+LLDB_PLUGIN(ABIPowerPC)
+
+void ABIPowerPC::Initialize() {
+  

[Lldb-commits] [PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

2020-02-12 Thread Djordje Todorovic via Phabricator via lldb-commits
djtodoro marked an inline comment as done.
djtodoro added inline comments.



Comment at: llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp:870
 if (MI->isCandidateForCallSiteEntry() &&
-DAG->getTarget().Options.EnableDebugEntryValues)
+DAG->getTarget().Options.SupportsDebugEntryValues)
   MF.addCallArgsForwardingRegs(MI, DAG->getSDCallSiteInfo(Node));

dstenb wrote:
> I'm sorry for commenting on this so late, but when now adapting this patch 
> for our downstream target, for which we will not enable call site info by 
> default yet, I noticed that the call site information wasn't added. I think 
> that this should be `ShouldEmitDebugEntryValues()`.
I thought to restrict the production of the call site info here and only to 
allow a hand-made testing, but I think you are right, we should use the 
`ShouldEmitDebugEntryValues()`. I'll update the patch, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73534



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


[Lldb-commits] [PATCH] D73665: Stop emitting a breakpoint for each location in a breakpoint when responding to breakpoint commands.

2020-02-12 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

Yes, I will add tests now that the code has been finalized! Thanks for your 
time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73665



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


[Lldb-commits] [PATCH] D70847: [lldb-vscode] Ensure that target matches the executable file

2020-02-12 Thread Greg Clayton via Phabricator via lldb-commits
clayborg accepted this revision.
clayborg added a comment.

Looks great, even more refactoring than asked. Thanks for sticking with this 
doing the work!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70847



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


[Lldb-commits] [PATCH] D70847: [lldb-vscode] Ensure that target matches the executable file

2020-02-12 Thread Anton Kolesov via Phabricator via lldb-commits
anton.kolesov updated this revision to Diff 244160.
anton.kolesov added a comment.

Updated in attempt to reduce amount of code duplication between request_attach 
and request_launch.
Built and tested with testsuite on Linux/x64, also built and manually tested on 
Windows/x64 host with a baremetal ARC cpu target.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70847

Files:
  lldb/tools/lldb-vscode/VSCode.cpp
  lldb/tools/lldb-vscode/VSCode.h
  lldb/tools/lldb-vscode/lldb-vscode.cpp
  lldb/tools/lldb-vscode/package.json

Index: lldb/tools/lldb-vscode/package.json
===
--- lldb/tools/lldb-vscode/package.json
+++ lldb/tools/lldb-vscode/package.json
@@ -122,6 +122,14 @@
 "type": "string",
 "description": "Specify a working directory to set the debug adaptor to so relative object files can be located."
 			},
+			"targetTriple": {
+"type": "string",
+"description": "Triplet of the target architecture to override value derived from the program file."
+			},
+			"platformName": {
+"type": "string",
+"description": "Name of the execution platform to override value derived from the program file."
+			},
 			"initCommands": {
 	"type": "array",
 	"description": "Initialization commands executed upon debugger startup.",
@@ -175,6 +183,14 @@
 "type": "string",
 "description": "Specify a working directory to set the debug adaptor to so relative object files can be located."
 			},
+			"targetTriple": {
+"type": "string",
+"description": "Triplet of the target architecture to override value derived from the program file."
+			},
+			"platformName": {
+"type": "string",
+"description": "Name of the execution platform to override value derived from the program file."
+			},
 			"attachCommands": {
 "type": "array",
 "description": "Custom commands that are executed instead of attaching to a process ID or to a process by name. These commands may optionally create a new target and must perform an attach. A valid process must exist after these commands complete or the \"attach\" will fail.",
Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -69,8 +69,6 @@
 
 enum LaunchMethod { Launch, Attach, AttachForSuspendedLaunch };
 
-enum VSCodeBroadcasterBits { eBroadcastBitStopEventThread = 1u << 0 };
-
 SOCKET AcceptConnection(int portno) {
   // Accept a socket connection from any host on "portno".
   SOCKET newsockfd = -1;
@@ -512,25 +510,13 @@
   // Run any initialize LLDB commands the user specified in the launch.json
   g_vsc.RunInitCommands();
 
-  // Grab the name of the program we need to debug and set it as the first
-  // argument that will be passed to the program we will debug.
-  const auto program = GetString(arguments, "program");
-  if (!program.empty()) {
-lldb::SBFileSpec program_fspec(program.data(), true /*resolve_path*/);
-
-g_vsc.launch_info.SetExecutableFile(program_fspec,
-false /*add_as_first_arg*/);
-const char *target_triple = nullptr;
-const char *uuid_cstr = nullptr;
-// Stand alone debug info file if different from executable
-const char *symfile = nullptr;
-g_vsc.target.AddModule(program.data(), target_triple, uuid_cstr, symfile);
-if (error.Fail()) {
-  response["success"] = llvm::json::Value(false);
-  EmplaceSafeString(response, "message", std::string(error.GetCString()));
-  g_vsc.SendJSON(llvm::json::Value(std::move(response)));
-  return;
-}
+  lldb::SBError status;
+  g_vsc.SetTarget(g_vsc.CreateTargetFromArguments(*arguments, status));
+  if (status.Fail()) {
+response["success"] = llvm::json::Value(false);
+EmplaceSafeString(response, "message", status.GetCString());
+g_vsc.SendJSON(llvm::json::Value(std::move(response)));
+return;
   }
 
   const bool detatchOnError = GetBoolean(arguments, "detachOnError", false);
@@ -543,7 +529,8 @@
 char attach_info[256];
 auto attach_info_len =
 snprintf(attach_info, sizeof(attach_info),
- "Waiting to attach to \"%s\"...", program.data());
+ "Waiting to attach to \"%s\"...",
+ g_vsc.target.GetExecutable().GetFilename());
 g_vsc.SendOutput(OutputType::Console, llvm::StringRef(attach_info,
   attach_info_len));
   }
@@ -1217,13 +1204,6 @@
 g_vsc.debugger.SetErrorFileHandle(out, false);
   }
 
-  g_vsc.target = g_vsc.debugger.CreateTarget(nullptr);
-  lldb::SBListener listener = g_vsc.debugger.GetListener();
-  listener.StartListeningForEvents(
-  

[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

2020-02-12 Thread Levon Ter-Grigoryan via Phabricator via lldb-commits
PatriosTheGreat updated this revision to Diff 244152.

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

https://reviews.llvm.org/D74217

Files:
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/registers-target-xml-reading/Makefile
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/registers-target-xml-reading/TestGdbRemoteTargetXmlPacket.py
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/registers-target-xml-reading/main.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h

Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -199,6 +199,8 @@
   static std::string XMLEncodeAttributeValue(llvm::StringRef value);
 
 private:
+  llvm::Expected> BuildTargetXml();
+
   void HandleInferiorState_Exited(NativeProcessProtocol *process);
 
   void HandleInferiorState_Stopped(NativeProcessProtocol *process);
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -377,6 +377,96 @@
   }
 }
 
+static llvm::StringRef GetEncodingNameOrEmpty(const RegisterInfo *reg_info) {
+  switch (reg_info->encoding) {
+  case eEncodingUint:
+return "uint";
+  case eEncodingSint:
+return "sint";
+  case eEncodingIEEE754:
+return "ieee754";
+  case eEncodingVector:
+return "vector";
+  default:
+return "";
+  }
+}
+
+static llvm::StringRef GetFormatNameOrEmpty(const RegisterInfo *reg_info) {
+  switch (reg_info->format) {
+  case eFormatBinary:
+return "binary";
+  case eFormatDecimal:
+return "decimal";
+  case eFormatHex:
+return "hex";
+  case eFormatFloat:
+return "float";
+  case eFormatVectorOfSInt8:
+return "vector-sint8";
+  case eFormatVectorOfUInt8:
+return "vector-uint8";
+  case eFormatVectorOfSInt16:
+return "vector-sint16";
+  case eFormatVectorOfUInt16:
+return "vector-uint16";
+  case eFormatVectorOfSInt32:
+return "vector-sint32";
+  case eFormatVectorOfUInt32:
+return "vector-uint32";
+  case eFormatVectorOfFloat32:
+return "vector-float32";
+  case eFormatVectorOfUInt64:
+return "vector-uint64";
+  case eFormatVectorOfUInt128:
+return "vector-uint128";
+  default:
+return "";
+  };
+}
+
+static llvm::StringRef GetKindGenericOrEmpty(const RegisterInfo *reg_info) {
+  switch (reg_info->kinds[RegisterKind::eRegisterKindGeneric]) {
+  case LLDB_REGNUM_GENERIC_PC:
+return "pc";
+  case LLDB_REGNUM_GENERIC_SP:
+return "sp";
+  case LLDB_REGNUM_GENERIC_FP:
+return "fp";
+  case LLDB_REGNUM_GENERIC_RA:
+return "ra";
+  case LLDB_REGNUM_GENERIC_FLAGS:
+return "flags";
+  case LLDB_REGNUM_GENERIC_ARG1:
+return "arg1";
+  case LLDB_REGNUM_GENERIC_ARG2:
+return "arg2";
+  case LLDB_REGNUM_GENERIC_ARG3:
+return "arg3";
+  case LLDB_REGNUM_GENERIC_ARG4:
+return "arg4";
+  case LLDB_REGNUM_GENERIC_ARG5:
+return "arg5";
+  case LLDB_REGNUM_GENERIC_ARG6:
+return "arg6";
+  case LLDB_REGNUM_GENERIC_ARG7:
+return "arg7";
+  case LLDB_REGNUM_GENERIC_ARG8:
+return "arg8";
+  default:
+return "";
+  }
+}
+
+static void CollectRegNums(const uint32_t *reg_num,
+   lldb_private::StreamString response) {
+  for (int i = 0; *reg_num != LLDB_INVALID_REGNUM; ++reg_num, ++i) {
+if (i > 0)
+  response.PutChar(',');
+response.Printf("%" PRIx32, *reg_num);
+  }
+}
+
 static void WriteRegisterValueInHexFixedWidth(
 StreamString , NativeRegisterContext _ctx,
 const RegisterInfo _info, const RegisterValue *reg_value_p,
@@ -1699,74 +1789,18 @@
   response.Printf("bitsize:%" PRIu32 ";offset:%" PRIu32 ";",
   reg_info->byte_size * 8, reg_info->byte_offset);
 
-  switch (reg_info->encoding) {
-  case eEncodingUint:
-response.PutCString("encoding:uint;");
-break;
-  case eEncodingSint:
-response.PutCString("encoding:sint;");
-break;
-  case eEncodingIEEE754:
-response.PutCString("encoding:ieee754;");
-break;
-  case eEncodingVector:
-response.PutCString("encoding:vector;");
-break;
-  default:
-break;
-  }
+  llvm::StringRef encoding = GetEncodingNameOrEmpty(reg_info);
+  if (!encoding.empty())
+response << "encoding:" << encoding << ';';
 
-  switch (reg_info->format) {
-  case eFormatBinary:
-response.PutCString("format:binary;");
-break;
-  case eFormatDecimal:
-

[Lldb-commits] [PATCH] D70840: [LLDB] [DWARF] Strip out the thumb bit from addresses on ARM

2020-02-12 Thread Martin Storsjö via Phabricator via lldb-commits
mstorsjo added a comment.

In D70840#1871932 , @labath wrote:

> In D70840#1871862 , @mstorsjo wrote:
>
> > > Now someone might argue that the looking up the address of the `ldr` 
> > > opcode is wrong, because that is not the actual pc value, and that we 
> > > should lookup the address of `ldr+1`. In that case we can point them to 
> > > the next function (`call_noreturn`), which ends with a call to a noreturn 
> > > function. Now if we get a crash in `does_not_return()`, the address we 
> > > will get from the backtrace will be one byte after the last opcode of 
> > > `call_noreturn` (`bl  does_not_return()`). This means that in the 
> > > **non-windows** case, it will not resolve correctly to the 
> > > `call_noreturn` even if we apply the usual trick of subtracting 1 from 
> > > the address to bring it into the range of the calling function.
> >
> > Hmm... I presume this issue is present now already (and doesn't really 
> > change due to normalizing the line tables and ranges on windows, to the 
> > same as linux). Not familiar with "the usual trick of subtracting 1 from 
> > the address to bring it into the range of the calling function", but 
> > wouldn't something like `GetOpcodeLoadAddress(pc) - 1` always bring you 
> > into the range of the calling function, given a return address?
>
>
> Lldb uses this when unwinding specifically to deal with the "call as a last 
> instruction" problem, and I'm pretty sure it's not the only tool doing that. 
> This is even described in the DWARF spec (non-normative text):
>
> > In most cases the return address is in the same context as the calling 
> > address, but that
> >  need not be the case, especially if the producer knows in some way the 
> > call never will
> >  return. The context of the ’return address’ might be on a different line, 
> > in a different
> >  lexical block, or past the end of the calling subroutine. If a consumer 
> > were to assume that
> >  it was in the same context as the calling address, the virtual unwind 
> > might fail.
> > 
> > For architectures with constant-length instructions where the return address
> >  immediately follows the call instruction, a simple solution is to subtract 
> > the length of an
> >  instruction from the return address to obtain the calling instruction. For 
> > architectures
> >  with variable-length instructions (for example, x86), this is not 
> > possible. However,
> >  subtracting 1 from the return address, although not guaranteed to provide 
> > the exact
> >  calling address, generally will produce an address within the same context 
> > as the calling
> >  address, and that usually is sufficient.
>
> Using `GetOpcodeLoadAddress(pc) - 1` would work, but this implies that the 
> caller should be passing in "load" addresses, which is in conflict with the 
> premise I have made at the beginning of the paragraph that one should be 
> passing in "code" addresses here. (My attempt at proof by contradiction.)


Ah, I see - so this also is another argument for making things work with "load" 
addresses - good then.


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

https://reviews.llvm.org/D70840



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


[Lldb-commits] [PATCH] D74255: [LLDB] Add support for AVR breakpoints

2020-02-12 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

The whole flow here is pretty nonsensical -- the only reason we ask for the 
opcode is to get its size so we can put it into the Z0 packet -- however this 
is only needed for targets like arm, which have mutiple ISAs/opcodes, and we've 
already gotten complaints about sending that unconditionally.

However, if we look at this locally, if the AVR architecture has a trap opcode 
(maybe to implement `__builtin_debugbreak()` -- I am assuming that's what 0x98 
0x95 is), then I don't see a problem with this function returning it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74255



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


[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

2020-02-12 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

Just one more thing I didn't catch in the previous round :(.




Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:461-481
+static void CollectRegNums(const RegisterInfo *reg_info,
+   lldb_private::StreamString response) {
+  int i = 0;
+  for (const uint32_t *reg_num = reg_info->value_regs;
+   *reg_num != LLDB_INVALID_REGNUM; ++reg_num, ++i) {
+if (i > 0)
+  response.PutChar(',');

Sorry I wasn't clear about this, but my idea was to make a _single_ utility 
function for this. If you pass in the `uint32_t*` to the function instead of 
the whole RegisterInfo struct, then you can use the same code in both places..


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

https://reviews.llvm.org/D74217



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


[Lldb-commits] [PATCH] D70840: [LLDB] [DWARF] Strip out the thumb bit from addresses on ARM

2020-02-12 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D70840#1871862 , @mstorsjo wrote:

> > Now someone might argue that the looking up the address of the `ldr` opcode 
> > is wrong, because that is not the actual pc value, and that we should 
> > lookup the address of `ldr+1`. In that case we can point them to the next 
> > function (`call_noreturn`), which ends with a call to a noreturn function. 
> > Now if we get a crash in `does_not_return()`, the address we will get from 
> > the backtrace will be one byte after the last opcode of `call_noreturn` 
> > (`bl  does_not_return()`). This means that in the **non-windows** case, 
> > it will not resolve correctly to the `call_noreturn` even if we apply the 
> > usual trick of subtracting 1 from the address to bring it into the range of 
> > the calling function.
>
> Hmm... I presume this issue is present now already (and doesn't really change 
> due to normalizing the line tables and ranges on windows, to the same as 
> linux). Not familiar with "the usual trick of subtracting 1 from the address 
> to bring it into the range of the calling function", but wouldn't something 
> like `GetOpcodeLoadAddress(pc) - 1` always bring you into the range of the 
> calling function, given a return address?


Lldb uses this when unwinding specifically to deal with the "call as a last 
instruction" problem, and I'm pretty sure it's not the only tool doing that. 
This is even described in the DWARF spec (non-normative text):

> In most cases the return address is in the same context as the calling 
> address, but that
>  need not be the case, especially if the producer knows in some way the call 
> never will
>  return. The context of the ’return address’ might be on a different line, in 
> a different
>  lexical block, or past the end of the calling subroutine. If a consumer were 
> to assume that
>  it was in the same context as the calling address, the virtual unwind might 
> fail.
> 
> For architectures with constant-length instructions where the return address
>  immediately follows the call instruction, a simple solution is to subtract 
> the length of an
>  instruction from the return address to obtain the calling instruction. For 
> architectures
>  with variable-length instructions (for example, x86), this is not possible. 
> However,
>  subtracting 1 from the return address, although not guaranteed to provide 
> the exact
>  calling address, generally will produce an address within the same context 
> as the calling
>  address, and that usually is sufficient.

Using `GetOpcodeLoadAddress(pc) - 1` would work, but this implies that the 
caller should be passing in "load" addresses, which is in conflict with the 
premise I have made at the beginning of the paragraph that one should be 
passing in "code" addresses here. (My attempt at proof by contradiction.)


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

https://reviews.llvm.org/D70840



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


[Lldb-commits] [PATCH] D74478: [lldb] Let TypeSystemClang::GetDisplayTypeName remove anonymous and inline namespaces.

2020-02-12 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor created this revision.
teemperor added reviewers: shafik, jingham.
Herald added subscribers: lldb-commits, JDevlieghere, christof.
Herald added a project: LLDB.

Currently when printing data types we include implicit scopes such as inline 
namespaces or anonymous namespaces.
This leads to command output like this (for `std::set`):

  (lldb) print my_set
  (std::__1::set, std::__1::allocator >) $0 = 
size=0 {}

This patch removes all the implicit scopes when printing type names in 
TypeSystemClang::GetDisplayTypeName
so that our output now looks like this:

  (lldb) print my_set
  (std::set, std::allocator >) $0 = size=0 {}

As previously GetDisplayTypeName and GetTypeName had the same output we 
actually often used the
two as if they are the same method (they were in fact using the same 
implementation), so this patch also
fixes the places where we actually want the display type name and not the 
actual type name.

Note that this doesn't touch the `GetTypeName` class that for example the data 
formatters use, so this patch
is only changes the way we display types to the user.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D74478

Files:
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/source/DataFormatters/FormatManager.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
  
lldb/test/API/commands/expression/import-std-module/basic/TestImportStdModule.py
  
lldb/test/API/commands/expression/import-std-module/conflicts/TestStdModuleWithConflicts.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/TestDataFormatterLibcxxForwardList.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/iterator/TestDataFormatterLibccIterator.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/map/TestDataFormatterLibccMap.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/multimap/TestDataFormatterLibccMultiMap.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/multiset/TestDataFormatterLibcxxMultiSet.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/multiset/main.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/queue/TestDataFormatterLibcxxQueue.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/set/TestDataFormatterLibcxxSet.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/set/main.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/tuple/TestDataFormatterLibcxxTuple.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unordered/TestDataFormatterUnordered.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py
  lldb/test/Shell/SymbolFile/NativePDB/ast-types.cpp

Index: lldb/test/Shell/SymbolFile/NativePDB/ast-types.cpp
===
--- lldb/test/Shell/SymbolFile/NativePDB/ast-types.cpp
+++ lldb/test/Shell/SymbolFile/NativePDB/ast-types.cpp
@@ -102,9 +102,9 @@
 // CHECK: (A::C<-1>::D) ACNeg1D = (ACDMember = 0, CPtr = 0x{{0+}})
 // CHECK: (A::D) AD = {}
 // CHECK: (A::D::E) ADE = (ADDMember = 0)
-// CHECK: ((anonymous namespace)::Anonymous) AnonInt = (AnonymousMember = 0)
-// CHECK: ((anonymous namespace)::Anonymous>) AnonABCVoid = (AnonymousMember = 0)
-// CHECK: ((anonymous namespace)::Anonymous>::D) AnonABCVoidD = (AnonymousDMember = 0)
+// CHECK: (Anonymous) AnonInt = (AnonymousMember = 0)
+// CHECK: (Anonymous>) AnonABCVoid = (AnonymousMember = 0)
+// CHECK: (Anonymous>::D) AnonABCVoidD = (AnonymousDMember = 0)
 // CHECK: Dumping clang ast for 1 modules.
 // CHECK: TranslationUnitDecl {{.*}}
 // CHECK: |-CXXRecordDecl {{.*}} class TrivialC definition
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py
@@ -51,7 +51,7 @@
'}'])
 
 self.expect("frame variable v_v1",
-substrs=['v_v1 =  Active Type = std::__1::variant  {',
+substrs=['v_v1 =  Active Type = std::variant  {',
  'Value =  Active Type = int  {',
'Value = 12',
  '}',
Index: 

[Lldb-commits] [PATCH] D73206: Pass `CompileUnit *` along `DWARFDIE` for DWZ

2020-02-12 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D73206#1866280 , @jankratochvil 
wrote:

> In D73206#1859944 , @labath wrote:
>
> > From a high-level, this approach looks like it could be viable. However, 
> > you have taken this a lot further than I would have expected. What I was 
> > expecting to see would be that a bunch of the DWARF internal functions 
> > would get an extra argument (be it a CompileUnit, DWARFUnit, or whatever), 
> > but I did not expect that this would also appear on the "generic" 
> > interfaces. My idea was that the external interface would keep getting just 
> > a single user_id_t, but then during the process of decoding that integer 
> > into a DWARFDie, we would also produce the additional CompileUnit object.
>
>
> In such case there need to be two kinds of `user_id_t`. One internal to 
> DWARF/ not containing MainCU and one for "generic" interfaces which does 
> contain MainCU. Because internally if `user_id_t` contains MainCU then 
> already also `DWARFDIE` and `DIERef` must contain MainCU - see for example 
> `ElaboratingDIEIterator` calling `DWARFDIE::GetID` where is no chance to know 
> the MainCU (without heavy refactorization to pass it down the call chain) - 
> and the code does not really need to know MainCU, it is just using 
> `user_id_t` as a unique DIE identifier.


Well.. I think we should avoid using user_id_t in places where we don't have 
enough information to construct one. I have removed the usage in 
ElaboratingDIEIterator with 034c2c6771d318f962bbf001f00ee11e542e1180 
.

> ...
>  The new plan:
> 
> - `SymbolFile` and `SB*` API should say unchanged with MainCUs encoded into 
> `user_id_t` they use
> - types like `Function` do contain `CompileUnit *` but still it may be easier 
> for the APIs to encode MainCU into their `user_id_t`.

SG, I agree that would be better, since it requires less refactoring throughout.

> - indexes contain now `DIERef`. They should contain rather a new 
> MainCU-extended variant of `DIERef`.

The indexes need to *produce* an "extended" DIERef. Internally, they can 
contain whatever they want. This is particularly important for the manual 
index, where simply introducing a new field would be a pretty big memory hit, I 
think. However, it should be fairly easy to design a more compact encoding 
scheme specifically for the use in the manual index.

In fact, if we make the index functions produce results piecemeal rather than 
in one big chunk, then we could make them return DWARFDIEs (and compile units 
or whatever) directly. The reason these functions return DIERefs now is that 
DIERefs are cheap to construct, and we don't want to construct all DWARFDIEs if 
the user only cares about one result. If the user can abort iteration, then we 
can get rid of DIERefs in these functions, as the first thing that the user 
does to them anyway is to convert them to a DWARFDIE.

> - `DWARFASTParser` (+`DWARFASTParserClang`) API - there must be an additional 
> `CompileUnit *` parameter as it uses `DWARFDIE`. (`DWARFDIE` cannot fit 
> MainCU into its 16 bytes.)

Yep. It may be interesting to try this out on regular dwo first. Right now, dwo 
dodges this problem by storing a CompileUnit pointer on both skeleton and split 
units. If we can make this work without the split unit back pointer, then we 
can come move closer to how things work in llvm, and also pave the way for the 
dwz stuff.

> - `DWARFMappedHash` also must have new MainCU field I think.

This is only used in conjunction with apple accelerators. I don't think dwz 
will ever be a thing there, so you can just ignore dwz here (type units and dwo 
are also ignored here).

> - `NameToDIE` should be using the new MainCU-enhanced `DIERef`.

This is only used in the manual index, where we can do something custom if 
needed.

> Given that `user_id_t` and `DIERef` will exist both with and without MainCU 
> (depending on which code is using them) it would be nice to make a new 
> MainCU-extended inherited type for them so that their mistaken use is 
> compile-time checked. But `user_id_t` is just an integer typedef so it would 
> need to be class-wrapped.  For `user_id_t` one needs to also make a new 
> MainCU-extended type of `UserID`.

I think it would be good to have only one kind of "user id". What are the cases 
where you need a main-cu-less user id?

>> PS. It would be easier to evaluate this if this was based on master instead 
>> of some patch which is taking the source in the direction we don't want to 
>> go it.
> 
> I need to verify the new refactorization satisfies all the requirements of my 
> existing functional non-regressing DWZ implementation I have. I would then 
> have to create a second proof of concept patch being to post here based on 
> `master`. Which I hope is not required for your review before its real 
> implementation. I understand the 

[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

2020-02-12 Thread Levon Ter-Grigoryan via Phabricator via lldb-commits
PatriosTheGreat updated this revision to Diff 244130.

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

https://reviews.llvm.org/D74217

Files:
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/registers-target-xml-reading/Makefile
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/registers-target-xml-reading/TestGdbRemoteTargetXmlPacket.py
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/registers-target-xml-reading/main.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h

Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -199,6 +199,8 @@
   static std::string XMLEncodeAttributeValue(llvm::StringRef value);
 
 private:
+  llvm::Expected> BuildTargetXml();
+
   void HandleInferiorState_Exited(NativeProcessProtocol *process);
 
   void HandleInferiorState_Stopped(NativeProcessProtocol *process);
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -377,6 +377,109 @@
   }
 }
 
+static llvm::StringRef GetEncodingNameOrEmpty(const RegisterInfo *reg_info) {
+  switch (reg_info->encoding) {
+  case eEncodingUint:
+return "uint";
+  case eEncodingSint:
+return "sint";
+  case eEncodingIEEE754:
+return "ieee754";
+  case eEncodingVector:
+return "vector";
+  default:
+return "";
+  }
+}
+
+static llvm::StringRef GetFormatNameOrEmpty(const RegisterInfo *reg_info) {
+  switch (reg_info->format) {
+  case eFormatBinary:
+return "binary";
+  case eFormatDecimal:
+return "decimal";
+  case eFormatHex:
+return "hex";
+  case eFormatFloat:
+return "float";
+  case eFormatVectorOfSInt8:
+return "vector-sint8";
+  case eFormatVectorOfUInt8:
+return "vector-uint8";
+  case eFormatVectorOfSInt16:
+return "vector-sint16";
+  case eFormatVectorOfUInt16:
+return "vector-uint16";
+  case eFormatVectorOfSInt32:
+return "vector-sint32";
+  case eFormatVectorOfUInt32:
+return "vector-uint32";
+  case eFormatVectorOfFloat32:
+return "vector-float32";
+  case eFormatVectorOfUInt64:
+return "vector-uint64";
+  case eFormatVectorOfUInt128:
+return "vector-uint128";
+  default:
+return "";
+  };
+}
+
+static llvm::StringRef GetKindGenericOrEmpty(const RegisterInfo *reg_info) {
+  switch (reg_info->kinds[RegisterKind::eRegisterKindGeneric]) {
+  case LLDB_REGNUM_GENERIC_PC:
+return "pc";
+  case LLDB_REGNUM_GENERIC_SP:
+return "sp";
+  case LLDB_REGNUM_GENERIC_FP:
+return "fp";
+  case LLDB_REGNUM_GENERIC_RA:
+return "ra";
+  case LLDB_REGNUM_GENERIC_FLAGS:
+return "flags";
+  case LLDB_REGNUM_GENERIC_ARG1:
+return "arg1";
+  case LLDB_REGNUM_GENERIC_ARG2:
+return "arg2";
+  case LLDB_REGNUM_GENERIC_ARG3:
+return "arg3";
+  case LLDB_REGNUM_GENERIC_ARG4:
+return "arg4";
+  case LLDB_REGNUM_GENERIC_ARG5:
+return "arg5";
+  case LLDB_REGNUM_GENERIC_ARG6:
+return "arg6";
+  case LLDB_REGNUM_GENERIC_ARG7:
+return "arg7";
+  case LLDB_REGNUM_GENERIC_ARG8:
+return "arg8";
+  default:
+return "";
+  }
+}
+
+static void CollectRegNums(const RegisterInfo *reg_info,
+   lldb_private::StreamString response) {
+  int i = 0;
+  for (const uint32_t *reg_num = reg_info->value_regs;
+   *reg_num != LLDB_INVALID_REGNUM; ++reg_num, ++i) {
+if (i > 0)
+  response.PutChar(',');
+response.Printf("%" PRIx32, *reg_num);
+  }
+}
+
+static void CollectInvalidateRegNums(const RegisterInfo *reg_info,
+ lldb_private::StreamString response) {
+  int i = 0;
+  for (const uint32_t *reg_num = reg_info->invalidate_regs;
+   *reg_num != LLDB_INVALID_REGNUM; ++reg_num, ++i) {
+if (i > 0)
+  response.PutChar(',');
+response.Printf("%" PRIx32, *reg_num);
+  }
+}
+
 static void WriteRegisterValueInHexFixedWidth(
 StreamString , NativeRegisterContext _ctx,
 const RegisterInfo _info, const RegisterValue *reg_value_p,
@@ -1699,74 +1802,18 @@
   response.Printf("bitsize:%" PRIu32 ";offset:%" PRIu32 ";",
   reg_info->byte_size * 8, reg_info->byte_offset);
 
-  switch (reg_info->encoding) {
-  case eEncodingUint:
-response.PutCString("encoding:uint;");
-break;
-  case eEncodingSint:
-response.PutCString("encoding:sint;");
-break;
-  case eEncodingIEEE754:
-

[Lldb-commits] [PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

2020-02-12 Thread David Stenberg via Phabricator via lldb-commits
dstenb added inline comments.



Comment at: llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp:870
 if (MI->isCandidateForCallSiteEntry() &&
-DAG->getTarget().Options.EnableDebugEntryValues)
+DAG->getTarget().Options.SupportsDebugEntryValues)
   MF.addCallArgsForwardingRegs(MI, DAG->getSDCallSiteInfo(Node));

I'm sorry for commenting on this so late, but when now adapting this patch for 
our downstream target, for which we will not enable call site info by default 
yet, I noticed that the call site information wasn't added. I think that this 
should be `ShouldEmitDebugEntryValues()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73534



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


[Lldb-commits] [PATCH] D70840: [LLDB] [DWARF] Strip out the thumb bit from addresses on ARM

2020-02-12 Thread Martin Storsjö via Phabricator via lldb-commits
mstorsjo added a comment.

In D70840#1871745 , @labath wrote:

> I did not send out any emails for this, but I did talk about this with 
> @dblaikie when I was in the US last week. My take from that is that we should 
> start by "fixing" llvm-symbolizer so that it works reasonably in these cases. 
> I'll use this  godbolt link to illustrate what 
> I mean.
>
> If we take a look at the `deref` function in that link, and try to pass the 
> address of the first instruction `ldr r0, [r0]` to llvm-symbolizer, then 
> we will get something reasonable in the non-windows case, but we will get 
> nothing in the windows case, because the debug info would claim that the 
> function starts halfway into that opcode. Fixing the tool so that it returns 
> the same thing everywhere sounds reasonable, and since it does both line 
> table and debug_info lookups, this will already answer the main questions we 
> have about where this logic should be implemented. Then we can just copy that 
> in lldb.


Thanks for keeping track of this! I'll look into fixing this at some suitable 
level for llvm-symbolizer for further discussion.

> Now someone might argue that the looking up the address of the `ldr` opcode 
> is wrong, because that is not the actual pc value, and that we should lookup 
> the address of `ldr+1`. In that case we can point them to the next function 
> (`call_noreturn`), which ends with a call to a noreturn function. Now if we 
> get a crash in `does_not_return()`, the address we will get from the 
> backtrace will be one byte after the last opcode of `call_noreturn` (`bl  
> does_not_return()`). This means that in the **non-windows** case, it will not 
> resolve correctly to the `call_noreturn` even if we apply the usual trick of 
> subtracting 1 from the address to bring it into the range of the calling 
> function.

Hmm... I presume this issue is present now already (and doesn't really change 
due to normalizing the line tables and ranges on windows, to the same as 
linux). Not familiar with "the usual trick of subtracting 1 from the address to 
bring it into the range of the calling function", but wouldn't something like 
`GetOpcodeLoadAddress(pc) - 1` always bring you into the range of the calling 
function, given a return address?


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

https://reviews.llvm.org/D70840



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


[Lldb-commits] [lldb] 034c2c6 - [lldb/DWARF] Use DWARFDebugInfoEntry * in ElaboratingDIEIterator

2020-02-12 Thread Pavel Labath via lldb-commits

Author: Pavel Labath
Date: 2020-02-12T12:48:49+01:00
New Revision: 034c2c6771d318f962bbf001f00ee11e542e1180

URL: 
https://github.com/llvm/llvm-project/commit/034c2c6771d318f962bbf001f00ee11e542e1180
DIFF: 
https://github.com/llvm/llvm-project/commit/034c2c6771d318f962bbf001f00ee11e542e1180.diff

LOG: [lldb/DWARF] Use DWARFDebugInfoEntry * in ElaboratingDIEIterator

This is simpler, faster, and sufficient to uniquely idenify a DIE.

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
index 5d8c522a4dcc..8e995e627978 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
@@ -32,7 +32,7 @@ class ElaboratingDIEIterator
   // Container sizes are optimized for the case of following 
DW_AT_specification
   // and DW_AT_abstract_origin just once.
   llvm::SmallVector m_worklist;
-  llvm::SmallSet m_seen;
+  llvm::SmallSet m_seen;
 
   void Next() {
 assert(!m_worklist.empty() && "Incrementing end iterator?");
@@ -44,7 +44,7 @@ class ElaboratingDIEIterator
 // And add back any items that elaborate it.
 for (dw_attr_t attr : {DW_AT_specification, DW_AT_abstract_origin}) {
   if (DWARFDIE d = die.GetReferencedDIE(attr))
-if (m_seen.insert(die.GetID()).second)
+if (m_seen.insert(die.GetDIE()).second)
   m_worklist.push_back(d);
 }
   }



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


[Lldb-commits] [PATCH] D74475: [lldb] Replace assertTrue(a == b, "msg") with assertEquals(a, b, "msg") in the test suite

2020-02-12 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.

I think this is a good idea. I have glanced over the patch, and I have found a 
couple of places where your script gets things wrong. They all involve doing 
boolean expressions in the assertion, which takes on a different meaning when 
you replace the comparison operator  with a comma.




Comment at: 
lldb/test/API/functionalities/process_group/TestChangeProcessGroup.py:70
 value = 
thread.GetSelectedFrame().EvaluateExpression("release_child_flag = 1")
-self.assertTrue(value.IsValid() and value.GetValueAsUnsigned(0) == 1)
+self.assertEquals(value.IsValid() and value.GetValueAsUnsigned(0), 1)
 process.Continue()

This is not right. It should probably be two asserts.



Comment at: lldb/test/API/macosx/thread-names/TestInterruptThreadNames.py:39
 
-self.assertTrue(inferior_set_up.IsValid() and 
inferior_set_up.GetValueAsSigned() == 1, "Check that the program was able to 
create its threads within the allotted time")
+self.assertEquals(inferior_set_up.IsValid() and 
inferior_set_up.GetValueAsSigned(), 1, "Check that the program was able to 
create its threads within the allotted time")
 

same here



Comment at: lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py:147
 output = self.get_stdout()
-self.assertTrue(output is None or len(output) == 0,
+self.assertEquals(output is None or len(output), 0,
 "expect no program output")

An equivalent expression would be `if output: assertEquals(len(output), 0)`, 
though I'm not sure why we'd need both checks tbh -- we should have a 
consistent way of reporting "no output"...


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D74475



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


[Lldb-commits] [lldb] 7002128 - [LLDB] Fix GCC warnings about extra semicolons. NFC.

2020-02-12 Thread Martin Storsjö via lldb-commits

Author: Martin Storsjö
Date: 2020-02-12T13:40:33+02:00
New Revision: 7002128ca935aa02be104e2a0c5812260990b074

URL: 
https://github.com/llvm/llvm-project/commit/7002128ca935aa02be104e2a0c5812260990b074
DIFF: 
https://github.com/llvm/llvm-project/commit/7002128ca935aa02be104e2a0c5812260990b074.diff

LOG: [LLDB] Fix GCC warnings about extra semicolons. NFC.

Added: 


Modified: 
lldb/source/API/SystemInitializerFull.cpp
lldb/tools/lldb-test/SystemInitializerTest.cpp

Removed: 




diff  --git a/lldb/source/API/SystemInitializerFull.cpp 
b/lldb/source/API/SystemInitializerFull.cpp
index cde820bbb76b..4290d9ef6f2e 100644
--- a/lldb/source/API/SystemInitializerFull.cpp
+++ b/lldb/source/API/SystemInitializerFull.cpp
@@ -24,97 +24,97 @@
 
 #include 
 
-LLDB_PLUGIN_DECLARE(ABIMacOSX_arm64);
-LLDB_PLUGIN_DECLARE(ABISysV_arm64);
-LLDB_PLUGIN_DECLARE(ABIMacOSX_arm);
-LLDB_PLUGIN_DECLARE(ABISysV_arm);
-LLDB_PLUGIN_DECLARE(ABISysV_arc);
-LLDB_PLUGIN_DECLARE(ABISysV_hexagon);
-LLDB_PLUGIN_DECLARE(ABISysV_mips);
-LLDB_PLUGIN_DECLARE(ABISysV_mips64);
-LLDB_PLUGIN_DECLARE(ABISysV_ppc);
-LLDB_PLUGIN_DECLARE(ABISysV_ppc64);
-LLDB_PLUGIN_DECLARE(ABISysV_s390x);
-LLDB_PLUGIN_DECLARE(ABIMacOSX_i386);
-LLDB_PLUGIN_DECLARE(ABISysV_i386);
-LLDB_PLUGIN_DECLARE(ABISysV_x86_64);
-LLDB_PLUGIN_DECLARE(ABIWindows_x86_64);
-LLDB_PLUGIN_DECLARE(ObjectFileBreakpad);
-LLDB_PLUGIN_DECLARE(ObjectFileELF);
-LLDB_PLUGIN_DECLARE(ObjectFileMachO);
-LLDB_PLUGIN_DECLARE(ObjectFilePECOFF);
-LLDB_PLUGIN_DECLARE(ObjectFileWasm);
-LLDB_PLUGIN_DECLARE(ObjectContainerBSDArchive);
-LLDB_PLUGIN_DECLARE(ObjectContainerUniversalMachO);
-LLDB_PLUGIN_DECLARE(ScriptInterpreterNone);
+LLDB_PLUGIN_DECLARE(ABIMacOSX_arm64)
+LLDB_PLUGIN_DECLARE(ABISysV_arm64)
+LLDB_PLUGIN_DECLARE(ABIMacOSX_arm)
+LLDB_PLUGIN_DECLARE(ABISysV_arm)
+LLDB_PLUGIN_DECLARE(ABISysV_arc)
+LLDB_PLUGIN_DECLARE(ABISysV_hexagon)
+LLDB_PLUGIN_DECLARE(ABISysV_mips)
+LLDB_PLUGIN_DECLARE(ABISysV_mips64)
+LLDB_PLUGIN_DECLARE(ABISysV_ppc)
+LLDB_PLUGIN_DECLARE(ABISysV_ppc64)
+LLDB_PLUGIN_DECLARE(ABISysV_s390x)
+LLDB_PLUGIN_DECLARE(ABIMacOSX_i386)
+LLDB_PLUGIN_DECLARE(ABISysV_i386)
+LLDB_PLUGIN_DECLARE(ABISysV_x86_64)
+LLDB_PLUGIN_DECLARE(ABIWindows_x86_64)
+LLDB_PLUGIN_DECLARE(ObjectFileBreakpad)
+LLDB_PLUGIN_DECLARE(ObjectFileELF)
+LLDB_PLUGIN_DECLARE(ObjectFileMachO)
+LLDB_PLUGIN_DECLARE(ObjectFilePECOFF)
+LLDB_PLUGIN_DECLARE(ObjectFileWasm)
+LLDB_PLUGIN_DECLARE(ObjectContainerBSDArchive)
+LLDB_PLUGIN_DECLARE(ObjectContainerUniversalMachO)
+LLDB_PLUGIN_DECLARE(ScriptInterpreterNone)
 #if LLDB_ENABLE_PYTHON
-LLDB_PLUGIN_DECLARE(OperatingSystemPython);
-LLDB_PLUGIN_DECLARE(ScriptInterpreterPython);
+LLDB_PLUGIN_DECLARE(OperatingSystemPython)
+LLDB_PLUGIN_DECLARE(ScriptInterpreterPython)
 #endif
 #if LLDB_ENABLE_LUA
-LLDB_PLUGIN_DECLARE(ScriptInterpreterLua);
+LLDB_PLUGIN_DECLARE(ScriptInterpreterLua)
 #endif
-LLDB_PLUGIN_DECLARE(PlatformFreeBSD);
-LLDB_PLUGIN_DECLARE(PlatformLinux);
-LLDB_PLUGIN_DECLARE(PlatformNetBSD);
-LLDB_PLUGIN_DECLARE(PlatformOpenBSD);
-LLDB_PLUGIN_DECLARE(PlatformWindows);
-LLDB_PLUGIN_DECLARE(PlatformAndroid);
-LLDB_PLUGIN_DECLARE(PlatformMacOSX);
-LLDB_PLUGIN_DECLARE(TypeSystemClang);
-LLDB_PLUGIN_DECLARE(ArchitectureArm);
-LLDB_PLUGIN_DECLARE(ArchitectureMips);
-LLDB_PLUGIN_DECLARE(ArchitecturePPC64);
-LLDB_PLUGIN_DECLARE(DisassemblerLLVMC);
-LLDB_PLUGIN_DECLARE(JITLoaderGDB);
-LLDB_PLUGIN_DECLARE(ProcessElfCore);
-LLDB_PLUGIN_DECLARE(ProcessMachCore);
-LLDB_PLUGIN_DECLARE(ProcessMinidump);
-LLDB_PLUGIN_DECLARE(MemoryHistoryASan);
-LLDB_PLUGIN_DECLARE(InstrumentationRuntimeASan);
-LLDB_PLUGIN_DECLARE(InstrumentationRuntimeTSan);
-LLDB_PLUGIN_DECLARE(InstrumentationRuntimeUBSan);
-LLDB_PLUGIN_DECLARE(InstrumentationRuntimeMainThreadChecker);
-LLDB_PLUGIN_DECLARE(SymbolVendorELF);
-LLDB_PLUGIN_DECLARE(SymbolFileBreakpad);
-LLDB_PLUGIN_DECLARE(SymbolFileDWARF);
-LLDB_PLUGIN_DECLARE(SymbolFilePDB);
-LLDB_PLUGIN_DECLARE(SymbolFileSymtab);
-LLDB_PLUGIN_DECLARE(SymbolVendorWasm);
-LLDB_PLUGIN_DECLARE(UnwindAssemblyInstEmulation);
-LLDB_PLUGIN_DECLARE(UnwindAssembly_x86);
-LLDB_PLUGIN_DECLARE(EmulateInstructionARM);
-LLDB_PLUGIN_DECLARE(EmulateInstructionARM64);
-LLDB_PLUGIN_DECLARE(EmulateInstructionMIPS);
-LLDB_PLUGIN_DECLARE(EmulateInstructionMIPS64);
-LLDB_PLUGIN_DECLARE(EmulateInstructionPPC64);
-LLDB_PLUGIN_DECLARE(SymbolFileDWARFDebugMap);
-LLDB_PLUGIN_DECLARE(ItaniumABILanguageRuntime);
-LLDB_PLUGIN_DECLARE(AppleObjCRuntime);
-LLDB_PLUGIN_DECLARE(SystemRuntimeMacOSX);
-LLDB_PLUGIN_DECLARE(RenderScriptRuntime);
-LLDB_PLUGIN_DECLARE(CPlusPlusLanguage);
-LLDB_PLUGIN_DECLARE(ObjCLanguage);
-LLDB_PLUGIN_DECLARE(ObjCPlusPlusLanguage);
+LLDB_PLUGIN_DECLARE(PlatformFreeBSD)
+LLDB_PLUGIN_DECLARE(PlatformLinux)
+LLDB_PLUGIN_DECLARE(PlatformNetBSD)
+LLDB_PLUGIN_DECLARE(PlatformOpenBSD)
+LLDB_PLUGIN_DECLARE(PlatformWindows)
+LLDB_PLUGIN_DECLARE(PlatformAndroid)

[Lldb-commits] [PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

2020-02-12 Thread Djordje Todorovic via Phabricator via lldb-commits
djtodoro added a comment.

Reverted due to 
http://lab.llvm.org:8011/builders/clang-with-thin-lto-ubuntu/builds/21373/steps/build-stage3-compiler/logs/stdio.

I will reland this as soon as I fix the issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73534



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


[Lldb-commits] [PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

2020-02-12 Thread Djordje Todorovic via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9f6ff07f8a39: [DebugInfo] Enable the debug entry values 
feature by default (authored by djtodoro).
Herald added subscribers: lldb-commits, cfe-commits, jrtc27.
Herald added projects: clang, LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D73534?vs=243759=244101#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73534

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/CC1Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/debug-info-extern-call.c
  clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
  lldb/packages/Python/lldbsuite/test/decorators.py
  
lldb/test/API/functionalities/param_entry_vals/basic_entry_values_x86_64/Makefile
  llvm/include/llvm/CodeGen/CommandFlags.inc
  llvm/include/llvm/Target/TargetMachine.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h
  llvm/lib/CodeGen/LiveDebugValues.cpp
  llvm/lib/CodeGen/MIRParser/MIRParser.cpp
  llvm/lib/CodeGen/MachineFunction.cpp
  llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
  llvm/lib/CodeGen/TargetOptionsImpl.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
  llvm/lib/Target/ARM/ARMISelLowering.cpp
  llvm/lib/Target/ARM/ARMTargetMachine.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86TargetMachine.cpp
  llvm/test/CodeGen/ARM/smml.ll
  llvm/test/CodeGen/MIR/Hexagon/bundled-call-site-info.mir
  llvm/test/CodeGen/MIR/X86/call-site-info-error1.mir
  llvm/test/CodeGen/MIR/X86/call-site-info-error2.mir
  llvm/test/CodeGen/MIR/X86/call-site-info-error3.mir
  llvm/test/CodeGen/MIR/X86/call-site-info-error4.mir
  llvm/test/CodeGen/X86/call-site-info-output.ll
  llvm/test/DebugInfo/AArch64/call-site-info-output.ll
  llvm/test/DebugInfo/ARM/call-site-info-output.ll
  llvm/test/DebugInfo/ARM/entry-value-multi-byte-expr.ll
  llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-interpret-movzxi.mir
  llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-interpretation.mir
  llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-orr-moves.mir
  llvm/test/DebugInfo/MIR/ARM/dbgcall-site-interpretation.mir
  llvm/test/DebugInfo/MIR/ARM/dbgcall-site-propagated-value.mir
  llvm/test/DebugInfo/MIR/ARM/if-coverter-call-site-info.mir
  llvm/test/DebugInfo/MIR/Hexagon/dbgcall-site-instr-before-bundled-call.mir
  llvm/test/DebugInfo/MIR/Hexagon/live-debug-values-bundled-entry-values.mir
  llvm/test/DebugInfo/MIR/SystemZ/call-site-lzer.mir
  llvm/test/DebugInfo/MIR/X86/DW_OP_entry_value.mir
  llvm/test/DebugInfo/MIR/X86/call-site-gnu-vs-dwarf5-attrs.mir
  llvm/test/DebugInfo/MIR/X86/dbg-call-site-spilled-arg.mir
  llvm/test/DebugInfo/MIR/X86/dbgcall-site-copy-super-sub.mir
  llvm/test/DebugInfo/MIR/X86/dbgcall-site-interpretation.mir
  llvm/test/DebugInfo/MIR/X86/dbgcall-site-lea-interpretation.mir
  llvm/test/DebugInfo/MIR/X86/dbgcall-site-partial-describe.mir
  llvm/test/DebugInfo/MIR/X86/dbgcall-site-reference.mir
  llvm/test/DebugInfo/MIR/X86/dbgcall-site-reg-shuffle.mir
  llvm/test/DebugInfo/MIR/X86/dbgcall-site-two-fwd-reg-defs.mir
  llvm/test/DebugInfo/MIR/X86/dbginfo-entryvals.mir
  llvm/test/DebugInfo/MIR/X86/debug-call-site-param.mir
  llvm/test/DebugInfo/MIR/X86/entry-value-of-modified-param.mir
  llvm/test/DebugInfo/MIR/X86/entry-values-diamond-bbs.mir
  llvm/test/DebugInfo/MIR/X86/kill-entry-value-after-diamond-bbs.mir
  llvm/test/DebugInfo/MIR/X86/multiple-param-dbg-value-entry.mir
  llvm/test/DebugInfo/MIR/X86/propagate-entry-value-cross-bbs.mir
  llvm/test/DebugInfo/MIR/X86/unreachable-block-call-site.mir
  llvm/test/DebugInfo/Sparc/entry-value-complex-reg-expr.ll
  llvm/test/DebugInfo/X86/dbg-value-range.ll
  llvm/test/DebugInfo/X86/dbg-value-regmask-clobber.ll
  llvm/test/DebugInfo/X86/dbgcall-site-64-bit-imms.ll
  llvm/test/DebugInfo/X86/dbgcall-site-zero-valued-imms.ll
  llvm/test/DebugInfo/X86/loclists-dwp.ll
  llvm/test/DebugInfo/X86/no-entry-values-with-O0.ll
  llvm/test/tools/llvm-dwarfdump/X86/locstats.ll
  llvm/test/tools/llvm-dwarfdump/X86/stats-dbg-callsite-info.ll
  llvm/test/tools/llvm-dwarfdump/X86/valid-call-site-GNU-extensions.ll
  llvm/test/tools/llvm-locstats/locstats.ll

Index: llvm/test/tools/llvm-locstats/locstats.ll
===
--- llvm/test/tools/llvm-locstats/locstats.ll
+++ llvm/test/tools/llvm-locstats/locstats.ll
@@ -9,9 +9,9 @@
 ; LOCSTATS: [10%,20%) 0 0%
 ; LOCSTATS: [20%,30%) 1 11%
 ; LOCSTATS: [30%,40%) 0 0%
-; LOCSTATS: [40%,50%) 1 11%
-; LOCSTATS: [50%,60%) 1 11%
-; LOCSTATS: [60%,70%) 1 11%
+; LOCSTATS: [40%,50%) 0 0%
+; LOCSTATS: [50%,60%) 0 0%
+; LOCSTATS: [60%,70%) 3 33%
 ; LOCSTATS: [70%,80%) 0 0%
 ; 

[Lldb-commits] [PATCH] D74136: [LLDB] WIP: Optionally follow DW_AT_decl_file when setting breakpoint

2020-02-12 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D74136#1870029 , @jingham wrote:

> In D74136#1869622 , @kwk wrote:
>
> > @labath @jingham to summarize from what I read here and what I chatted 
> > about with @labath , the following  is a possible way to go for now, right?
> >
> > 1. We're not going to introduce my flag.
> > 2. You're both not perfectly happy with the way things are documented at 
> > the moment and dislike some of the implementation as in in LLDB but chaning 
> > it should not be part of this patch.
> > 3. @jingham wouldn't want to introduce `--compile-unit` as a flag that 
> > @labath proposed.
> > 4. You want `breakpoint set --file` to search everything, that is to say 
> > compilation units and files referenced in `DW_AT_decl_file`.
> >
> >   If you can confirm that this is correct, then I can refactor this patch 
> > to remove the switch and change the default behavior for `breakpoint set 
> > --file`. Especially point 4. is important I guess.
>
>
> There's a point (5) which is that the search in 4 should be conditioned on 
> the setting of the "target.inline-breakpoint-strategy".  That way if people 
> have big projects but don't ever #include source files, and don't build with 
> LTO, they can turn off these extra searches, which might end up being costly 
> and to no purpose for them.


I think we're on the same page, but I'll just try to rephrase this in my own 
words.

Basically, I'm hoping that we could get the `--file+--function` combo to behave 
the same way `--file+--line`. I.e., that the --file argument should specify the 
file that the function is defined in (i.e. the DW_AT_decl_file attribute of the 
function), and not the compile unit (DW_AT_name of the compile unit containing 
the function). The way the `inline-breakpoint-strategy` setting comes into play 
is that if the setting value is "headers" and the user specifies a `.cpp` file, 
then we will assume that the .cpp file also matches the name of one of the 
compile units, and we will not attempt to search compile units with different 
names (which is the same thing we do for line breakpoints, I believe).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74136



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


[Lldb-commits] [lldb] 97ed706 - Revert "[DebugInfo] Enable the debug entry values feature by default"

2020-02-12 Thread Djordje Todorovic via lldb-commits

Author: Djordje Todorovic
Date: 2020-02-12T11:59:04+01:00
New Revision: 97ed706a962af7c6835c7b6716207c4072011ac1

URL: 
https://github.com/llvm/llvm-project/commit/97ed706a962af7c6835c7b6716207c4072011ac1
DIFF: 
https://github.com/llvm/llvm-project/commit/97ed706a962af7c6835c7b6716207c4072011ac1.diff

LOG: Revert "[DebugInfo] Enable the debug entry values feature by default"

This reverts commit rG9f6ff07f8a39.

Found a test failure on clang-with-thin-lto-ubuntu buildbot.

Added: 


Modified: 
clang/include/clang/Basic/CodeGenOptions.def
clang/include/clang/Driver/CC1Options.td
clang/lib/CodeGen/BackendUtil.cpp
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/CodeGen/debug-info-extern-call.c
clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
lldb/packages/Python/lldbsuite/test/decorators.py

lldb/test/API/functionalities/param_entry_vals/basic_entry_values_x86_64/Makefile
llvm/include/llvm/CodeGen/CommandFlags.inc
llvm/include/llvm/Target/TargetMachine.h
llvm/include/llvm/Target/TargetOptions.h
llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h
llvm/lib/CodeGen/LiveDebugValues.cpp
llvm/lib/CodeGen/MIRParser/MIRParser.cpp
llvm/lib/CodeGen/MachineFunction.cpp
llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
llvm/lib/CodeGen/TargetOptionsImpl.cpp
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
llvm/lib/Target/ARM/ARMISelLowering.cpp
llvm/lib/Target/ARM/ARMTargetMachine.cpp
llvm/lib/Target/X86/X86ISelLowering.cpp
llvm/lib/Target/X86/X86TargetMachine.cpp
llvm/test/CodeGen/ARM/smml.ll
llvm/test/CodeGen/MIR/Hexagon/bundled-call-site-info.mir
llvm/test/CodeGen/MIR/X86/call-site-info-error1.mir
llvm/test/CodeGen/MIR/X86/call-site-info-error2.mir
llvm/test/CodeGen/MIR/X86/call-site-info-error3.mir
llvm/test/CodeGen/MIR/X86/call-site-info-error4.mir
llvm/test/CodeGen/X86/call-site-info-output.ll
llvm/test/DebugInfo/AArch64/call-site-info-output.ll
llvm/test/DebugInfo/ARM/call-site-info-output.ll
llvm/test/DebugInfo/ARM/entry-value-multi-byte-expr.ll
llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-interpret-movzxi.mir
llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-interpretation.mir
llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-orr-moves.mir
llvm/test/DebugInfo/MIR/ARM/dbgcall-site-interpretation.mir
llvm/test/DebugInfo/MIR/ARM/dbgcall-site-propagated-value.mir
llvm/test/DebugInfo/MIR/ARM/if-coverter-call-site-info.mir
llvm/test/DebugInfo/MIR/Hexagon/dbgcall-site-instr-before-bundled-call.mir
llvm/test/DebugInfo/MIR/Hexagon/live-debug-values-bundled-entry-values.mir
llvm/test/DebugInfo/MIR/SystemZ/call-site-lzer.mir
llvm/test/DebugInfo/MIR/X86/DW_OP_entry_value.mir
llvm/test/DebugInfo/MIR/X86/call-site-gnu-vs-dwarf5-attrs.mir
llvm/test/DebugInfo/MIR/X86/dbg-call-site-spilled-arg.mir
llvm/test/DebugInfo/MIR/X86/dbgcall-site-copy-super-sub.mir
llvm/test/DebugInfo/MIR/X86/dbgcall-site-interpretation.mir
llvm/test/DebugInfo/MIR/X86/dbgcall-site-lea-interpretation.mir
llvm/test/DebugInfo/MIR/X86/dbgcall-site-partial-describe.mir
llvm/test/DebugInfo/MIR/X86/dbgcall-site-reference.mir
llvm/test/DebugInfo/MIR/X86/dbgcall-site-reg-shuffle.mir
llvm/test/DebugInfo/MIR/X86/dbgcall-site-two-fwd-reg-defs.mir
llvm/test/DebugInfo/MIR/X86/dbginfo-entryvals.mir
llvm/test/DebugInfo/MIR/X86/debug-call-site-param.mir
llvm/test/DebugInfo/MIR/X86/entry-value-of-modified-param.mir
llvm/test/DebugInfo/MIR/X86/entry-values-diamond-bbs.mir
llvm/test/DebugInfo/MIR/X86/kill-entry-value-after-diamond-bbs.mir
llvm/test/DebugInfo/MIR/X86/multiple-param-dbg-value-entry.mir
llvm/test/DebugInfo/MIR/X86/propagate-entry-value-cross-bbs.mir
llvm/test/DebugInfo/MIR/X86/unreachable-block-call-site.mir
llvm/test/DebugInfo/Sparc/entry-value-complex-reg-expr.ll
llvm/test/DebugInfo/X86/dbg-value-range.ll
llvm/test/DebugInfo/X86/dbg-value-regmask-clobber.ll
llvm/test/DebugInfo/X86/dbgcall-site-64-bit-imms.ll
llvm/test/DebugInfo/X86/dbgcall-site-zero-valued-imms.ll
llvm/test/DebugInfo/X86/loclists-dwp.ll
llvm/test/tools/llvm-dwarfdump/X86/locstats.ll
llvm/test/tools/llvm-dwarfdump/X86/stats-dbg-callsite-info.ll
llvm/test/tools/llvm-dwarfdump/X86/valid-call-site-GNU-extensions.ll
llvm/test/tools/llvm-locstats/locstats.ll

Removed: 
llvm/test/DebugInfo/X86/no-entry-values-with-O0.ll



diff  --git a/clang/include/clang/Basic/CodeGenOptions.def 
b/clang/include/clang/Basic/CodeGenOptions.def
index fa450724ddd4..48c0df49e32d 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -63,6 +63,7 @@ 

[Lldb-commits] [PATCH] D70840: [LLDB] [DWARF] Strip out the thumb bit from addresses on ARM

2020-02-12 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

I did not send out any emails for this, but I did talk about this with 
@dblaikie when I was in the US last week. My take from that is that we should 
start by "fixing" llvm-symbolizer so that it works reasonably in these cases. 
I'll use this  godbolt link to illustrate what I 
mean.

If we take a look at the `deref` function in that link, and try to pass the 
address of the first instruction `ldr r0, [r0]` to llvm-symbolizer, then we 
will get something reasonable in the non-windows case, but we will get nothing 
in the windows case, because the debug info would claim that the function 
starts halfway into that opcode. Fixing the tool so that it returns the same 
thing everywhere sounds reasonable, and since it does both line table and 
debug_info lookups, this will already answer the main questions we have about 
where this logic should be implemented. Then we can just copy that in lldb.

Now someone might argue that the looking up the address of the `ldr` opcode is 
wrong, because that is not the actual pc value, and that we should lookup the 
address of `ldr+1`. In that case we can point them to the next function 
(`call_noreturn`), which ends with a call to a noreturn function. Now if we get 
a crash in `does_not_return()`, the address we will get from the backtrace will 
be one byte after the last opcode of `call_noreturn` (`bl  
does_not_return()`). This means that in the **non-windows** case, it will not 
resolve correctly to the `call_noreturn` even if we apply the usual trick of 
subtracting 1 from the address to bring it into the range of the calling 
function. Either way, there's some fixup that needs to happen somewhere...


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

https://reviews.llvm.org/D70840



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


[Lldb-commits] [lldb] 320647c - [lldb] Pass a valid SourceLocation to Sema::RequireCompleteType in ASTResultSynthesizer

2020-02-12 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2020-02-12T11:22:56+01:00
New Revision: 320647c02ae9827cae0824c6aec2bcdfcf482e44

URL: 
https://github.com/llvm/llvm-project/commit/320647c02ae9827cae0824c6aec2bcdfcf482e44
DIFF: 
https://github.com/llvm/llvm-project/commit/320647c02ae9827cae0824c6aec2bcdfcf482e44.diff

LOG: [lldb] Pass a valid SourceLocation to Sema::RequireCompleteType in 
ASTResultSynthesizer

Sema::RequireCompleteTypeImpl is supposed to have an assert that checks that the
SourceLocation that is passed in is always valid. It's currently commented out, 
but
as soon as this assert goes active, nearly every LLDB expression will start 
crashing as
we always pass in an invalid SourceLocation from the ASTResultSynthesizer.

This patch just passes in the valid SourceLocation of the expression (which is
the SourceLocation where the complete type is required) to prevent that from 
happening.

Added: 


Modified: 
lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
index f0e1b5dd1d02..bd705af0efcd 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
@@ -325,7 +325,8 @@ bool 
ASTResultSynthesizer::SynthesizeBodyResult(CompoundStmt *Body,
 else
   result_ptr_id = ("$__lldb_expr_result_ptr");
 
-m_sema->RequireCompleteType(SourceLocation(), expr_qual_type,
+m_sema->RequireCompleteType(last_expr->getSourceRange().getBegin(),
+expr_qual_type,
 clang::diag::err_incomplete_type);
 
 QualType ptr_qual_type;



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


[Lldb-commits] [PATCH] D73665: Stop emitting a breakpoint for each location in a breakpoint when responding to breakpoint commands.

2020-02-12 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

I'm sorry about the delay, I'm quite busy these days.

This code looks good to me now, but it's lacking test coverage. Can you add 
some test where new breakpoint locations get added or removed and check that 
the lldb-vscode generates the appropriate kind of dap message. And maybe 
another where a breakpoint is set via the the command line and check that we 
_don't_ emit any events...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73665



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


[Lldb-commits] [PATCH] D70847: [lldb-vscode] Ensure that target matches the executable file

2020-02-12 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

This approach looks good to me. It'd just be good to do something about the 
code duplication.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70847



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


[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

2020-02-12 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

Just a couple of comments to make this code more llvm-y. Without those, we have 
just moved the place which constructs the temporary std::string. I haven't 
highlighted all the uses, but overall I'd recommend using the `Format` function 
or something similar instead of `Printf`, as the latter can be error-prone 
(because of the need to pass the right PRIx macro to get the integer size 
right) and/or inefficient (need to do the `.str().c_str()` dance on StringRefs).




Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1808
+response.PutCString("encoding:");
+response.PutCString(encoding.str().c_str());
+response.PutChar(';');

Despite the name, PutCString actually takes a StringRef, so you can just drop 
the `.str().c_str()` thingy. In fact, I'd consider just writing `response << 
"encoding:" << encoding << ';';`



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:2814
+if (!encoding.empty())
+  response.Printf("encoding=\"%s\" ", encoding.str().c_str());
+

Similarly, `response << "encoding='" << encoding << "' "`, or 
`response.Format("encoding='{0}'", encoding)` would be shorter, and avoid 
string copying.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:2906
+return BuildTargetXml();
+  }
+

no braces


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

https://reviews.llvm.org/D74217



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


[Lldb-commits] [lldb] 9f6ff07 - [DebugInfo] Enable the debug entry values feature by default

2020-02-12 Thread Djordje Todorovic via lldb-commits

Author: Djordje Todorovic
Date: 2020-02-12T10:25:14+01:00
New Revision: 9f6ff07f8a396dfc736c4cb6f9fba9a203531329

URL: 
https://github.com/llvm/llvm-project/commit/9f6ff07f8a396dfc736c4cb6f9fba9a203531329
DIFF: 
https://github.com/llvm/llvm-project/commit/9f6ff07f8a396dfc736c4cb6f9fba9a203531329.diff

LOG: [DebugInfo] Enable the debug entry values feature by default

This patch enables the debug entry values feature.

  - Remove the (CC1) experimental -femit-debug-entry-values option
  - Enable it for x86, arm and aarch64 targets
  - Resolve the test failures
  - Leave the llc experimental option for targets that do not
support the CallSiteInfo yet

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

Added: 
llvm/test/DebugInfo/X86/no-entry-values-with-O0.ll

Modified: 
clang/include/clang/Basic/CodeGenOptions.def
clang/include/clang/Driver/CC1Options.td
clang/lib/CodeGen/BackendUtil.cpp
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/CodeGen/debug-info-extern-call.c
clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
lldb/packages/Python/lldbsuite/test/decorators.py

lldb/test/API/functionalities/param_entry_vals/basic_entry_values_x86_64/Makefile
llvm/include/llvm/CodeGen/CommandFlags.inc
llvm/include/llvm/Target/TargetMachine.h
llvm/include/llvm/Target/TargetOptions.h
llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h
llvm/lib/CodeGen/LiveDebugValues.cpp
llvm/lib/CodeGen/MIRParser/MIRParser.cpp
llvm/lib/CodeGen/MachineFunction.cpp
llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
llvm/lib/CodeGen/TargetOptionsImpl.cpp
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
llvm/lib/Target/ARM/ARMISelLowering.cpp
llvm/lib/Target/ARM/ARMTargetMachine.cpp
llvm/lib/Target/X86/X86ISelLowering.cpp
llvm/lib/Target/X86/X86TargetMachine.cpp
llvm/test/CodeGen/ARM/smml.ll
llvm/test/CodeGen/MIR/Hexagon/bundled-call-site-info.mir
llvm/test/CodeGen/MIR/X86/call-site-info-error1.mir
llvm/test/CodeGen/MIR/X86/call-site-info-error2.mir
llvm/test/CodeGen/MIR/X86/call-site-info-error3.mir
llvm/test/CodeGen/MIR/X86/call-site-info-error4.mir
llvm/test/CodeGen/X86/call-site-info-output.ll
llvm/test/DebugInfo/AArch64/call-site-info-output.ll
llvm/test/DebugInfo/ARM/call-site-info-output.ll
llvm/test/DebugInfo/ARM/entry-value-multi-byte-expr.ll
llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-interpret-movzxi.mir
llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-interpretation.mir
llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-orr-moves.mir
llvm/test/DebugInfo/MIR/ARM/dbgcall-site-interpretation.mir
llvm/test/DebugInfo/MIR/ARM/dbgcall-site-propagated-value.mir
llvm/test/DebugInfo/MIR/ARM/if-coverter-call-site-info.mir
llvm/test/DebugInfo/MIR/Hexagon/dbgcall-site-instr-before-bundled-call.mir
llvm/test/DebugInfo/MIR/Hexagon/live-debug-values-bundled-entry-values.mir
llvm/test/DebugInfo/MIR/SystemZ/call-site-lzer.mir
llvm/test/DebugInfo/MIR/X86/DW_OP_entry_value.mir
llvm/test/DebugInfo/MIR/X86/call-site-gnu-vs-dwarf5-attrs.mir
llvm/test/DebugInfo/MIR/X86/dbg-call-site-spilled-arg.mir
llvm/test/DebugInfo/MIR/X86/dbgcall-site-copy-super-sub.mir
llvm/test/DebugInfo/MIR/X86/dbgcall-site-interpretation.mir
llvm/test/DebugInfo/MIR/X86/dbgcall-site-lea-interpretation.mir
llvm/test/DebugInfo/MIR/X86/dbgcall-site-partial-describe.mir
llvm/test/DebugInfo/MIR/X86/dbgcall-site-reference.mir
llvm/test/DebugInfo/MIR/X86/dbgcall-site-reg-shuffle.mir
llvm/test/DebugInfo/MIR/X86/dbgcall-site-two-fwd-reg-defs.mir
llvm/test/DebugInfo/MIR/X86/dbginfo-entryvals.mir
llvm/test/DebugInfo/MIR/X86/debug-call-site-param.mir
llvm/test/DebugInfo/MIR/X86/entry-value-of-modified-param.mir
llvm/test/DebugInfo/MIR/X86/entry-values-diamond-bbs.mir
llvm/test/DebugInfo/MIR/X86/kill-entry-value-after-diamond-bbs.mir
llvm/test/DebugInfo/MIR/X86/multiple-param-dbg-value-entry.mir
llvm/test/DebugInfo/MIR/X86/propagate-entry-value-cross-bbs.mir
llvm/test/DebugInfo/MIR/X86/unreachable-block-call-site.mir
llvm/test/DebugInfo/Sparc/entry-value-complex-reg-expr.ll
llvm/test/DebugInfo/X86/dbg-value-range.ll
llvm/test/DebugInfo/X86/dbg-value-regmask-clobber.ll
llvm/test/DebugInfo/X86/dbgcall-site-64-bit-imms.ll
llvm/test/DebugInfo/X86/dbgcall-site-zero-valued-imms.ll
llvm/test/DebugInfo/X86/loclists-dwp.ll
llvm/test/tools/llvm-dwarfdump/X86/locstats.ll
llvm/test/tools/llvm-dwarfdump/X86/stats-dbg-callsite-info.ll
llvm/test/tools/llvm-dwarfdump/X86/valid-call-site-GNU-extensions.ll
llvm/test/tools/llvm-locstats/locstats.ll

Removed: 




diff  --git 

[Lldb-commits] [lldb] 30ce956 - [lldb][NFC] Remove GetConstTypeName and GetConstQualifiedTypeName from CompilerType

2020-02-12 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2020-02-12T09:49:39+01:00
New Revision: 30ce956aec94ab3785025346a890f4a8cbbe1c58

URL: 
https://github.com/llvm/llvm-project/commit/30ce956aec94ab3785025346a890f4a8cbbe1c58
DIFF: 
https://github.com/llvm/llvm-project/commit/30ce956aec94ab3785025346a890f4a8cbbe1c58.diff

LOG: [lldb][NFC] Remove GetConstTypeName and GetConstQualifiedTypeName from 
CompilerType

Beside these two functions just being wrappers around GetTypeName they are also
just a leftover from migrating the CompilerType interface to ConstString.

Added: 


Modified: 
lldb/include/lldb/DataFormatters/FormatClasses.h
lldb/include/lldb/Symbol/CompilerType.h
lldb/source/Core/ValueObject.cpp
lldb/source/Core/ValueObjectChild.cpp
lldb/source/Core/ValueObjectConstResult.cpp
lldb/source/Core/ValueObjectMemory.cpp
lldb/source/Core/ValueObjectRegister.cpp
lldb/source/DataFormatters/FormatManager.cpp

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
lldb/source/Symbol/CompilerType.cpp
lldb/source/Symbol/Type.cpp

Removed: 




diff  --git a/lldb/include/lldb/DataFormatters/FormatClasses.h 
b/lldb/include/lldb/DataFormatters/FormatClasses.h
index 344a2d70ac53..41b9539803aa 100644
--- a/lldb/include/lldb/DataFormatters/FormatClasses.h
+++ b/lldb/include/lldb/DataFormatters/FormatClasses.h
@@ -126,7 +126,7 @@ class TypeNameSpecifierImpl {
 
   TypeNameSpecifierImpl(CompilerType type) : m_is_regex(false), m_type() {
 if (type.IsValid()) {
-  m_type.m_type_name.assign(type.GetConstTypeName().GetCString());
+  m_type.m_type_name.assign(type.GetTypeName().GetCString());
   m_type.m_compiler_type = type;
 }
   }

diff  --git a/lldb/include/lldb/Symbol/CompilerType.h 
b/lldb/include/lldb/Symbol/CompilerType.h
index 3885283e8ecf..8399166b4dd4 100644
--- a/lldb/include/lldb/Symbol/CompilerType.h
+++ b/lldb/include/lldb/Symbol/CompilerType.h
@@ -149,10 +149,6 @@ class CompilerType {
 
   TypeSystem *GetTypeSystem() const { return m_type_system; }
 
-  ConstString GetConstQualifiedTypeName() const;
-
-  ConstString GetConstTypeName() const;
-
   ConstString GetTypeName() const;
 
   ConstString GetDisplayTypeName() const;

diff  --git a/lldb/source/Core/ValueObject.cpp 
b/lldb/source/Core/ValueObject.cpp
index 80e41c91e68b..71ceef7798d3 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -1650,13 +1650,13 @@ bool ValueObject::GetDeclaration(Declaration ) {
 }
 
 ConstString ValueObject::GetTypeName() {
-  return GetCompilerType().GetConstTypeName();
+  return GetCompilerType().GetTypeName();
 }
 
 ConstString ValueObject::GetDisplayTypeName() { return GetTypeName(); }
 
 ConstString ValueObject::GetQualifiedTypeName() {
-  return GetCompilerType().GetConstQualifiedTypeName();
+  return GetCompilerType().GetTypeName();
 }
 
 LanguageType ValueObject::GetObjectRuntimeLanguage() {

diff  --git a/lldb/source/Core/ValueObjectChild.cpp 
b/lldb/source/Core/ValueObjectChild.cpp
index 778bcecf05d3..6205ed32c615 100644
--- a/lldb/source/Core/ValueObjectChild.cpp
+++ b/lldb/source/Core/ValueObjectChild.cpp
@@ -70,14 +70,14 @@ static void AdjustForBitfieldness(ConstString ,
 
 ConstString ValueObjectChild::GetTypeName() {
   if (m_type_name.IsEmpty()) {
-m_type_name = GetCompilerType().GetConstTypeName();
+m_type_name = GetCompilerType().GetTypeName();
 AdjustForBitfieldness(m_type_name, m_bitfield_bit_size);
   }
   return m_type_name;
 }
 
 ConstString ValueObjectChild::GetQualifiedTypeName() {
-  ConstString qualified_name = GetCompilerType().GetConstTypeName();
+  ConstString qualified_name = GetCompilerType().GetTypeName();
   AdjustForBitfieldness(qualified_name, m_bitfield_bit_size);
   return qualified_name;
 }

diff  --git a/lldb/source/Core/ValueObjectConstResult.cpp 
b/lldb/source/Core/ValueObjectConstResult.cpp
index 80d7783ea267..8d84f8e62ccc 100644
--- a/lldb/source/Core/ValueObjectConstResult.cpp
+++ b/lldb/source/Core/ValueObjectConstResult.cpp
@@ -228,7 +228,7 @@ size_t 
ValueObjectConstResult::CalculateNumChildren(uint32_t max) {
 
 ConstString ValueObjectConstResult::GetTypeName() {
   if (m_type_name.IsEmpty())
-m_type_name = GetCompilerType().GetConstTypeName();
+m_type_name = GetCompilerType().GetTypeName();
   return m_type_name;
 }
 

diff  --git a/lldb/source/Core/ValueObjectMemory.cpp 
b/lldb/source/Core/ValueObjectMemory.cpp
index b1fd51386682..91b2c6084928 100644
--- a/lldb/source/Core/ValueObjectMemory.cpp
+++ b/lldb/source/Core/ValueObjectMemory.cpp
@@ -117,7 +117,7 @@ CompilerType ValueObjectMemory::GetCompilerTypeImpl() {
 ConstString ValueObjectMemory::GetTypeName() {
   if (m_type_sp)
 return m_type_sp->GetName();
-  return m_compiler_type.GetConstTypeName();
+  return m_compiler_type.GetTypeName();
 }
 
 

[Lldb-commits] [lldb] 440460f - [lldb][NFC] Move common_completions mapping out of CommandCompletions header.

2020-02-12 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2020-02-12T09:48:51+01:00
New Revision: 440460f1e70193f5805e2fdbc6d91ab5d5d0bbab

URL: 
https://github.com/llvm/llvm-project/commit/440460f1e70193f5805e2fdbc6d91ab5d5d0bbab
DIFF: 
https://github.com/llvm/llvm-project/commit/440460f1e70193f5805e2fdbc6d91ab5d5d0bbab.diff

LOG: [lldb][NFC] Move common_completions mapping out of CommandCompletions 
header.

Added: 


Modified: 
lldb/include/lldb/Interpreter/CommandCompletions.h
lldb/source/Commands/CommandCompletions.cpp

Removed: 




diff  --git a/lldb/include/lldb/Interpreter/CommandCompletions.h 
b/lldb/include/lldb/Interpreter/CommandCompletions.h
index c4eb1ae878f4..a678a1173c36 100644
--- a/lldb/include/lldb/Interpreter/CommandCompletions.h
+++ b/lldb/include/lldb/Interpreter/CommandCompletions.h
@@ -23,13 +23,6 @@ namespace lldb_private {
 class TildeExpressionResolver;
 class CommandCompletions {
 public:
-  // This is the command completion callback that is used to complete the
-  // argument of the option it is bound to (in the OptionDefinition table
-  // below).  Return the total number of matches.
-  typedef void (*CompletionCallback)(CommandInterpreter ,
- CompletionRequest ,
- // A search filter to limit the search...
- lldb_private::SearchFilter *searcher);
   enum CommonCompletionTypes {
 eNoCompletion = 0u,
 eSourceFileCompletion = (1u << 0),
@@ -47,11 +40,6 @@ class CommandCompletions {
 eCustomCompletion = (1u << 9)
   };
 
-  struct CommonCompletionElement {
-uint32_t type;
-CompletionCallback callback;
-  };
-
   static bool InvokeCommonCompletionCallbacks(
   CommandInterpreter , uint32_t completion_mask,
   lldb_private::CompletionRequest , SearchFilter *searcher);
@@ -93,9 +81,6 @@ class CommandCompletions {
 
   static void VariablePath(CommandInterpreter ,
CompletionRequest , SearchFilter *searcher);
-
-private:
-  static CommonCompletionElement g_common_completions[];
 };
 
 } // namespace lldb_private

diff  --git a/lldb/source/Commands/CommandCompletions.cpp 
b/lldb/source/Commands/CommandCompletions.cpp
index 31c8e6ab17cc..bc3a2ec4bd9e 100644
--- a/lldb/source/Commands/CommandCompletions.cpp
+++ b/lldb/source/Commands/CommandCompletions.cpp
@@ -27,18 +27,17 @@
 
 using namespace lldb_private;
 
-CommandCompletions::CommonCompletionElement
-CommandCompletions::g_common_completions[] = {
-{eSourceFileCompletion, CommandCompletions::SourceFiles},
-{eDiskFileCompletion, CommandCompletions::DiskFiles},
-{eDiskDirectoryCompletion, CommandCompletions::DiskDirectories},
-{eSymbolCompletion, CommandCompletions::Symbols},
-{eModuleCompletion, CommandCompletions::Modules},
-{eSettingsNameCompletion, CommandCompletions::SettingsNames},
-{ePlatformPluginCompletion, CommandCompletions::PlatformPluginNames},
-{eArchitectureCompletion, CommandCompletions::ArchitectureNames},
-{eVariablePathCompletion, CommandCompletions::VariablePath},
-{eNoCompletion, nullptr} // This one has to be last in the list.
+// This is the command completion callback that is used to complete the
+// argument of the option it is bound to (in the OptionDefinition table
+// below).
+typedef void (*CompletionCallback)(CommandInterpreter ,
+   CompletionRequest ,
+   // A search filter to limit the search...
+   lldb_private::SearchFilter *searcher);
+
+struct CommonCompletionElement {
+  uint32_t type;
+  CompletionCallback callback;
 };
 
 bool CommandCompletions::InvokeCommonCompletionCallbacks(
@@ -46,14 +45,27 @@ bool CommandCompletions::InvokeCommonCompletionCallbacks(
 CompletionRequest , SearchFilter *searcher) {
   bool handled = false;
 
+  const CommonCompletionElement common_completions[] = {
+  {eSourceFileCompletion, CommandCompletions::SourceFiles},
+  {eDiskFileCompletion, CommandCompletions::DiskFiles},
+  {eDiskDirectoryCompletion, CommandCompletions::DiskDirectories},
+  {eSymbolCompletion, CommandCompletions::Symbols},
+  {eModuleCompletion, CommandCompletions::Modules},
+  {eSettingsNameCompletion, CommandCompletions::SettingsNames},
+  {ePlatformPluginCompletion, CommandCompletions::PlatformPluginNames},
+  {eArchitectureCompletion, CommandCompletions::ArchitectureNames},
+  {eVariablePathCompletion, CommandCompletions::VariablePath},
+  {eNoCompletion, nullptr} // This one has to be last in the list.
+  };
+
   for (int i = 0;; i++) {
-if (g_common_completions[i].type == eNoCompletion)
+if (common_completions[i].type == eNoCompletion)
   break;
-else if ((g_common_completions[i].type & completion_mask) ==
- 

[Lldb-commits] [lldb] 4617fb0 - [lldb] Move implementation of GetDisplayName to TypeSystem class

2020-02-12 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2020-02-12T09:32:09+01:00
New Revision: 4617fb0b7fd444dca9bf7612dd632ef3feb61d48

URL: 
https://github.com/llvm/llvm-project/commit/4617fb0b7fd444dca9bf7612dd632ef3feb61d48
DIFF: 
https://github.com/llvm/llvm-project/commit/4617fb0b7fd444dca9bf7612dd632ef3feb61d48.diff

LOG: [lldb] Move implementation of GetDisplayName to TypeSystem class

CompilerType doesn't implement logic.

Added: 


Modified: 
lldb/include/lldb/Symbol/TypeSystem.h
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
lldb/source/Symbol/CompilerType.cpp

Removed: 




diff  --git a/lldb/include/lldb/Symbol/TypeSystem.h 
b/lldb/include/lldb/Symbol/TypeSystem.h
index 811b99028f63..188b276b7c48 100644
--- a/lldb/include/lldb/Symbol/TypeSystem.h
+++ b/lldb/include/lldb/Symbol/TypeSystem.h
@@ -199,6 +199,8 @@ class TypeSystem : public PluginInterface {
 
   virtual ConstString GetTypeName(lldb::opaque_compiler_type_t type) = 0;
 
+  virtual ConstString GetDisplayTypeName(lldb::opaque_compiler_type_t type) = 
0;
+
   virtual uint32_t
   GetTypeInfo(lldb::opaque_compiler_type_t type,
   CompilerType *pointee_or_element_compiler_type) = 0;

diff  --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
index db03595e970b..049975a3547b 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -586,6 +586,10 @@ class TypeSystemClang : public TypeSystem {
 
   ConstString GetTypeName(lldb::opaque_compiler_type_t type) override;
 
+  ConstString GetDisplayTypeName(lldb::opaque_compiler_type_t type) override {
+return GetTypeName(type);
+  }
+
   uint32_t GetTypeInfo(lldb::opaque_compiler_type_t type,
CompilerType *pointee_or_element_compiler_type) 
override;
 

diff  --git a/lldb/source/Symbol/CompilerType.cpp 
b/lldb/source/Symbol/CompilerType.cpp
index a0f38981e2db..ba69b6b9f0c2 100644
--- a/lldb/source/Symbol/CompilerType.cpp
+++ b/lldb/source/Symbol/CompilerType.cpp
@@ -288,7 +288,11 @@ ConstString CompilerType::GetTypeName() const {
   return ConstString("");
 }
 
-ConstString CompilerType::GetDisplayTypeName() const { return GetTypeName(); }
+ConstString CompilerType::GetDisplayTypeName() const {
+  if (IsValid())
+return m_type_system->GetDisplayTypeName(m_type);
+  return ConstString("");
+}
 
 uint32_t CompilerType::GetTypeInfo(
 CompilerType *pointee_or_element_compiler_type) const {



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


[Lldb-commits] [lldb] 572fc89 - [lldb][NFC] Move all completer subclasses into source file

2020-02-12 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2020-02-12T09:08:44+01:00
New Revision: 572fc8974277e5def25029219daec20d08f85030

URL: 
https://github.com/llvm/llvm-project/commit/572fc8974277e5def25029219daec20d08f85030
DIFF: 
https://github.com/llvm/llvm-project/commit/572fc8974277e5def25029219daec20d08f85030.diff

LOG: [lldb][NFC] Move all completer subclasses into source file

They are all implementation details so let's keep them out of the interface.
Also makes this code more readable by keeping these small classes
not spread over header and source file.

Added: 


Modified: 
lldb/include/lldb/Interpreter/CommandCompletions.h
lldb/source/Commands/CommandCompletions.cpp

Removed: 




diff  --git a/lldb/include/lldb/Interpreter/CommandCompletions.h 
b/lldb/include/lldb/Interpreter/CommandCompletions.h
index 39061d9db8bc..c4eb1ae878f4 100644
--- a/lldb/include/lldb/Interpreter/CommandCompletions.h
+++ b/lldb/include/lldb/Interpreter/CommandCompletions.h
@@ -94,94 +94,6 @@ class CommandCompletions {
   static void VariablePath(CommandInterpreter ,
CompletionRequest , SearchFilter *searcher);
 
-  // The Completer class is a convenient base class for building searchers that
-  // go along with the SearchFilter passed to the standard Completer functions.
-  class Completer : public Searcher {
-  public:
-Completer(CommandInterpreter , CompletionRequest );
-
-~Completer() override;
-
-CallbackReturn SearchCallback(SearchFilter , SymbolContext ,
-  Address *addr) override = 0;
-
-lldb::SearchDepth GetDepth() override = 0;
-
-virtual void DoCompletion(SearchFilter *filter) = 0;
-
-  protected:
-CommandInterpreter _interpreter;
-CompletionRequest _request;
-
-  private:
-DISALLOW_COPY_AND_ASSIGN(Completer);
-  };
-
-  // SourceFileCompleter implements the source file completer
-  class SourceFileCompleter : public Completer {
-  public:
-SourceFileCompleter(CommandInterpreter ,
-CompletionRequest );
-
-lldb::SearchDepth GetDepth() override;
-
-Searcher::CallbackReturn SearchCallback(SearchFilter ,
-SymbolContext ,
-Address *addr) override;
-
-void DoCompletion(SearchFilter *filter) override;
-
-  private:
-FileSpecList m_matching_files;
-const char *m_file_name;
-const char *m_dir_name;
-
-DISALLOW_COPY_AND_ASSIGN(SourceFileCompleter);
-  };
-
-  // ModuleCompleter implements the module completer
-  class ModuleCompleter : public Completer {
-  public:
-ModuleCompleter(CommandInterpreter ,
-CompletionRequest );
-
-lldb::SearchDepth GetDepth() override;
-
-Searcher::CallbackReturn SearchCallback(SearchFilter ,
-SymbolContext ,
-Address *addr) override;
-
-void DoCompletion(SearchFilter *filter) override;
-
-  private:
-const char *m_file_name;
-const char *m_dir_name;
-
-DISALLOW_COPY_AND_ASSIGN(ModuleCompleter);
-  };
-
-  // SymbolCompleter implements the symbol completer
-  class SymbolCompleter : public Completer {
-  public:
-SymbolCompleter(CommandInterpreter ,
-CompletionRequest );
-
-lldb::SearchDepth GetDepth() override;
-
-Searcher::CallbackReturn SearchCallback(SearchFilter ,
-SymbolContext ,
-Address *addr) override;
-
-void DoCompletion(SearchFilter *filter) override;
-
-  private:
-RegularExpression m_regex;
-typedef std::set collection;
-collection m_match_set;
-
-DISALLOW_COPY_AND_ASSIGN(SymbolCompleter);
-  };
-
 private:
   static CommonCompletionElement g_common_completions[];
 };

diff  --git a/lldb/source/Commands/CommandCompletions.cpp 
b/lldb/source/Commands/CommandCompletions.cpp
index 0e35e0d1123f..31c8e6ab17cc 100644
--- a/lldb/source/Commands/CommandCompletions.cpp
+++ b/lldb/source/Commands/CommandCompletions.cpp
@@ -59,6 +59,208 @@ bool CommandCompletions::InvokeCommonCompletionCallbacks(
   return handled;
 }
 
+namespace {
+// The Completer class is a convenient base class for building searchers that
+// go along with the SearchFilter passed to the standard Completer functions.
+class Completer : public Searcher {
+public:
+  Completer(CommandInterpreter , CompletionRequest )
+  : m_interpreter(interpreter), m_request(request) {}
+
+  ~Completer() override = default;
+
+  CallbackReturn SearchCallback(SearchFilter , SymbolContext ,
+Address *addr) override = 0;
+
+  lldb::SearchDepth GetDepth() override = 0;
+
+  virtual void DoCompletion(SearchFilter *filter) = 0;
+
+protected:
+  CommandInterpreter _interpreter;
+  CompletionRequest _request;
+
+private:
+