date:20211130

[Lldb-commits] [PATCH] D114467: [LLDB][NativePDB] Allow find functions by full names

2021-11-30 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

What is the reason for these differences? The test in question appears to be 
fairly hermetic (in-tree compiler and linker), so it sounds like something is 
going wrong inside lldb...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114467

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

[Lldb-commits] [PATCH] D114746: [lldb] Generalize ParsedDWARFTypeAttributes

2021-11-30 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

I don't think we should be putting this into the DWARFAttribute file. It is 
substantially higher-level than the rest of the file, and the DWARFAttribute 
file is destined to be merged with the llvm implementation at some point. Is 
there a reason for not putting it into `DWARFASTParser` (like the other patch)?

In D114746#3160331 , @ljmf00 wrote:

> I'm not sure if this falls into NFC category since I'm changing how flags are 
> stored.

There's no formal definition of "NFC", so it does not really matter.

> This patch also reduces the memory footprint of the struct by using bit flags 
> instead of individual booleans.

The class was never meant to be stored anywhere else except the stack of the 
function doing the parsing, so it's not really necessary to optimize it that 
much. But since, you've already done it, I suppose it can stay...

Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h:83

+FLAGS_ENUM(DWARFAttributeFlags)
+{

This macro was created specifically for using in lldb public api. Elsewhere you 
can just use the standard llvm solution (search for LLVM_MARK_AS_BITMASK_ENUM).

Since this attribute is not supposed to be visible/useful outside the 
`ParsedDWARFTypeAttributes` class, I think it'd be better to declare it there.

Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h:123
+
+  inline bool is_artificial() const {
+return attr_flags & eDWARFAttributeIsArtificial;

`inline` on a method declared inside the class is redundant

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

https://reviews.llvm.org/D114746

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

[Lldb-commits] [lldb] 9a14ade - [lldb] Remove 'extern "C"' from the lldb-swig-python interface

2021-11-30 Thread Pavel Labath via lldb-commits


Author: Pavel Labath
Date: 2021-11-30T11:06:09+01:00
New Revision: 9a14adeae00015798843ff5cad987e5fdbdddb34

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

LOG: [lldb] Remove 'extern "C"' from the lldb-swig-python interface

The LLDBSWIGPython functions had (at least) two problems:
- There wasn't a single source of truth (a header file) for the
  prototypes of these functions. This meant that subtle differences
  in copies of function declarations could go by undetected. And
  not-so-subtle differences would result in strange runtime failures.
- All of the declarations had to have an extern "C" interface, because
  the function definitions were being placed inside and extert "C" block
  generated by swig.

This patch fixes both problems by moving the function definitions to the
%header block of the swig files. This block is not surrounded by extern
"C", and seems more appropriate anyway, as swig docs say it is meant for
"user-defined support code" (whereas the previous %wrapper code was for
automatically-generated wrappers).

It also puts the declarations into the SWIGPythonBridge header file
(which seems to have been created for this purpose), and ensures it is
included by all code wishing to define or use these functions. This
means that any differences in the declaration become a compiler error
instead of a runtime failure.

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

Added: 


Modified: 
lldb/bindings/python/python-wrapper.swig
lldb/bindings/python/python.swig
lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Removed: 




diff  --git a/lldb/bindings/python/python-wrapper.swig 
b/lldb/bindings/python/python-wrapper.swig
index 8159423c62bbd..079f8d12dafab 100644
--- a/lldb/bindings/python/python-wrapper.swig
+++ b/lldb/bindings/python/python-wrapper.swig
@@ -22,32 +22,8 @@ private:
 bool m_print;
 };
 
-%}
-
-%wrapper %{
-
-// resolve a dotted Python name in the form
-// foo.bar.baz.Foobar to an actual Python object
-// if pmodule is NULL, the __main__ module will be used
-// as the starting point for the search
-
-
-// This function is called by 
lldb_private::ScriptInterpreterPython::BreakpointCallbackFunction(...)
-// and is used when a script command is attached to a breakpoint for execution.
-
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wreturn-type-c-linkage"
-
-// Disable warning C4190: 'LLDBSwigPythonBreakpointCallbackFunction' has
-// C-linkage specified, but returns UDT 'llvm::Expected' which is
-// incompatible with C
-#if _MSC_VER
-#pragma warning (push)
-#pragma warning (disable : 4190)
-#endif
-
-SWIGEXPORT llvm::Expected
-LLDBSwigPythonBreakpointCallbackFunction
+llvm::Expected
+lldb_private::LLDBSwigPythonBreakpointCallbackFunction
 (
 const char *python_function_name,
 const char *session_dictionary_name,
@@ -93,17 +69,20 @@ LLDBSwigPythonBreakpointCallbackFunction
 return result.get().get() != Py_False;
 }
 
-#if _MSC_VER
-#pragma warning (pop)
-#endif
+// resolve a dotted Python name in the form
+// foo.bar.baz.Foobar to an actual Python object
+// if pmodule is NULL, the __main__ module will be used
+// as the starting point for the search
 
-#pragma clang diagnostic pop
+
+// This function is called by 
lldb_private::ScriptInterpreterPython::BreakpointCallbackFunction(...)
+// and is used when a script command is attached to a breakpoint for execution.
 
 // This function is called by 
lldb_private::ScriptInterpreterPython::WatchpointCallbackFunction(...)
 // and is used when a script command is attached to a watchpoint for execution.
 
-SWIGEXPORT bool
-LLDBSwigPythonWatchpointCallbackFunction
+bool
+lldb_private::LLDBSwigPythonWatchpointCallbackFunction
 (
 const char *python_function_name,
 const char *session_dictionary_name,
@@ -134,8 +113,8 @@ LLDBSwigPythonWatchpointCallbackFunction
 return stop_at_watchpoint;
 }
 
-SWIGEXPORT bool
-LLDBSwigPythonCallTypeScript
+bool
+lldb_private::LLDBSwigPythonCallTypeScript
 (
 const char *python_function_name,
 const void *session_dictionary,
@@ -207,8 +186,8 @@ LLDBSwigPythonCallTypeScript
 return true;
 }
 
-SWIGEXPORT void*
-LLDBSwigPythonCreateSyntheticProvider
+void*
+lldb_private::LLDBSwigPythonCreateSyntheticProvider
 (
 const char *python_class_name,
 const char *session_dictionary_name,
@@ -241,8 +220,8 @@ LLDBSwigPythonCreateSyntheticProvider
 Py_RETURN_NONE;
 }
 
-SWIGEXPORT void*
-LLDBSwigPythonCreateCommandObject
+void*
+lldb_private::LLDBSwigPythonCreateCommandObject
 (
 const char *python_class_name,
 const char *session_dictionary_name,

[Lldb-commits] [PATCH] D114369: [lldb] Remove 'extern "C"' from the lldb-swig-python interface

2021-11-30 Thread Pavel Labath via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9a14adeae000: [lldb] Remove 'extern "C"' 
from the lldb-swig-python interface (authored by labath).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114369

Files:
  lldb/bindings/python/python-wrapper.swig
  lldb/bindings/python/python.swig
  lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
@@ -8,10 +8,10 @@
 
 #include "gtest/gtest.h"
 
-#include "Plugins/ScriptInterpreter/Python/lldb-python.h"
-
+#include "Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h"
 #include "Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h"
 #include "Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h"
+#include "Plugins/ScriptInterpreter/Python/lldb-python.h"
 
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/HostInfo.h"
@@ -55,24 +55,11 @@
 
 #if PY_MAJOR_VERSION >= 3
 extern "C" PyObject *PyInit__lldb(void) { return nullptr; }
-#define LLDBSwigPyInit PyInit__lldb
 #else
 extern "C" void init_lldb(void) {}
-#define LLDBSwigPyInit init_lldb
-#endif
-
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wreturn-type-c-linkage"
-
-// Disable warning C4190: 'LLDBSwigPythonBreakpointCallbackFunction' has
-// C-linkage specified, but returns UDT 'llvm::Expected' which is
-// incompatible with C
-#if _MSC_VER
-#pragma warning (push)
-#pragma warning (disable : 4190)
 #endif
 
-extern "C" llvm::Expected LLDBSwigPythonBreakpointCallbackFunction(
+llvm::Expected lldb_private::LLDBSwigPythonBreakpointCallbackFunction(
 const char *python_function_name, const char *session_dictionary_name,
 const lldb::StackFrameSP &sb_frame,
 const lldb::BreakpointLocationSP &sb_bp_loc,
@@ -80,218 +67,205 @@
   return false;
 }
 
-#if _MSC_VER
-#pragma warning (pop)
-#endif
-
-#pragma clang diagnostic pop
-
-extern "C" bool LLDBSwigPythonWatchpointCallbackFunction(
+bool lldb_private::LLDBSwigPythonWatchpointCallbackFunction(
 const char *python_function_name, const char *session_dictionary_name,
 const lldb::StackFrameSP &sb_frame, const lldb::WatchpointSP &sb_wp) {
   return false;
 }
 
-extern "C" bool LLDBSwigPythonCallTypeScript(
-const char *python_function_name, void *session_dictionary,
+bool lldb_private::LLDBSwigPythonCallTypeScript(
+const char *python_function_name, const void *session_dictionary,
 const lldb::ValueObjectSP &valobj_sp, void **pyfunct_wrapper,
 const lldb::TypeSummaryOptionsSP &options_sp, std::string &retval) {
   return false;
 }
 
-extern "C" void *
-LLDBSwigPythonCreateSyntheticProvider(const char *python_class_name,
-  const char *session_dictionary_name,
-  const lldb::ValueObjectSP &valobj_sp) {
+void *lldb_private::LLDBSwigPythonCreateSyntheticProvider(
+const char *python_class_name, const char *session_dictionary_name,
+const lldb::ValueObjectSP &valobj_sp) {
   return nullptr;
 }
 
-extern "C" void *
-LLDBSwigPythonCreateCommandObject(const char *python_class_name,
-  const char *session_dictionary_name,
-  const lldb::DebuggerSP debugger_sp) {
+void *lldb_private::LLDBSwigPythonCreateCommandObject(
+const char *python_class_name, const char *session_dictionary_name,
+const lldb::DebuggerSP debugger_sp) {
   return nullptr;
 }
 
-extern "C" void *LLDBSwigPythonCreateScriptedThreadPlan(
+void *lldb_private::LLDBSwigPythonCreateScriptedThreadPlan(
 const char *python_class_name, const char *session_dictionary_name,
-StructuredDataImpl *args_data,
-std::string &error_string,
+StructuredDataImpl *args_data, std::string &error_string,
 const lldb::ThreadPlanSP &thread_plan_sp) {
   return nullptr;
 }
 
-extern "C" bool LLDBSWIGPythonCallThreadPlan(void *implementor,
- const char *method_name,
- Event *event_sp, bool &got_error) {
+bool lldb_private::LLDBSWIGPythonCallThreadPlan(void *implementor,
+const char *method_name,
+Event *event_sp,
+bool &got_error) {
   return false;
 }
 
-extern "C" void *LLDBSwigPythonCreateScriptedBreakpointResolver(
+void *lldb_private::LLDBSwigPythonCreateScriptedBreakpointResolver(
 const char *python_class_name, const char *se

[Lldb-commits] [PATCH] D114675: [lldb] [Target] Support fallback to file address in ReadMemory()

2021-11-30 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

In D114675#3160513 , @mgorny wrote:

> Well, that's what happens with the core file I have and what you're asking 
> for is really above my paygrade.

Maybe you should ask for a raise then. :P

> That said, this is kernel core dump, so load addresses should probably be the 
> same as vm-addresses in the file.

Yes, and the real question why doesn't lldb know that. The point of the load 
addresses is to tell lldb where a particular section is loaded, so the 
problem/bug is that that is not happening. Normally this is the job of the 
dynamic loader plugin. ProcessElfCore is using `DynamicLoaderPOSIXDYLD`, but 
that is probably not correct for the kernel cores. If the file addresses in the 
kernel image are always correct, then it may be sufficient to just make it use 
`DynamicLoaderStatic` for these kinds of cores. If not, you may want to try 
implementing something like the existing minidump plugin, which directly sets 
load addresses based on the information in the minidump file (bypassing the 
dynamic loader plugin, which wouldn't really work anyway, as the relevant 
information would not be captured in the minidump).

Blindly falling back to file addresses is not a solution, because it will be 
wrong for the cases where the file and load addresses do not match (e.g. an elf 
pie binary will have a file address of zero, but that's definitely not where it 
will be loaded).

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

https://reviews.llvm.org/D114675

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

[Lldb-commits] [lldb] a6e6736 - [lldb] Inline Platform::LoadCachedExecutable into its (single) caller

2021-11-30 Thread Pavel Labath via lldb-commits


Author: Pavel Labath
Date: 2021-11-30T14:15:49+01:00
New Revision: a6e673643c44f94557fa09022a3c6edf76167871

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

LOG: [lldb] Inline Platform::LoadCachedExecutable into its (single) caller

Added: 


Modified: 
lldb/include/lldb/Target/Platform.h
lldb/source/Target/Platform.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/Platform.h 
b/lldb/include/lldb/Target/Platform.h
index e645e3ca95bee..26127359a3224 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -953,10 +953,6 @@ class Platform : public PluginInterface {
   bool GetCachedSharedModule(const ModuleSpec &module_spec,
  lldb::ModuleSP &module_sp, bool *did_create_ptr);
 
-  Status LoadCachedExecutable(const ModuleSpec &module_spec,
-  lldb::ModuleSP &module_sp,
-  const FileSpecList *module_search_paths_ptr);
-
   FileSpec GetModuleCacheRoot();
 };
 

diff  --git a/lldb/source/Target/Platform.cpp b/lldb/source/Target/Platform.cpp
index d75e11b0ab450..af5ca0225169f 100644
--- a/lldb/source/Target/Platform.cpp
+++ b/lldb/source/Target/Platform.cpp
@@ -1547,28 +1547,20 @@ Status
 Platform::GetCachedExecutable(ModuleSpec &module_spec,
   lldb::ModuleSP &module_sp,
   const FileSpecList *module_search_paths_ptr) {
-  const auto platform_spec = module_spec.GetFileSpec();
-  const auto error =
-  LoadCachedExecutable(module_spec, module_sp, module_search_paths_ptr);
-  if (error.Success()) {
-module_spec.GetFileSpec() = module_sp->GetFileSpec();
-module_spec.GetPlatformFileSpec() = platform_spec;
-  }
-
-  return error;
-}
-
-Status
-Platform::LoadCachedExecutable(const ModuleSpec &module_spec,
-   lldb::ModuleSP &module_sp,
-   const FileSpecList *module_search_paths_ptr) {
-  return GetRemoteSharedModule(
+  FileSpec platform_spec = module_spec.GetFileSpec();
+  Status error = GetRemoteSharedModule(
   module_spec, nullptr, module_sp,
   [&](const ModuleSpec &spec) {
 return ResolveRemoteExecutable(spec, module_sp,
module_search_paths_ptr);
   },
   nullptr);
+  if (error.Success()) {
+module_spec.GetFileSpec() = module_sp->GetFileSpec();
+module_spec.GetPlatformFileSpec() = platform_spec;
+  }
+
+  return error;
 }
 
 Status Platform::GetRemoteSharedModule(const ModuleSpec &module_spec,



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

[Lldb-commits] [lldb] 1408684 - [lldb] Introduce PlatformQemuUser

2021-11-30 Thread Pavel Labath via lldb-commits


Author: Pavel Labath
Date: 2021-11-30T14:16:08+01:00
New Revision: 1408684957bbfb5b412e0ef3c027c88daa1058eb

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

LOG: [lldb] Introduce PlatformQemuUser

This adds a new platform class, whose job is to enable running
(debugging) executables under qemu.

(For general information about qemu, I recommend reading the RFC thread
on lldb-dev
.)

This initial patch implements the necessary boilerplate as well as the
minimal amount of functionality needed to actually be able to do
something useful (which, in this case means debugging a fully statically
linked executable).

The knobs necessary to emulate dynamically linked programs, as well as
to control other aspects of qemu operation (the emulated cpu, for
instance) will be added in subsequent patches. Same goes for the ability
to automatically bind to the executables of the emulated architecture.

Currently only two settings are available:
- architecture: the architecture that we should emulate
- emulator-path: the path to the emulator

Even though this patch is relatively small, it doesn't lack subtleties
that are worth calling out explicitly:
- named sockets: qemu supports tcp and unix socket connections, both of
  them in the "forward connect" mode (qemu listening, lldb connecting).
  Forward TCP connections are impossible to realise in a race-free way.
  This is the reason why I chose unix sockets as they have larger, more
  structured names, which can guarantee that there are no collisions
  between concurrent connection attempts.
- the above means that this code will not work on windows. I don't think
  that's an issue since user mode qemu does not support windows anyway.
- Right now, I am leaving the code enabled for windows, but maybe it
  would be better to disable it (otoh, disabling it means windows
  developers can't check they don't break it)
- qemu-user also does not support macOS, so one could contemplate
  disabling it there too. However, macOS does support named sockets, so
  one can even run the (mock) qemu tests there, and I think it'd be a
  shame to lose that.

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

Added: 
lldb/source/Plugins/Platform/QemuUser/CMakeLists.txt
lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.h
lldb/source/Plugins/Platform/QemuUser/PlatformQemuUserProperties.td
lldb/test/API/qemu/Makefile
lldb/test/API/qemu/TestQemuLaunch.py
lldb/test/API/qemu/main.c
lldb/test/API/qemu/qemu.py

Modified: 
lldb/packages/Python/lldbsuite/test/gdbclientutils.py
lldb/source/Plugins/Platform/CMakeLists.txt

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/gdbclientutils.py 
b/lldb/packages/Python/lldbsuite/test/gdbclientutils.py
index 6b4650eda0735..78854bb9dae87 100644
--- a/lldb/packages/Python/lldbsuite/test/gdbclientutils.py
+++ b/lldb/packages/Python/lldbsuite/test/gdbclientutils.py
@@ -339,7 +339,7 @@ class UnexpectedPacketException(Exception):
 pass
 
 
-class ServerSocket:
+class ServerChannel:
 """
 A wrapper class for TCP or pty-based server.
 """
@@ -366,22 +366,14 @@ def sendall(self, data):
 """Send the data to the connected client."""
 
 
-class TCPServerSocket(ServerSocket):
-def __init__(self):
-family, type, proto, _, addr = socket.getaddrinfo(
-"localhost", 0, proto=socket.IPPROTO_TCP)[0]
+class ServerSocket(ServerChannel):
+def __init__(self, family, type, proto, addr):
 self._server_socket = socket.socket(family, type, proto)
 self._connection = None
 
 self._server_socket.bind(addr)
 self._server_socket.listen(1)
 
-def get_connect_address(self):
-return "[{}]:{}".format(*self._server_socket.getsockname())
-
-def get_connect_url(self):
-return "connect://" + self.get_connect_address()
-
 def close_server(self):
 self._server_socket.close()
 
@@ -410,7 +402,31 @@ def sendall(self, data):
 return self._connection.sendall(data)
 
 
-class PtyServerSocket(ServerSocket):
+class TCPServerSocket(ServerSocket):
+def __init__(self):
+family, type, proto, _, addr = socket.getaddrinfo(
+"localhost", 0, proto=socket.IPPROTO_TCP)[0]
+super().__init__(family, type, proto, addr)
+
+def get_connect_address(self):
+return "[{}]:{}".format(*self._server_socket.getsockname())
+
+def get_connect_url(self):
+return "connect://" + self.get_connect_address()
+
+
+class UnixServerSocket(ServerSocket):
+def __init__(self, addr):
+super().__init__(socket.AF_UNIX, socket.SOCK_STREAM,

[Lldb-commits] [PATCH] D114509: [lldb] Introduce PlatformQemuUser

2021-11-30 Thread Pavel Labath via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1408684957bb: [lldb] Introduce PlatformQemuUser (authored by 
labath).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114509

Files:
  lldb/packages/Python/lldbsuite/test/gdbclientutils.py
  lldb/source/Plugins/Platform/CMakeLists.txt
  lldb/source/Plugins/Platform/QemuUser/CMakeLists.txt
  lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
  lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.h
  lldb/source/Plugins/Platform/QemuUser/PlatformQemuUserProperties.td
  lldb/test/API/qemu/Makefile
  lldb/test/API/qemu/TestQemuLaunch.py
  lldb/test/API/qemu/main.c
  lldb/test/API/qemu/qemu.py

Index: lldb/test/API/qemu/qemu.py
===
--- /dev/null
+++ lldb/test/API/qemu/qemu.py
@@ -0,0 +1,37 @@
+from textwrap import dedent
+import argparse
+import socket
+import json
+
+import use_lldb_suite
+from lldbsuite.test.gdbclientutils import *
+
+class MyResponder(MockGDBServerResponder):
+def cont(self):
+return "W47"
+
+class FakeEmulator(MockGDBServer):
+def __init__(self, addr):
+super().__init__(UnixServerSocket(addr))
+self.responder = MyResponder()
+
+def main():
+parser = argparse.ArgumentParser(description=dedent("""\
+Implements a fake qemu for testing purposes. The executable program
+is not actually run. Instead a very basic mock process is presented
+to lldb. The emulated program must accept at least one argument.
+This should be a path where the emulator will dump its state. This
+allows us to check the invocation parameters.
+"""))
+parser.add_argument('-g', metavar="unix-socket", required=True)
+parser.add_argument('program', help="The program to 'emulate'.")
+parser.add_argument('state_file', help="Where to dump the emulator state.")
+parsed, rest = parser.parse_known_args()
+with open(parsed.state_file, "w") as f:
+json.dump({"program":parsed.program, "rest":rest}, f)
+
+emulator = FakeEmulator(parsed.g)
+emulator.run()
+
+if __name__ == "__main__":
+main()
Index: lldb/test/API/qemu/main.c
===
--- /dev/null
+++ lldb/test/API/qemu/main.c
@@ -0,0 +1,3 @@
+// NB: This code will never be run, but we do need a realistic-looking
+// executable for the tests.
+int main() {}
Index: lldb/test/API/qemu/TestQemuLaunch.py
===
--- /dev/null
+++ lldb/test/API/qemu/TestQemuLaunch.py
@@ -0,0 +1,83 @@
+from __future__ import print_function
+import lldb
+import unittest
+import os
+import json
+import stat
+import sys
+from textwrap import dedent
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from lldbsuite.test.gdbclientutils import *
+
+
+@skipIfRemote
+@skipIfWindows
+class TestQemuLaunch(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+def set_emulator_setting(self, name, value):
+self.runCmd("settings set platform.plugin.qemu-user.%s %s" %
+(name, value))
+
+def setUp(self):
+super().setUp()
+emulator = self.getBuildArtifact("qemu.py")
+with os.fdopen(os.open(emulator, os.O_WRONLY|os.O_CREAT, stat.S_IRWXU),
+"w") as e:
+
+e.write(dedent("""\
+#! {exec!s}
+
+import runpy
+import sys
+
+sys.path = {path!r}
+runpy.run_path({source!r}, run_name='__main__')
+""").format(exec=sys.executable, path=sys.path,
+source=self.getSourcePath("qemu.py")))
+
+self.set_emulator_setting("architecture", self.getArchitecture())
+self.set_emulator_setting("emulator-path", emulator)
+
+def test_basic_launch(self):
+self.build()
+exe = self.getBuildArtifact()
+
+# Create a target using out platform
+error = lldb.SBError()
+target = self.dbg.CreateTarget(exe, '', 'qemu-user', False, error)
+self.assertSuccess(error)
+self.assertEqual(target.GetPlatform().GetName(), "qemu-user")
+
+# "Launch" the process. Our fake qemu implementation will pretend it
+# immediately exited.
+process = target.LaunchSimple(
+[self.getBuildArtifact("state.log"), "arg2", "arg3"], None, None)
+self.assertIsNotNone(process)
+self.assertEqual(process.GetState(), lldb.eStateExited)
+self.assertEqual(process.GetExitStatus(), 0x47)
+
+# Verify the qemu invocation parameters.
+with open(self.getBuildArtifact("state.log")) as s:
+state = json.load(s)
+self.assertEqual(state["p

[Lldb-commits] [PATCH] D114791: [lldb] Clarify StructuredDataImpl ownership

2021-11-30 Thread Pavel Labath via Phabricator via lldb-commits

labath created this revision.
labath added reviewers: JDevlieghere, mib, jingham.
Herald added a subscriber: yaxunl.
labath requested review of this revision.
Herald added a project: LLDB.

StructuredDataImpl ownership semantics is unclear at best. Various
structures were holding a non-owning pointer to it, with a comment that
the object is owned somewhere else. From what I was able to gather that
"somewhere else" was the SBStructuredData object, but I am not sure that
all created object eventually made its way there. (It wouldn't matter
even if they did, as we are leaking most of our SBStructuredData
objects.)

Since StructuredDataImpl is just a collection of two (shared) pointers,
there's really no point in elaborate lifetime management, so this patch
replaces all StructuredDataImpl pointers with actual objects or
unique_ptrs to it. This makes it much easier to resolve SBStructuredData
leaks in a follow-up patch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114791

Files:
  lldb/bindings/lua/lua-wrapper.swig
  lldb/bindings/python/python-wrapper.swig
  lldb/include/lldb/API/SBStructuredData.h
  lldb/include/lldb/Breakpoint/BreakpointResolverScripted.h
  lldb/include/lldb/Core/StructuredDataImpl.h
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  lldb/include/lldb/Target/Target.h
  lldb/include/lldb/Target/ThreadPlanPython.h
  lldb/source/API/SBStructuredData.cpp
  lldb/source/API/SBThreadPlan.cpp
  lldb/source/Breakpoint/BreakpointResolverScripted.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
  
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/Thread.cpp
  lldb/source/Target/ThreadPlanPython.cpp
  lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
@@ -63,7 +63,7 @@
 const char *python_function_name, const char *session_dictionary_name,
 const lldb::StackFrameSP &sb_frame,
 const lldb::BreakpointLocationSP &sb_bp_loc,
-StructuredDataImpl *args_impl) {
+const StructuredDataImpl &args_impl) {
   return false;
 }
 
@@ -94,7 +94,7 @@
 
 void *lldb_private::LLDBSwigPythonCreateScriptedThreadPlan(
 const char *python_class_name, const char *session_dictionary_name,
-StructuredDataImpl *args_data, std::string &error_string,
+const StructuredDataImpl &args_data, std::string &error_string,
 const lldb::ThreadPlanSP &thread_plan_sp) {
   return nullptr;
 }
@@ -108,7 +108,7 @@
 
 void *lldb_private::LLDBSwigPythonCreateScriptedBreakpointResolver(
 const char *python_class_name, const char *session_dictionary_name,
-lldb_private::StructuredDataImpl *args, const lldb::BreakpointSP &bkpt_sp) {
+const StructuredDataImpl &args, const lldb::BreakpointSP &bkpt_sp) {
   return nullptr;
 }
 
@@ -200,14 +200,14 @@
 
 void *lldb_private::LLDBSwigPythonCreateScriptedProcess(
 const char *python_class_name, const char *session_dictionary_name,
-const lldb::TargetSP &target_sp, StructuredDataImpl *args_impl,
+const lldb::TargetSP &target_sp, const StructuredDataImpl &args_impl,
 std::string &error_string) {
   return nullptr;
 }
 
 void *lldb_private::LLDBSwigPythonCreateScriptedThread(
 const char *python_class_name, const char *session_dictionary_name,
-const lldb::ProcessSP &process_sp, StructuredDataImpl *args_impl,
+const lldb::ProcessSP &process_sp, const StructuredDataImpl &args_impl,
 std::string &error_string) {
   return nullptr;
 }
@@ -259,8 +259,8 @@
 
 void *lldb_private::LLDBSwigPythonCreateScriptedStopHook(
 lldb::TargetSP target_sp, const char *python_class_name,
-const char *session_dictionary_name,
-lldb_private::StructuredDataImpl *args_impl, Status &error) {
+const char *session_dictionary_name, const StructuredDataImpl &args_impl,
+Status &error) {
   return nullptr;
 }
 
Index: lldb/source/Target/ThreadPlanPython.cpp
===
--- lldb/source/Target/ThreadPlanPython.cpp
+++ lldb/source/Target/ThreadPlanPython.cpp
@@ -26,7 +26,7 @@
 // ThreadPlanPython
 
 ThreadPlanPython::ThreadPlanPython(Thread &thread, const char *class_name,
-   StructuredDataImpl *args_data)
+   const StructuredDataImpl &args_data)
 : ThreadPlan(ThreadPlan::eKindPython, "Python based Thread Plan", thread,

[Lldb-commits] [PATCH] D114791: [lldb] Clarify StructuredDataImpl ownership

2021-11-30 Thread Pavel Labath via Phabricator via lldb-commits

labath added inline comments.



Comment at: lldb/source/API/SBThreadPlan.cpp:72-73
   if (thread)
-m_opaque_wp =
-std::make_shared(*thread, class_name, nullptr);
+m_opaque_wp = std::make_shared(*thread, class_name,
+ StructuredDataImpl());
 }

I haven't been able to figure out how/if this works. As far as I can tell, this 
object will get destroyed immediately after construction due to lack of any 
shared_ptrs pointing to it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114791

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

[Lldb-commits] [PATCH] D114796: [lldb/qemu] Add support for pty redirection

2021-11-30 Thread Pavel Labath via Phabricator via lldb-commits

labath created this revision.
labath added reviewers: mgorny, DavidSpickett, JDevlieghere.
labath requested review of this revision.
Herald added a project: LLDB.

Lldb uses a pty to read/write to the standard input and output of the
debugged process. For host processes this would be automatically set up
by Target::FinalizeFileActions. The Qemu platform is in a unique
position of not really being a host platform, but not being remote
either. It reports IsHost() = false, but it is sufficiently host-like
that we can use the usual pty mechanism.

This patch adds the necessary glue code to enable pty redirection. It
includes a small refactor of Target::FinalizeFileActions and
ProcessLaunchInfo::SetUpPtyRedirection to reduce the amount of
boilerplate that would need to be copied.

I will note that qemu is not able to separate output from the emulated
program from the output of the emulator itself, so the two will arrive
intertwined. Normally this should not be a problem since qemu should not
produce any output during regular operation, but some output can slip
through in case of errors. This situation should be pretty obvious (to a
human), and it is the best we can do anyway.

For testing purposes, and inspired by lldb-server tests, I have extended
the mock emulator with the ability "program" the behavior of the
"emulated" program via command-line arguments.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114796

Files:
  lldb/source/Host/common/ProcessLaunchInfo.cpp
  lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
  lldb/source/Target/Target.cpp
  lldb/test/API/qemu/TestQemuLaunch.py
  lldb/test/API/qemu/qemu.py

Index: lldb/test/API/qemu/qemu.py
===
--- lldb/test/API/qemu/qemu.py
+++ lldb/test/API/qemu/qemu.py
@@ -1,36 +1,63 @@
-from textwrap import dedent
 import argparse
 import socket
 import json
+import sys
 
 import use_lldb_suite
 from lldbsuite.test.gdbclientutils import *
 
+_description = """\
+Implements a fake qemu for testing purposes. The executable program
+is not actually run. Instead a very basic mock process is presented
+to lldb. This allows us to check the invocation parameters.
+
+The behavior of the emulated "process" is controlled via its command line
+arguments, which should take the form of key:value pairs. Currently supported
+actions are:
+- dump: Dump the state of the emulator as a json dictionary.  specifies
+  the target filename.
+- stdout: Write  to program stdout.
+- stderr: Write  to program stderr.
+- stdin: Read a line from stdin and store it in the emulator state. 
+  specifies the dictionary key.
+"""
+
 class MyResponder(MockGDBServerResponder):
+def __init__(self, state):
+super().__init__()
+self._state = state
+
 def cont(self):
+for a in self._state["args"]:
+action, data = a.split(":", 1)
+if action == "dump":
+with open(data, "w") as f:
+json.dump(self._state, f)
+elif action == "stdout":
+sys.stdout.write(data)
+elif action == "stderr":
+sys.stderr.write(data)
+elif action == "stdin":
+self._state[data] = sys.stdin.readline()
+else:
+print("Unknown action: %r\n" % a)
+return "X01"
 return "W47"
 
 class FakeEmulator(MockGDBServer):
-def __init__(self, addr):
+def __init__(self, addr, state):
 super().__init__(UnixServerSocket(addr))
-self.responder = MyResponder()
+self.responder = MyResponder(state)
 
 def main():
-parser = argparse.ArgumentParser(description=dedent("""\
-Implements a fake qemu for testing purposes. The executable program
-is not actually run. Instead a very basic mock process is presented
-to lldb. The emulated program must accept at least one argument.
-This should be a path where the emulator will dump its state. This
-allows us to check the invocation parameters.
-"""))
+parser = argparse.ArgumentParser(description=_description,
+formatter_class=argparse.RawDescriptionHelpFormatter)
 parser.add_argument('-g', metavar="unix-socket", required=True)
 parser.add_argument('program', help="The program to 'emulate'.")
-parser.add_argument('state_file', help="Where to dump the emulator state.")
-parsed, rest = parser.parse_known_args()
-with open(parsed.state_file, "w") as f:
-json.dump({"program":parsed.program, "rest":rest}, f)
+parser.add_argument("args", nargs=argparse.REMAINDER)
+args = parser.parse_args()
 
-emulator = FakeEmulator(parsed.g)
+emulator = FakeEmulator(args.g, vars(args))
 emulator.run()
 
 if __name__ == "__main__":
Index: lldb/test/API/qemu/TestQemuLaunch.py
===
--- lldb/test/API/

[Lldb-commits] [PATCH] D114796: [lldb/qemu] Add support for pty redirection

2021-11-30 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett added inline comments.



Comment at: lldb/source/Target/Target.cpp:3324
 
-  if (default_to_use_pty &&
-  (!in_file_spec || !out_file_spec || !err_file_spec)) {
+  if (default_to_use_pty) {
 llvm::Error Err = info.SetUpPtyRedirection();

What's the logic to this change? I thought maybe the `!in_file_spec...` were 
redundant but not 100% sure.



Comment at: lldb/test/API/qemu/TestQemuLaunch.py:75
+
+# Create a target using out platform
+error = lldb.SBError()

out -> our?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114796

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

[Lldb-commits] [PATCH] D114796: [lldb/qemu] Add support for pty redirection

2021-11-30 Thread Pavel Labath via Phabricator via lldb-commits

labath marked an inline comment as done.
labath added inline comments.



Comment at: lldb/source/Target/Target.cpp:3324
 
-  if (default_to_use_pty &&
-  (!in_file_spec || !out_file_spec || !err_file_spec)) {
+  if (default_to_use_pty) {
 llvm::Error Err = info.SetUpPtyRedirection();

DavidSpickett wrote:
> What's the logic to this change? I thought maybe the `!in_file_spec...` were 
> redundant but not 100% sure.
The original code was trying to avoid calling this function in case all three 
actions were set. It was almost redundant, as `SetUpPtyRedirection` had the 
same checks already -- the only difference was that the function would 
unconditionally create the pty master (and then not use it). So I've added an 
early exit to that function and that way I could avoid doing the same checks in 
the new platform.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114796

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

[Lldb-commits] [PATCH] D114796: [lldb/qemu] Add support for pty redirection

2021-11-30 Thread Pavel Labath via Phabricator via lldb-commits

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

fix typo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114796

Files:
  lldb/source/Host/common/ProcessLaunchInfo.cpp
  lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
  lldb/source/Target/Target.cpp
  lldb/test/API/qemu/TestQemuLaunch.py
  lldb/test/API/qemu/qemu.py

Index: lldb/test/API/qemu/qemu.py
===
--- lldb/test/API/qemu/qemu.py
+++ lldb/test/API/qemu/qemu.py
@@ -1,36 +1,63 @@
-from textwrap import dedent
 import argparse
 import socket
 import json
+import sys
 
 import use_lldb_suite
 from lldbsuite.test.gdbclientutils import *
 
+_description = """\
+Implements a fake qemu for testing purposes. The executable program
+is not actually run. Instead a very basic mock process is presented
+to lldb. This allows us to check the invocation parameters.
+
+The behavior of the emulated "process" is controlled via its command line
+arguments, which should take the form of key:value pairs. Currently supported
+actions are:
+- dump: Dump the state of the emulator as a json dictionary.  specifies
+  the target filename.
+- stdout: Write  to program stdout.
+- stderr: Write  to program stderr.
+- stdin: Read a line from stdin and store it in the emulator state. 
+  specifies the dictionary key.
+"""
+
 class MyResponder(MockGDBServerResponder):
+def __init__(self, state):
+super().__init__()
+self._state = state
+
 def cont(self):
+for a in self._state["args"]:
+action, data = a.split(":", 1)
+if action == "dump":
+with open(data, "w") as f:
+json.dump(self._state, f)
+elif action == "stdout":
+sys.stdout.write(data)
+elif action == "stderr":
+sys.stderr.write(data)
+elif action == "stdin":
+self._state[data] = sys.stdin.readline()
+else:
+print("Unknown action: %r\n" % a)
+return "X01"
 return "W47"
 
 class FakeEmulator(MockGDBServer):
-def __init__(self, addr):
+def __init__(self, addr, state):
 super().__init__(UnixServerSocket(addr))
-self.responder = MyResponder()
+self.responder = MyResponder(state)
 
 def main():
-parser = argparse.ArgumentParser(description=dedent("""\
-Implements a fake qemu for testing purposes. The executable program
-is not actually run. Instead a very basic mock process is presented
-to lldb. The emulated program must accept at least one argument.
-This should be a path where the emulator will dump its state. This
-allows us to check the invocation parameters.
-"""))
+parser = argparse.ArgumentParser(description=_description,
+formatter_class=argparse.RawDescriptionHelpFormatter)
 parser.add_argument('-g', metavar="unix-socket", required=True)
 parser.add_argument('program', help="The program to 'emulate'.")
-parser.add_argument('state_file', help="Where to dump the emulator state.")
-parsed, rest = parser.parse_known_args()
-with open(parsed.state_file, "w") as f:
-json.dump({"program":parsed.program, "rest":rest}, f)
+parser.add_argument("args", nargs=argparse.REMAINDER)
+args = parser.parse_args()
 
-emulator = FakeEmulator(parsed.g)
+emulator = FakeEmulator(args.g, vars(args))
 emulator.run()
 
 if __name__ == "__main__":
Index: lldb/test/API/qemu/TestQemuLaunch.py
===
--- lldb/test/API/qemu/TestQemuLaunch.py
+++ lldb/test/API/qemu/TestQemuLaunch.py
@@ -6,6 +6,7 @@
 import stat
 import sys
 from textwrap import dedent
+import lldbsuite.test.lldbutil
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test.decorators import *
 from lldbsuite.test.gdbclientutils import *
@@ -46,7 +47,7 @@
 self.build()
 exe = self.getBuildArtifact()
 
-# Create a target using out platform
+# Create a target using our platform
 error = lldb.SBError()
 target = self.dbg.CreateTarget(exe, '', 'qemu-user', False, error)
 self.assertSuccess(error)
@@ -55,7 +56,7 @@
 # "Launch" the process. Our fake qemu implementation will pretend it
 # immediately exited.
 process = target.LaunchSimple(
-[self.getBuildArtifact("state.log"), "arg2", "arg3"], None, None)
+["dump:" + self.getBuildArtifact("state.log")], None, None)
 self.assertIsNotNone(process)
 self.assertEqual(process.GetState(), lldb.eStateExited)
 self.assertEqual(process.GetExitStatus(), 0x47)
@@ -64,7 +65,84 @@
 with open(self.getBuildArtifact("state.log")) as s:
 state = json.load(s)
 self.assertE

[Lldb-commits] [PATCH] D114719: [lldb][NFC] Move non-clang specific method to the generic DWARF Parser

2021-11-30 Thread Shafik Yaghmour via Phabricator via lldb-commits

shafik accepted this revision.
shafik added a comment.

LGTM


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

https://reviews.llvm.org/D114719

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

[Lldb-commits] [PATCH] D114668: [lldb][NFC] Move generic DWARFASTParser code out of Clang-specific code

2021-11-30 Thread Shafik Yaghmour via Phabricator via lldb-commits

shafik added a subscriber: clayborg.
shafik added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.cpp:77
+  default:
+  case DW_AT_abstract_origin:
+  case DW_AT_accessibility:

ljmf00 wrote:
> bulbazord wrote:
> > ljmf00 wrote:
> > > Why we are including just these specific attributes? Maybe we should add 
> > > a comment explaining it. According to the DWARF standard, any attribute 
> > > is eligible for any tag.
> > I'm not sure why. Possibly they were added to make sure the switch was 
> > fully covered (potentially to silence a warning)? You could add a `FIXME` 
> > or a `TODO` if you feel that these attributes should have functionality 
> > associated with them like the ones above.
> I don't think it is to mark it as fully covered since there are much more 
> attributes, the default label will address it anyway, and according to the 
> DWARF standard, any attribute can be in a type tag (realistically, any tag). 
> We can take the example of `DW_AT_description` which is just a description 
> associated with the symbol. I feel like this can be safely deleted but I'm 
> afraid to do it in favour of some other rationale I'm not seeing.
@clayborg it looks like this has been this way since you put this in: 
https://github.com/llvm/llvm-project/commit/261ac3f4b5b98d02dd8718078015a92cf07df736

Do you agree this is dead code or is there something we are missing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114668

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

[Lldb-commits] [PATCH] D114746: [lldb] Generalize ParsedDWARFTypeAttributes

2021-11-30 Thread Luís Ferreira via Phabricator via lldb-commits

ljmf00 planned changes to this revision.
ljmf00 added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h:86
+  /// Whether it is an artificially generated symbol.
+  eDWARFAttributeIsArtificial = (1u << 0),
+  eDWARFAttributeIsExplicit = (1u << 1),

I forgot to add the rest of the documentation. Leaving a note here for myself


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

https://reviews.llvm.org/D114746

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

[Lldb-commits] [PATCH] D114627: [lldb] add new overload for SymbolFile::FindTypes that accepts a scope

2021-11-30 Thread Shafik Yaghmour via Phabricator via lldb-commits

shafik added a comment.

Is there a specific use case that motivated this feature?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114627

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

[Lldb-commits] [PATCH] D114742: [lldb] Search PrivateFrameworks when using an internal SDK

2021-11-30 Thread Adrian Prantl via Phabricator via lldb-commits

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



Comment at: lldb/packages/Python/lldbsuite/test/builders/darwin.py:86
+private_frameworks = os.path.join(sdk_root, 
'System','Library','PrivateFrameworks')
+args['FRAMEWORK_INCLUDES'] = '-F{}'.format(private_frameworks)
+

Just curious: why not 
```
 args['FRAMEWORK_INCLUDES'] = '-F' + private_frameworks
```
?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114742

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

[Lldb-commits] [PATCH] D114742: [lldb] Search PrivateFrameworks when using an internal SDK

2021-11-30 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added inline comments.



Comment at: lldb/packages/Python/lldbsuite/test/builders/darwin.py:86
+private_frameworks = os.path.join(sdk_root, 
'System','Library','PrivateFrameworks')
+args['FRAMEWORK_INCLUDES'] = '-F{}'.format(private_frameworks)
+

aprantl wrote:
> Just curious: why not 
> ```
>  args['FRAMEWORK_INCLUDES'] = '-F' + private_frameworks
> ```
> ?
Mostly for consistency with the rest of the file, but also personal preference. 
It's no coincidence that the rest of the file uses `format` too. I like 
f-strings even more, but they're Python 3.something only. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114742

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

[Lldb-commits] [lldb] 0a302f6 - [lldb] Search PrivateFrameworks when using an internal SDK

2021-11-30 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2021-11-30T11:28:19-08:00
New Revision: 0a302f66673720a0d3fd3f0ce32ec9cfda428cf1

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

LOG: [lldb] Search PrivateFrameworks when using an internal SDK

Make sure to add the PrivateFrameworks directory to the frameworks path
when using an internal SDK. This is necessary for the "on-device" test
suite.

rdar://84519268

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

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/builders/darwin.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/builders/darwin.py 
b/lldb/packages/Python/lldbsuite/test/builders/darwin.py
index 6e5ba391a7b70..acde678ba87a0 100644
--- a/lldb/packages/Python/lldbsuite/test/builders/darwin.py
+++ b/lldb/packages/Python/lldbsuite/test/builders/darwin.py
@@ -79,6 +79,14 @@ def getExtraMakeArgs(self):
 if configuration.dsymutil:
 args['DSYMUTIL'] = configuration.dsymutil
 
+if 'internal' in configuration.apple_sdk:
+sdk_root = lldbutil.get_xcode_sdk_root(configuration.apple_sdk)
+if sdk_root:
+private_frameworks = os.path.join(sdk_root, 'System',
+  'Library',
+  'PrivateFrameworks')
+args['FRAMEWORK_INCLUDES'] = '-F{}'.format(private_frameworks)
+
 operating_system, env = get_os_and_env()
 if operating_system and operating_system != "macosx":
 builder_dir = os.path.dirname(os.path.abspath(__file__))



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

[Lldb-commits] [lldb] 4fa9e43 - [lldb] Fix indentation in builders/darwin.py

2021-11-30 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2021-11-30T11:28:52-08:00
New Revision: 4fa9e435209e6067b9d5921647429053ae7a8e0c

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

LOG: [lldb] Fix indentation in builders/darwin.py

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/builders/darwin.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/builders/darwin.py 
b/lldb/packages/Python/lldbsuite/test/builders/darwin.py
index acde678ba87a..33a925c2d73a 100644
--- a/lldb/packages/Python/lldbsuite/test/builders/darwin.py
+++ b/lldb/packages/Python/lldbsuite/test/builders/darwin.py
@@ -92,9 +92,9 @@ def getExtraMakeArgs(self):
 builder_dir = os.path.dirname(os.path.abspath(__file__))
 test_dir = os.path.dirname(builder_dir)
 if env == "simulator":
-  entitlements_file = 'entitlements-simulator.plist'
+entitlements_file = 'entitlements-simulator.plist'
 else:
-  entitlements_file = 'entitlements.plist'
+entitlements_file = 'entitlements.plist'
 entitlements = os.path.join(test_dir, 'make', entitlements_file)
 args['CODESIGN'] = 'codesign --entitlements {}'.format(
 entitlements)



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

[Lldb-commits] [PATCH] D114742: [lldb] Search PrivateFrameworks when using an internal SDK

2021-11-30 Thread Jonas Devlieghere via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0a302f666737: [lldb] Search PrivateFrameworks when using an 
internal SDK (authored by JDevlieghere).

Changed prior to commit:
  https://reviews.llvm.org/D114742?vs=390473&id=390763#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114742

Files:
  lldb/packages/Python/lldbsuite/test/builders/darwin.py


Index: lldb/packages/Python/lldbsuite/test/builders/darwin.py
===
--- lldb/packages/Python/lldbsuite/test/builders/darwin.py
+++ lldb/packages/Python/lldbsuite/test/builders/darwin.py
@@ -79,6 +79,14 @@
 if configuration.dsymutil:
 args['DSYMUTIL'] = configuration.dsymutil
 
+if 'internal' in configuration.apple_sdk:
+sdk_root = lldbutil.get_xcode_sdk_root(configuration.apple_sdk)
+if sdk_root:
+private_frameworks = os.path.join(sdk_root, 'System',
+  'Library',
+  'PrivateFrameworks')
+args['FRAMEWORK_INCLUDES'] = '-F{}'.format(private_frameworks)
+
 operating_system, env = get_os_and_env()
 if operating_system and operating_system != "macosx":
 builder_dir = os.path.dirname(os.path.abspath(__file__))


Index: lldb/packages/Python/lldbsuite/test/builders/darwin.py
===
--- lldb/packages/Python/lldbsuite/test/builders/darwin.py
+++ lldb/packages/Python/lldbsuite/test/builders/darwin.py
@@ -79,6 +79,14 @@
 if configuration.dsymutil:
 args['DSYMUTIL'] = configuration.dsymutil
 
+if 'internal' in configuration.apple_sdk:
+sdk_root = lldbutil.get_xcode_sdk_root(configuration.apple_sdk)
+if sdk_root:
+private_frameworks = os.path.join(sdk_root, 'System',
+  'Library',
+  'PrivateFrameworks')
+args['FRAMEWORK_INCLUDES'] = '-F{}'.format(private_frameworks)
+
 operating_system, env = get_os_and_env()
 if operating_system and operating_system != "macosx":
 builder_dir = os.path.dirname(os.path.abspath(__file__))
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D114742: [lldb] Search PrivateFrameworks when using an internal SDK

2021-11-30 Thread Dave Lee via Phabricator via lldb-commits

kastiglione added inline comments.



Comment at: lldb/packages/Python/lldbsuite/test/builders/darwin.py:86
+private_frameworks = os.path.join(sdk_root, 
'System','Library','PrivateFrameworks')
+args['FRAMEWORK_INCLUDES'] = '-F{}'.format(private_frameworks)
+

JDevlieghere wrote:
> aprantl wrote:
> > Just curious: why not 
> > ```
> >  args['FRAMEWORK_INCLUDES'] = '-F' + private_frameworks
> > ```
> > ?
> Mostly for consistency with the rest of the file, but also personal 
> preference. It's no coincidence that the rest of the file uses `format` too. 
> I like f-strings even more, but they're Python 3.something only. 
Maybe we can use format strings now? LLVM requires python >= 3.6, does that 
mean lldb does too? https://llvm.org/docs/GettingStarted.html#software


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114742

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

[Lldb-commits] [lldb] c471359 - [lldb] Fix TypeError: argument of type 'NoneType' is not iterable

2021-11-30 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2021-11-30T12:41:45-08:00
New Revision: c47135949779904542ee1e6d5486609ebf0da1b5

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

LOG: [lldb] Fix TypeError: argument of type 'NoneType' is not iterable

Check if we have an apple_sdk before checking if it contains "internal".

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/builders/darwin.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/builders/darwin.py 
b/lldb/packages/Python/lldbsuite/test/builders/darwin.py
index 33a925c2d73a..fd97ef88d927 100644
--- a/lldb/packages/Python/lldbsuite/test/builders/darwin.py
+++ b/lldb/packages/Python/lldbsuite/test/builders/darwin.py
@@ -79,7 +79,7 @@ def getExtraMakeArgs(self):
 if configuration.dsymutil:
 args['DSYMUTIL'] = configuration.dsymutil
 
-if 'internal' in configuration.apple_sdk:
+if configuration.apple_sdk and 'internal' in configuration.apple_sdk:
 sdk_root = lldbutil.get_xcode_sdk_root(configuration.apple_sdk)
 if sdk_root:
 private_frameworks = os.path.join(sdk_root, 'System',



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

[Lldb-commits] [lldb] 5f2e8f5 - [lldb] Mark TestTsanBasic and TestUbsanBasic as "no debug info" tests

2021-11-30 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2021-11-30T12:40:50-08:00
New Revision: 5f2e8f579697492fab572796f99bc713f349e6fe

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

LOG: [lldb] Mark TestTsanBasic and TestUbsanBasic as "no debug info" tests

Speed up testing by not rerunning the test for all debug info variants.

Added: 


Modified: 
lldb/test/API/functionalities/tsan/basic/TestTsanBasic.py
lldb/test/API/functionalities/ubsan/basic/TestUbsanBasic.py

Removed: 




diff  --git a/lldb/test/API/functionalities/tsan/basic/TestTsanBasic.py 
b/lldb/test/API/functionalities/tsan/basic/TestTsanBasic.py
index e783ae0e8d893..17684050572ba 100644
--- a/lldb/test/API/functionalities/tsan/basic/TestTsanBasic.py
+++ b/lldb/test/API/functionalities/tsan/basic/TestTsanBasic.py
@@ -20,6 +20,7 @@ class TsanBasicTestCase(TestBase):
 @skipIfFreeBSD  # llvm.org/pr21136 runtimes not yet available by default
 @skipIfRemote
 @skipUnlessThreadSanitizer
+@no_debug_info_test
 def test(self):
 self.build()
 self.tsan_tests()

diff  --git a/lldb/test/API/functionalities/ubsan/basic/TestUbsanBasic.py 
b/lldb/test/API/functionalities/ubsan/basic/TestUbsanBasic.py
index 23f1296bf12e7..038b37b57d568 100644
--- a/lldb/test/API/functionalities/ubsan/basic/TestUbsanBasic.py
+++ b/lldb/test/API/functionalities/ubsan/basic/TestUbsanBasic.py
@@ -15,6 +15,7 @@ class UbsanBasicTestCase(TestBase):
 mydir = TestBase.compute_mydir(__file__)
 
 @skipUnlessUndefinedBehaviorSanitizer
+@no_debug_info_test
 def test(self):
 self.build()
 self.ubsan_tests()



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

[Lldb-commits] [lldb] d1326a3 - [lldb] Fix broken skipUnlessUndefinedBehaviorSanitizer decorator

2021-11-30 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2021-11-30T13:03:33-08:00
New Revision: d1326a3b10054dd89be46f51f857d009cf8ae14f

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

LOG: [lldb] Fix broken skipUnlessUndefinedBehaviorSanitizer decorator

727bd89b605b broke the UBSan decorator. The decorator compiles a custom
source code snippet that exposes UB and verifies the presence of a UBSan
symbol in the generated binary. The aforementioned commit broke both by
compiling a snippet without UB and discarding the result.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/decorators.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/decorators.py 
b/lldb/packages/Python/lldbsuite/test/decorators.py
index 972a98a13079..b6ef3f08df22 100644
--- a/lldb/packages/Python/lldbsuite/test/decorators.py
+++ b/lldb/packages/Python/lldbsuite/test/decorators.py
@@ -87,13 +87,16 @@ def _match_decorator_property(expected, actual):
 return expected == actual
 
 
-def _compiler_supports(compiler, flag):
+def _compiler_supports(compiler,
+   flag,
+   source='int main() {}',
+   output_file=tempfile.NamedTemporaryFile()):
 """Test whether the compiler supports the given flag."""
 if platform.system() == 'Darwin':
 compiler = "xcrun " + compiler
-f = tempfile.NamedTemporaryFile()
 try:
-cmd = "echo 'int main() {}' | %s %s -x c -o %s -" % (compiler, flag, 
f.name)
+cmd = "echo '%s' | %s %s -x c -o %s -" % (source, compiler, flag,
+  output_file.name)
 subprocess.check_call(cmd, shell=True)
 except subprocess.CalledProcessError:
 return False
@@ -755,16 +758,13 @@ def is_compiler_clang_with_ubsan(self):
 if is_running_under_asan():
 return "Undefined behavior sanitizer tests are disabled when 
runing under ASAN"
 
-# Write out a temp file which exhibits UB.
-inputf = tempfile.NamedTemporaryFile(suffix='.c', mode='w')
-inputf.write('int main() { int x = 0; return x / x; }\n')
-inputf.flush()
-
 # We need to write out the object into a named temp file for 
inspection.
 outputf = tempfile.NamedTemporaryFile()
 
 # Try to compile with ubsan turned on.
-if not _compiler_supports(self.getCompiler(), '-fsanitize=undefined'):
+if not _compiler_supports(self.getCompiler(), '-fsanitize=undefined',
+  'int main() { int x = 0; return x / x; }',
+  outputf):
 return "Compiler cannot compile with -fsanitize=undefined"
 
 # Check that we actually see ubsan instrumentation in the binary.



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

[Lldb-commits] [PATCH] D114819: [lldb] Split TestCxxChar8_t

2021-11-30 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere created this revision.
JDevlieghere added reviewers: labath, jingham.
JDevlieghere requested review of this revision.

Split TestCxxChar8_t into two parts: one that check reading variables without a 
process and another part with. This allows us to skip the former on Apple 
Silicon, where lack of support for chained fix-ups causes the test to fail.


https://reviews.llvm.org/D114819

Files:
  lldb/test/API/lang/cpp/char8_t/TestCxxChar8_t.py


Index: lldb/test/API/lang/cpp/char8_t/TestCxxChar8_t.py
===
--- lldb/test/API/lang/cpp/char8_t/TestCxxChar8_t.py
+++ lldb/test/API/lang/cpp/char8_t/TestCxxChar8_t.py
@@ -15,13 +15,12 @@
 mydir = TestBase.compute_mydir(__file__)
 
 @skipIf(compiler="clang", compiler_version=['<', '7.0'])
-def test(self):
-"""Test that C++ supports char8_t correctly."""
+@expectedFailureDarwin(archs=["arm64", "arm64e"]) # 

+def test_without_process(self):
+"""Test that C++ supports char8_t without a running process."""
 self.build()
-
 lldbutil.run_to_breakpoint_make_target(self)
 
-# Make sure the variables can be printed without a running process.
 self.expect("target variable a", substrs=["char8_t", "0x61 u8'a'"])
 self.expect("target variable ab",
 substrs=["const char8_t *", 'u8"你好"'])
@@ -29,9 +28,17 @@
 
 self.expect_expr("a", result_type="char8_t", result_summary="0x61 
u8'a'")
 self.expect_expr("ab", result_type="const char8_t *", 
result_summary='u8"你好"')
+
 # FIXME: This should work too.
 self.expect("expr abc", substrs=['u8"你好"'], matching=False)
 
+
+@skipIf(compiler="clang", compiler_version=['<', '7.0'])
+def test_with_process(self):
+"""Test that C++ supports char8_t with a running process."""
+self.build()
+lldbutil.run_to_breakpoint_make_target(self)
+
 lldbutil.run_break_set_by_source_regexp(self, "// break here", "-f 
main.cpp")
 self.runCmd("run")
 


Index: lldb/test/API/lang/cpp/char8_t/TestCxxChar8_t.py
===
--- lldb/test/API/lang/cpp/char8_t/TestCxxChar8_t.py
+++ lldb/test/API/lang/cpp/char8_t/TestCxxChar8_t.py
@@ -15,13 +15,12 @@
 mydir = TestBase.compute_mydir(__file__)
 
 @skipIf(compiler="clang", compiler_version=['<', '7.0'])
-def test(self):
-"""Test that C++ supports char8_t correctly."""
+@expectedFailureDarwin(archs=["arm64", "arm64e"]) # 
+def test_without_process(self):
+"""Test that C++ supports char8_t without a running process."""
 self.build()
-
 lldbutil.run_to_breakpoint_make_target(self)
 
-# Make sure the variables can be printed without a running process.
 self.expect("target variable a", substrs=["char8_t", "0x61 u8'a'"])
 self.expect("target variable ab",
 substrs=["const char8_t *", 'u8"ä½ å¥½"'])
@@ -29,9 +28,17 @@
 
 self.expect_expr("a", result_type="char8_t", result_summary="0x61 u8'a'")
 self.expect_expr("ab", result_type="const char8_t *", result_summary='u8"ä½ å¥½"')
+
 # FIXME: This should work too.
 self.expect("expr abc", substrs=['u8"ä½ å¥½"'], matching=False)
 
+
+@skipIf(compiler="clang", compiler_version=['<', '7.0'])
+def test_with_process(self):
+"""Test that C++ supports char8_t with a running process."""
+self.build()
+lldbutil.run_to_breakpoint_make_target(self)
+
 lldbutil.run_break_set_by_source_regexp(self, "// break here", "-f main.cpp")
 self.runCmd("run")
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D114819: [lldb] Split TestCxxChar8_t

2021-11-30 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added inline comments.



Comment at: lldb/test/API/lang/cpp/char8_t/TestCxxChar8_t.py:22
 self.build()
-
 lldbutil.run_to_breakpoint_make_target(self)
 

The name of this helper method is terribly confusing. I had to look at the 
source to realize it is meant to specify that it's the `make_target` helper 
function of `run_to_breakpoint`. Something for a subsequent patch.


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

https://reviews.llvm.org/D114819

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

[Lldb-commits] [lldb] 7e6df41 - [NFC] Refactor symbol table parsing.

2021-11-30 Thread Greg Clayton via lldb-commits


Author: Greg Clayton
Date: 2021-11-30T13:54:32-08:00
New Revision: 7e6df41f655e51c984b92bbd9c19a6fb851f35d4

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

LOG: [NFC] Refactor symbol table parsing.

Symbol table parsing has evolved over the years and many plug-ins contained 
duplicate code in the ObjectFile::GetSymtab() that used to be pure virtual. 
With this change, the "Symbtab *ObjectFile::GetSymtab()" is no longer virtual 
and will end up calling a new "void ObjectFile::ParseSymtab(Symtab &symtab)" 
pure virtual function to actually do the parsing. This helps centralize the 
code for parsing the symbol table and allows the ObjectFile base class to do 
all of the common work, like taking the necessary locks and creating the symbol 
table object itself. Plug-ins now just need to parse when they are asked to 
parse as the ParseSymtab function will only get called once.

This is a retry of the original patch https://reviews.llvm.org/D113965 which 
was reverted. There was a deadlock in the Manual DWARF indexing code during 
symbol preloading where the module was asked on the main thread to preload its 
symbols, and this would in turn cause the DWARF manual indexing to use a thread 
pool to index all of the compile units, and if there were relocations on the 
debug information sections, these threads could ask the ObjectFile to load 
section contents, which could cause a call to ObjectFileELF::RelocateSection() 
which would ask for the symbol table from the module and it would deadlock. We 
can't lock the module in ObjectFile::GetSymtab(), so the solution I am using is 
to use a llvm::once_flag to create the symbol table object once and then lock 
the Symtab object. Since all APIs on the symbol table use this lock, this will 
prevent anyone from using the symbol table before it is parsed and finalized 
and will avoid the deadlock I mentioned. ObjectFileELF::GetSymtab() was never 
locking the module lock before and would put off creating the symbol table 
until somewhere inside ObjectFileELF::GetSymtab(). Now we create it one time 
inside of the ObjectFile::GetSymtab() and immediately lock it which should be 
safe enough. This avoids the deadlocks and still provides safety.

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

Added: 


Modified: 
lldb/include/lldb/Symbol/ObjectFile.h
lldb/include/lldb/Symbol/Symtab.h
lldb/source/Core/Module.cpp
lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h
lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
lldb/source/Plugins/ObjectFile/JIT/ObjectFileJIT.cpp
lldb/source/Plugins/ObjectFile/JIT/ObjectFileJIT.h
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.h
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
lldb/source/Symbol/ObjectFile.cpp
lldb/source/Symbol/Symtab.cpp

Removed: 




diff  --git a/lldb/include/lldb/Symbol/ObjectFile.h 
b/lldb/include/lldb/Symbol/ObjectFile.h
index 4ccd7f92064d5..0a8b38b2c6428 100644
--- a/lldb/include/lldb/Symbol/ObjectFile.h
+++ b/lldb/include/lldb/Symbol/ObjectFile.h
@@ -19,6 +19,7 @@
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/UUID.h"
 #include "lldb/lldb-private.h"
+#include "llvm/Support/Threading.h"
 #include "llvm/Support/VersionTuple.h"
 
 namespace lldb_private {
@@ -322,12 +323,26 @@ class ObjectFile : public 
std::enable_shared_from_this,
   /// Gets the symbol table for the currently selected architecture (and
   /// object for archives).
   ///
-  /// Symbol table parsing can be deferred by ObjectFile instances until this
-  /// accessor is called the first time.
+  /// This function will manage when ParseSymtab(...) is called to actually do
+  /// the symbol table parsing in each plug-in. This function will take care of
+  /// taking all the necessary locks and finalizing the symbol table when the
+  /// symbol table does get parsed.
   ///
   /// \return
   /// The symbol table for this object file.
-  virtual Symtab *GetSymtab() = 0;
+  Symtab *GetSymtab();
+
+  /// Parse the symbol table into the provides symbol table object.
+  ///
+  /// Symbol tab

[Lldb-commits] [PATCH] D114288: [NFC] Refactor symbol table parsing.

2021-11-30 Thread Greg Clayton via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7e6df41f655e: [NFC] Refactor symbol table parsing. (authored 
by clayborg).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114288

Files:
  lldb/include/lldb/Symbol/ObjectFile.h
  lldb/include/lldb/Symbol/Symtab.h
  lldb/source/Core/Module.cpp
  lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
  lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
  lldb/source/Plugins/ObjectFile/JIT/ObjectFileJIT.cpp
  lldb/source/Plugins/ObjectFile/JIT/ObjectFileJIT.h
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
  lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.h
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
  lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Symbol/ObjectFile.cpp
  lldb/source/Symbol/Symtab.cpp

Index: lldb/source/Symbol/Symtab.cpp
===
--- lldb/source/Symbol/Symtab.cpp
+++ lldb/source/Symbol/Symtab.cpp
@@ -997,10 +997,15 @@
   }
 }
 
-void Symtab::CalculateSymbolSizes() {
+void Symtab::Finalize() {
   std::lock_guard guard(m_mutex);
-  // Size computation happens inside InitAddressIndexes.
+  // Calculate the size of symbols inside InitAddressIndexes.
   InitAddressIndexes();
+  // Shrink to fit the symbols so we don't waste memory
+  if (m_symbols.capacity() > m_symbols.size()) {
+collection new_symbols(m_symbols.begin(), m_symbols.end());
+m_symbols.swap(new_symbols);
+  }
 }
 
 Symbol *Symtab::FindSymbolAtFileAddress(addr_t file_addr) {
Index: lldb/source/Symbol/ObjectFile.cpp
===
--- lldb/source/Symbol/ObjectFile.cpp
+++ lldb/source/Symbol/ObjectFile.cpp
@@ -244,7 +244,7 @@
   m_type(eTypeInvalid), m_strata(eStrataInvalid),
   m_file_offset(file_offset), m_length(length), m_data(), m_process_wp(),
   m_memory_addr(LLDB_INVALID_ADDRESS), m_sections_up(), m_symtab_up(),
-  m_synthetic_symbol_idx(0) {
+  m_symtab_once_up(new llvm::once_flag()) {
   if (file_spec_ptr)
 m_file = *file_spec_ptr;
   if (data_sp)
@@ -265,7 +265,7 @@
 : ModuleChild(module_sp), m_file(), m_type(eTypeInvalid),
   m_strata(eStrataInvalid), m_file_offset(0), m_length(0), m_data(),
   m_process_wp(process_sp), m_memory_addr(header_addr), m_sections_up(),
-  m_symtab_up(), m_synthetic_symbol_idx(0) {
+  m_symtab_up(), m_symtab_once_up(new llvm::once_flag()) {
   if (header_data_sp)
 m_data.SetData(header_data_sp, 0, header_data_sp->GetByteSize());
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_OBJECT));
@@ -571,11 +571,13 @@
 void ObjectFile::ClearSymtab() {
   ModuleSP module_sp(GetModule());
   if (module_sp) {
-std::lock_guard guard(module_sp->GetMutex());
 Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_OBJECT));
 LLDB_LOGF(log, "%p ObjectFile::ClearSymtab () symtab = %p",
   static_cast(this),
   static_cast(m_symtab_up.get()));
+// Since we need to clear the symbol table, we need a new llvm::once_flag
+// instance so we can safely create another symbol table
+m_symtab_once_up.reset(new llvm::once_flag());
 m_symtab_up.reset();
   }
 }
@@ -715,3 +717,33 @@
 break;
   }
 }
+
+
+Symtab *ObjectFile::GetSymtab() {
+  ModuleSP module_sp(GetModule());
+  if (module_sp) {
+// We can't take the module lock in ObjectFile::GetSymtab() or we can
+// deadlock in DWARF indexing when any file asks for the symbol table from
+// an object file. This currently happens in the preloading of symbols in
+// SymbolFileDWARF::PreloadSymbols() because the main thread will take the
+// module lock, and then threads will be spun up to index the DWARF and
+// any of those threads might end up trying to relocate items in the DWARF
+// sections which causes ObjectFile::GetSectionData(...) to relocate section
+// data which requires the symbol table.
+//
+// So to work around this, we create the symbol table one time using
+// llvm::once_flag, lock it, and then set the unique pointer. Any other
+// thread that gets ahold of the symbol table before parsing is done, will
+// not be able to access the symbol table contents since all APIs in Symtab
+// are protected by a mutex in the Sym

[Lldb-commits] [PATCH] D114668: [lldb][NFC] Move generic DWARFASTParser code out of Clang-specific code

2021-11-30 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.cpp:77
+  default:
+  case DW_AT_abstract_origin:
+  case DW_AT_accessibility:

shafik wrote:
> ljmf00 wrote:
> > bulbazord wrote:
> > > ljmf00 wrote:
> > > > Why we are including just these specific attributes? Maybe we should 
> > > > add a comment explaining it. According to the DWARF standard, any 
> > > > attribute is eligible for any tag.
> > > I'm not sure why. Possibly they were added to make sure the switch was 
> > > fully covered (potentially to silence a warning)? You could add a `FIXME` 
> > > or a `TODO` if you feel that these attributes should have functionality 
> > > associated with them like the ones above.
> > I don't think it is to mark it as fully covered since there are much more 
> > attributes, the default label will address it anyway, and according to the 
> > DWARF standard, any attribute can be in a type tag (realistically, any 
> > tag). We can take the example of `DW_AT_description` which is just a 
> > description associated with the symbol. I feel like this can be safely 
> > deleted but I'm afraid to do it in favour of some other rationale I'm not 
> > seeing.
> @clayborg it looks like this has been this way since you put this in: 
> https://github.com/llvm/llvm-project/commit/261ac3f4b5b98d02dd8718078015a92cf07df736
> 
> Do you agree this is dead code or is there something we are missing?
Fine to remove these extra attributes that are in the default case as we 
definitely don't have all of the attributes listed here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114668

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

[Lldb-commits] [PATCH] D114746: [lldb] Generalize ParsedDWARFTypeAttributes

2021-11-30 Thread Shafik Yaghmour via Phabricator via lldb-commits

shafik added a comment.

I also echo Pavel's question, why not in `DWARFASTParser`?




Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1140
   : containing_decl_ctx,
-GetOwningClangModule(die), name, clang_type, attrs.storage,
-attrs.is_inline);
+GetOwningClangModule(die), name, clang_type, attrs.is_external() ? 
clang::SC_Extern : clang::SC_None,
+attrs.is_inline());

Is there a reason not to use an attribute `storage` like before? This feels 
like we are leaking out of the abstraction where before we were not.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1493
 // and the byte size is zero.
-attrs.is_forward_declaration = true;
+attrs.attr_flags |= eDWARFAttributeIsForwardDecl;
   }

if we are going to have getter abstraction why not have a 
`setIsForwardDeclaration()` or something similar.


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

https://reviews.llvm.org/D114746

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

[Lldb-commits] [PATCH] D113930: [LLDB][NativePDB] Fix function decl creation for class methods

2021-11-30 Thread Zequan Wu via Phabricator via lldb-commits

zequanwu updated this revision to Diff 390845.
zequanwu added a comment.

Handle OverloadedMethodRecord.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113930

Files:
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
  lldb/test/Shell/SymbolFile/NativePDB/find-functions.cpp

Index: lldb/test/Shell/SymbolFile/NativePDB/find-functions.cpp
===
--- lldb/test/Shell/SymbolFile/NativePDB/find-functions.cpp
+++ lldb/test/Shell/SymbolFile/NativePDB/find-functions.cpp
@@ -1,7 +1,7 @@
 // clang-format off
 // REQUIRES: lld, x86
 
-// RUN: %clang_cl --target=x86_64-windows-msvc -Od -Z7 -c /Fo%t.obj -- %s
+// RUN: %clang_cl --target=x86_64-windows-msvc -Od -Z7 -c /GR- /Fo%t.obj -- %s
 // RUN: lld-link -debug:full -nodefaultlib -entry:main %t.obj -out:%t.exe -pdb:%t.pdb
 
 // RUN: lldb-test symbols --find=function --name=main --function-flags=full %t.exe \
@@ -13,6 +13,46 @@
 // RUN: lldb-test symbols --find=function --name=varargs_fn --function-flags=full %t.exe \
 // RUN: | FileCheck %s --check-prefix=FIND-VAR
 
+// RUN: lldb-test symbols --find=function --name=Struct::simple_method --function-flags=full %t.exe \
+// RUN: | FileCheck %s --check-prefix=FIND-SIMPLE
+
+// RUN: lldb-test symbols --find=function --name=Struct::virtual_method --function-flags=full %t.exe \
+// RUN: | FileCheck %s --check-prefix=FIND-VIRTUAL
+
+// RUN: lldb-test symbols --find=function --name=Struct::static_method --function-flags=full %t.exe \
+// RUN: | FileCheck %s --check-prefix=FIND-STATIC-METHOD
+
+// RUN: lldb-test symbols --find=function --name=Struct::overloaded_method --function-flags=full %t.exe \
+// RUN: | FileCheck %s --check-prefix=FIND-OVERLOAD
+
+struct Struct {
+  int simple_method() {
+return 1;
+  }
+
+  virtual int virtual_method() {
+return 2;
+  }
+
+  static int static_method() {
+return 3;
+  }
+
+  int overloaded_method() {
+return 4 + overloaded_method('a') + overloaded_method('a', 1);
+  }
+protected:
+  virtual int overloaded_method(char c) {
+return 5;
+  }
+private:
+  static int overloaded_method(char c, int i, ...) {
+return 6;
+  }
+};
+
+Struct s;
+
 static int static_fn() {
   return 42;
 }
@@ -22,7 +62,8 @@
 }
 
 int main(int argc, char **argv) {
-  return static_fn() + varargs_fn(argc, argc);
+  return static_fn() + varargs_fn(argc, argc) + s.simple_method() +
+  Struct::static_method() + s.virtual_method() + s.overloaded_method();
 }
 
 // FIND-MAIN:  Function: id = {{.*}}, name = "main"
@@ -33,3 +74,17 @@
 
 // FIND-VAR:  Function: id = {{.*}}, name = "{{.*}}varargs_fn{{.*}}"
 // FIND-VAR-NEXT: FuncType: id = {{.*}}, compiler_type = "int (int, int, ...)"
+
+// FIND-SIMPLE:  Function: id = {{.*}}, name = "{{.*}}Struct::simple_method{{.*}}"
+// FIND-SIMPLE-NEXT: FuncType: id = {{.*}}, compiler_type = "int (void)"
+
+// FIND-VIRTUAL:  Function: id = {{.*}}, name = "{{.*}}Struct::virtual_method{{.*}}"
+// FIND-VIRTUAL-NEXT: FuncType: id = {{.*}}, compiler_type = "int (void)"
+
+// FIND-STATIC-METHOD:  Function: id = {{.*}}, name = "{{.*}}Struct::static_method{{.*}}"
+// FIND-STATIC-METHOD-NEXT: FuncType: id = {{.*}}, compiler_type = "int (void)"
+
+// FIND-OVERLOAD: Function: id = {{.*}}, name = "{{.*}}Struct::overloaded_method{{.*}}"
+// FIND-OVERLOAD: FuncType: id = {{.*}}, compiler_type = "int (void)"
+// FIND-OVERLOAD: FuncType: id = {{.*}}, compiler_type = "int (char)"
+// FIND-OVERLOAD: FuncType: id = {{.*}}, compiler_type = "int (char, int, ...)"
Index: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
===
--- lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
+++ lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
@@ -1014,8 +1014,112 @@
   proc_name.consume_front(context_name);
   proc_name.consume_front("::");
 
-  clang::FunctionDecl *function_decl = m_clang.CreateFunctionDeclaration(
-  parent, OptionalClangModuleID(), proc_name, func_ct, storage, false);
+  clang::FunctionDecl *function_decl = nullptr;
+  if (parent->isRecord()) {
+struct ProcessOneMethodRecord : public TypeVisitorCallbacks {
+  ProcessOneMethodRecord(PdbIndex &m_index, TypeSystemClang &m_clang,
+ clang::FunctionDecl *&function_decl,
+ lldb::opaque_compiler_type_t parent_ty,
+ llvm::StringRef proc_name, CompilerType func_ct)
+  : m_index(m_index), m_clang(m_clang), function_decl(function_decl),
+parent_ty(parent_ty), proc_name(proc_name), func_ct(func_ct) {}
+  PdbIndex &m_index;
+  TypeSystemClang &m_clang;
+  clang::FunctionDecl *&function_decl;
+  lldb::opaque_compiler_type_t parent_ty;
+  llvm::StringRef proc_name;
+  CompilerType func_ct;
+
+  llvm::Error
+  visitKnown

[Lldb-commits] [PATCH] D113930: [LLDB][NativePDB] Fix function decl creation for class methods

2021-11-30 Thread Zequan Wu via Phabricator via lldb-commits

zequanwu updated this revision to Diff 390847.
zequanwu added a comment.

use lower case for parameters.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113930

Files:
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
  lldb/test/Shell/SymbolFile/NativePDB/find-functions.cpp

Index: lldb/test/Shell/SymbolFile/NativePDB/find-functions.cpp
===
--- lldb/test/Shell/SymbolFile/NativePDB/find-functions.cpp
+++ lldb/test/Shell/SymbolFile/NativePDB/find-functions.cpp
@@ -1,7 +1,7 @@
 // clang-format off
 // REQUIRES: lld, x86
 
-// RUN: %clang_cl --target=x86_64-windows-msvc -Od -Z7 -c /Fo%t.obj -- %s
+// RUN: %clang_cl --target=x86_64-windows-msvc -Od -Z7 -c /GR- /Fo%t.obj -- %s
 // RUN: lld-link -debug:full -nodefaultlib -entry:main %t.obj -out:%t.exe -pdb:%t.pdb
 
 // RUN: lldb-test symbols --find=function --name=main --function-flags=full %t.exe \
@@ -13,6 +13,46 @@
 // RUN: lldb-test symbols --find=function --name=varargs_fn --function-flags=full %t.exe \
 // RUN: | FileCheck %s --check-prefix=FIND-VAR
 
+// RUN: lldb-test symbols --find=function --name=Struct::simple_method --function-flags=full %t.exe \
+// RUN: | FileCheck %s --check-prefix=FIND-SIMPLE
+
+// RUN: lldb-test symbols --find=function --name=Struct::virtual_method --function-flags=full %t.exe \
+// RUN: | FileCheck %s --check-prefix=FIND-VIRTUAL
+
+// RUN: lldb-test symbols --find=function --name=Struct::static_method --function-flags=full %t.exe \
+// RUN: | FileCheck %s --check-prefix=FIND-STATIC-METHOD
+
+// RUN: lldb-test symbols --find=function --name=Struct::overloaded_method --function-flags=full %t.exe \
+// RUN: | FileCheck %s --check-prefix=FIND-OVERLOAD
+
+struct Struct {
+  int simple_method() {
+return 1;
+  }
+
+  virtual int virtual_method() {
+return 2;
+  }
+
+  static int static_method() {
+return 3;
+  }
+
+  int overloaded_method() {
+return 4 + overloaded_method('a') + overloaded_method('a', 1);
+  }
+protected:
+  virtual int overloaded_method(char c) {
+return 5;
+  }
+private:
+  static int overloaded_method(char c, int i, ...) {
+return 6;
+  }
+};
+
+Struct s;
+
 static int static_fn() {
   return 42;
 }
@@ -22,7 +62,8 @@
 }
 
 int main(int argc, char **argv) {
-  return static_fn() + varargs_fn(argc, argc);
+  return static_fn() + varargs_fn(argc, argc) + s.simple_method() +
+  Struct::static_method() + s.virtual_method() + s.overloaded_method();
 }
 
 // FIND-MAIN:  Function: id = {{.*}}, name = "main"
@@ -33,3 +74,17 @@
 
 // FIND-VAR:  Function: id = {{.*}}, name = "{{.*}}varargs_fn{{.*}}"
 // FIND-VAR-NEXT: FuncType: id = {{.*}}, compiler_type = "int (int, int, ...)"
+
+// FIND-SIMPLE:  Function: id = {{.*}}, name = "{{.*}}Struct::simple_method{{.*}}"
+// FIND-SIMPLE-NEXT: FuncType: id = {{.*}}, compiler_type = "int (void)"
+
+// FIND-VIRTUAL:  Function: id = {{.*}}, name = "{{.*}}Struct::virtual_method{{.*}}"
+// FIND-VIRTUAL-NEXT: FuncType: id = {{.*}}, compiler_type = "int (void)"
+
+// FIND-STATIC-METHOD:  Function: id = {{.*}}, name = "{{.*}}Struct::static_method{{.*}}"
+// FIND-STATIC-METHOD-NEXT: FuncType: id = {{.*}}, compiler_type = "int (void)"
+
+// FIND-OVERLOAD: Function: id = {{.*}}, name = "{{.*}}Struct::overloaded_method{{.*}}"
+// FIND-OVERLOAD: FuncType: id = {{.*}}, compiler_type = "int (void)"
+// FIND-OVERLOAD: FuncType: id = {{.*}}, compiler_type = "int (char)"
+// FIND-OVERLOAD: FuncType: id = {{.*}}, compiler_type = "int (char, int, ...)"
Index: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
===
--- lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
+++ lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
@@ -1014,8 +1014,112 @@
   proc_name.consume_front(context_name);
   proc_name.consume_front("::");
 
-  clang::FunctionDecl *function_decl = m_clang.CreateFunctionDeclaration(
-  parent, OptionalClangModuleID(), proc_name, func_ct, storage, false);
+  clang::FunctionDecl *function_decl = nullptr;
+  if (parent->isRecord()) {
+struct ProcessOneMethodRecord : public TypeVisitorCallbacks {
+  ProcessOneMethodRecord(PdbIndex &m_index, TypeSystemClang &m_clang,
+ clang::FunctionDecl *&function_decl,
+ lldb::opaque_compiler_type_t parent_ty,
+ llvm::StringRef proc_name, CompilerType func_ct)
+  : m_index(m_index), m_clang(m_clang), function_decl(function_decl),
+parent_ty(parent_ty), proc_name(proc_name), func_ct(func_ct) {}
+  PdbIndex &m_index;
+  TypeSystemClang &m_clang;
+  clang::FunctionDecl *&function_decl;
+  lldb::opaque_compiler_type_t parent_ty;
+  llvm::StringRef proc_name;
+  CompilerType func_ct;
+
+  llvm::Error
+  visitKnown

[Lldb-commits] [PATCH] D114819: [lldb] Split TestCxxChar8_t

2021-11-30 Thread Shafik Yaghmour via Phabricator via lldb-commits

shafik added a comment.

I recently ran into this, thanks for fixing the test.


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

https://reviews.llvm.org/D114819

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

[Lldb-commits] [lldb] 92a8dc0 - [lldb] Temporarily skip TestTsanBasic on Darwin

2021-11-30 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2021-11-30T20:11:26-08:00
New Revision: 92a8dc0735cfb3f296f0c487b20d8fa8474e3e40

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

LOG: [lldb] Temporarily skip TestTsanBasic on Darwin

See ongoing discussion in https://reviews.llvm.org/D112603.

Added: 


Modified: 
lldb/test/API/functionalities/tsan/basic/TestTsanBasic.py

Removed: 




diff  --git a/lldb/test/API/functionalities/tsan/basic/TestTsanBasic.py 
b/lldb/test/API/functionalities/tsan/basic/TestTsanBasic.py
index 17684050572ba..556551ead8e6c 100644
--- a/lldb/test/API/functionalities/tsan/basic/TestTsanBasic.py
+++ b/lldb/test/API/functionalities/tsan/basic/TestTsanBasic.py
@@ -20,6 +20,7 @@ class TsanBasicTestCase(TestBase):
 @skipIfFreeBSD  # llvm.org/pr21136 runtimes not yet available by default
 @skipIfRemote
 @skipUnlessThreadSanitizer
+@expectedFailureDarwin # FIXME: https://reviews.llvm.org/D112603
 @no_debug_info_test
 def test(self):
 self.build()



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

[Lldb-commits] [PATCH] D114627: [lldb] add new overload for SymbolFile::FindTypes that accepts a scope

2021-11-30 Thread Lasse Folger via Phabricator via lldb-commits

lassefolger added a comment.

In D114627#3162109 , @shafik wrote:

> Is there a specific use case that motivated this feature?

The motivation is performance. If there are many (>1000) types with the same 
base name this implementation performs much better than the current 
implementation.
This is mostly important for the first access (because later ones use the 
caches results). In our scenarios it reduced the time for the first access from 
86s to 6s and from 12s to 2s. And the access for subsequest accesses from 0.15 
to 0.08 and 0.12 to 0.06.

The primary reason is that Type::GetQualifiedName is executed for every type 
with the same basename when the types are filtered at the end. This triggers 
type completion each type which is very expensive.

I'm still working on this commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114627

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

38 matches

Mail list logo