[Lldb-commits] [PATCH] D65185: Let tablegen generate property definitions

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: teemperor, friss, xiaobai, labath, clayborg.
Herald added a subscriber: mgorny.
Herald added a reviewer: jdoerfert.
Herald added a project: LLDB.

Property definitions are currently defined in a `PropertyDefinition` array and 
have a corresponding enum to index in this array. Unfortunately this is quite 
error prone. Indeed, just today we found an incorrect merge where a discrepancy 
between the order of the enum values and their definition caused the test suite 
to fail spectacularly.

Tablegen can streamline the process of generating the property definition table 
while at the same time guaranteeing that the enums stay in sync. That's exactly 
what this patch does. It adds a new tablegen file for the properties, building 
on top of the infrastructure that Raphael added recently for the command 
options. It also introduces two new tablegen backends: one for the property 
definitions and one for their corresponding enums.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D65185

Files:
  lldb/CMakeLists.txt
  lldb/include/lldb/Core/CMakeLists.txt
  lldb/include/lldb/Core/Properties.td
  lldb/include/lldb/Core/PropertiesBase.td
  lldb/source/Core/CMakeLists.txt
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/ModuleList.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
  lldb/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
  lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Target/CMakeLists.txt
  lldb/source/Target/Platform.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/Thread.cpp
  lldb/utils/TableGen/CMakeLists.txt
  lldb/utils/TableGen/LLDBPropertyDefEmitter.cpp
  lldb/utils/TableGen/LLDBTableGen.cpp
  lldb/utils/TableGen/LLDBTableGenBackends.h

Index: lldb/utils/TableGen/LLDBTableGenBackends.h
===
--- lldb/utils/TableGen/LLDBTableGenBackends.h
+++ lldb/utils/TableGen/LLDBTableGenBackends.h
@@ -28,6 +28,8 @@
 namespace lldb_private {
 
 void EmitOptionDefs(RecordKeeper , raw_ostream );
+void EmitPropertyDefs(RecordKeeper , raw_ostream );
+void EmitPropertyEnumDefs(RecordKeeper , raw_ostream );
 
 } // namespace lldb_private
 
Index: lldb/utils/TableGen/LLDBTableGen.cpp
===
--- lldb/utils/TableGen/LLDBTableGen.cpp
+++ lldb/utils/TableGen/LLDBTableGen.cpp
@@ -25,6 +25,8 @@
   PrintRecords,
   DumpJSON,
   GenOptionDefs,
+  GenPropertyDefs,
+  GenPropertyEnumDefs,
 };
 
 static cl::opt
@@ -34,7 +36,11 @@
   clEnumValN(DumpJSON, "dump-json",
  "Dump all records as machine-readable JSON"),
   clEnumValN(GenOptionDefs, "gen-lldb-option-defs",
- "Generate lldb option definitions")));
+ "Generate lldb option definitions"),
+  clEnumValN(GenPropertyDefs, "gen-lldb-property-defs",
+ "Generate lldb property definitions"),
+  clEnumValN(GenPropertyEnumDefs, "gen-lldb-property-enum-defs",
+ "Generate lldb property enum definitions")));
 
 static bool LLDBTableGenMain(raw_ostream , RecordKeeper ) {
   switch (Action) {
@@ -47,6 +53,12 @@
   case GenOptionDefs:
 EmitOptionDefs(Records, OS);
 break;
+  case GenPropertyDefs:
+EmitPropertyDefs(Records, OS);
+break;
+  case GenPropertyEnumDefs:
+EmitPropertyEnumDefs(Records, OS);
+break;
   }
   return false;
 }
Index: lldb/utils/TableGen/LLDBPropertyDefEmitter.cpp
===
--- /dev/null
+++ lldb/utils/TableGen/LLDBPropertyDefEmitter.cpp
@@ -0,0 +1,165 @@
+//===- LLDBPropertyDefEmitter.cpp -===//
+//
+// 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
+//
+//===--===//
+//
+// These tablegen backends emits LLDB's PropertyDefinition values.
+//
+//===--===//
+
+#include "LLDBTableGenBackends.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/TableGen/Record.h"
+#include "llvm/TableGen/StringMatcher.h"
+#include "llvm/TableGen/TableGenBackend.h"
+#include 
+#include 
+
+using namespace llvm;
+
+/// Map of properties definitions to their 

[Lldb-commits] [PATCH] D65171: [LLDB] Find debugserver in Command Line Tools as well

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

LGTM.  There was another problem I was discussing with someone the other week 
(I can't remember exactly the details) where we'd need to retrieve the Xcode 
install path so this would have been needed there too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65171



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

[Lldb-commits] [PATCH] D65171: [LLDB] Find debugserver in Command Line Tools as well

[António Afonso via Phabricator via lldb-commits]

aadsm created this revision.
aadsm added reviewers: clayborg, JDevlieghere.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This might be an edge case in regular use but if you're shipping an lldb 
version with no debugserver lldb will try to use the System one.
However, lldb only knows how to find the Xcode one and not the Command Line 
Tools one. This diff fixes that.

We try to find debugserver with 
`PlatformDarwin::LocateExecutable("debugserver")`, we call `xcode-select -p` to 
get the path and then assume this path is of Xcode.

The changes I did are:

- Change `PlatformDarwin::LocateExecutable` to also add the Command Line Tools 
directory to the list of paths to search for debugserver.
- Created a new function to find the Command Line Tools directory named 
`GetCommandLineToolsLibraryPath`.
- Refactored the code that calls `xcode-select -p` into its own function 
`GetXcodeSelectPath()`. There were 2 identical pieces of code for this so I 
reduced it to one and used this function everywhere instead.
- I also changed `PlatformDarwin::GetSDKDirectoryForModules` to use the `SDKs` 
directory that exists in the Command Line Tools installation.

I'm not sure how to create tests for this. PlatformDarwinTest is really limited 
and I couldn't find how to mock Filesystem::Instance() so I could create a 
virtual file system.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65171

Files:
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp

Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -1107,6 +1107,34 @@
   return false;
 }
 
+static FileSpec GetXcodeSelectPath() {
+  static FileSpec g_xcode_select_filespec;
+
+  if (!g_xcode_select_filespec) {
+FileSpec xcode_select_cmd("/usr/bin/xcode-select");
+if (FileSystem::Instance().Exists(xcode_select_cmd)) {
+  int exit_status = -1;
+  int signo = -1;
+  std::string command_output;
+  Status status =
+  Host::RunShellCommand("/usr/bin/xcode-select --print-path",
+nullptr, // current working directory
+_status, , _output,
+std::chrono::seconds(2), // short timeout
+false);  // don't run in a shell
+  if (status.Success() && exit_status == 0 && !command_output.empty()) {
+size_t first_non_newline = command_output.find_last_not_of("\r\n");
+if (first_non_newline != std::string::npos) {
+  command_output.erase(first_non_newline + 1);
+}
+g_xcode_select_filespec = FileSpec(command_output);
+  }
+}
+  }
+
+  return g_xcode_select_filespec;
+}
+
 // Return a directory path like /Applications/Xcode.app/Contents/Developer
 const char *PlatformDarwin::GetDeveloperDirectory() {
   std::lock_guard guard(m_mutex);
@@ -1164,34 +1192,10 @@
 }
 
 if (!developer_dir_path_valid) {
-  FileSpec xcode_select_cmd("/usr/bin/xcode-select");
-  if (FileSystem::Instance().Exists(xcode_select_cmd)) {
-int exit_status = -1;
-int signo = -1;
-std::string command_output;
-Status error =
-Host::RunShellCommand("/usr/bin/xcode-select --print-path",
-  nullptr, // current working directory
-  _status, , _output,
-  std::chrono::seconds(2), // short timeout
-  false); // don't run in a shell
-if (error.Success() && exit_status == 0 && !command_output.empty()) {
-  const char *cmd_output_ptr = command_output.c_str();
-  developer_dir_path[sizeof(developer_dir_path) - 1] = '\0';
-  size_t i;
-  for (i = 0; i < sizeof(developer_dir_path) - 1; i++) {
-if (cmd_output_ptr[i] == '\r' || cmd_output_ptr[i] == '\n' ||
-cmd_output_ptr[i] == '\0')
-  break;
-developer_dir_path[i] = cmd_output_ptr[i];
-  }
-  developer_dir_path[i] = '\0';
-
-  FileSpec devel_dir(developer_dir_path);
-  if (FileSystem::Instance().IsDirectory(devel_dir)) {
-developer_dir_path_valid = true;
-  }
-}
+  FileSpec devel_dir = GetXcodeSelectPath();
+  if (FileSystem::Instance().IsDirectory(devel_dir)) {
+devel_dir.GetPath(_dir_path[0], sizeof(developer_dir_path));
+developer_dir_path_valid = true;
   }
 }
 
@@ -1333,29 +1337,11 @@
 g_xcode_filespec = CheckPathForXcode(developer_dir_spec);
   }
 
-  // Fall back to using "xcrun" to find the selected Xcode
+  // Fall back to using "xcode-select" to find the selected Xcode
   if (!g_xcode_filespec) {
-int status = 0;
-  

[Lldb-commits] [PATCH] D65122: [Symbol] Use llvm::Expected when getting TypeSystems

[Alex Langford via Phabricator via lldb-commits]

xiaobai marked 4 inline comments as done.
xiaobai added inline comments.



Comment at: source/Core/ValueObjectRegister.cpp:261
+  if (auto *exe_module = target->GetExecutableModulePointer()) {
+if (auto type_system_or_err =
+exe_module->GetTypeSystemForLanguage(eLanguageTypeC)) {

labath wrote:
> JDevlieghere wrote:
> > As a general note: I think it's good practice to use `llvm::consumeError` 
> > when discarding the error to make it explicit that it's not handled. This 
> > also makes it easier down the line to handle the error, e.g. by logging it.
> This is not just good practice, it is actually required. An simply checking 
> the bool-ness of the Expected object will not set the "checked" flag of the 
> error object, and it will assert when it is destroyed (if asserts are 
> enabled).
So what you're saying is do not define it in an if condition, but rather get 
the Expected and if it isn't valid then we consume the error, else use the type 
system. Is that right?



Comment at: source/Expression/Materializer.cpp:875
+if (!type_system_or_err) {
+  err.SetErrorStringWithFormat(
+  "Couldn't dematerialize a result variable: "

JDevlieghere wrote:
> Another possible alternative would be to join the errors (llvm::joinErrors) 
> and use the resulting error to initialize the status. Not sure if that makes 
> things better here though. 
I'm not sure either, I'll try it and see what it looks like.



Comment at: source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp:105
 
-lldb_private::TypeSystem *DWARFBaseDIE::GetTypeSystem() const {
-  if (m_cu)
+llvm::Expected DWARFBaseDIE::GetTypeSystem() const 
{
+  llvm::Error err = llvm::Error::success();

JDevlieghere wrote:
> Seems like this could be a lot less complex with an early return:
> 
> ```
> if (!m_cu) {
> return llvm::make_error(
> "Unable to get TypeSystem, no compilation unit available",
> llvm::inconvertibleErrorCode());
> }
> return m_cu->GetTypeSystem();
> ```
Hmm, yeah that looks better.


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

https://reviews.llvm.org/D65122



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

[Lldb-commits] [lldb] r366863 - [lldb] Fix enum value description

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Tue Jul 23 15:46:16 2019
New Revision: 366863

URL: http://llvm.org/viewvc/llvm-project?rev=366863=rev
Log:
[lldb] Fix enum value description

Modified:
lldb/trunk/utils/TableGen/LLDBTableGen.cpp

Modified: lldb/trunk/utils/TableGen/LLDBTableGen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/utils/TableGen/LLDBTableGen.cpp?rev=366863=366862=366863=diff
==
--- lldb/trunk/utils/TableGen/LLDBTableGen.cpp (original)
+++ lldb/trunk/utils/TableGen/LLDBTableGen.cpp Tue Jul 23 15:46:16 2019
@@ -34,7 +34,7 @@ static cl::opt
   clEnumValN(DumpJSON, "dump-json",
  "Dump all records as machine-readable JSON"),
   clEnumValN(GenOptionDefs, "gen-lldb-option-defs",
- "Generate clang attribute clases")));
+ "Generate lldb option definitions")));
 
 static bool LLDBTableGenMain(raw_ostream , RecordKeeper ) {
   switch (Action) {


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

[Lldb-commits] [PATCH] D64767: [lldb][test_suite] Update tests with unexpected pass on Android aarch64

[Alex Langford via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL366858: [lldb][test_suite] Update tests with unexpected pass 
on Android aarch64 (authored by xiaobai, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64767?vs=209946=211360#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64767

Files:
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/hardware_breakpoints/hardware_breakpoint_on_multiple_threads/TestHWBreakMultiThread.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/deleted-executable/TestDeletedExecutable.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/inferior-assert/TestInferiorAssert.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/load_unload/TestLoadUnload.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/target_command/TestTargetCommand.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/target_create_deps/TestTargetCreateDeps.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_commands/command/TestWatchpointCommandLLDB.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_commands/command/TestWatchpointCommandPython.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_commands/condition/TestWatchpointConditionCmd.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_events/TestWatchpointEvents.py
  
lldb/trunk/packages/Python/lldbsuite/test/lang/c/const_variables/TestConstVariables.py
  
lldb/trunk/packages/Python/lldbsuite/test/lang/c/global_variables/TestGlobalVariables.py
  
lldb/trunk/packages/Python/lldbsuite/test/python_api/watchpoint/condition/TestWatchpointConditionAPI.py
  
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py

Index: lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py
+++ lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py
@@ -1279,7 +1279,6 @@
 
 @llgs_test
 @skipUnlessPlatform(oslist=['linux'])
-@expectedFailureAndroid
 @skipIf(archs=no_match(['arm', 'aarch64']))
 def test_hardware_breakpoint_set_and_remove_work_llgs(self):
 self.init_llgs_test()
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/inferior-assert/TestInferiorAssert.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/inferior-assert/TestInferiorAssert.py
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/inferior-assert/TestInferiorAssert.py
@@ -51,6 +51,7 @@
 archs=[
 "aarch64",
 "arm"],
+triple=no_match(".*-android"),
 bugnumber="llvm.org/pr25338")
 @expectedFailureAll(bugnumber="llvm.org/pr26592", triple='^mips')
 @expectedFailureNetBSD
@@ -76,6 +77,7 @@
 archs=[
 "aarch64",
 "arm"],
+triple=no_match(".*-android"),
 bugnumber="llvm.org/pr25338")
 @expectedFailureAll(bugnumber="llvm.org/pr26592", triple='^mips')
 @expectedFailureNetBSD
@@ -92,6 +94,7 @@
 archs=[
 "aarch64",
 "arm"],
+triple=no_match(".*-android"),
 bugnumber="llvm.org/pr25338")
 @expectedFailureAll(bugnumber="llvm.org/pr26592", triple='^mips')
 @expectedFailureNetBSD
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/hardware_breakpoints/hardware_breakpoint_on_multiple_threads/TestHWBreakMultiThread.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/hardware_breakpoints/hardware_breakpoint_on_multiple_threads/TestHWBreakMultiThread.py
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/hardware_breakpoints/hardware_breakpoint_on_multiple_threads/TestHWBreakMultiThread.py
@@ -22,7 +22,6 @@
 
 # LLDB supports hardware breakpoints for arm and aarch64 architectures.
 @skipIf(archs=no_match(['arm', 'aarch64']))
-@expectedFailureAndroid
 def test_hw_break_set_delete_multi_thread(self):
 self.build()
 self.setTearDownCleanup()
@@ -30,7 +29,6 @@
 
 # LLDB supports hardware breakpoints for arm and aarch64 architectures.
 @skipIf(archs=no_match(['arm', 'aarch64']))
-@expectedFailureAndroid
 def test_hw_break_set_disable_multi_thread(self):
 self.build()
 self.setTearDownCleanup()
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/target_command/TestTargetCommand.py

[Lldb-commits] [lldb] r366858 - [lldb][test_suite] Update tests with unexpected pass on Android aarch64

[Alex Langford via lldb-commits]

Author: xiaobai
Date: Tue Jul 23 15:12:16 2019
New Revision: 366858

URL: http://llvm.org/viewvc/llvm-project?rev=366858=rev
Log:
[lldb][test_suite] Update tests with unexpected pass on Android aarch64

Summary: update some test decorates that can actually pass on andriod aarch64

Patch by Wanyi Ye 

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

Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/hardware_breakpoints/hardware_breakpoint_on_multiple_threads/TestHWBreakMultiThread.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/deleted-executable/TestDeletedExecutable.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/inferior-assert/TestInferiorAssert.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/load_unload/TestLoadUnload.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/target_command/TestTargetCommand.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/target_create_deps/TestTargetCreateDeps.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_commands/command/TestWatchpointCommandLLDB.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_commands/command/TestWatchpointCommandPython.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_commands/condition/TestWatchpointConditionCmd.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_events/TestWatchpointEvents.py

lldb/trunk/packages/Python/lldbsuite/test/lang/c/const_variables/TestConstVariables.py

lldb/trunk/packages/Python/lldbsuite/test/lang/c/global_variables/TestGlobalVariables.py

lldb/trunk/packages/Python/lldbsuite/test/python_api/watchpoint/condition/TestWatchpointConditionAPI.py

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/hardware_breakpoints/hardware_breakpoint_on_multiple_threads/TestHWBreakMultiThread.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/hardware_breakpoints/hardware_breakpoint_on_multiple_threads/TestHWBreakMultiThread.py?rev=366858=366857=366858=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/hardware_breakpoints/hardware_breakpoint_on_multiple_threads/TestHWBreakMultiThread.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/hardware_breakpoints/hardware_breakpoint_on_multiple_threads/TestHWBreakMultiThread.py
 Tue Jul 23 15:12:16 2019
@@ -22,7 +22,6 @@ class HardwareBreakpointMultiThreadTestC
 
 # LLDB supports hardware breakpoints for arm and aarch64 architectures.
 @skipIf(archs=no_match(['arm', 'aarch64']))
-@expectedFailureAndroid
 def test_hw_break_set_delete_multi_thread(self):
 self.build()
 self.setTearDownCleanup()
@@ -30,7 +29,6 @@ class HardwareBreakpointMultiThreadTestC
 
 # LLDB supports hardware breakpoints for arm and aarch64 architectures.
 @skipIf(archs=no_match(['arm', 'aarch64']))
-@expectedFailureAndroid
 def test_hw_break_set_disable_multi_thread(self):
 self.build()
 self.setTearDownCleanup()

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/deleted-executable/TestDeletedExecutable.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/deleted-executable/TestDeletedExecutable.py?rev=366858=366857=366858=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/deleted-executable/TestDeletedExecutable.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/deleted-executable/TestDeletedExecutable.py
 Tue Jul 23 15:12:16 2019
@@ -18,7 +18,9 @@ class TestDeletedExecutable(TestBase):
 NO_DEBUG_INFO_TESTCASE = True
 
 @skipIfWindows # cannot delete a running executable
-@expectedFailureAll(oslist=["linux"]) # determining the architecture of 
the process fails
+@expectedFailureAll(oslist=["linux"],
+triple=no_match('aarch64-.*-android'))
+# determining the architecture of the process fails
 @expectedFailureNetBSD
 def test(self):
 self.build()

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/inferior-assert/TestInferiorAssert.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/inferior-assert/TestInferiorAssert.py?rev=366858=366857=366858=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/inferior-assert/TestInferiorAssert.py
 (original)
+++ 

[Lldb-commits] [lldb] r366852 - [ExpressionParser] Fix formatting and whitespace (NFC)

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Tue Jul 23 14:14:23 2019
New Revision: 366852

URL: http://llvm.org/viewvc/llvm-project?rev=366852=rev
Log:
[ExpressionParser] Fix formatting and whitespace (NFC)

Fix formatting and whitespace before making changes to this file.

Modified:
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp

Modified: 
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp?rev=366852=366851=366852=diff
==
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp 
(original)
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp 
Tue Jul 23 14:14:23 2019
@@ -196,9 +196,12 @@ namespace {
 /// the type.
 class Completer : public clang::RecursiveASTVisitor {
 private:
-  clang::ASTImporter _exporter; /// Used to import Decl contents
-  clang::FileID m_file;   /// The file that's going away
-  llvm::DenseSet m_completed;  /// Visited Decls, to avoid 
cycles
+  /// Used to import Decl contents
+  clang::ASTImporter _exporter;
+  /// The file that's going away
+  clang::FileID m_file;
+  /// Visited Decls, to avoid cycles
+  llvm::DenseSet m_completed;
 
   bool ImportAndCheckCompletable(clang::Decl *decl) {
 (void)m_exporter.Import(decl);
@@ -237,7 +240,7 @@ private:
 public:
   Completer(clang::ASTImporter , clang::FileID file)
   : m_exporter(exporter), m_file(file) {}
-  
+
   // Implements the RecursiveASTVisitor's core API.  It is called on each Decl
   // that the RecursiveASTVisitor encounters, and returns true if the traversal
   // should continue.
@@ -246,11 +249,10 @@ public:
 return true;
   }
 };
-}
+} // namespace
 
 static void CompleteAllDeclContexts(clang::ASTImporter ,
-clang::FileID file,
-clang::QualType root) {
+clang::FileID file, clang::QualType root) {
   clang::QualType canonical_type = root.getCanonicalType();
   if (clang::TagDecl *tag_decl = canonical_type->getAsTagDecl()) {
 Completer(exporter, file).TraverseDecl(tag_decl);
@@ -263,12 +265,12 @@ static void CompleteAllDeclContexts(clan
 }
 
 static clang::QualType ExportAllDeclaredTypes(
-clang::ExternalASTMerger ,
-clang::ASTContext , clang::FileManager _file_manager,
+clang::ExternalASTMerger , clang::ASTContext ,
+clang::FileManager _file_manager,
 const clang::ExternalASTMerger::OriginMap _origin_map,
 clang::FileID file, clang::QualType root) {
-  clang::ExternalASTMerger::ImporterSource importer_source =
-  { source, source_file_manager, source_origin_map };
+  clang::ExternalASTMerger::ImporterSource importer_source = {
+  source, source_file_manager, source_origin_map};
   merger.AddSources(importer_source);
   clang::ASTImporter  = merger.ImporterForOrigin(source);
   CompleteAllDeclContexts(exporter, file, root);
@@ -286,10 +288,10 @@ static clang::QualType ExportAllDeclared
 TypeFromUser ClangExpressionDeclMap::DeportType(ClangASTContext ,
 ClangASTContext ,
 TypeFromParser parser_type) {
-  assert ( == m_target->GetScratchClangASTContext());
-  assert ((TypeSystem*) == parser_type.GetTypeSystem());
-  assert (source.getASTContext() == m_ast_context);
-  
+  assert( == m_target->GetScratchClangASTContext());
+  assert((TypeSystem *) == parser_type.GetTypeSystem());
+  assert(source.getASTContext() == m_ast_context);
+
   if (m_ast_importer_sp) {
 return TypeFromUser(m_ast_importer_sp->DeportType(
 target.getASTContext(), source.getASTContext(),
@@ -299,13 +301,11 @@ TypeFromUser ClangExpressionDeclMap::Dep
 clang::FileID source_file =
 source.getASTContext()->getSourceManager().getFileID(
 source.getASTContext()->getTranslationUnitDecl()->getLocation());
-auto scratch_ast_context = static_cast(
+auto scratch_ast_context = static_cast(
 m_target->GetScratchClangASTContext());
 clang::QualType exported_type = ExportAllDeclaredTypes(
-scratch_ast_context->GetMergerUnchecked(),
-*source.getASTContext(), *source.getFileManager(),
-m_merger_up->GetOrigins(),
-source_file,
+scratch_ast_context->GetMergerUnchecked(), *source.getASTContext(),
+*source.getFileManager(), m_merger_up->GetOrigins(), source_file,
 clang::QualType::getFromOpaquePtr(parser_type.GetOpaqueQualType()));
 return TypeFromUser(exported_type.getAsOpaquePtr(), );
   } else {
@@ -828,7 +828,7 @@ void ClangExpressionDeclMap::FindExterna
   log->Printf("  CEDM::FEVD[%u] Inspecting (NamespaceMap*)%p (%d entries)",
   current_id, 

[Lldb-commits] [lldb] r366853 - [ExpressionParser] Handle llvm::Expected result

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Tue Jul 23 14:14:41 2019
New Revision: 366853

URL: http://llvm.org/viewvc/llvm-project?rev=366853=rev
Log:
[ExpressionParser] Handle llvm::Expected result

This fixes the unchecked-error assertion at runtime.

  Expected must be checked before access or destruction. Expected
  value was in success state. (Note: Expected values in success mode
  must still be checked prior to being destroyed).

Modified:
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp

Modified: 
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp?rev=366853=366852=366853=diff
==
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp 
(original)
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp 
Tue Jul 23 14:14:41 2019
@@ -204,7 +204,11 @@ private:
   llvm::DenseSet m_completed;
 
   bool ImportAndCheckCompletable(clang::Decl *decl) {
-(void)m_exporter.Import(decl);
+llvm::Expected imported_decl = m_exporter.Import(decl);
+if (!imported_decl) {
+  llvm::consumeError(imported_decl.takeError());
+  return false;
+}
 if (m_completed.count(decl))
   return false;
 if (!llvm::isa(decl))
@@ -225,7 +229,11 @@ private:
   void Complete(clang::Decl *decl) {
 m_completed.insert(decl);
 auto *decl_context = llvm::cast(decl);
-(void)m_exporter.Import(decl);
+llvm::Expected imported_decl = m_exporter.Import(decl);
+if (!imported_decl) {
+  llvm::consumeError(imported_decl.takeError());
+  return;
+}
 m_exporter.CompleteDecl(decl);
 for (Decl *child : decl_context->decls())
   if (ImportAndCheckCompletable(child))


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

[Lldb-commits] [PATCH] D65152: Fix issues with inferior stdout coming out of order

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

I agree with Greg, having one function that can do any of the combinations of 
stdout & stderr seems more convenient.

Other than that, this is fine.


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

https://reviews.llvm.org/D65152



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

[Lldb-commits] [PATCH] D65163: Fix occasional hangs of VSCode testcases

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added a comment.

One heisenbug down!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65163



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

[Lldb-commits] [PATCH] D65163: Fix occasional hangs of VSCode testcases

[Jan Kratochvil via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL366850: [lldb] Fix occasional hangs of VSCode testcases 
(authored by jankratochvil, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65163?vs=211345=211353#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65163

Files:
  lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py


Index: lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
+++ lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
@@ -498,13 +498,7 @@
 'arguments': args_dict
 }
 response = self.send_recv(command_dict)
-recv_packets = []
-self.recv_condition.acquire()
-for event in self.recv_packets:
-if event['event'] != 'stopped':
-recv_packets.append(event)
-self.recv_packets = recv_packets
-self.recv_condition.release()
+# Caller must still call wait_for_stopped.
 return response
 
 def request_disconnect(self, terminateDebuggee=None):


Index: lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
+++ lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
@@ -498,13 +498,7 @@
 'arguments': args_dict
 }
 response = self.send_recv(command_dict)
-recv_packets = []
-self.recv_condition.acquire()
-for event in self.recv_packets:
-if event['event'] != 'stopped':
-recv_packets.append(event)
-self.recv_packets = recv_packets
-self.recv_condition.release()
+# Caller must still call wait_for_stopped.
 return response
 
 def request_disconnect(self, terminateDebuggee=None):
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r366850 - [lldb] Fix occasional hangs of VSCode testcases

[Jan Kratochvil via lldb-commits]

Author: jankratochvil
Date: Tue Jul 23 13:45:03 2019
New Revision: 366850

URL: http://llvm.org/viewvc/llvm-project?rev=366850=rev
Log:
[lldb] Fix occasional hangs of VSCode testcases

On slower machines the vscode testcases were sometimes hanging:

  1910 ? Sl 0:00 |   \_ /usr/bin/python .../llvm/tools/lldb/test/dotest.py ... 
-p TestVSCode_setBreakpoints.py
  2649 ? Sl 0:00 |   \_ .../build/bin/lldb-vscode
  2690 ? S  0:00 |   \_ .../build/bin/lldb-server gdbserver --fd=9 
--native-regs --setsid
  2708 ? t  0:00 |   \_ 
.../build/lldb-test-build.noindex/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.test_functionality/a.out

A reproducer of the racy bug for send_recv():
# self.send_packet(command)
#+import time
#+time.sleep(1)
# done = False

I guess `request_continue` was probably originally intended to be synchronous
but then it isn't and this code has been leftover there.

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

Modified:
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py

Modified: lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py?rev=366850=366849=366850=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py 
(original)
+++ lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py Tue 
Jul 23 13:45:03 2019
@@ -498,13 +498,7 @@ class DebugCommunication(object):
 'arguments': args_dict
 }
 response = self.send_recv(command_dict)
-recv_packets = []
-self.recv_condition.acquire()
-for event in self.recv_packets:
-if event['event'] != 'stopped':
-recv_packets.append(event)
-self.recv_packets = recv_packets
-self.recv_condition.release()
+# Caller must still call wait_for_stopped.
 return response
 
 def request_disconnect(self, terminateDebuggee=None):


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

[Lldb-commits] [PATCH] D62503: Add ReadCStringFromMemory for faster string reads

[António Afonso via Phabricator via lldb-commits]

aadsm added a comment.

I've just done these 2 steps:

- Revert rL364751  (which is a revert itself)
- Revert 9c10b620c061 
 (will 
re-apply this one)

Should be fine since now we have an option that is false by default. Will then 
land D64013 .


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62503



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

[Lldb-commits] [PATCH] D64251: Don't depend on psutil on AIX

[Dan Liew via Phabricator via lldb-commits]

delcypher accepted this revision.
delcypher added a comment.

LGTM.

@davide While I agree with sentiment of fixing psutil, the use of psutil was 
never meant to be permanent. As the owner of this code I don't mind making 
accommodations for other platforms provided we keep implementation details well 
contained as this patch now does.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64251



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

[Lldb-commits] [PATCH] D65123: Restore tests for lldb-server and lldb-vscode removed at rL366590

[António Afonso via Phabricator via lldb-commits]

aadsm added a comment.

@Hui what do you mean? the history is still there:

  $ git log 
lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  commit a61c247ce189efe7641dccb8ad7f85f60fc72220 (master)
  Author: Antonio Afonso 
  Date:   Mon Jul 22 23:35:05 2019 +
  
  Restore tests for lldb-server and lldb-vscode removed at rL366590
  
  commit b45853f173139c7c3078b97f53e7a6eba6148c13
  Author: Raphael Isemann 
  Date:   Fri Jul 19 15:55:23 2019 +
  
  [lldb][NFC] Cleanup mentions and code related to lldb-mi
  
  commit 08c38f77c5fb4d3735ec215032fed8ee6730b3db
  Author: Pavel Labath 
  Date:   Mon Jul 1 12:41:20 2019 +
  
  Revert "Implement xfer:libraries-svr4:read packet"
  ...


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65123



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

[Lldb-commits] [PATCH] D65165: [Symbol] Fix some botched logic in Variable::GetLanguage

[Alex Langford via Phabricator via lldb-commits]

xiaobai created this revision.
xiaobai added a reviewer: labath.
Herald added a reviewer: jdoerfert.

I messed up the logic for this. Fixing with some improvements suggested
by Pavel.


https://reviews.llvm.org/D65165

Files:
  source/Symbol/Variable.cpp


Index: source/Symbol/Variable.cpp
===
--- source/Symbol/Variable.cpp
+++ source/Symbol/Variable.cpp
@@ -59,12 +59,12 @@
 return lang;
 
   if (auto *func = m_owner_scope->CalculateSymbolContextFunction()) {
-if ((lang = func->GetLanguage()) && lang != lldb::eLanguageTypeUnknown)
+if ((lang = func->GetLanguage()))
+  return lang;
+  } else if (auto *comp_unit =
+ m_owner_scope->CalculateSymbolContextCompileUnit()) {
+if ((lang = comp_unit->GetLanguage()))
   return lang;
-else if (auto *comp_unit =
- m_owner_scope->CalculateSymbolContextCompileUnit())
-  if ((lang = func->GetLanguage()) && lang != lldb::eLanguageTypeUnknown)
-return lang;
   }
 
   return lldb::eLanguageTypeUnknown;


Index: source/Symbol/Variable.cpp
===
--- source/Symbol/Variable.cpp
+++ source/Symbol/Variable.cpp
@@ -59,12 +59,12 @@
 return lang;
 
   if (auto *func = m_owner_scope->CalculateSymbolContextFunction()) {
-if ((lang = func->GetLanguage()) && lang != lldb::eLanguageTypeUnknown)
+if ((lang = func->GetLanguage()))
+  return lang;
+  } else if (auto *comp_unit =
+ m_owner_scope->CalculateSymbolContextCompileUnit()) {
+if ((lang = comp_unit->GetLanguage()))
   return lang;
-else if (auto *comp_unit =
- m_owner_scope->CalculateSymbolContextCompileUnit())
-  if ((lang = func->GetLanguage()) && lang != lldb::eLanguageTypeUnknown)
-return lang;
   }
 
   return lldb::eLanguageTypeUnknown;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D65135: SymbolVendor: Remove the type list member

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added inline comments.
This revision is now accepted and ready to land.



Comment at: source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp:1286
 
-  size_t old_count = m_obj_file->GetModule()->GetTypeList()->GetSize();
+  size_t old_count = GetTypeList().GetSize();
   LazyRandomTypeCollection  = m_index->tpi().typeCollection();

nit: maybe we can `const` this and the one below?


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

https://reviews.llvm.org/D65135



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

[Lldb-commits] [PATCH] D65128: [Logging] Replace Log::Printf with LLDB_LOG macro (NFC)

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 211350.
JDevlieghere edited the summary of this revision.
JDevlieghere added a comment.

Introduce `LLDB_LOGF` macro to support printf-style format strings.


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

https://reviews.llvm.org/D65128

Files:
  lldb/include/lldb/Utility/Log.h
  lldb/source/API/SBDebugger.cpp
  lldb/source/API/SBFrame.cpp
  lldb/source/API/SBTarget.cpp
  lldb/source/Breakpoint/Breakpoint.cpp
  lldb/source/Breakpoint/BreakpointLocation.cpp
  lldb/source/Breakpoint/BreakpointResolver.cpp
  lldb/source/Breakpoint/BreakpointResolverAddress.cpp
  lldb/source/Breakpoint/BreakpointResolverName.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Core/AddressResolverFileLine.cpp
  lldb/source/Core/Communication.cpp
  lldb/source/Core/FormatEntity.cpp
  lldb/source/Core/Mangled.cpp
  lldb/source/Core/Module.cpp
  lldb/source/Core/ModuleList.cpp
  lldb/source/Core/ValueObject.cpp
  lldb/source/Core/ValueObjectDynamicValue.cpp
  lldb/source/Core/ValueObjectSyntheticFilter.cpp
  lldb/source/DataFormatters/FormatManager.cpp
  lldb/source/DataFormatters/TypeCategoryMap.cpp
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Expression/ExpressionVariable.cpp
  lldb/source/Expression/FunctionCaller.cpp
  lldb/source/Expression/IRExecutionUnit.cpp
  lldb/source/Expression/IRInterpreter.cpp
  lldb/source/Expression/IRMemoryMap.cpp
  lldb/source/Expression/LLVMUserExpression.cpp
  lldb/source/Expression/Materializer.cpp
  lldb/source/Expression/UserExpression.cpp
  lldb/source/Host/common/Host.cpp
  lldb/source/Host/common/HostInfoBase.cpp
  lldb/source/Host/common/HostNativeThreadBase.cpp
  lldb/source/Host/common/NativeProcessProtocol.cpp
  lldb/source/Host/common/NativeRegisterContext.cpp
  lldb/source/Host/common/Socket.cpp
  lldb/source/Host/common/TCPSocket.cpp
  lldb/source/Host/common/UDPSocket.cpp
  lldb/source/Host/linux/HostInfoLinux.cpp
  lldb/source/Host/macosx/objcxx/Host.mm
  lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
  lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
  lldb/source/Host/windows/ConnectionGenericFileWindows.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Plugins/ABI/MacOSX-arm64/ABIMacOSX_arm64.cpp
  lldb/source/Plugins/ABI/SysV-arm64/ABISysV_arm64.cpp
  lldb/source/Plugins/ABI/SysV-mips/ABISysV_mips.cpp
  lldb/source/Plugins/ABI/SysV-mips64/ABISysV_mips64.cpp
  lldb/source/Plugins/ABI/SysV-ppc/ABISysV_ppc.cpp
  lldb/source/Plugins/ABI/SysV-ppc64/ABISysV_ppc64.cpp
  lldb/source/Plugins/ABI/SysV-s390x/ABISysV_s390x.cpp
  lldb/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
  lldb/source/Plugins/ABI/Windows-x86_64/ABIWindows_x86_64.cpp
  lldb/source/Plugins/Architecture/Mips/ArchitectureMips.cpp
  lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
  lldb/source/Plugins/DynamicLoader/Hexagon-DYLD/DynamicLoaderHexagonDYLD.cpp
  lldb/source/Plugins/DynamicLoader/Hexagon-DYLD/HexagonDYLDRendezvous.cpp
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  lldb/source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ASTDumper.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangFunctionCaller.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
  lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp
  lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
  lldb/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp
  
lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
  
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptExpressionOpts.cpp
  
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
  

[Lldb-commits] [PATCH] D65163: Fix occasional hangs of VSCode testcases

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D65163#1597902 , @labath wrote:

> Thank you very much for tracking this down. Maybe wait a while to see if 
> @clayborg has any thoughts on this, but otherwise, this looks good to me.


haha, I already accepted. So good to go.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D65163



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

[Lldb-commits] [PATCH] D65152: Fix issues with inferior stdout coming out of order

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: include/lldb/Core/Debugger.h:350
+  std::mutex m_output_flush_mutex;
+  void FlushProcessOutput(Process , bool is_stdout);
 

What about making this function:

```
void Debugger::FlushProcessOutput(Process , bool flush_stdout, bool 
flush_stderr);
```

Then we don't need to call this twice everywhere.




Comment at: source/Core/Debugger.cpp:1416-1421
+if (got_stdout || got_state_changed)
+  FlushProcessOutput(*process_sp, /*is_stdout*/ true);
 
 // Now display and STDERR
-if (got_stderr || got_state_changed) {
-  GetProcessSTDERR(process_sp.get(), error_stream_sp.get());
-}
+if (got_stderr || got_state_changed)
+  FlushProcessOutput(*process_sp, /*is_stdout*/ false);

If we change FlushProcessOutput as mentioned above this would be:

```
FlushProcessOutput(Process , got_stdout || got_state_changed, 
got_stderr || got_state_changed);
```





Comment at: source/Interpreter/CommandInterpreter.cpp:2712-2713
+return;
+  m_debugger.FlushProcessOutput(*process_sp, /*is_stdout*/ true);
+  m_debugger.FlushProcessOutput(*process_sp, /*is_stdout*/ false);
 }

```
m_debugger.FlushProcessOutput(*process_sp, true, true);
```


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

https://reviews.llvm.org/D65152



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

[Lldb-commits] [PATCH] D65109: [LLDB] Remove the Xcode project

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

In D65109#1597492 , @sgraenitz wrote:

> Seriously, discussion for changes like this should be open for more than 
> 1h15min! I am in favor of the change in principle, but there's a number of 
> things that have been rushed over here,


My two cents: altho this went through really fast, the only remaining users of 
the Xcode project file were JimI and myself.  Greg Clayton got tired of having 
to update it (or waiting for Jim or I to update it) a while ago.  And it hasn't 
been buildable for over a week now - neither Jim nor I had updated it to 
process the CommandOptions tablegen .td file since that was added. I don't 
think removing it was especially controversial - but good point on the 
ancillary files that needed to also be cleaned up.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65109



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

[Lldb-commits] [PATCH] D65163: Fix occasional hangs of VSCode testcases

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.

Thank you very much for tracking this down. Maybe wait a while to see if 
@clayborg has any thoughts on this, but otherwise, this looks good to me.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D65163



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

[Lldb-commits] [PATCH] D65163: Fix occasional hangs of VSCode testcases

[Greg Clayton via Phabricator via lldb-commits]

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

If all tests pass, then I am good with this.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D65163



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

[Lldb-commits] [PATCH] D65123: Restore tests for lldb-server and lldb-vscode removed at rL366590

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added a comment.

In D65123#1597229 , @jankratochvil 
wrote:

> I was glad the `vscode` testcases are gone as they are racy causing many 
> false failures on our Fedora buildbot always hanging until the whole 
> testsuite gets killed:


That should be fixed now by D65163 .


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65123



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

[Lldb-commits] [PATCH] D65163: Fix occasional hangs of VSCode testcases

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil created this revision.
jankratochvil added reviewers: clayborg, labath, stella.stamenova, gkistanova.
jankratochvil added a project: LLDB.
Herald added a reviewer: JDevlieghere.
Herald added a subscriber: lldb-commits.

On slower machines the vscode testcases were sometimes hanging:

  1910 ?Sl 0:00  |   \_ /usr/bin/python 
/home/jkratoch/slave-lldb-x86_64-fedora/lldb-x86_64-fedora/llvm/tools/lldb/test/dotest.py
 -q --arch=x86_64 -s 
/home/jkratoch/slave-lldb-x86_64-fedora/lldb-x86_64-fedora/build/lldb-test-traces
 --build-dir 
/home/jkratoch/slave-lldb-x86_64-fedora/lldb-x86_64-fedora/build/lldb-test-build.noindex
 -S nm -u CXXFLAGS -u CFLAGS --executable 
/home/jkratoch/slave-lldb-x86_64-fedora/lldb-x86_64-fedora/build/./bin/lldb 
--dsymutil 
/home/jkratoch/slave-lldb-x86_64-fedora/lldb-x86_64-fedora/build/./bin/dsymutil 
--filecheck 
/home/jkratoch/slave-lldb-x86_64-fedora/lldb-x86_64-fedora/build/./bin/FileCheck
 -C 
/home/jkratoch/slave-lldb-x86_64-fedora/lldb-x86_64-fedora/build/./bin/clang 
--env ARCHIVER=/usr/bin/ar --env OBJCOPY=/usr/bin/objcopy --env 
LLVM_LIBS_DIR=/home/jkratoch/slave-lldb-x86_64-fedora/lldb-x86_64-fedora/build/./lib
 
/home/jkratoch/slave-lldb-x86_64-fedora/lldb-x86_64-fedora/llvm/tools/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/breakpoint
 -p TestVSCode_setBreakpoints.py
  2649 ?Sl 0:00  |   \_ 
/home/jkratoch/slave-lldb-x86_64-fedora/lldb-x86_64-fedora/build/bin/lldb-vscode
  2690 ?S  0:00  |   \_ 
/home/jkratoch/slave-lldb-x86_64-fedora/lldb-x86_64-fedora/build/bin/lldb-server
 gdbserver --fd=9 --native-regs --setsid
  2708 ?t  0:00  |   \_ 
/home/jkratoch/slave-lldb-x86_64-fedora/lldb-x86_64-fedora/build/lldb-test-build.noindex/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.test_functionality/a.out

Here is also a reproducer of the racy bug 
.
I guess `request_continue` was probably originally intended to be synchronous 
but then it isn't and this code has been leftover there.
I think it is an obvious fix but I have requested a review.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D65163

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py


Index: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
===
--- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
@@ -498,13 +498,7 @@
 'arguments': args_dict
 }
 response = self.send_recv(command_dict)
-recv_packets = []
-self.recv_condition.acquire()
-for event in self.recv_packets:
-if event['event'] != 'stopped':
-recv_packets.append(event)
-self.recv_packets = recv_packets
-self.recv_condition.release()
+# Caller must still call wait_for_stopped.
 return response
 
 def request_disconnect(self, terminateDebuggee=None):


Index: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
===
--- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
@@ -498,13 +498,7 @@
 'arguments': args_dict
 }
 response = self.send_recv(command_dict)
-recv_packets = []
-self.recv_condition.acquire()
-for event in self.recv_packets:
-if event['event'] != 'stopped':
-recv_packets.append(event)
-self.recv_packets = recv_packets
-self.recv_condition.release()
+# Caller must still call wait_for_stopped.
 return response
 
 def request_disconnect(self, terminateDebuggee=None):
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D65123: Restore tests for lldb-server and lldb-vscode removed at rL366590

[Hui Huang via Phabricator via lldb-commits]

Hui added a comment.

Any chance to restore the tracking history as well?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65123



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

[Lldb-commits] [PATCH] D65123: Restore tests for lldb-server and lldb-vscode removed at rL366590

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added a comment.

In D65123#1597742 , @aadsm wrote:

> @jankratochvil interesting, we're also not able to run the lldb-vscode tests 
> and it was on my todo list to take a look.


It is hanging in `vscode.py` function `recv_packet` somehow despite the packet 
`type=event event=stopped` is received by the other Python thread (and put into 
`recv_packets`) it somehow never quits the loop.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65123



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

[Lldb-commits] [lldb] r366833 - [utils] Remove sync-source (with SVN)

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Tue Jul 23 11:09:13 2019
New Revision: 366833

URL: http://llvm.org/viewvc/llvm-project?rev=366833=rev
Log:
[utils] Remove sync-source (with SVN)

Removed:
lldb/trunk/utils/sync-source/

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

[Lldb-commits] [PATCH] D65109: [LLDB] Remove the Xcode project

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D65109#1597492 , @sgraenitz wrote:

> Seriously, discussion for changes like this should be open for more than 
> 1h15min!


We discussed this and came to an agreement only a few hours before in the team 
meeting, which you were part of. As far as we know, Greg was the only user of 
the Xcode project outside of Apple, and he gave the thumbs up.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65109



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

[Lldb-commits] [PATCH] D65155: [lldb] Remove Xcode project legacy

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D65155#1597750 , @JDevlieghere 
wrote:

> In D65155#1597744 , @xiaobai wrote:
>
> > In D65155#1597710 , @JDevlieghere 
> > wrote:
> >
> > > In D65155#1597657 , @lanza wrote:
> > >
> > > > Not directly related, but there's some other legacy cruft I figure are 
> > > > valid for removal. e.g.
> > > >
> > > >   utils/sync-source
> > > >   misc/grep-svn-log.py
> > > >   git-svn/convert.py
> > >
> > >
> > > r366827.
> >
> >
> > This removed the entire utils directory. Perhaps related to the same thing 
> > that happened in D65123 ?
>
>
> Yeah, just noticed this as well...


r366830.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65155



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

[Lldb-commits] [PATCH] D65155: [lldb] Remove Xcode project legacy

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D65155#1597744 , @xiaobai wrote:

> In D65155#1597710 , @JDevlieghere 
> wrote:
>
> > In D65155#1597657 , @lanza wrote:
> >
> > > Not directly related, but there's some other legacy cruft I figure are 
> > > valid for removal. e.g.
> > >
> > >   utils/sync-source
> > >   misc/grep-svn-log.py
> > >   git-svn/convert.py
> >
> >
> > r366827.
>
>
> This removed the entire utils directory. Perhaps related to the same thing 
> that happened in D65123 ?


Yeah, just noticed this as well...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65155



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

[Lldb-commits] [PATCH] D65155: [lldb] Remove Xcode project legacy

[Alex Langford via Phabricator via lldb-commits]

xiaobai added a comment.

In D65155#1597710 , @JDevlieghere 
wrote:

> In D65155#1597657 , @lanza wrote:
>
> > Not directly related, but there's some other legacy cruft I figure are 
> > valid for removal. e.g.
> >
> >   utils/sync-source
> >   misc/grep-svn-log.py
> >   git-svn/convert.py
>
>
> r366827.


This removed the entire utils directory. Perhaps related to the same thing that 
happened in D65123 ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65155



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

[Lldb-commits] [PATCH] D65152: Fix issues with inferior stdout coming out of order

[Pavel Labath via Phabricator via lldb-commits]

labath updated this revision to Diff 211328.
labath added a comment.

Remove sleep, left over from testing.


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

https://reviews.llvm.org/D65152

Files:
  include/lldb/Core/Debugger.h
  include/lldb/Interpreter/CommandInterpreter.h
  source/Core/Debugger.cpp
  source/Interpreter/CommandInterpreter.cpp

Index: source/Interpreter/CommandInterpreter.cpp
===
--- source/Interpreter/CommandInterpreter.cpp
+++ source/Interpreter/CommandInterpreter.cpp
@@ -2701,32 +2701,16 @@
   }
 }
 
-size_t CommandInterpreter::GetProcessOutput() {
-  //  The process has stuff waiting for stderr; get it and write it out to the
-  //  appropriate place.
-  char stdio_buffer[1024];
-  size_t len;
-  size_t total_bytes = 0;
-  Status error;
+void CommandInterpreter::GetProcessOutput() {
   TargetSP target_sp(m_debugger.GetTargetList().GetSelectedTarget());
-  if (target_sp) {
-ProcessSP process_sp(target_sp->GetProcessSP());
-if (process_sp) {
-  while ((len = process_sp->GetSTDOUT(stdio_buffer, sizeof(stdio_buffer),
-  error)) > 0) {
-size_t bytes_written = len;
-m_debugger.GetOutputFile()->Write(stdio_buffer, bytes_written);
-total_bytes += len;
-  }
-  while ((len = process_sp->GetSTDERR(stdio_buffer, sizeof(stdio_buffer),
-  error)) > 0) {
-size_t bytes_written = len;
-m_debugger.GetErrorFile()->Write(stdio_buffer, bytes_written);
-total_bytes += len;
-  }
-}
-  }
-  return total_bytes;
+  if (!target_sp)
+return;
+
+  ProcessSP process_sp(target_sp->GetProcessSP());
+  if (!process_sp)
+return;
+  m_debugger.FlushProcessOutput(*process_sp, /*is_stdout*/ true);
+  m_debugger.FlushProcessOutput(*process_sp, /*is_stdout*/ false);
 }
 
 void CommandInterpreter::StartHandlingCommand() {
Index: source/Core/Debugger.cpp
===
--- source/Core/Debugger.cpp
+++ source/Core/Debugger.cpp
@@ -1360,60 +1360,19 @@
   //}
 }
 
-size_t Debugger::GetProcessSTDOUT(Process *process, Stream *stream) {
-  size_t total_bytes = 0;
-  if (stream == nullptr)
-stream = GetOutputFile().get();
-
-  if (stream) {
-//  The process has stuff waiting for stdout; get it and write it out to the
-//  appropriate place.
-if (process == nullptr) {
-  TargetSP target_sp = GetTargetList().GetSelectedTarget();
-  if (target_sp)
-process = target_sp->GetProcessSP().get();
-}
-if (process) {
-  Status error;
-  size_t len;
-  char stdio_buffer[1024];
-  while ((len = process->GetSTDOUT(stdio_buffer, sizeof(stdio_buffer),
-   error)) > 0) {
-stream->Write(stdio_buffer, len);
-total_bytes += len;
-  }
-}
-stream->Flush();
-  }
-  return total_bytes;
-}
-
-size_t Debugger::GetProcessSTDERR(Process *process, Stream *stream) {
-  size_t total_bytes = 0;
-  if (stream == nullptr)
-stream = GetOutputFile().get();
+void Debugger::FlushProcessOutput(Process , bool is_stdout) {
+  std::lock_guard guard(m_output_flush_mutex);
+  StreamSP stream = is_stdout ? GetAsyncOutputStream() : GetAsyncErrorStream();
+  auto get = is_stdout ? ::GetSTDOUT : ::GetSTDERR;
 
-  if (stream) {
-//  The process has stuff waiting for stderr; get it and write it out to the
-//  appropriate place.
-if (process == nullptr) {
-  TargetSP target_sp = GetTargetList().GetSelectedTarget();
-  if (target_sp)
-process = target_sp->GetProcessSP().get();
-}
-if (process) {
-  Status error;
-  size_t len;
-  char stdio_buffer[1024];
-  while ((len = process->GetSTDERR(stdio_buffer, sizeof(stdio_buffer),
-   error)) > 0) {
-stream->Write(stdio_buffer, len);
-total_bytes += len;
-  }
-}
-stream->Flush();
+  Status error;
+  size_t len;
+  char stdio_buffer[1024];
+  while ((len = (process.*get)(stdio_buffer, sizeof(stdio_buffer), error)) >
+ 0) {
+stream->Write(stdio_buffer, len);
   }
-  return total_bytes;
+  stream->Flush();
 }
 
 // This function handles events that were broadcast by the process.
@@ -1454,14 +1413,12 @@
 }
 
 // Now display and STDOUT
-if (got_stdout || got_state_changed) {
-  GetProcessSTDOUT(process_sp.get(), output_stream_sp.get());
-}
+if (got_stdout || got_state_changed)
+  FlushProcessOutput(*process_sp, /*is_stdout*/ true);
 
 // Now display and STDERR
-if (got_stderr || got_state_changed) {
-  GetProcessSTDERR(process_sp.get(), error_stream_sp.get());
-}
+if (got_stderr || got_state_changed)
+  FlushProcessOutput(*process_sp, /*is_stdout*/ false);
 
 // Give structured data events an opportunity to display.
 if 

[Lldb-commits] [PATCH] D65155: [lldb] Remove Xcode project legacy

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D65155#1597657 , @lanza wrote:

> Not directly related, but there's some other legacy cruft I figure are valid 
> for removal. e.g.
>
>   utils/sync-source
>   misc/grep-svn-log.py
>   git-svn/convert.py


r366827.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65155



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

[Lldb-commits] [lldb] r366827 - [Utils] Remove legacy scripts

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Tue Jul 23 10:23:36 2019
New Revision: 366827

URL: http://llvm.org/viewvc/llvm-project?rev=366827=rev
Log:
[Utils] Remove legacy scripts

As pointed out by Nathan in D65155, these scrips don't seem to serve any
real need anymore.

Removed:
lldb/trunk/utils/

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

[Lldb-commits] [lldb] r366824 - [Logging] Fix format strings

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Tue Jul 23 10:03:37 2019
New Revision: 366824

URL: http://llvm.org/viewvc/llvm-project?rev=366824=rev
Log:
[Logging] Fix format strings

Change format strings to use the `{}` syntax instead of the printf
syntax when using LLDB_LOG.

Modified:
lldb/trunk/source/Core/Communication.cpp
lldb/trunk/source/Target/Target.cpp

Modified: lldb/trunk/source/Core/Communication.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Communication.cpp?rev=366824=366823=366824=diff
==
--- lldb/trunk/source/Core/Communication.cpp (original)
+++ lldb/trunk/source/Core/Communication.cpp Tue Jul 23 10:03:37 2019
@@ -49,7 +49,7 @@ Communication::Communication(const char
 
   LLDB_LOG(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_OBJECT |
   LIBLLDB_LOG_COMMUNICATION),
-   "%p Communication::Communication (name = %s)", this, name);
+   "{0} Communication::Communication (name = {1})", this, name);
 
   SetEventName(eBroadcastBitDisconnected, "disconnected");
   SetEventName(eBroadcastBitReadThreadGotBytes, "got bytes");
@@ -64,7 +64,7 @@ Communication::Communication(const char
 Communication::~Communication() {
   LLDB_LOG(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_OBJECT |
   LIBLLDB_LOG_COMMUNICATION),
-   "%p Communication::~Communication (name = %s)", this,
+   "{0} Communication::~Communication (name = {1})", this,
GetBroadcasterName().AsCString());
   Clear();
 }
@@ -79,7 +79,7 @@ ConnectionStatus Communication::Connect(
   Clear();
 
   LLDB_LOG(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_COMMUNICATION),
-   "%p Communication::Connect (url = %s)", this, url);
+   "{0} Communication::Connect (url = {1})", this, url);
 
   lldb::ConnectionSP connection_sp(m_connection_sp);
   if (connection_sp)
@@ -91,7 +91,7 @@ ConnectionStatus Communication::Connect(
 
 ConnectionStatus Communication::Disconnect(Status *error_ptr) {
   LLDB_LOG(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_COMMUNICATION),
-   "%p Communication::Disconnect ()", this);
+   "{0} Communication::Disconnect ()", this);
 
   lldb::ConnectionSP connection_sp(m_connection_sp);
   if (connection_sp) {
@@ -174,8 +174,8 @@ size_t Communication::Write(const void *
 
   std::lock_guard guard(m_write_mutex);
   LLDB_LOG(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_COMMUNICATION),
-   "%p Communication::Write (src = %p, src_len = %" PRIu64
-   ") connection = %p",
+   "{0} Communication::Write (src = {1}, src_len = %" PRIu64
+   ") connection = {2}",
this, src, (uint64_t)src_len, connection_sp.get());
 
   if (connection_sp)
@@ -195,7 +195,7 @@ bool Communication::StartReadThread(Stat
 return true;
 
   LLDB_LOG(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_COMMUNICATION),
-   "%p Communication::StartReadThread ()", this);
+   "{0} Communication::StartReadThread ()", this);
 
   char thread_name[1024];
   snprintf(thread_name, sizeof(thread_name), "",
@@ -228,7 +228,7 @@ bool Communication::StopReadThread(Statu
 return true;
 
   LLDB_LOG(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_COMMUNICATION),
-   "%p Communication::StopReadThread ()", this);
+   "{0} Communication::StopReadThread ()", this);
 
   m_read_thread_enabled = false;
 
@@ -270,8 +270,8 @@ void Communication::AppendBytesToCache(c
bool broadcast,
ConnectionStatus status) {
   LLDB_LOG(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_COMMUNICATION),
-   "%p Communication::AppendBytesToCache (src = %p, src_len = %" PRIu64
-   ", broadcast = %i)",
+   "{0} Communication::AppendBytesToCache (src = {1}, src_len = {2}, "
+   "broadcast = {3})",
this, bytes, (uint64_t)len, broadcast);
   if ((bytes == nullptr || len == 0) &&
   (status != lldb::eConnectionStatusEndOfFile))

Modified: lldb/trunk/source/Target/Target.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Target.cpp?rev=366824=366823=366824=diff
==
--- lldb/trunk/source/Target/Target.cpp (original)
+++ lldb/trunk/source/Target/Target.cpp Tue Jul 23 10:03:37 2019
@@ -106,12 +106,11 @@ Target::Target(Debugger , const
 
   CheckInWithManager();
 
-  Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_OBJECT));
-  if (log)
-log->Printf("%p Target::Target()", static_cast(this));
+  LLDB_LOG(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_OBJECT),
+   "{0} Target::Target()", static_cast(this));
   if (target_arch.IsValid()) {
 LLDB_LOG(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_TARGET),

[Lldb-commits] [PATCH] D65155: [lldb] Remove Xcode project legacy

[Nathan Lanza via Phabricator via lldb-commits]

lanza accepted this revision.
lanza added a comment.

utils/sync-source
misc/grep-svn-log.py


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65155



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

[Lldb-commits] [PATCH] D64881: [Cmake] Use the modern way to find Python when possible

[Hans Wennborg via Phabricator via lldb-commits]

hans added a comment.

In D64881#1596920 , @labath wrote:

> I think setting 
> -DPYTHON_EXECUTABLE=C:\Users\hwennborg\AppData\Local\Programs\Python\Python36-32\python.exe
>  (or something) should be enough to get cmake to select python3 for the 
> interpreter as well.


Cool, that works. Thanks!


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D64881



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

[Lldb-commits] [PATCH] D65109: [LLDB] Remove the Xcode project

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz added a comment.

[lldb] Remove Xcode project legacy: https://reviews.llvm.org/D65155


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65109



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

[Lldb-commits] [PATCH] D65155: [lldb] Remove Xcode project legacy

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz created this revision.
sgraenitz added reviewers: JDevlieghere, jasonmolenda, clayborg, jingham, 
lanza, teemperor.
Herald added a subscriber: mgorny.
Herald added a project: LLDB.

Since D65109  removed the manually maintained 
Xcode project, there's a few things we don't need anymore. Anything here we 
should keep or anything more to remove?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65155

Files:
  lldb/cmake/XcodeHeaderGenerator/CMakeLists.txt
  lldb/scripts/Python/finish-swig-Python-LLDB.sh
  lldb/scripts/Xcode/build-llvm.py
  lldb/scripts/Xcode/lldbbuild.py
  lldb/scripts/Xcode/package-clang-resource-headers.py
  lldb/scripts/Xcode/prepare-gtest-run-dir.sh
  lldb/scripts/Xcode/repo.py
  lldb/scripts/Xcode/repos/FALLBACK
  lldb/scripts/Xcode/repos/svn-trunk.json
  lldb/scripts/build-lldb-llvm-clang
  lldb/scripts/checkpoint-llvm.pl
  lldb/scripts/finish-swig-wrapper-classes.sh
  lldb/scripts/install-lldb.sh
  lldb/scripts/sed-sources

Index: lldb/scripts/sed-sources
===
--- lldb/scripts/sed-sources
+++ /dev/null
@@ -1,251 +0,0 @@
-#!/usr/bin/perl
-
-use strict;
-use File::Find;
-use File::Temp qw/ tempfile tempdir /;
-use Getopt::Std;
-use Pod::Usage;
-use Text::Tabs;
-
-=head1 NAME
-
-B -- Performs multiple sed commands on files with the ability to expand or unexpand tabs.
-
-=head1 SYNOPSIS
-
-B [options] [file dir ...]
-
-=head1 DESCRIPTION
-
-Performs multiple sed commands (modify builtin %seds hash) on source files
-or any sources in directories. If no arguments are given, STDIN will be used
-as the source. If source files or directories are specified as arguments,
-all files will be transformed and overwritten with new versions. Use the B<-p>
-option to preview changes to STDOUT, or use the B<-b> option to make a backup
-or the original files.
-
-=head1 OPTIONS
-
-=over
-
-=item B<-b>
-
-Backup original source file by appending ".bak" before overwriting with the
-newly transformed file.
-
-=item B<-g>
-
-Display verbose debug logging.
-
-=item B<-e>
-
-Expand tabs to spaces (in addition to doing sed substitutions).
-
-=item B<-u>
-
-Unexpand spaces to tabs (in addition to doing sed substitutions).
-
-=item B<-p>
-
-Preview changes to STDOUT without modifying original source files.
-
-=item B<-r>
-
-Skip variants when doing multiple files (no _profile or _debug variants). 
-
-=item B<-t N>
-
-Set the number of spaces per tab (default is 4) to use when expanding or
-unexpanding.
-
-=back
-
-=head1 EXAMPLES
- 
-# Recursively process all source files in the current working directory 
-# and subdirectories and also expand tabs to spaces. All source files 
-# will be overwritten with the newly transformed source files.
-
-% sed-sources -e $cwd
-
-# Recursively process all source files in the current working directory 
-# and subdirectories and also unexpand spaces to tabs and preview the
-# results to STDOUT
-
-% sed-sources -p -u $cwd
-
-# Same as above except use 8 spaces per tab. 
-
-% sed-sources -p -u -t8 $cwd
-
-=cut
-
-
-our $opt_b = 0;	# Backup original file?
-our $opt_g = 0;	# Verbose debug output?
-our $opt_e = 0;	# Expand tabs to spaces?
-our $opt_h = 0; # Show help?
-our $opt_m = 0;	# Show help manpage style?
-our $opt_p = 0;	# Preview changes to STDOUT?
-our $opt_t = 4;	# Number of spaces per tab?
-our $opt_u = 0;	# Unexpand spaces to tabs?
-getopts('eghmpt:u'); 
-
-$opt_m and show_manpage();
-$opt_h and help();
-
-our %seds = (
-	'\s+$' => "\n",		# Get rid of spaces at the end of a line
-	'^\s+$' => "\n",	# Get rid spaces on lines that are all spaces
-);
-
-
-sub show_manpage { exit pod2usage( verbose => 2 ); };
-sub help { exit pod2usage( verbose => 3, noperldoc => 1 ); };
-
-
-#--
-# process_opened_file_handle
-#-- 
-sub process_opened_file_handle
-{
-	my $in_fh = shift;
-	my $out_fh = shift;
-
-	# Set the number of spaces per tab for expand/unexpand
-	$tabstop = $opt_t; 
-	
-	while (my $line = <$in_fh>) 
-	{
-		foreach my $key (keys %seds)
-		{
-			my $value = $seds{"$key"};
-			$line =~ s/$key/$value/g;
-		}	
-		if ($opt_e) {
-			print $out_fh expand $line;
-		} elsif ($opt_u) {
-			print $out_fh unexpand $line;
-		} else {
-			print $out_fh $line;
-		}
-	}
-}
-
-#--
-# process_file
-#-- 
-sub process_file
-{
-	my $in_path = shift;
-	if (-T $in_path) 
-	{ 
-		my $out_fh;
-		my $out_path;
-		if ($opt_p)
-		{
-			# Preview to STDOUT
-			$out_fh = *STDOUT;
-			print "#-- \n";
-			print "# BEGIN: '$in_path'\n";
-			print "#-- \n";
-		}
-		else
-		{
-			

[Lldb-commits] [PATCH] D65152: Fix issues with inferior stdout coming out of order

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: clayborg, jingham.
Herald added a subscriber: jfb.

We've had a bug where two pieces of code, executing on two threads were
attempting to write inferior output simultaneously. The first one was in
Debugger::HandleProcessEvent, which handled the cases where stdout was
coming while the process was running. The second was in
CommandInterpreter::IOHandlerInputComplete, which was ensuring that any
output is printed before the command which caused process to run
terminates.

Both of these things make sense, but the fact they were implemented as
two independent functions without any synchronization meant that race
conditions could occur (e.g. both threads call process->GetSTDOUT, get
two chunks of data, but then end up calling stream->Write in opposite
order). This was most apparent in situations where a process quickly
writes a bunch of output and then exits (as all our register tests do).

This patch adds a mutex to ensure that stdout forwarding happens
atomically. It also refactors a code somewhat in order to reduce code
duplication.


https://reviews.llvm.org/D65152

Files:
  include/lldb/Core/Debugger.h
  include/lldb/Interpreter/CommandInterpreter.h
  source/Core/Debugger.cpp
  source/Interpreter/CommandInterpreter.cpp

Index: source/Interpreter/CommandInterpreter.cpp
===
--- source/Interpreter/CommandInterpreter.cpp
+++ source/Interpreter/CommandInterpreter.cpp
@@ -2701,32 +2701,16 @@
   }
 }
 
-size_t CommandInterpreter::GetProcessOutput() {
-  //  The process has stuff waiting for stderr; get it and write it out to the
-  //  appropriate place.
-  char stdio_buffer[1024];
-  size_t len;
-  size_t total_bytes = 0;
-  Status error;
+void CommandInterpreter::GetProcessOutput() {
   TargetSP target_sp(m_debugger.GetTargetList().GetSelectedTarget());
-  if (target_sp) {
-ProcessSP process_sp(target_sp->GetProcessSP());
-if (process_sp) {
-  while ((len = process_sp->GetSTDOUT(stdio_buffer, sizeof(stdio_buffer),
-  error)) > 0) {
-size_t bytes_written = len;
-m_debugger.GetOutputFile()->Write(stdio_buffer, bytes_written);
-total_bytes += len;
-  }
-  while ((len = process_sp->GetSTDERR(stdio_buffer, sizeof(stdio_buffer),
-  error)) > 0) {
-size_t bytes_written = len;
-m_debugger.GetErrorFile()->Write(stdio_buffer, bytes_written);
-total_bytes += len;
-  }
-}
-  }
-  return total_bytes;
+  if (!target_sp)
+return;
+
+  ProcessSP process_sp(target_sp->GetProcessSP());
+  if (!process_sp)
+return;
+  m_debugger.FlushProcessOutput(*process_sp, /*is_stdout*/ true);
+  m_debugger.FlushProcessOutput(*process_sp, /*is_stdout*/ false);
 }
 
 void CommandInterpreter::StartHandlingCommand() {
Index: source/Core/Debugger.cpp
===
--- source/Core/Debugger.cpp
+++ source/Core/Debugger.cpp
@@ -1360,60 +1360,20 @@
   //}
 }
 
-size_t Debugger::GetProcessSTDOUT(Process *process, Stream *stream) {
-  size_t total_bytes = 0;
-  if (stream == nullptr)
-stream = GetOutputFile().get();
-
-  if (stream) {
-//  The process has stuff waiting for stdout; get it and write it out to the
-//  appropriate place.
-if (process == nullptr) {
-  TargetSP target_sp = GetTargetList().GetSelectedTarget();
-  if (target_sp)
-process = target_sp->GetProcessSP().get();
-}
-if (process) {
-  Status error;
-  size_t len;
-  char stdio_buffer[1024];
-  while ((len = process->GetSTDOUT(stdio_buffer, sizeof(stdio_buffer),
-   error)) > 0) {
-stream->Write(stdio_buffer, len);
-total_bytes += len;
-  }
-}
-stream->Flush();
-  }
-  return total_bytes;
-}
-
-size_t Debugger::GetProcessSTDERR(Process *process, Stream *stream) {
-  size_t total_bytes = 0;
-  if (stream == nullptr)
-stream = GetOutputFile().get();
+void Debugger::FlushProcessOutput(Process , bool is_stdout) {
+  std::lock_guard guard(m_output_flush_mutex);
+  StreamSP stream = is_stdout ? GetAsyncOutputStream() : GetAsyncErrorStream();
+  auto get = is_stdout ? ::GetSTDOUT : ::GetSTDERR;
 
-  if (stream) {
-//  The process has stuff waiting for stderr; get it and write it out to the
-//  appropriate place.
-if (process == nullptr) {
-  TargetSP target_sp = GetTargetList().GetSelectedTarget();
-  if (target_sp)
-process = target_sp->GetProcessSP().get();
-}
-if (process) {
-  Status error;
-  size_t len;
-  char stdio_buffer[1024];
-  while ((len = process->GetSTDERR(stdio_buffer, sizeof(stdio_buffer),
-   error)) > 0) {
-stream->Write(stdio_buffer, len);
-total_bytes += len;
-  }
-}
-stream->Flush();
+  Status 

[Lldb-commits] [PATCH] D65114: [LLDB] Add utility to streamline Xcode project generation.

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz resigned from this revision.
sgraenitz added a comment.

> Can we just use the mono-repo style build and use "cmake -G Xcode"?

Yes you can, but then you have all of llvm/clang/libcxx/lldb in one project. 
For LLDB development that's usually not necessary. (And also, for Xcode it's a 
bit overwhelming.)

> I've included a new script to streamline the process

It's not so much of my business and we may disagree, but I am not sure it's a 
good idea to add an automation layer on-top of a meta-build-system. Remember 
build-script.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D65114



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

[Lldb-commits] [PATCH] D65109: [LLDB] Remove the Xcode project

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz added a comment.
Herald added a subscriber: ormris.

Seriously, discussion for changes like this should be open for more than 
1h15min! I am in favor of the change in principle, but there's a number of 
things that have been rushed over here, e.g.:

  cmake/XcodeHeaderGenerator/CMakeLists.txt
  scripts/finish-swig-wrapper-classes.sh
  scripts/Xcode/build-llvm.py
  scripts/Xcode/lldbbuild.py
  scripts/Xcode/package-clang-resource-headers.py
  scripts/Xcode/prepare-gtest-run-dir.sh
  scripts/Xcode/repo.py
  scripts/Xcode/repos/FALLBACK
  scripts/Xcode/repos/svn-trunk.json


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65109



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

[Lldb-commits] [PATCH] D65128: [Logging] Replace Log::Printf with LLDB_LOG macro (NFC)

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D65128#1596964 , @labath wrote:

> I don't think you can make a change like this mechanically, and the reason is 
> not the file-function prepending, but the fact that the two styles use a 
> different format specifier syntax (printf vs. the llvm formatv thingy). If 
> you change the function without changing the format strings, you just create 
> a bunch of static log messages...


Thanks for pointing this out. Somehow I convinced myself that formatv supported 
both, but on further inspection that is not true. I guess we could change the 
format strings too, but I'm afraid that is a more controversial change.

> 
> 
> In D65128#1596813 , @JDevlieghere 
> wrote:
> 
>> In D65128#1596809 , @jingham wrote:
>>
>> > Actually, I don't want this change as is.  Some logs - like the expression 
>> > and step logs - are laid out for readability, and LLDB_LOG automatically 
>> > adds the file & function which will make them much harder to read.
>> >
>> > We need a variant that doesn't inject this information, and then this 
>> > change should use that variant.  Or we can remove the file & function from 
>> > LLDB_LOG and decide to let the people add those as they wish.  I don't 
>> > have a strong opinion one way or the other, but I certainly don't want 
>> > this extra noise in the step or expression logs.
>>
>>
>> I agree with you. I always use LLDB_LOG, but not because it includes the 
>> file and function, but because it's consistent, it gives me the ability to 
>> use the LLVM formatter, and I don't have to explicitly check whether the log 
>> is NULL or not. I can only speak for myself of course.
>>
>> @labath Since you added the macro, what's your take on this? Do you care a 
>> lot about the file and function, and should we have two macros, one that 
>> includes this and one that doesn't? Or should we have just one macro, and 
>> have users decide whether they need this information or not?
> 
> 
> The reason I added the file-function prepending stuff was because a lot of 
> our logging statements do things like:
> 
>   if(log)
> log->Printf("MyUsuallyVeryLongClassName::%s real stuff...", args..., 
> __FUNCTION__);
> 
> 
> This means that the Printf call usually does not fit a single line, and so 
> each logging statement ends up taking at least three lines (together with the 
> if(log) stuff). By making sure the origin of the log statement is 
> automatically added to the message, I was hoping that we'd only log the 
> actual interesting stuff, and so we'd have succinct log statements which 
> don't interfere with the flow of the rest of the code (`LLDB_LOG(log, "real 
> stuff...", args...)`).
> 
> So, that was the direction I wanted to take things in. However, I wouldn't 
> say that has been entirely successful, so if you want to take a different 
> direction now, that's fine with me. I just want to point out two things:
> 
> - The file-function prepending stuff is optional. In fact it's not even 
> enabled by default -- you have to pass a `--file-function` flag to the "log 
> enable" command to get this behavior
> - The prepending code goes through a lot of trouble to make sure the 
> file-function field is always of the same width. This means that if you have 
> a log file which contains these things, you can just scroll to the right to 
> avoid seeing them, or (assuming your editor supports block operations) easily 
> cut them out.

That explains my confusion as to it being part of the macro. I never noticed it 
showing up in the log, and this explains why.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D65128



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

[Lldb-commits] [PATCH] D65123: Restore tests for lldb-server and lldb-vscode removed at rL366590

[James Y Knight via Phabricator via lldb-commits]

jyknight added a comment.

In D65123#1596983 , @labath wrote:

> In D65123#1596935 , @teemperor wrote:
>
> > Thanks for restoring this, but I'm a bit confused how this has happened. My 
> > original diff  didn't remove 
> > these folders, but it seems when I committed the diff via arcanist and `git 
> > llvm` it also removed these other folders. I assume `git llvm` is when 
> > translating the change to SVN not just removing a folder but also all 
> > folders in the same directory?
>
>
> Interesting.. I wouldn't be totally surprised by that as git-llvm does some 
> magic to svn check out only the files/directories that have been modified. 
> Combining that with the different treatment of directories in svn and git 
> (explicit modelling vs. implicit "it exists if it contains a file"), I can 
> see how things might go south in some corner cases like this. But that's just 
> speculation -- I'm generally trying to avoid git-llvm as it is too magical 
> for my taste. Maybe @jyknight or @mehdi_amini can shed more light on this?


Reproduced this bug, and working on a fix.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65123



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

[Lldb-commits] [lldb] r366804 - [lldb][NFC] Tablegenify process

[Raphael Isemann via lldb-commits]

Author: teemperor
Date: Tue Jul 23 05:54:33 2019
New Revision: 366804

URL: http://llvm.org/viewvc/llvm-project?rev=366804=rev
Log:
[lldb][NFC] Tablegenify process

Modified:
lldb/trunk/source/Commands/CommandObjectProcess.cpp
lldb/trunk/source/Commands/Options.td

Modified: lldb/trunk/source/Commands/CommandObjectProcess.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectProcess.cpp?rev=366804=366803=366804=diff
==
--- lldb/trunk/source/Commands/CommandObjectProcess.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectProcess.cpp Tue Jul 23 05:54:33 2019
@@ -256,14 +256,8 @@ protected:
 };
 
 static constexpr OptionDefinition g_process_attach_options[] = {
-// clang-format off
-  { LLDB_OPT_SET_ALL, false, "continue", 'c', 
OptionParser::eNoArgument,   nullptr, {}, 0, eArgTypeNone, 
"Immediately continue the process once attached." },
-  { LLDB_OPT_SET_ALL, false, "plugin",   'P', 
OptionParser::eRequiredArgument, nullptr, {}, 0, eArgTypePlugin,   "Name of 
the process plugin you want to use." },
-  { LLDB_OPT_SET_1,   false, "pid",  'p', 
OptionParser::eRequiredArgument, nullptr, {}, 0, eArgTypePid,  "The 
process ID of an existing process to attach to." },
-  { LLDB_OPT_SET_2,   false, "name", 'n', 
OptionParser::eRequiredArgument, nullptr, {}, 0, eArgTypeProcessName,  "The 
name of the process to attach to." },
-  { LLDB_OPT_SET_2,   false, "include-existing", 'i', 
OptionParser::eNoArgument,   nullptr, {}, 0, eArgTypeNone, "Include 
existing processes when doing attach -w." },
-  { LLDB_OPT_SET_2,   false, "waitfor",  'w', 
OptionParser::eNoArgument,   nullptr, {}, 0, eArgTypeNone, "Wait 
for the process with  to launch." },
-// clang-format on
+#define LLDB_OPTIONS_process_attach
+#include "CommandOptions.inc"
 };
 
 #pragma mark CommandObjectProcessAttach
@@ -506,9 +500,8 @@ protected:
 // CommandObjectProcessContinue
 
 static constexpr OptionDefinition g_process_continue_options[] = {
-// clang-format off
-  { LLDB_OPT_SET_ALL, false, "ignore-count",'i', 
OptionParser::eRequiredArgument, nullptr, {}, 0, eArgTypeUnsignedInteger, 
"Ignore  crossings of the breakpoint (if it exists) for the currently 
selected thread." }
-// clang-format on
+#define LLDB_OPTIONS_process_continue
+#include "CommandOptions.inc"
 };
 
 #pragma mark CommandObjectProcessContinue
@@ -667,9 +660,8 @@ protected:
 
 // CommandObjectProcessDetach
 static constexpr OptionDefinition g_process_detach_options[] = {
-// clang-format off
-  { LLDB_OPT_SET_1, false, "keep-stopped", 's', 
OptionParser::eRequiredArgument, nullptr, {}, 0, eArgTypeBoolean, "Whether or 
not the process should be kept stopped on detach (if possible)." },
-// clang-format on
+#define LLDB_OPTIONS_process_detach
+#include "CommandOptions.inc"
 };
 
 #pragma mark CommandObjectProcessDetach
@@ -764,9 +756,8 @@ protected:
 // CommandObjectProcessConnect
 
 static constexpr OptionDefinition g_process_connect_options[] = {
-// clang-format off
-  { LLDB_OPT_SET_ALL, false, "plugin", 'p', OptionParser::eRequiredArgument, 
nullptr, {}, 0, eArgTypePlugin, "Name of the process plugin you want to use." },
-// clang-format on
+#define LLDB_OPTIONS_process_connect
+#include "CommandOptions.inc"
 };
 
 #pragma mark CommandObjectProcessConnect
@@ -889,9 +880,8 @@ public:
 // CommandObjectProcessLoad
 
 static constexpr OptionDefinition g_process_load_options[] = {
-// clang-format off
-  { LLDB_OPT_SET_ALL, false, "install", 'i', OptionParser::eOptionalArgument, 
nullptr, {}, 0, eArgTypePath, "Install the shared library to the target. If 
specified without an argument then the library will installed in the current 
working directory." },
-// clang-format on
+#define LLDB_OPTIONS_process_load
+#include "CommandOptions.inc"
 };
 
 #pragma mark CommandObjectProcessLoad
@@ -1273,11 +1263,8 @@ public:
 // CommandObjectProcessHandle
 
 static constexpr OptionDefinition g_process_handle_options[] = {
-// clang-format off
-  { LLDB_OPT_SET_1, false, "stop",   's', OptionParser::eRequiredArgument, 
nullptr, {}, 0, eArgTypeBoolean, "Whether or not the process should be stopped 
if the signal is received." },
-  { LLDB_OPT_SET_1, false, "notify", 'n', OptionParser::eRequiredArgument, 
nullptr, {}, 0, eArgTypeBoolean, "Whether or not the debugger should notify the 
user if the signal is received." },
-  { LLDB_OPT_SET_1, false, "pass",   'p', OptionParser::eRequiredArgument, 
nullptr, {}, 0, eArgTypeBoolean, "Whether or not the signal should be passed to 
the process." }
-// clang-format on
+#define LLDB_OPTIONS_process_handle
+#include "CommandOptions.inc"
 };
 
 #pragma mark CommandObjectProcessHandle

Modified: lldb/trunk/source/Commands/Options.td
URL: 

[Lldb-commits] [lldb] r366803 - ProcessMachCore: Fix a -Wmisleading-indentation warning

[Pavel Labath via lldb-commits]

Author: labath
Date: Tue Jul 23 05:53:45 2019
New Revision: 366803

URL: http://llvm.org/viewvc/llvm-project?rev=366803=rev
Log:
ProcessMachCore: Fix a -Wmisleading-indentation warning

Modified:
lldb/trunk/source/Plugins/Process/mach-core/ProcessMachCore.cpp

Modified: lldb/trunk/source/Plugins/Process/mach-core/ProcessMachCore.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/mach-core/ProcessMachCore.cpp?rev=366803=366802=366803=diff
==
--- lldb/trunk/source/Plugins/Process/mach-core/ProcessMachCore.cpp (original)
+++ lldb/trunk/source/Plugins/Process/mach-core/ProcessMachCore.cpp Tue Jul 23 
05:53:45 2019
@@ -338,44 +338,43 @@ Status ProcessMachCore::DoLoadCore() {
   "from LC_IDENT/LC_NOTE 'kern ver str' string: '%s'", 
   corefile_identifier.c_str());
 
-  // We're only given a UUID here, not a load address.
-  // But there are python scripts in the EFI binary's dSYM which
-  // know how to relocate the binary to the correct load address.
-  // lldb only needs to locate & load the binary + dSYM.
-  ModuleSpec module_spec;
-  module_spec.GetUUID() = uuid;
-  module_spec.GetArchitecture() = GetTarget().GetArchitecture();
-
-  // Lookup UUID locally, before attempting dsymForUUID like action
-  FileSpecList search_paths =
-  Target::GetDefaultDebugFileSearchPaths();
-  module_spec.GetSymbolFileSpec() =
-  Symbols::LocateExecutableSymbolFile(module_spec, search_paths);
-  if (module_spec.GetSymbolFileSpec()) {
-ModuleSpec executable_module_spec =
-Symbols::LocateExecutableObjectFile(module_spec);
-if 
(FileSystem::Instance().Exists(executable_module_spec.GetFileSpec())) {
-  module_spec.GetFileSpec() =
-  executable_module_spec.GetFileSpec();
-}
+// We're only given a UUID here, not a load address.
+// But there are python scripts in the EFI binary's dSYM which
+// know how to relocate the binary to the correct load address.
+// lldb only needs to locate & load the binary + dSYM.
+ModuleSpec module_spec;
+module_spec.GetUUID() = uuid;
+module_spec.GetArchitecture() = GetTarget().GetArchitecture();
+
+// Lookup UUID locally, before attempting dsymForUUID like action
+FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths();
+module_spec.GetSymbolFileSpec() =
+Symbols::LocateExecutableSymbolFile(module_spec, search_paths);
+if (module_spec.GetSymbolFileSpec()) {
+  ModuleSpec executable_module_spec =
+  Symbols::LocateExecutableObjectFile(module_spec);
+  if (FileSystem::Instance().Exists(
+  executable_module_spec.GetFileSpec())) {
+module_spec.GetFileSpec() = executable_module_spec.GetFileSpec();
   }
+}
 
-  // Force a a dsymForUUID lookup, if that tool is available.
-  if (!module_spec.GetSymbolFileSpec())
-Symbols::DownloadObjectAndSymbolFile(module_spec, true);
-
-  if (FileSystem::Instance().Exists(module_spec.GetFileSpec())) {
-ModuleSP module_sp(new Module(module_spec));
-if (module_sp.get() && module_sp->GetObjectFile()) {
-  // Get the current target executable
-  ModuleSP exe_module_sp(GetTarget().GetExecutableModule());
-
-  // Make sure you don't already have the right module loaded
-  // and they will be uniqued
-  if (exe_module_sp.get() != module_sp.get())
-GetTarget().SetExecutableModule(module_sp, eLoadDependentsNo);
-}
+// Force a a dsymForUUID lookup, if that tool is available.
+if (!module_spec.GetSymbolFileSpec())
+  Symbols::DownloadObjectAndSymbolFile(module_spec, true);
+
+if (FileSystem::Instance().Exists(module_spec.GetFileSpec())) {
+  ModuleSP module_sp(new Module(module_spec));
+  if (module_sp.get() && module_sp->GetObjectFile()) {
+// Get the current target executable
+ModuleSP exe_module_sp(GetTarget().GetExecutableModule());
+
+// Make sure you don't already have the right module loaded
+// and they will be uniqued
+if (exe_module_sp.get() != module_sp.get())
+  GetTarget().SetExecutableModule(module_sp, eLoadDependentsNo);
   }
+}
   }
   }
 


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

[Lldb-commits] [PATCH] D65123: Restore tests for lldb-server and lldb-vscode removed at rL366590

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added subscribers: kwk, jankratochvil.
jankratochvil added a comment.

I was glade the `vscode` testcases are gone as they are racy causing many false 
failures on our Fedora buildbot always hanging until the whole testsuite gets 
killed:

  1910 ?Sl 0:00  |   \_ /usr/bin/python 
/home/jkratoch/slave-lldb-x86_64-fedora/lldb-x86_64-fedora/llvm/tools/lldb/test/dotest.py
 -q --arch=x86_64 -s 
/home/jkratoch/slave-lldb-x86_64-fedora/lldb-x86_64-fedora/build/lldb-test-traces
 --build-dir 
/home/jkratoch/slave-lldb-x86_64-fedora/lldb-x86_64-fedora/build/lldb-test-build.noindex
 -S nm -u CXXFLAGS -u CFLAGS --executable 
/home/jkratoch/slave-lldb-x86_64-fedora/lldb-x86_64-fedora/build/./bin/lldb 
--dsymutil 
/home/jkratoch/slave-lldb-x86_64-fedora/lldb-x86_64-fedora/build/./bin/dsymutil 
--filecheck 
/home/jkratoch/slave-lldb-x86_64-fedora/lldb-x86_64-fedora/build/./bin/FileCheck
 -C 
/home/jkratoch/slave-lldb-x86_64-fedora/lldb-x86_64-fedora/build/./bin/clang 
--env ARCHIVER=/usr/bin/ar --env OBJCOPY=/usr/bin/objcopy --env 
LLVM_LIBS_DIR=/home/jkratoch/slave-lldb-x86_64-fedora/lldb-x86_64-fedora/build/./lib
 
/home/jkratoch/slave-lldb-x86_64-fedora/lldb-x86_64-fedora/llvm/tools/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/breakpoint
 -p TestVSCode_setBreakpoints.py
  2649 ?Sl 0:00  |   \_ 
/home/jkratoch/slave-lldb-x86_64-fedora/lldb-x86_64-fedora/build/bin/lldb-vscode
  2690 ?S  0:00  |   \_ 
/home/jkratoch/slave-lldb-x86_64-fedora/lldb-x86_64-fedora/build/bin/lldb-server
 gdbserver --fd=9 --native-regs --setsid
  2708 ?t  0:00  |   \_ 
/home/jkratoch/slave-lldb-x86_64-fedora/lldb-x86_64-fedora/build/lldb-test-build.noindex/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.test_functionality/a.out

I have checked the `lldb-vscode` server does print:

  
{"body":{"allThreadsStopped":true,"preserveFocusHint":false,"reason":"breakpoint","threadCausedFocus":true,"threadId":10215},"event":"stopped","seq":0,"type":"event"}

But then `lldb` itself waits forever for the breakpoint hit, unaware why so 
far. The problem debugging it is that it happens only on slow hosts where it is 
difficult to run the testsuite on LLDB built with full DWARF.
If someone has an idea, otherwise we (with @kwk) will continue debugging it.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65123



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

[Lldb-commits] [lldb] r366798 - Fix windows build after r366791

[Pavel Labath via lldb-commits]

Author: labath
Date: Tue Jul 23 05:26:09 2019
New Revision: 366798

URL: http://llvm.org/viewvc/llvm-project?rev=366798=rev
Log:
Fix windows build after r366791

A side effect of this commit was that it exchanged the order of types
and compile units in the output of SymbolVendor::Dump. A couple of PDB
tests dependened on that to assert the links between the two.

While it wouldn't be too hard to update the tests, the change of
ordering was not something I intended to do with that patch, and is easy
to restore the original order, so I do just that.

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

Modified: lldb/trunk/source/Symbol/SymbolVendor.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/SymbolVendor.cpp?rev=366798=366797=366798=diff
==
--- lldb/trunk/source/Symbol/SymbolVendor.cpp (original)
+++ lldb/trunk/source/Symbol/SymbolVendor.cpp Tue Jul 23 05:26:09 2019
@@ -354,10 +354,12 @@ void SymbolVendor::Dump(Stream *s) {
   }
 }
 s->EOL();
+s->PutCString("Types:\n");
+m_type_list.Dump(s, show_context);
+s->EOL();
 if (m_sym_file_up)
   m_sym_file_up->Dump(*s);
 s->IndentMore();
-m_type_list.Dump(s, show_context);
 
 if (Symtab *symtab = GetSymtab())
   symtab->Dump(s, nullptr, eSortOrderNone);


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

[Lldb-commits] [lldb] r366795 - [lldb][NFC] Tablegenify source

[Raphael Isemann via lldb-commits]

Author: teemperor
Date: Tue Jul 23 04:08:12 2019
New Revision: 366795

URL: http://llvm.org/viewvc/llvm-project?rev=366795=rev
Log:
[lldb][NFC] Tablegenify source

Modified:
lldb/trunk/source/Commands/CommandObjectSource.cpp
lldb/trunk/source/Commands/Options.td

Modified: lldb/trunk/source/Commands/CommandObjectSource.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectSource.cpp?rev=366795=366794=366795=diff
==
--- lldb/trunk/source/Commands/CommandObjectSource.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectSource.cpp Tue Jul 23 04:08:12 2019
@@ -35,15 +35,8 @@ using namespace lldb_private;
 // CommandObjectSourceInfo - debug line entries dumping command
 
 static constexpr OptionDefinition g_source_info_options[] = {
-// clang-format off
-  { LLDB_OPT_SET_ALL,false, "count",'c', 
OptionParser::eRequiredArgument, nullptr, {}, 0,
 eArgTypeCount,   "The number of line entries to display." 
},
-  { LLDB_OPT_SET_1 | LLDB_OPT_SET_2, false, "shlib",'s', 
OptionParser::eRequiredArgument, nullptr, {}, 
CommandCompletions::eModuleCompletion, eArgTypeShlibName,   "Look 
up the source in the given module or shared library (can be specified more than 
once)." },
-  { LLDB_OPT_SET_1,  false, "file", 'f', 
OptionParser::eRequiredArgument, nullptr, {}, 
CommandCompletions::eSourceFileCompletion, eArgTypeFilename,"The 
file from which to display source." },
-  { LLDB_OPT_SET_1,  false, "line", 'l', 
OptionParser::eRequiredArgument, nullptr, {}, 0,
 eArgTypeLineNum, "The line number at which to start the 
displaying lines." },
-  { LLDB_OPT_SET_1,  false, "end-line", 'e', 
OptionParser::eRequiredArgument, nullptr, {}, 0,
 eArgTypeLineNum, "The line number at which to stop 
displaying lines." },
-  { LLDB_OPT_SET_2,  false, "name", 'n', 
OptionParser::eRequiredArgument, nullptr, {}, 
CommandCompletions::eSymbolCompletion, eArgTypeSymbol,  "The 
name of a function whose source to display." },
-  { LLDB_OPT_SET_3,  false, "address",  'a', 
OptionParser::eRequiredArgument, nullptr, {}, 0,
 eArgTypeAddressOrExpression, "Lookup the address and display the 
source information for the corresponding file and line." },
-// clang-format on
+#define LLDB_OPTIONS_source_info
+#include "CommandOptions.inc"
 };
 
 class CommandObjectSourceInfo : public CommandObjectParsed {
@@ -645,16 +638,8 @@ protected:
 // CommandObjectSourceList
 
 static constexpr OptionDefinition g_source_list_options[] = {
-// clang-format off
-  { LLDB_OPT_SET_ALL,false, "count",'c', 
OptionParser::eRequiredArgument, nullptr, {}, 0,
 eArgTypeCount,   "The number of source lines to display." 
},
-  { LLDB_OPT_SET_1 | LLDB_OPT_SET_2, false, "shlib",'s', 
OptionParser::eRequiredArgument, nullptr, {}, 
CommandCompletions::eModuleCompletion, eArgTypeShlibName,   "Look 
up the source file in the given shared library." },
-  { LLDB_OPT_SET_ALL,false, "show-breakpoints", 'b', 
OptionParser::eNoArgument,   nullptr, {}, 0,
 eArgTypeNone,"Show the line table locations from the 
debug information that indicate valid places to set source level breakpoints." 
},
-  { LLDB_OPT_SET_1,  false, "file", 'f', 
OptionParser::eRequiredArgument, nullptr, {}, 
CommandCompletions::eSourceFileCompletion, eArgTypeFilename,"The 
file from which to display source." },
-  { LLDB_OPT_SET_1,  false, "line", 'l', 
OptionParser::eRequiredArgument, nullptr, {}, 0,
 eArgTypeLineNum, "The line number at which to start the 
display source." },
-  { LLDB_OPT_SET_2,  false, "name", 'n', 
OptionParser::eRequiredArgument, nullptr, {}, 
CommandCompletions::eSymbolCompletion, eArgTypeSymbol,  "The 
name of a function whose source to display." },
-  { LLDB_OPT_SET_3,  false, "address",  'a', 
OptionParser::eRequiredArgument, nullptr, {}, 0,
 eArgTypeAddressOrExpression, "Lookup the address and display the 
source information for the corresponding file and line." },
-  { LLDB_OPT_SET_4,  false, "reverse",  'r', 
OptionParser::eNoArgument,   nullptr, {}, 0,
 eArgTypeNone,"Reverse the listing to look backwards 
from the last displayed block 

[Lldb-commits] [PATCH] D64042: [Symbol] Improve Variable::GetLanguage

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.
Herald added a reviewer: jdoerfert.



Comment at: lldb/trunk/source/Symbol/Variable.cpp:61-69
+  if (auto *func = m_owner_scope->CalculateSymbolContextFunction()) {
+if ((lang = func->GetLanguage()) && lang != lldb::eLanguageTypeUnknown)
+  return lang;
+else if (auto *comp_unit =
+ m_owner_scope->CalculateSymbolContextCompileUnit())
+  if ((lang = func->GetLanguage()) && lang != lldb::eLanguageTypeUnknown)
+return lang;

I get a warning here (at least with gcc) about `comp_unit` variable being 
unused. Did you perhaps mean to do `lang = comp_unit->GetLanguage()` on the 
line below?

I would have fixed this myself, but when I started looking at this, I became 
unsure of that is this code exactly supposed to do. Is the check for the 
CompileUnit really supposed to be nested inside the check the existence of a 
function. I would have kind of expected it to be located outside the function 
if statement (`if (func) stuff(func); else if (cu) stuff(cu);`

I also find the `if( lang = ... && lang != Unknown)` pattern very confusing. As 
it stands now, it checks for the zero value twice (once due to the `(lang = 
...)` part and once because eLanguageTypeUnknown is zero), but that part is 
very unobvious...


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64042



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

[Lldb-commits] [PATCH] D65135: SymbolVendor: Remove the type list member

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: clayborg, JDevlieghere, jingham.

Similarly to the compile unit lists, the list of types can also be
managed by the symbol file itself.

Since the only purpose of this list seems to be to maintain an owning
reference to all the types a symbol file has created (items are only
ever added to the list, never retrieved), I remove the passthrough
functions in SymbolVendor and Module. I also tighten the interface of
the function (return a reference instead of a pointer, make it protected
instead of public).


https://reviews.llvm.org/D65135

Files:
  include/lldb/Core/Module.h
  include/lldb/Symbol/SymbolFile.h
  include/lldb/Symbol/SymbolVendor.h
  include/lldb/Symbol/Type.h
  source/API/SBCompileUnit.cpp
  source/Core/Module.cpp
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  source/Symbol/SymbolFile.cpp
  source/Symbol/SymbolVendor.cpp
  source/Symbol/Type.cpp

Index: source/Symbol/Type.cpp
===
--- source/Symbol/Type.cpp
+++ source/Symbol/Type.cpp
@@ -425,8 +425,6 @@
   return false;
 }
 
-TypeList *Type::GetTypeList() { return GetSymbolFile()->GetTypeList(); }
-
 const Declaration ::GetDeclaration() const { return m_decl; }
 
 bool Type::ResolveClangType(ResolveState compiler_type_resolve_state) {
Index: source/Symbol/SymbolVendor.cpp
===
--- source/Symbol/SymbolVendor.cpp
+++ source/Symbol/SymbolVendor.cpp
@@ -58,7 +58,7 @@
 
 // SymbolVendor constructor
 SymbolVendor::SymbolVendor(const lldb::ModuleSP _sp)
-: ModuleChild(module_sp), m_type_list(), m_sym_file_up(), m_symtab() {}
+: ModuleChild(module_sp), m_sym_file_up(), m_symtab() {}
 
 // Destructor
 SymbolVendor::~SymbolVendor() {}
@@ -336,8 +336,6 @@
   if (module_sp) {
 std::lock_guard guard(module_sp->GetMutex());
 
-bool show_context = false;
-
 s->Printf("%p: ", static_cast(this));
 s->Indent();
 s->PutCString("SymbolVendor");
@@ -357,7 +355,6 @@
 if (m_sym_file_up)
   m_sym_file_up->Dump(*s);
 s->IndentMore();
-m_type_list.Dump(s, show_context);
 
 if (Symtab *symtab = GetSymtab())
   symtab->Dump(s, nullptr, eSortOrderNone);
Index: source/Symbol/SymbolFile.cpp
===
--- source/Symbol/SymbolFile.cpp
+++ source/Symbol/SymbolFile.cpp
@@ -82,12 +82,6 @@
   return best_symfile_up.release();
 }
 
-TypeList *SymbolFile::GetTypeList() {
-  if (m_obj_file)
-return m_obj_file->GetModule()->GetTypeList();
-  return nullptr;
-}
-
 TypeSystem *SymbolFile::GetTypeSystemForLanguage(lldb::LanguageType language) {
   TypeSystem *type_system =
   m_obj_file->GetModule()->GetTypeSystemForLanguage(language);
@@ -215,6 +209,10 @@
 }
   }
   s.PutChar('\n');
+
+  s.PutCString("Types:\n");
+  m_type_list.Dump(, /*show_context*/ false);
+  s.PutChar('\n');
 }
 
 SymbolFile::RegisterInfoResolver::~RegisterInfoResolver() = default;
Index: source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
===
--- source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
+++ source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
@@ -138,8 +138,6 @@
   void FindTypesByRegex(const lldb_private::RegularExpression ,
 uint32_t max_matches, lldb_private::TypeMap );
 
-  lldb_private::TypeList *GetTypeList() override;
-
   size_t GetTypes(lldb_private::SymbolContextScope *sc_scope,
   lldb::TypeClass type_mask,
   lldb_private::TypeList _list) override;
Index: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
===
--- source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -557,9 +557,7 @@
   lldb::TypeSP result = pdb->CreateLLDBTypeFromPDBType(*pdb_type);
   if (result) {
 m_types.insert(std::make_pair(type_uid, result));
-auto type_list = GetTypeList();
-if (type_list)
-  type_list->Insert(result);
+GetTypeList().Insert(result);
   }
   return result.get();
 }
@@ -1516,10 +1514,6 @@
   return 0;
 }
 
-lldb_private::TypeList *SymbolFilePDB::GetTypeList() {
-  return m_obj_file->GetModule()->GetTypeList();
-}
-
 void SymbolFilePDB::GetTypesForPDBSymbol(const llvm::pdb::PDBSymbol _symbol,
  uint32_t type_mask,
  TypeCollection _collection) {
Index: source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
===
--- 

[Lldb-commits] [PATCH] D65089: SymbolVendor: Move compile unit handling into the SymbolFile class

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL366791: SymbolVendor: Move compile unit handling into the 
SymbolFile class (authored by labath, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65089

Files:
  lldb/trunk/include/lldb/Symbol/SymbolFile.h
  lldb/trunk/include/lldb/Symbol/SymbolVendor.h
  lldb/trunk/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
  lldb/trunk/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  lldb/trunk/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
  lldb/trunk/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
  lldb/trunk/source/Symbol/SymbolFile.cpp
  lldb/trunk/source/Symbol/SymbolVendor.cpp
  lldb/trunk/tools/lldb-test/lldb-test.cpp

Index: lldb/trunk/source/Symbol/SymbolFile.cpp
===
--- lldb/trunk/source/Symbol/SymbolFile.cpp
+++ lldb/trunk/source/Symbol/SymbolFile.cpp
@@ -10,6 +10,7 @@
 
 #include "lldb/Core/Module.h"
 #include "lldb/Core/PluginManager.h"
+#include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Symbol/TypeMap.h"
 #include "lldb/Symbol/TypeSystem.h"
@@ -21,6 +22,7 @@
 #include 
 
 using namespace lldb_private;
+using namespace lldb;
 
 void SymbolFile::PreloadSymbols() {
   // No-op for most implementations.
@@ -169,4 +171,50 @@
 #endif
 }
 
+uint32_t SymbolFile::GetNumCompileUnits() {
+  std::lock_guard guard(GetModuleMutex());
+  if (!m_compile_units) {
+// Create an array of compile unit shared pointers -- which will each
+// remain NULL until someone asks for the actual compile unit information.
+m_compile_units.emplace(CalculateNumCompileUnits());
+  }
+  return m_compile_units->size();
+}
+
+CompUnitSP SymbolFile::GetCompileUnitAtIndex(uint32_t idx) {
+  uint32_t num = GetNumCompileUnits();
+  if (idx >= num)
+return nullptr;
+  lldb::CompUnitSP _sp = (*m_compile_units)[idx];
+  if (!cu_sp)
+cu_sp = ParseCompileUnitAtIndex(idx);
+  return cu_sp;
+}
+
+void SymbolFile::SetCompileUnitAtIndex(uint32_t idx, const CompUnitSP _sp) {
+  std::lock_guard guard(GetModuleMutex());
+  const size_t num_compile_units = GetNumCompileUnits();
+  assert(idx < num_compile_units);
+
+  // Fire off an assertion if this compile unit already exists for now. The
+  // partial parsing should take care of only setting the compile unit
+  // once, so if this assertion fails, we need to make sure that we don't
+  // have a race condition, or have a second parse of the same compile
+  // unit.
+  assert((*m_compile_units)[idx] == nullptr);
+  (*m_compile_units)[idx] = cu_sp;
+}
+
+void SymbolFile::Dump(Stream ) {
+  s.PutCString("Compile units:\n");
+  if (m_compile_units) {
+for (const CompUnitSP _sp : *m_compile_units) {
+  // We currently only dump the compile units that have been parsed
+  if (cu_sp)
+cu_sp->Dump(, /*show_context*/ false);
+}
+  }
+  s.PutChar('\n');
+}
+
 SymbolFile::RegisterInfoResolver::~RegisterInfoResolver() = default;
Index: lldb/trunk/source/Symbol/SymbolVendor.cpp
===
--- lldb/trunk/source/Symbol/SymbolVendor.cpp
+++ lldb/trunk/source/Symbol/SymbolVendor.cpp
@@ -58,8 +58,7 @@
 
 // SymbolVendor constructor
 SymbolVendor::SymbolVendor(const lldb::ModuleSP _sp)
-: ModuleChild(module_sp), m_type_list(), m_compile_units(), m_sym_file_up(),
-  m_symtab() {}
+: ModuleChild(module_sp), m_type_list(), m_sym_file_up(), m_symtab() {}
 
 // Destructor
 SymbolVendor::~SymbolVendor() {}
@@ -76,44 +75,14 @@
   }
 }
 
-bool SymbolVendor::SetCompileUnitAtIndex(size_t idx, const CompUnitSP _sp) {
-  ModuleSP module_sp(GetModule());
-  if (module_sp) {
-std::lock_guard guard(module_sp->GetMutex());
-const size_t num_compile_units = GetNumCompileUnits();
-if (idx < num_compile_units) {
-  // Fire off an assertion if this compile unit already exists for now. The
-  // partial parsing should take care of only setting the compile unit
-  // once, so if this assertion fails, we need to make sure that we don't
-  // have a race condition, or have a second parse of the same compile
-  // unit.
-  assert(m_compile_units[idx].get() == nullptr);
-  m_compile_units[idx] = 

[Lldb-commits] [lldb] r366791 - SymbolVendor: Move compile unit handling into the SymbolFile class

[Pavel Labath via lldb-commits]

Author: labath
Date: Tue Jul 23 02:24:02 2019
New Revision: 366791

URL: http://llvm.org/viewvc/llvm-project?rev=366791=rev
Log:
SymbolVendor: Move compile unit handling into the SymbolFile class

Summary:
SymbolFile classes are responsible for creating CompileUnit instances
and they already need to have a notion of the id<->CompileUnit mapping
(because of APIs like ParseCompileUnitAtIndex). However, the
SymbolVendor has remained as the thing responsible for caching created
units (which the SymbolFiles were calling via convoluted constructs like
"m_obj_file->GetModule()->GetSymbolVendor()->SetCompileUnitAtIndex(...)").

This patch moves the responsibility of caching the units into the
SymbolFile class. It does this by moving the implementation of
SymbolVendor::{GetNumCompileUnits,GetCompileUnitAtIndex} into the
equivalent SymbolFile functions. The SymbolVendor functions become just
a passthrough much like the rest of SymbolVendor.

The original implementations of SymbolFile::GetNumCompileUnits is moved
to "CalculateNumCompileUnits", and are made protected, as the "Get"
function is the external api of the class.
SymbolFile::ParseCompileUnitAtIndex is made protected for the same
reason.

This is the first step in removing the SymbolVendor indirection, as
proposed in
. After
removing all interesting logic from the SymbolVendor class, I'll proceed
with removing the indirection itself.

Reviewers: clayborg, jingham, JDevlieghere

Subscribers: jdoerfert, lldb-commits

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

Modified:
lldb/trunk/include/lldb/Symbol/SymbolFile.h
lldb/trunk/include/lldb/Symbol/SymbolVendor.h
lldb/trunk/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
lldb/trunk/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
lldb/trunk/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
lldb/trunk/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
lldb/trunk/source/Symbol/SymbolFile.cpp
lldb/trunk/source/Symbol/SymbolVendor.cpp
lldb/trunk/tools/lldb-test/lldb-test.cpp

Modified: lldb/trunk/include/lldb/Symbol/SymbolFile.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/SymbolFile.h?rev=366791=366790=366791=diff
==
--- lldb/trunk/include/lldb/Symbol/SymbolFile.h (original)
+++ lldb/trunk/include/lldb/Symbol/SymbolFile.h Tue Jul 23 02:24:02 2019
@@ -110,8 +110,8 @@ public:
 
   // Compile Unit function calls
   // Approach 1 - iterator
-  virtual uint32_t GetNumCompileUnits() = 0;
-  virtual lldb::CompUnitSP ParseCompileUnitAtIndex(uint32_t index) = 0;
+  uint32_t GetNumCompileUnits();
+  lldb::CompUnitSP GetCompileUnitAtIndex(uint32_t idx);
 
   virtual lldb::LanguageType ParseLanguage(CompileUnit _unit) = 0;
   virtual size_t ParseFunctions(CompileUnit _unit) = 0;
@@ -235,12 +235,17 @@ public:
 return nullptr;
   }
 
-  virtual void Dump(Stream ) {}
+  virtual void Dump(Stream );
 
 protected:
   void AssertModuleLock();
+  virtual uint32_t CalculateNumCompileUnits() = 0;
+  virtual lldb::CompUnitSP ParseCompileUnitAtIndex(uint32_t idx) = 0;
+
+  void SetCompileUnitAtIndex(uint32_t idx, const lldb::CompUnitSP _sp);
 
   ObjectFile *m_obj_file; // The object file that symbols can be extracted 
from.
+  llvm::Optional> m_compile_units;
   uint32_t m_abilities;
   bool m_calculated_abilities;
 

Modified: lldb/trunk/include/lldb/Symbol/SymbolVendor.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/SymbolVendor.h?rev=366791=366790=366791=diff
==
--- lldb/trunk/include/lldb/Symbol/SymbolVendor.h (original)
+++ lldb/trunk/include/lldb/Symbol/SymbolVendor.h Tue Jul 23 02:24:02 2019
@@ -110,9 +110,6 @@ public:
 
   virtual size_t GetNumCompileUnits();
 
-  virtual bool SetCompileUnitAtIndex(size_t cu_idx,
- const lldb::CompUnitSP _sp);
-
   virtual lldb::CompUnitSP GetCompileUnitAtIndex(size_t idx);
 
   TypeList () { return m_type_list; }
@@ -142,13 +139,7 @@ public:
   uint32_t GetPluginVersion() override;
 
 protected:
-  // Classes that inherit from SymbolVendor can see and modify these
-  typedef std::vector CompileUnits;
-  typedef CompileUnits::iterator CompileUnitIter;
-  typedef 

[Lldb-commits] [PATCH] D65089: SymbolVendor: Move compile unit handling into the SymbolFile class

[Pavel Labath via Phabricator via lldb-commits]

labath updated this revision to Diff 211265.
labath added a comment.
Herald added a reviewer: jdoerfert.

- Make the compile unit list an Optional to have an explicit way of 
saying "we've already tried to compute the number of units and it was zero"


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

https://reviews.llvm.org/D65089

Files:
  include/lldb/Symbol/SymbolFile.h
  include/lldb/Symbol/SymbolVendor.h
  source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
  source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
  source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
  source/Symbol/SymbolFile.cpp
  source/Symbol/SymbolVendor.cpp
  tools/lldb-test/lldb-test.cpp

Index: tools/lldb-test/lldb-test.cpp
===
--- tools/lldb-test/lldb-test.cpp
+++ tools/lldb-test/lldb-test.cpp
@@ -558,7 +558,7 @@
   outs() << "Found " << comp_units_count << " compile units.\n";
 
   for (uint32_t i = 0; i < comp_units_count; i++) {
-lldb::CompUnitSP comp_unit = symfile->ParseCompileUnitAtIndex(i);
+lldb::CompUnitSP comp_unit = symfile->GetCompileUnitAtIndex(i);
 if (!comp_unit)
   return make_string_error("Connot parse compile unit {0}.", i);
 
Index: source/Symbol/SymbolVendor.cpp
===
--- source/Symbol/SymbolVendor.cpp
+++ source/Symbol/SymbolVendor.cpp
@@ -58,8 +58,7 @@
 
 // SymbolVendor constructor
 SymbolVendor::SymbolVendor(const lldb::ModuleSP _sp)
-: ModuleChild(module_sp), m_type_list(), m_compile_units(), m_sym_file_up(),
-  m_symtab() {}
+: ModuleChild(module_sp), m_type_list(), m_sym_file_up(), m_symtab() {}
 
 // Destructor
 SymbolVendor::~SymbolVendor() {}
@@ -76,44 +75,14 @@
   }
 }
 
-bool SymbolVendor::SetCompileUnitAtIndex(size_t idx, const CompUnitSP _sp) {
-  ModuleSP module_sp(GetModule());
-  if (module_sp) {
-std::lock_guard guard(module_sp->GetMutex());
-const size_t num_compile_units = GetNumCompileUnits();
-if (idx < num_compile_units) {
-  // Fire off an assertion if this compile unit already exists for now. The
-  // partial parsing should take care of only setting the compile unit
-  // once, so if this assertion fails, we need to make sure that we don't
-  // have a race condition, or have a second parse of the same compile
-  // unit.
-  assert(m_compile_units[idx].get() == nullptr);
-  m_compile_units[idx] = cu_sp;
-  return true;
-} else {
-  // This should NOT happen, and if it does, we want to crash and know
-  // about it
-  assert(idx < num_compile_units);
-}
-  }
-  return false;
-}
-
 size_t SymbolVendor::GetNumCompileUnits() {
   ModuleSP module_sp(GetModule());
   if (module_sp) {
 std::lock_guard guard(module_sp->GetMutex());
-if (m_compile_units.empty()) {
-  if (m_sym_file_up) {
-// Resize our array of compile unit shared pointers -- which will each
-// remain NULL until someone asks for the actual compile unit
-// information. When this happens, the symbol file will be asked to
-// parse this compile unit information.
-m_compile_units.resize(m_sym_file_up->GetNumCompileUnits());
-  }
-}
+if (m_sym_file_up)
+  return m_sym_file_up->GetNumCompileUnits();
   }
-  return m_compile_units.size();
+  return 0;
 }
 
 lldb::LanguageType SymbolVendor::ParseLanguage(CompileUnit _unit) {
@@ -390,14 +359,6 @@
 s->IndentMore();
 m_type_list.Dump(s, show_context);
 
-CompileUnitConstIter cu_pos, cu_end;
-cu_end = m_compile_units.end();
-for (cu_pos = m_compile_units.begin(); cu_pos != cu_end; ++cu_pos) {
-  // We currently only dump the compile units that have been parsed
-  if (*cu_pos)
-(*cu_pos)->Dump(s, show_context);
-}
-
 if (Symtab *symtab = GetSymtab())
   symtab->Dump(s, nullptr, eSortOrderNone);
 
@@ -406,20 +367,13 @@
 }
 
 CompUnitSP SymbolVendor::GetCompileUnitAtIndex(size_t idx) {
-  CompUnitSP cu_sp;
   ModuleSP module_sp(GetModule());
   if (module_sp) {
 std::lock_guard guard(module_sp->GetMutex());
-const size_t num_compile_units = GetNumCompileUnits();
-if (idx < num_compile_units) {
-  cu_sp = m_compile_units[idx];
-  if (cu_sp.get() == nullptr) {
-m_compile_units[idx] = m_sym_file_up->ParseCompileUnitAtIndex(idx);
-cu_sp = m_compile_units[idx];
-  }
-}
+if (m_sym_file_up)
+  

[Lldb-commits] [PATCH] D65123: Restore tests for lldb-server and lldb-vscode removed at rL366590

[Pavel Labath via Phabricator via lldb-commits]

labath added subscribers: jyknight, mehdi_amini.
labath added a comment.

In D65123#1596935 , @teemperor wrote:

> Thanks for restoring this, but I'm a bit confused how this has happened. My 
> original diff  didn't remove these 
> folders, but it seems when I committed the diff via arcanist and `git llvm` 
> it also removed these other folders. I assume `git llvm` is when translating 
> the change to SVN not just removing a folder but also all folders in the same 
> directory?


Interesting.. I wouldn't be totally surprised by that as git-llvm does some 
magic to svn check out only the files/directories that have been modified. 
Combining that with the different treatment of directories in svn and git 
(explicit modelling vs. implicit "it exists if it contains a file"), I can see 
how things might go south in some corner cases like this. But that's just 
speculation -- I'm generally trying to avoid git-llvm as it is too magical for 
my taste. Maybe @jyknight or @mehdi_amini can shed more light on this?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65123



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

[Lldb-commits] [PATCH] D65129: Test load unloading of modules with libraries-svr4

[Pavel Labath via Phabricator via lldb-commits]

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

This is python, so there are always ways to make this work without 
copy-pasting, but I'm not sure that's such a good idea in this case. Doing 
things this way is probably fine...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65129



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

[Lldb-commits] [PATCH] D65128: [Logging] Replace Log::Printf with LLDB_LOG macro (NFC)

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I don't think you can make a change like this mechanically, and the reason is 
not the file-function prepending, but the fact that the two styles use a 
different format specifier syntax (printf vs. the llvm formatv thingy). If you 
change the function without changing the format strings, you just create a 
bunch of static log messages...

In D65128#1596813 , @JDevlieghere 
wrote:

> In D65128#1596809 , @jingham wrote:
>
> > Actually, I don't want this change as is.  Some logs - like the expression 
> > and step logs - are laid out for readability, and LLDB_LOG automatically 
> > adds the file & function which will make them much harder to read.
> >
> > We need a variant that doesn't inject this information, and then this 
> > change should use that variant.  Or we can remove the file & function from 
> > LLDB_LOG and decide to let the people add those as they wish.  I don't have 
> > a strong opinion one way or the other, but I certainly don't want this 
> > extra noise in the step or expression logs.
>
>
> I agree with you. I always use LLDB_LOG, but not because it includes the file 
> and function, but because it's consistent, it gives me the ability to use the 
> LLVM formatter, and I don't have to explicitly check whether the log is NULL 
> or not. I can only speak for myself of course.
>
> @labath Since you added the macro, what's your take on this? Do you care a 
> lot about the file and function, and should we have two macros, one that 
> includes this and one that doesn't? Or should we have just one macro, and 
> have users decide whether they need this information or not?


The reason I added the file-function prepending stuff was because a lot of our 
logging statements do things like:

  if(log)
log->Printf("MyUsuallyVeryLongClassName::%s real stuff...", args..., 
__FUNCTION__);

This means that the Printf call usually does not fit a single line, and so each 
logging statement ends up taking at least three lines (together with the 
if(log) stuff). By making sure the origin of the log statement is automatically 
added to the message, I was hoping that we'd only log the actual interesting 
stuff, and so we'd have succinct log statements which don't interfere with the 
flow of the rest of the code (`LLDB_LOG(log, "real stuff...", args...)`).

So, that was the direction I wanted to take things in. However, I wouldn't say 
that has been entirely successful, so if you want to take a different direction 
now, that's fine with me. I just want to point out two things:

- The file-function prepending stuff is optional. In fact it's not even enabled 
by default -- you have to pass a `--file-function` flag to the "log enable" 
command to get this behavior
- The prepending code goes through a lot of trouble to make sure the 
file-function field is always of the same width. This means that if you have a 
log file which contains these things, you can just scroll to the right to avoid 
seeing them, or (assuming your editor supports block operations) easily cut 
them out.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D65128



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

[Lldb-commits] [PATCH] D65123: Restore tests for lldb-server and lldb-vscode removed at rL366590

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

Thanks for restoring this, but I'm a bit confused how this has happened. My 
original diff  didn't remove these 
folders, but it seems when I committed the diff via arcanist and `git llvm` it 
also removed these other folders. I assume `git llvm` is when translating the 
change to SVN not just removing a folder but also all folders in the same 
directory?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65123



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

[Lldb-commits] [PATCH] D65122: [Symbol] Use llvm::Expected when getting TypeSystems

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.
Herald added a reviewer: jdoerfert.

The intention is for the pointer in `Expected` to be non-null in 
the success case, right? If that's true, then I'd suggest to make that explicit 
by returning a reference (`Expected`) instead.




Comment at: source/Core/ValueObjectRegister.cpp:261
+  if (auto *exe_module = target->GetExecutableModulePointer()) {
+if (auto type_system_or_err =
+exe_module->GetTypeSystemForLanguage(eLanguageTypeC)) {

JDevlieghere wrote:
> As a general note: I think it's good practice to use `llvm::consumeError` 
> when discarding the error to make it explicit that it's not handled. This 
> also makes it easier down the line to handle the error, e.g. by logging it.
This is not just good practice, it is actually required. An simply checking the 
bool-ness of the Expected object will not set the "checked" flag of the error 
object, and it will assert when it is destroyed (if asserts are enabled).



Comment at: source/Core/ValueObjectRegister.cpp:263
+exe_module->GetTypeSystemForLanguage(eLanguageTypeC)) {
+  auto *type_system = *type_system_or_err;
   m_compiler_type = type_system->GetBuiltinTypeForEncodingAndBitSize(

If you returned a reference then you also wouldn't need this extra variable to 
avoid weird double-deref.


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

https://reviews.llvm.org/D65122



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

[Lldb-commits] [PATCH] D65123: Restore tests for lldb-server and lldb-vscode removed at rL366590

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.
Herald added a subscriber: ormris.

Thanks.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65123



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

[Lldb-commits] [lldb] r366783 - [lldb][NFC] Tablegenify disassemble

[Raphael Isemann via lldb-commits]

Author: teemperor
Date: Tue Jul 23 00:15:54 2019
New Revision: 366783

URL: http://llvm.org/viewvc/llvm-project?rev=366783=rev
Log:
[lldb][NFC] Tablegenify disassemble

Modified:
lldb/trunk/source/Commands/CommandObjectDisassemble.cpp
lldb/trunk/source/Commands/Options.td

Modified: lldb/trunk/source/Commands/CommandObjectDisassemble.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectDisassemble.cpp?rev=366783=366782=366783=diff
==
--- lldb/trunk/source/Commands/CommandObjectDisassemble.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectDisassemble.cpp Tue Jul 23 00:15:54 
2019
@@ -31,30 +31,8 @@ using namespace lldb;
 using namespace lldb_private;
 
 static constexpr OptionDefinition g_disassemble_options[] = {
-// clang-format off
-  { LLDB_OPT_SET_ALL, false, "bytes", 'b', OptionParser::eNoArgument,  
 nullptr, {}, 0, eArgTypeNone,  
  "Show opcode bytes when disassembling." },
-  { LLDB_OPT_SET_ALL, false, "context",   'C', 
OptionParser::eRequiredArgument, nullptr, {}, 0,
 eArgTypeNumLines,"Number of context lines of source to show." 
},
-  { LLDB_OPT_SET_ALL, false, "mixed", 'm', OptionParser::eNoArgument,  
 nullptr, {}, 0, eArgTypeNone,  
  "Enable mixed source and assembly display." },
-  { LLDB_OPT_SET_ALL, false, "raw",   'r', OptionParser::eNoArgument,  
 nullptr, {}, 0, eArgTypeNone,  
  "Print raw disassembly with no symbol information." },
-  { LLDB_OPT_SET_ALL, false, "plugin",'P', 
OptionParser::eRequiredArgument, nullptr, {}, 0,
 eArgTypePlugin,  "Name of the disassembler plugin you want to 
use." },
-  { LLDB_OPT_SET_ALL, false, "flavor",'F', 
OptionParser::eRequiredArgument, nullptr, {}, 0,
 eArgTypeDisassemblyFlavor,   "Name of the disassembly flavor you want to 
use.  "
-  "Currently the only valid options are default, and for Intel "
-  "architectures, att and intel." },
-  { LLDB_OPT_SET_ALL, false, "arch",  'A', 
OptionParser::eRequiredArgument, nullptr, {}, 0,
 eArgTypeArchitecture,"Specify the architecture to use from cross 
disassembly." },
-  { LLDB_OPT_SET_1 |
-  LLDB_OPT_SET_2,   true,  "start-address", 's', 
OptionParser::eRequiredArgument, nullptr, {}, 0,
 eArgTypeAddressOrExpression, "Address at which to start disassembling." },
-  { LLDB_OPT_SET_1,   false, "end-address",   'e', 
OptionParser::eRequiredArgument, nullptr, {}, 0,
 eArgTypeAddressOrExpression, "Address at which to end disassembling." },
-  { LLDB_OPT_SET_2 |
-  LLDB_OPT_SET_3 |
-  LLDB_OPT_SET_4 |
-  LLDB_OPT_SET_5,   false, "count", 'c', 
OptionParser::eRequiredArgument, nullptr, {}, 0,
 eArgTypeNumLines,"Number of instructions to display." },
-  { LLDB_OPT_SET_3,   false, "name",  'n', 
OptionParser::eRequiredArgument, nullptr, {}, 
CommandCompletions::eSymbolCompletion, eArgTypeFunctionName,
"Disassemble entire contents of the given function name." },
-  { LLDB_OPT_SET_4,   false, "frame", 'f', OptionParser::eNoArgument,  
 nullptr, {}, 0, eArgTypeNone,  
  "Disassemble from the start of the current frame's function." },
-  { LLDB_OPT_SET_5,   false, "pc",'p', OptionParser::eNoArgument,  
 nullptr, {}, 0, eArgTypeNone,  
  "Disassemble around the current pc." },
-  { LLDB_OPT_SET_6,   false, "line",  'l', OptionParser::eNoArgument,  
 nullptr, {}, 0, eArgTypeNone,  
  "Disassemble the current frame's current source line instructions if 
there is debug line "
-  "table information, else disassemble around the pc." },
-  { LLDB_OPT_SET_7,   false, "address",   'a', 
OptionParser::eRequiredArgument, nullptr, {}, 0,
 eArgTypeAddressOrExpression, "Disassemble function containing this 
address." },
-// clang-format on
+#define LLDB_OPTIONS_disassemble
+#include "CommandOptions.inc"
 };
 
 CommandObjectDisassemble::CommandOptions::CommandOptions()

Modified: lldb/trunk/source/Commands/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/Options.td?rev=366783=366782=366783=diff
==
--- lldb/trunk/source/Commands/Options.td (original)
+++ lldb/trunk/source/Commands/Options.td Tue Jul 23 00:15:54 2019
@@ -295,6 +295,45 @@ let Command = 

[Lldb-commits] [PATCH] D64881: [Cmake] Use the modern way to find Python when possible

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I think setting 
-DPYTHON_EXECUTABLE=C:\Users\hwennborg\AppData\Local\Programs\Python\Python36-32\python.exe
 (or something) should be enough to get cmake to select python3 for the 
interpreter as well.

As for the version mismatch, the one of the reasons why the mismatched version 
did not work on unix is because some of our python build scripts assume that 
lldb will be running the same version of python as they are and derive python 
site-packages dir based on that. This is bad practice because of 
cross-compiling, but it may not be an issue on windows because there the 
site-packages dir does not include the python version (lib/site-packages vs 
lib/pythonX.Y/site-packages). So it's possible we can skip the version mismatch 
check on windows.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D64881



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

[Lldb-commits] [PATCH] D65114: [LLDB] Add utility to streamline Xcode project generation.

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/utils/xcode.py:33
+mkdir(build_path)
+with cwd(build_path):
+cmake_cmd = [

You can get rid of the chdir stuff by just running `cmake -S$SOURCE_DIR 
-B$BUILD_DIR && ninja -C $BUILD_DIR`.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D65114



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