[Lldb-commits] [PATCH] D139251: [lldb/Interpreter] Introduce ScriptedPlatform{, Python}Interface

[Med Ismail Bennani via Phabricator via lldb-commits]

mib created this revision.
mib added reviewers: JDevlieghere, bulbazord.
mib added a project: LLDB.
Herald added a project: All.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch introduces both the ScriptedPlatformInterface and the
ScriptedPlatformPythonInterface. As the name suggests, these calls will
be used to call into the Scripted Platform python implementation from
the C++ Scripted Platform plugin instance.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139251

Files:
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  lldb/include/lldb/Interpreter/ScriptedPlatformInterface.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/Interpreter/ScriptInterpreter.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPlatformPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPlatformPythonInterface.h

Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPlatformPythonInterface.h
===
--- /dev/null
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPlatformPythonInterface.h
@@ -0,0 +1,43 @@
+//===-- ScriptedPlatformPythonInterface.h *- C++
+//-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_SCRIPTEDPLATFORMPYTHONINTERFACE_H
+#define LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_SCRIPTEDPLATFORMPYTHONINTERFACE_H
+
+#include "lldb/Host/Config.h"
+
+#if LLDB_ENABLE_PYTHON
+
+#include "ScriptedPythonInterface.h"
+#include "lldb/Interpreter/ScriptedPlatformInterface.h"
+
+namespace lldb_private {
+class ScriptedPlatformPythonInterface : public ScriptedPlatformInterface,
+public ScriptedPythonInterface {
+public:
+  ScriptedPlatformPythonInterface(ScriptInterpreterPythonImpl );
+
+  StructuredData::GenericSP
+  CreatePluginObject(const llvm::StringRef class_name,
+ ExecutionContext _ctx,
+ StructuredData::DictionarySP args_sp,
+ StructuredData::Generic *script_obj = nullptr) override;
+
+  StructuredData::DictionarySP ListProcesses() override;
+
+  StructuredData::DictionarySP GetProcessInfo(lldb::pid_t) override;
+
+  Status LaunchProcess() override;
+
+  Status KillProcess(lldb::pid_t pid) override;
+};
+} // namespace lldb_private
+
+#endif // LLDB_ENABLE_PYTHON
+#endif // LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_SCRIPTEDPLATFORMPYTHONINTERFACE_H
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPlatformPythonInterface.cpp
===
--- /dev/null
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPlatformPythonInterface.cpp
@@ -0,0 +1,98 @@
+//===-- ScriptedPlatformPythonInterface.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
+//
+//===--===//
+
+#include "lldb/Host/Config.h"
+#include "lldb/Utility/Log.h"
+#include "lldb/Utility/Status.h"
+#include "lldb/lldb-enumerations.h"
+
+#if LLDB_ENABLE_PYTHON
+
+// LLDB Python header must be included first
+#include "lldb-python.h"
+
+#include "SWIGPythonBridge.h"
+#include "ScriptInterpreterPythonImpl.h"
+#include "ScriptedPlatformPythonInterface.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::python;
+using Locker = ScriptInterpreterPythonImpl::Locker;
+
+ScriptedPlatformPythonInterface::ScriptedPlatformPythonInterface(
+ScriptInterpreterPythonImpl )
+: ScriptedPlatformInterface(), ScriptedPythonInterface(interpreter) {}
+
+StructuredData::GenericSP ScriptedPlatformPythonInterface::CreatePluginObject(
+llvm::StringRef class_name, ExecutionContext _ctx,
+StructuredData::DictionarySP args_sp, StructuredData::Generic *script_obj) {
+  if (class_name.empty())
+return {};
+
+  StructuredDataImpl args_impl(args_sp);
+  std::string error_string;
+
+  Locker py_lock(_interpreter, Locker::AcquireLock | Locker::NoSTDIN,
+ Locker::FreeLock);
+
+  PythonObject ret_val = LLDBSwigPythonCreateScriptedPlatform(
+  class_name.str().c_str(), m_interpreter.GetDictionaryName(), args_impl,
+  error_string);
+
+  m_object_instance_sp =
+  StructuredData::GenericSP(new StructuredPythonObject(std::move(ret_val)));
+
+ 

[Lldb-commits] [PATCH] D139250: [lldb] Add ScriptedPlatform python implementation

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 479809.
mib added a comment.

Fix typo


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

https://reviews.llvm.org/D139250

Files:
  lldb/bindings/python/CMakeLists.txt
  lldb/bindings/python/python-wrapper.swig
  lldb/examples/python/scripted_process/scripted_platform.py
  lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
  lldb/test/API/functionalities/scripted_platform/my_scripted_platform.py
  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
@@ -213,6 +213,12 @@
   return python::PythonObject();
 }
 
+python::PythonObject lldb_private::LLDBSwigPythonCreateScriptedPlatform(
+const char *python_class_name, const char *session_dictionary_name,
+const StructuredDataImpl _impl, std::string _string) {
+  return python::PythonObject();
+}
+
 python::PythonObject lldb_private::LLDBSWIGPython_CreateFrameRecognizer(
 const char *python_class_name, const char *session_dictionary_name) {
   return python::PythonObject();
Index: lldb/test/API/functionalities/scripted_platform/my_scripted_platform.py
===
--- /dev/null
+++ lldb/test/API/functionalities/scripted_platform/my_scripted_platform.py
@@ -0,0 +1,38 @@
+import os
+
+import lldb
+from lldb.plugins.scripted_platform import ScriptedPlatform
+
+class MyScriptedPlatform(ScriptedPlatform):
+
+def __init__(self, args):
+self.processes = {}
+
+proc = {}
+proc['name'] = 'a.out'
+proc['arch'] = 'arm64-apple-macosx'
+proc['pid'] = 420
+proc['parent'] = 42
+proc['uid'] = 501
+proc['gid'] = 20
+self.processes[420] = proc
+
+def list_processes(self):
+return self.processes
+
+def get_process_info(self, pid):
+return self.processes[pid]
+
+def launch_process(self, launch_info):
+return lldb.SBError()
+
+def kill_process(self, pid):
+return lldb.SBError()
+
+def __lldb_init_module(debugger, dict):
+if not 'SKIP_SCRIPTED_PLATFORM_SELECT' in os.environ:
+debugger.HandleCommand(
+"platform select scripted-platform -C %s.%s" % (__name__, MyScriptedPlatform.__name__))
+else:
+print("Name of the class that will manage the scripted platform: '%s.%s'"
+% (__name__, MyScriptedPlatform.__name__))
Index: lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
@@ -105,6 +105,10 @@
 const lldb::ProcessSP _sp, const StructuredDataImpl _impl,
 std::string _string);
 
+python::PythonObject LLDBSwigPythonCreateScriptedPlatform(
+const char *python_class_name, const char *session_dictionary_name,
+const StructuredDataImpl _impl, std::string _string);
+
 llvm::Expected LLDBSwigPythonBreakpointCallbackFunction(
 const char *python_function_name, const char *session_dictionary_name,
 const lldb::StackFrameSP _frame,
Index: lldb/examples/python/scripted_process/scripted_platform.py
===
--- /dev/null
+++ lldb/examples/python/scripted_process/scripted_platform.py
@@ -0,0 +1,81 @@
+from abc import ABCMeta, abstractmethod
+
+import lldb
+
+class ScriptedPlatform(metaclass=ABCMeta):
+
+"""
+The base class for a scripted platform.
+
+Most of the base class methods are `@abstractmethod` that need to be
+overwritten by the inheriting class.
+
+DISCLAIMER: THIS INTERFACE IS STILL UNDER DEVELOPMENT AND NOT STABLE.
+THE METHODS EXPOSED MIGHT CHANGE IN THE FUTURE.
+"""
+
+processes = None
+
+@abstractmethod
+def __init__(self, args):
+""" Construct a scripted platform.
+
+Args:
+args (lldb.SBStructuredData): A Dictionary holding arbitrary
+key/value pairs used by the scripted platform.
+"""
+processes = []
+
+@abstractmethod
+def list_processes(self):
+""" Get a list of processes that can be ran on the platform.
+
+process_info = {
+name = a.out,
+arch = aarch64,
+pid = 420
+parent_pid = 42 (optional)
+uid = 0 (optional)
+gid = 0 (optional)
+}
+
+Returns:
+Dict: The processes represented as a dictionary, with at least the
+process ID, name, architecture. Optionally, the user can also
+provide the parent process ID and the user and group IDs.
+The 

[Lldb-commits] [PATCH] D139250: [lldb] Add ScriptedPlatform python implementation

[Med Ismail Bennani via Phabricator via lldb-commits]

mib created this revision.
mib added reviewers: JDevlieghere, bulbazord.
mib added a project: LLDB.
Herald added a project: All.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch introduces both the Scripted Platform python base
implementation and an example for it.

The base implementation is embedded in lldb python module under
`lldb.plugins.scripted_platform`.

This patch also adds the SWIG methods to instanciate the
ScriptedPlatform python object from C++

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139250

Files:
  lldb/bindings/python/CMakeLists.txt
  lldb/bindings/python/python-wrapper.swig
  lldb/examples/python/scripted_process/scripted_platform.py
  lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
  lldb/test/API/functionalities/scripted_platform/my_scripted_platform.py
  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
@@ -213,6 +213,12 @@
   return python::PythonObject();
 }
 
+python::PythonObject lldb_private::LLDBSwigPythonCreateScriptedPlatform(
+const char *python_class_name, const char *session_dictionary_name,
+const StructuredDataImpl _impl, std::string _string) {
+  return python::PythonObject();
+}
+
 python::PythonObject lldb_private::LLDBSWIGPython_CreateFrameRecognizer(
 const char *python_class_name, const char *session_dictionary_name) {
   return python::PythonObject();
Index: lldb/test/API/functionalities/scripted_platform/my_scripted_platform.py
===
--- /dev/null
+++ lldb/test/API/functionalities/scripted_platform/my_scripted_platform.py
@@ -0,0 +1,38 @@
+import os
+
+import lldb
+from lldb.plugins.scripted_platform import ScriptedPlatform
+
+class MyScriptedPlatform(ScriptedPlatform):
+
+def __init__(self, args):
+self.processes = {}
+
+proc = {}
+proc['name'] = 'a.out'
+proc['arch'] = 'arm64-apple-macosx'
+proc['pid'] = 420
+proc['parent'] = 42
+proc['uid'] = 501
+proc['gid'] = 20
+self.processes[420] = proc
+
+def list_processes(self):
+return self.processes
+
+def get_process_info(self, pid):
+return self.processes[pid]
+
+def launch_process(self, launch_info):
+return lldb.SBError()
+
+def kill_process(self, pid):
+return lldb.SBError()
+
+def __lldb_init_module(debugger, dict):
+if not 'SKIP_SCRIPTED_PLATFORM_SELECT' in os.environ:
+debugger.HandleCommand(
+"platform select scripted-platform -C %s.%s" % (__name__, MyScriptedPlatform.__name__))
+else:
+print("Name of the class that will manage the scripted platform: '%s.%s'"
+% (__name__, MyScriptedPlatform.__name__))
Index: lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
@@ -105,6 +105,10 @@
 const lldb::ProcessSP _sp, const StructuredDataImpl _impl,
 std::string _string);
 
+python::PythonObject LLDBSwigPythonCreateScriptedPlatform(
+const char *python_class_name, const char *session_dictionary_name,
+const StructuredDataImpl _impl, std::string _string);
+
 llvm::Expected LLDBSwigPythonBreakpointCallbackFunction(
 const char *python_function_name, const char *session_dictionary_name,
 const lldb::StackFrameSP _frame,
Index: lldb/examples/python/scripted_process/scripted_platform.py
===
--- /dev/null
+++ lldb/examples/python/scripted_process/scripted_platform.py
@@ -0,0 +1,83 @@
+from abc import ABCMeta, abstractmethod
+
+import lldb
+from lldb.plugins.scripted_process import ScriptedProcess
+from lldb.plugins.scripted_process import ScriptedThread
+
+class ScriptedPlatform(metaclass=ABCMeta):
+
+"""
+The base class for a scripted platform.
+
+Most of the base class methods are `@abstractmethod` that need to be
+overwritten by the inheriting class.
+
+DISCLAIMER: THIS INTERFACE IS STILL UNDER DEVELOPMENT AND NOT STABLE.
+THE METHODS EXPOSED MIGHT CHANGE IN THE FUTURE.
+"""
+
+processes = None
+
+@abstractmethod
+def __init__(self, args):
+""" Construct a scripted process.
+
+Args:
+args (lldb.SBStructuredData): A Dictionary holding arbitrary
+key/value pairs used by the scripted process.
+"""
+processes = []
+
+@abstractmethod
+def list_processes(self):
+""" Get 

[Lldb-commits] [PATCH] D139249: [lldb] Add Debugger & ScriptedMetadata reference to Platform::CreateInstance

[Med Ismail Bennani via Phabricator via lldb-commits]

mib created this revision.
mib added reviewers: JDevlieghere, jingham, labath.
mib added a project: LLDB.
Herald added a subscriber: emaste.
Herald added a project: All.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch is preparatory work for Scripted Platform support and does
multiple things:

First, it introduces new options for the `platform select` command and
`SBPlatform::Create` API, to hold a reference to the debugger object,
the name of the python script managing the Scripted Platform and a
structured data dictionary that the user can use to pass arbitrary data.

Then, it updates the various `Create` and `GetOrCreate` methods for
the `Platform` and `PlatformList` classes to pass down the new parameter
to the `Platform::CreateInstance` callbacks.

Finally, it updates every callback to reflect these changes.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139249

Files:
  lldb/bindings/interface/SBPlatform.i
  lldb/include/lldb/API/SBDebugger.h
  lldb/include/lldb/API/SBPlatform.h
  lldb/include/lldb/API/SBStructuredData.h
  lldb/include/lldb/Interpreter/OptionGroupPlatform.h
  lldb/include/lldb/Target/Platform.h
  lldb/include/lldb/lldb-private-interfaces.h
  lldb/source/API/SBDebugger.cpp
  lldb/source/API/SBPlatform.cpp
  lldb/source/Commands/CommandObjectPlatform.cpp
  lldb/source/Commands/CommandObjectPlatform.h
  lldb/source/Core/Debugger.cpp
  lldb/source/Interpreter/OptionGroupPlatform.cpp
  lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
  lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
  lldb/source/Plugins/Platform/Android/PlatformAndroid.h
  lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp
  lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.h
  lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
  lldb/source/Plugins/Platform/Linux/PlatformLinux.h
  lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.h
  lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.h
  lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleBridge.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleBridge.h
  lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleTV.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleTV.h
  lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleWatch.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleWatch.h
  lldb/source/Plugins/Platform/MacOSX/PlatformRemoteMacOSX.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformRemoteMacOSX.h
  lldb/source/Plugins/Platform/MacOSX/PlatformRemoteiOS.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformRemoteiOS.h
  lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
  lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.h
  lldb/source/Plugins/Platform/OpenBSD/PlatformOpenBSD.cpp
  lldb/source/Plugins/Platform/OpenBSD/PlatformOpenBSD.h
  lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
  lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
  lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.h
  lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
  lldb/source/Plugins/Platform/Windows/PlatformWindows.h
  lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
  lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h
  lldb/source/Target/Platform.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/TargetList.cpp
  lldb/unittests/Platform/PlatformTest.cpp

Index: lldb/unittests/Platform/PlatformTest.cpp
===
--- lldb/unittests/Platform/PlatformTest.cpp
+++ lldb/unittests/Platform/PlatformTest.cpp
@@ -59,7 +59,9 @@
 PluginManager::UnregisterPlugin(PlatformThumb::CreateInstance);
   }
 
-  static PlatformSP CreateInstance(bool force, const ArchSpec *arch) {
+  static PlatformSP CreateInstance(bool force, const ArchSpec *arch,
+   const Debugger *debugger,
+   const ScriptedMetadata *metadata) {
 return std::make_shared();
   }
 
Index: lldb/source/Target/TargetList.cpp
===
--- lldb/source/Target/TargetList.cpp
+++ lldb/source/Target/TargetList.cpp
@@ -185,7 +185,7 @@
 for (const ModuleSpec  : module_specs.ModuleSpecs())
   archs.push_back(spec.GetArchitecture());
 if (PlatformSP platform_for_archs_sp =
-platform_list.GetOrCreate(archs, {}, candidates)) {
+platform_list.GetOrCreate(archs, {}, candidates, nullptr)) {
   platform_sp = 

[Lldb-commits] [PATCH] D139248: [lldb/Interpreter] Improve ScriptedPythonInterface::GetStatusFromMethod

[Med Ismail Bennani via Phabricator via lldb-commits]

mib created this revision.
mib added reviewers: JDevlieghere, kastiglione.
mib added a project: LLDB.
Herald added a project: All.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch makes `ScriptedPythonInterface::GetStatusFromMethod` take a
parameter pack as an argument. That will allow it to pass arbitrary
arguments to the python method.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139248

Files:
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h


Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
@@ -99,7 +99,13 @@
 return ExtractValueFromPythonObject(py_return, error);
   }
 
-  Status GetStatusFromMethod(llvm::StringRef method_name);
+  template 
+  Status GetStatusFromMethod(llvm::StringRef method_name, Args &&...args) {
+Status error;
+Dispatch(method_name, error, args...);
+
+return error;
+  }
 
   template  T Transform(T object) {
 // No Transformation for generic usage
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.cpp
@@ -26,14 +26,6 @@
 ScriptInterpreterPythonImpl )
 : ScriptedInterface(), m_interpreter(interpreter) {}
 
-Status
-ScriptedPythonInterface::GetStatusFromMethod(llvm::StringRef method_name) {
-  Status error;
-  Dispatch(method_name, error);
-
-  return error;
-}
-
 template <>
 StructuredData::ArraySP
 ScriptedPythonInterface::ExtractValueFromPythonObject(


Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
@@ -99,7 +99,13 @@
 return ExtractValueFromPythonObject(py_return, error);
   }
 
-  Status GetStatusFromMethod(llvm::StringRef method_name);
+  template 
+  Status GetStatusFromMethod(llvm::StringRef method_name, Args &&...args) {
+Status error;
+Dispatch(method_name, error, args...);
+
+return error;
+  }
 
   template  T Transform(T object) {
 // No Transformation for generic usage
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.cpp
@@ -26,14 +26,6 @@
 ScriptInterpreterPythonImpl )
 : ScriptedInterface(), m_interpreter(interpreter) {}
 
-Status
-ScriptedPythonInterface::GetStatusFromMethod(llvm::StringRef method_name) {
-  Status error;
-  Dispatch(method_name, error);
-
-  return error;
-}
-
 template <>
 StructuredData::ArraySP
 ScriptedPythonInterface::ExtractValueFromPythonObject(
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D139247: [lldb/Utility] Make ScriptedProcessInfo more generic

[Med Ismail Bennani via Phabricator via lldb-commits]

mib created this revision.
mib added reviewers: JDevlieghere, bulbazord.
mib added a project: LLDB.
Herald added a project: All.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch moves the ScriptedProcessInfo class out of the
ScriptedProcess and hoist it as a standalone utility class, so it can be
reused with the Scripted Platform.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139247

Files:
  lldb/include/lldb/Utility/ScriptedMetadata.h
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.h
  lldb/source/Plugins/Process/scripted/ScriptedThread.cpp

Index: lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
===
--- lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
@@ -57,8 +57,8 @@
   ExecutionContext exe_ctx(process);
   StructuredData::GenericSP owned_script_object_sp =
   scripted_thread_interface->CreatePluginObject(
-  thread_class_name, exe_ctx,
-  process.m_scripted_process_info.GetArgsSP(), script_object);
+  thread_class_name, exe_ctx, process.m_scripted_metadata.GetArgsSP(),
+  script_object);
 
   if (!owned_script_object_sp)
 return llvm::createStringError(llvm::inconvertibleErrorCode(),
Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.h
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.h
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.h
@@ -11,6 +11,7 @@
 
 #include "lldb/Target/Process.h"
 #include "lldb/Utility/ConstString.h"
+#include "lldb/Utility/ScriptedMetadata.h"
 #include "lldb/Utility/Status.h"
 
 #include "ScriptedThread.h"
@@ -18,24 +19,7 @@
 #include 
 
 namespace lldb_private {
-
 class ScriptedProcess : public Process {
-protected:
-  class ScriptedProcessInfo {
-  public:
-ScriptedProcessInfo(const ProcessLaunchInfo _info) {
-  m_class_name = launch_info.GetScriptedProcessClassName();
-  m_args_sp = launch_info.GetScriptedProcessDictionarySP();
-}
-
-std::string GetClassName() const { return m_class_name; }
-StructuredData::DictionarySP GetArgsSP() const { return m_args_sp; }
-
-  private:
-std::string m_class_name;
-StructuredData::DictionarySP m_args_sp;
-  };
-
 public:
   static lldb::ProcessSP CreateInstance(lldb::TargetSP target_sp,
 lldb::ListenerSP listener_sp,
@@ -90,8 +74,7 @@
 
 protected:
   ScriptedProcess(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp,
-  const ScriptedProcess::ScriptedProcessInfo _info,
-  Status );
+  const ScriptedMetadata _metadata, Status );
 
   Status DoStop();
 
@@ -111,7 +94,7 @@
   static bool IsScriptLanguageSupported(lldb::ScriptLanguage language);
 
   // Member variables.
-  const ScriptedProcessInfo m_scripted_process_info;
+  const ScriptedMetadata m_scripted_metadata;
   lldb_private::ScriptInterpreter *m_interpreter = nullptr;
   lldb_private::StructuredData::ObjectSP m_script_object_sp = nullptr;
   //@}
Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
@@ -58,12 +58,11 @@
   !IsScriptLanguageSupported(target_sp->GetDebugger().GetScriptLanguage()))
 return nullptr;
 
-  Status error;
-  ScriptedProcess::ScriptedProcessInfo scripted_process_info(
-  target_sp->GetProcessLaunchInfo());
+  ScriptedMetadata scripted_metadata(target_sp->GetProcessLaunchInfo());
 
-  auto process_sp = std::shared_ptr(new ScriptedProcess(
-  target_sp, listener_sp, scripted_process_info, error));
+  Status error;
+  auto process_sp = std::shared_ptr(
+  new ScriptedProcess(target_sp, listener_sp, scripted_metadata, error));
 
   if (error.Fail() || !process_sp || !process_sp->m_script_object_sp ||
   !process_sp->m_script_object_sp->IsValid()) {
@@ -79,12 +78,11 @@
   return true;
 }
 
-ScriptedProcess::ScriptedProcess(
-lldb::TargetSP target_sp, lldb::ListenerSP listener_sp,
-const ScriptedProcess::ScriptedProcessInfo _process_info,
-Status )
-: Process(target_sp, listener_sp),
-  m_scripted_process_info(scripted_process_info) {
+ScriptedProcess::ScriptedProcess(lldb::TargetSP target_sp,
+ lldb::ListenerSP listener_sp,
+ const ScriptedMetadata _metadata,
+ Status )
+: Process(target_sp, listener_sp), m_scripted_metadata(scripted_metadata) {
 
   if (!target_sp) {
 error.SetErrorStringWithFormat("ScriptedProcess::%s () - ERROR: %s",
@@ -104,8 +102,8 @@
   

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

[Christopher Di Bella via Phabricator via lldb-commits]

cjdb added a comment.

In D138939#3967397 , @tschuett wrote:

> I do not ask you to do anything! I just noticed that you add a lot of 
> `FormatXXXDiagnostic` functions. An alternativ design is to have one 
> `FormatDiagnostic` function with a mode parameter. Then you can decide 
> whether to print legacy or user-oriented reasons.
>
> If next year you invent another diagnostic, you can extend the enum and the 
> `FormatDiagnostic` function.

Makes sense to me! Thank you for clarifying.

In D138939#3967715 , @dblaikie wrote:

> Sorry if this is derailing, but I wonder/worry about a few things here:
>
> 1. Compounding structured output with phraseology - it seems like it might be 
> worthwhile for these to be separate issues

I agree with you. The reason I've compounded the two is to demonstrate how this 
is supposed to be used (otherwise it really feels like a nothing burger where 
I've made change for the sake of change).

> (doesn't mean the terminal output has to say exactly the same thing - as 
> you've mentioned, we could have some fields that aren't rendered in some 
> modes (eg: maybe a terminal only gets the summary, and not the reason - or 
> perhaps you can ask for reasons to be provided if you don't mind the 
> verbosity)). In the example case include here - describing the template 
> parameter kind mismatch seems OK for Clang's current textual diagnostic 
> experience & I don't think that would need to be limited to only the SARIF 
> output?

@rsmith and I spent a fair amount of time chatting about giving Clang a verbose 
diagnostic mode, and we agreed that would have issues:

1. If you wanted to use unstructured text in brief mode 90% of the time and 
then change when you need to clarify something, build systems are going to need 
to rebuild everything.
2. Because scripts almost certainly exist that consume the current diagnostics, 
I'm reluctant to alter those in any meaningful way. I even note this in the 
RFC, where I explicitly state that there are no current plans to bin the 
existing model.

> 2. Could the new phraseology be included in a separate/parallel diagnostic 
> file, essentially akin to a translation? Clang never did get support for 
> multiple languages in its diagnostics, but maybe this is the time to do that 
> work? And then have this new phrasing as the first "translation" of sorts?

Yes and no. For strict a rephrasing, this would be possible, but reasons aren't 
necessarily going to be just a rephrasing (the one in this patch is not). I 
intend for the reason section to be a far richer field that contains 
information that we've never previously provided before: a translation file 
can't help there. However, using parallel diagnostic files might not be such a 
bad idea. How do we ensure the two don't get out of sync?

> 3. I'm not sure I'd commit to calling the current diagnostic experience 
> "legacy" just yet &

You mentioned "terminal diagnostics" below. I like that a lot more than the 
original "unstructured reason" and "traditional reason", so let's go with that 
for now.

> maybe this is my broadest feedback: It'd be great to avoid a two-system 
> situation with the intent to merge things back in later.

I don't know how we can without breaking the promise of backwards compatibility.

> I think what I'd like to see (though I'm far from the decider in any of this) 
> would be that the existing underlying structured diagnostics system could 
> have a new emission system, that produces SARIF instead of text on a terminal 
> - same underlying structured representation, but providing that more directly 
> (in SARIF/JSON/etc) so that it's not lossy in the conversion to an output 
> format - essentially a machine-readable mode for clang's diagnostic output. 
> (to a file, to the terminal, beside the existing terminal output or instead 
> of it, whatever folks prefer) - this would, ideally, initially require no 
> changes to diagnostic API usage across clang.

I //think// you may be asking for what I mentioned here 
? That's doable and probably wise: I 
just need to add `-fdiagnostics-format=sarif-compatibility` (flag spelling to 
be bikeshed). (@tschuett it seems that this flag may happen regardless of 
whether you intended for it!)

> Diagnostic quality improvements, like the template parameter kind mismatch 
> can be committed independently as desired.

I'll coordinate with you next week to address my concerns here, but if we 
//can// achieve this, I'll be really happy. I'll be happier again if we can 
make it so both formats can have the change too :)

> Pretty much all of these features could be implemented somewhat in parallel, 
> each feature would be of value independently to varying degrees, and we 
> wouldn't end up with a deprecated/legacy/not ready yet situation (the only 
> "not ready yet" part might be adding the new diagnostic phrasings 

[Lldb-commits] [PATCH] D138618: [LLDB] Enable 64 bit debug/type offset

[Alexander Yermolovich via Phabricator via lldb-commits]

ayermolo added a comment.

In D138618#3966539 , @labath wrote:

> I don't believe there's no way to split this patch up. I mean, just half of 
> it is dedicated to changing PRIx32 into PRIx64. Surely that can be a patch of 
> it's own, even if it meant that DWARFDIE::GetOffset temporarily returns 
> something other than dw_offset_t. And if we first changed that to use the 
> llvm::formatv function (which does not require width specifiers), then 
> changing the size of dw_offset_t would be a no-op there. And the fact that 
> the patch description can be summed up into one sentence (enable 64-bit 
> offsets) means that I as a reviewer have to reverse-engineer from scratch 
> what all of these (very subtle) changes do and how they interact.
>
> For example, I only now realized that this patch makes the DIERef class 
> essentially interchangable with the `user_id_t` type (in has an (implicit!) 
> constructor and a get_id() function). That's not necessarily bad, if every 
> DIERef can really be converted to a user_id_t. However, if that's the case, 
> then why does `SymbolFileDWARF::GetUID` still exist?
>
> Previously the DIERef class did not encode information about which "OSO" 
> DWARF file in the debug map it is referring to, and they way that was 
> enforced was by having all conversions go through that function (which was 
> the only way to convert from one to the other). If every DIERef really does 
> carry the OSO information then this function (and it's counterpart, 
> `DecodeUID`) should not exist. If it doesn't carry that information, then 
> we're losing some type safety because we now have two ways to do the 
> conversion, and one of them is (sometimes?) incorrect. Maybe there's no way 
> to avoid that, it's definitely worth discussing, and it that would be a lot 
> easier without the other changes in the way.
>
> As for the discussion, I am still undecided about whether encoding the OSO 
> information into the DIERef is a good thing. In some ways, it is very similar 
> to dwo files (whose information is encoded there), but OTOH, they are also 
> very different. An OSO is essentially a completely self-contained dwarf file, 
> and we represent it in lldb as such (with its own Module, SymbolFile objects, 
> etc.). A DWO file is only syntactically independent (e.g. its DIE tree can be 
> parsed independently), but there's no way to interpret the information inside 
> it without accessing the parent object file (as that contains all the address 
> information). This is also reflected in how they're represented in LLDB. The 
> don't have their own Module objects, and the SymbolFileDWARFDwo class is just 
> a very thin wrapper that forwards everything to the real symbol file. 
> Therefore, it does not seem *un*reasonable to have one way/object to 
> reference a DIE inside a single SymbolFileDWARF (and all the DWO files it 
> references), and another to reference *any* DIE in the set of all 
> SymbolFileDWARFs (either a single object, or multiple objects managed by a 
> SymbolFileDWARFDebugMap) which provide the information for this module.

How would you like me to break up this diff? Is factoring OSO into another diff 
enough, or do you want more granular one?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138618

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

[Lldb-commits] [PATCH] D138618: [LLDB] Enable 64 bit debug/type offset

[Alexander Yermolovich via Phabricator via lldb-commits]

ayermolo updated this revision to Diff 479769.
ayermolo added a comment.

Addressed inlined comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138618

Files:
  lldb/include/lldb/Core/dwarf.h
  lldb/include/lldb/Symbol/DWARFCallFrameInfo.h
  lldb/source/Plugins/SymbolFile/DWARF/DIERef.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DIERef.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFTypeUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
  lldb/source/Symbol/DWARFCallFrameInfo.cpp
  lldb/test/Shell/SymbolFile/DWARF/DW_AT_range-DW_FORM_sec_offset.s
  lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists-dwo.s
  lldb/unittests/SymbolFile/DWARF/DWARFIndexCachingTest.cpp

Index: lldb/unittests/SymbolFile/DWARF/DWARFIndexCachingTest.cpp
===
--- lldb/unittests/SymbolFile/DWARF/DWARFIndexCachingTest.cpp
+++ lldb/unittests/SymbolFile/DWARF/DWARFIndexCachingTest.cpp
@@ -45,6 +45,26 @@
   EncodeDecode(DIERef(200, DIERef::Section::DebugTypes, 0x11223344));
 }
 
+TEST(DWARFIndexCachingTest, DIERefEncodeDecodeMax) {
+  // Tests DIERef::Encode(...) and DIERef::Decode(...)
+  EncodeDecode(DIERef(llvm::None, DIERef::Section::DebugInfo,
+  DIERef::k_die_offset_mask));
+  EncodeDecode(DIERef(llvm::None, DIERef::Section::DebugTypes,
+  DIERef::k_die_offset_mask));
+  EncodeDecode(
+  DIERef(100, DIERef::Section::DebugInfo, DIERef::k_die_offset_mask));
+  EncodeDecode(
+  DIERef(200, DIERef::Section::DebugTypes, DIERef::k_die_offset_mask));
+  EncodeDecode(DIERef(DIERef::k_dwo_num_mask, DIERef::Section::DebugInfo,
+  DIERef::k_dwo_num_mask));
+  EncodeDecode(DIERef(DIERef::k_dwo_num_mask, DIERef::Section::DebugTypes,
+  DIERef::k_dwo_num_mask));
+  EncodeDecode(
+  DIERef(DIERef::k_dwo_num_mask, DIERef::Section::DebugInfo, 0x11223344));
+  EncodeDecode(
+  DIERef(DIERef::k_dwo_num_mask, DIERef::Section::DebugTypes, 0x11223344));
+}
+
 static void EncodeDecode(const NameToDIE , ByteOrder byte_order) {
   const uint8_t addr_size = 8;
   DataEncoder encoder(byte_order, addr_size);
Index: lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists-dwo.s
===
--- lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists-dwo.s
+++ lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists-dwo.s
@@ -4,9 +4,9 @@
 # RUN:   -o exit | FileCheck %s
 
 # CHECK-LABEL: image lookup -v -s lookup_rnglists
-# CHECK:  Function: id = {0x4028}, name = "rnglists", range = [0x-0x0003)
-# CHECK:Blocks: id = {0x4028}, range = [0x-0x0003)
-# CHECK-NEXT:   id = {0x4037}, range = [0x0001-0x0002)
+# CHECK:  Function: id = {0x2028}, name = "rnglists", range = [0x-0x0003)
+# CHECK:Blocks: id = {0x2028}, range = [0x-0x0003)
+# CHECK-NEXT:   id = {0x2037}, range = [0x0001-0x0002)
 
 .text
 rnglists:
Index: lldb/test/Shell/SymbolFile/DWARF/DW_AT_range-DW_FORM_sec_offset.s
===
--- lldb/test/Shell/SymbolFile/DWARF/DW_AT_range-DW_FORM_sec_offset.s
+++ lldb/test/Shell/SymbolFile/DWARF/DW_AT_range-DW_FORM_sec_offset.s
@@ -8,7 +8,7 @@
 # RUN:   -o exit | FileCheck %s
 
 # Failure was the block range 1..2 was not printed plus:
-# error: DW_AT_range-DW_FORM_sec_offset.s.tmp {0x003f}: DIE has DW_AT_ranges(0xc) attribute, but range extraction failed (missing or invalid range list table), please file a bug and attach the file at the start of this error message
+# error: DW_AT_range-DW_FORM_sec_offset.s.tmp {0x003f}: DIE has DW_AT_ranges(0xc) attribute, but range extraction failed (missing or invalid range list table), please file a bug and attach the file at the start of this error message
 
 # CHECK-LABEL: image lookup -v -s lookup_rnglists
 # CHECK:  Function: id = {0x0029}, name = "rnglists", range = [0x-0x0003)
@@ -22,7 +22,7 @@
 # RUN: cat %t.error | 

[Lldb-commits] [PATCH] D138834: [lldb] Fix simple template names interaction with debug info declarations

[Arthur Eubanks via Phabricator via lldb-commits]

aeubanks added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:765
+m_ast.CreateClassTemplateSpecializationType(class_specialization_decl);
+return clang_type.GetTypeName(/*BaseOnly*/ true);
+  }

Michael137 wrote:
> Michael137 wrote:
> > Ok so what we're doing is:
> > 1. Create a `ClassTemplateSpecializationDecl` with an empty basename
> > 2. Return the type-name and since the basename is empty we end up with just 
> > the template arguments
> > 
> > Why can't we just call `GetTemplateParametersString(die)` instead of 
> > creating this function?
> Can confirm that this works locally. Though required moving that function out 
> of the DWARFASTParserClang, which seems OK
`GetTemplateParametersString` is specifically only used for 
`GetCPlusPlusQualifiedName`, which is used below
```
  // For C++, we rely solely upon the one definition rule that says
  // only one thing can exist at a given decl context. We ignore the
  // file and line that things are declared on.
  std::string qualified_name = GetCPlusPlusQualifiedName(die);
```
so it doesn't have to match clang's printing. but for the simple template name 
stuff, we are comparing clang-generated names, so everything needs to go 
through clang.

I've replaced `GetTemplateParametersString` with this code that goes through 
clang as well



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2988
+template_params =
+dwarf_ast->GetForwardDeclarationDIETemplateParams(die);
+}

Michael137 wrote:
> So after this call we'll have an unnamed `ClassTemplateDecl` hanging off the 
> AST? Doing this inside `FindDefinitionTypeForDWARFDeclContext` instead of 
> `ParseStructureLikeDIE` feels a bit off. But will have to understand this 
> change a bit deeper to know whether there is something we can do about that.
> 
> As a side-note, we seem to be accumulating a lot of blocks around LLDB that 
> look like:
> ```
> // Do X because -gsimple-template-names
> if (name.contains('<')) {
>// Do something
> }
> ```
> Still wonder if there isn't some flag that can be set on, e.g., a CU, that 
> would tell us this.
the effect is not really per-CU but rather per-DIE because in some cases (where 
DWARF roundtripping is lossy, @dblaikie would know best) we still put template 
params in the name. so it needs to be checked per-DIE

I agree that it's weird to create clang AST nodes here and then forget about 
them, but clang uses a bump allocator and we can't easily delete it. As long as 
we call this at most once or twice per DIE, I don't think it's worth caching 
the results, but if we do end up using this more, we do need to cache the 
created AST nodes. I've added a comment to the docstring.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138834

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

[Lldb-commits] [PATCH] D138834: [lldb] Fix simple template names interaction with debug info declarations

[Arthur Eubanks via Phabricator via lldb-commits]

aeubanks updated this revision to Diff 479759.
aeubanks marked 2 inline comments as done.
aeubanks added a comment.

address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138834

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/test/API/lang/cpp/unique-types3/Makefile
  lldb/test/API/lang/cpp/unique-types3/TestUniqueTypes3.py
  lldb/test/API/lang/cpp/unique-types3/a.cpp
  lldb/test/API/lang/cpp/unique-types3/a.h
  lldb/test/API/lang/cpp/unique-types3/main.cpp

Index: lldb/test/API/lang/cpp/unique-types3/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/unique-types3/main.cpp
@@ -0,0 +1,9 @@
+#include "a.h"
+
+S a1;
+S a2;
+S a3;
+
+void f(S &);
+
+int main() { f(a2); }
Index: lldb/test/API/lang/cpp/unique-types3/a.h
===
--- /dev/null
+++ lldb/test/API/lang/cpp/unique-types3/a.h
@@ -0,0 +1,3 @@
+template  struct S {
+  T t;
+};
Index: lldb/test/API/lang/cpp/unique-types3/a.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/unique-types3/a.cpp
@@ -0,0 +1,5 @@
+#include "a.h"
+
+void f(S ) {
+  (void)a; // Set breakpoint here
+}
Index: lldb/test/API/lang/cpp/unique-types3/TestUniqueTypes3.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/unique-types3/TestUniqueTypes3.py
@@ -0,0 +1,27 @@
+"""
+Test that we return only the requested template instantiation.
+"""
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+class UniqueTypesTestCase3(TestBase):
+def do_test(self, debug_flags):
+"""Test that we display the correct template instantiation."""
+self.build(dictionary=debug_flags)
+lldbutil.run_to_source_breakpoint(self, "// Set breakpoint here", lldb.SBFileSpec("a.cpp"))
+self.expect_expr("a", result_type="S")
+
+@skipIf(compiler=no_match("clang"))
+@skipIf(compiler_version=["<", "15.0"])
+def test_simple_template_names(self):
+# Can't directly set CFLAGS_EXTRAS here because the Makefile can't
+# override an environment variable.
+self.do_test(dict(TEST_CFLAGS_EXTRAS="-gsimple-template-names"))
+
+@skipIf(compiler=no_match("clang"))
+@skipIf(compiler_version=["<", "15.0"])
+def test_no_simple_template_names(self):
+self.do_test(dict(CFLAGS_EXTRAS="-gno-simple-template-names"))
Index: lldb/test/API/lang/cpp/unique-types3/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/cpp/unique-types3/Makefile
@@ -0,0 +1,5 @@
+CXX_SOURCES := main.cpp a.cpp
+
+CFLAGS_EXTRAS = $(TEST_CFLAGS_EXTRAS) $(LIMIT_DEBUG_INFO_FLAGS)
+
+include Makefile.rules
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2978,6 +2978,15 @@
   }
 }
 
+// See comments below about -gsimple-template-names for why we attempt to
+// compute missing template parameter names.
+ConstString template_params;
+if (type_system) {
+  DWARFASTParser *dwarf_ast = type_system->GetDWARFParser();
+  if (dwarf_ast)
+template_params = dwarf_ast->GetDIEClassTemplateParams(die);
+}
+
 m_index->GetTypes(GetDWARFDeclContext(die), [&](DWARFDIE type_die) {
   // Make sure type_die's language matches the type system we are
   // looking for. We don't want to find a "Foo" type from Java if we
@@ -3049,6 +3058,27 @@
   if (!resolved_type || resolved_type == DIE_IS_BEING_PARSED)
 return true;
 
+  // With -gsimple-template-names, the DIE name may not contain the template
+  // parameters. If the declaration has template parameters but doesn't
+  // contain '<', check that the child template parameters match.
+  if (template_params) {
+llvm::StringRef test_base_name =
+GetTypeForDIE(type_die)->GetBaseName().GetStringRef();
+auto i = test_base_name.find('<');
+
+// Full name from clang AST doesn't contain '<' so this type_die isn't
+// a template parameter, but we're expecting template parameters, so
+// bail.
+if (i == llvm::StringRef::npos)
+  return true;
+
+llvm::StringRef test_template_params =
+test_base_name.slice(i, test_base_name.size());
+// Bail if template parameters don't match.
+if 

[Lldb-commits] [PATCH] D139226: Don't mark DW_OP_addr addresses as file addresses while converting them to load addresses sometimes

[Jason Molenda via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0c2b7fa8691e: Leave DW_OP_addr addresses as load addresses 
in DWARFExpression (authored by jasonmolenda).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139226

Files:
  lldb/source/Expression/DWARFExpression.cpp
  lldb/test/API/lang/c/high-mem-global/Makefile
  lldb/test/API/lang/c/high-mem-global/TestHighMemGlobal.py
  lldb/test/API/lang/c/high-mem-global/main.c

Index: lldb/test/API/lang/c/high-mem-global/main.c
===
--- lldb/test/API/lang/c/high-mem-global/main.c
+++ /dev/null
@@ -1,9 +0,0 @@
-
-struct mystruct {
-  int c, d, e;
-} global = {1, 2, 3};
-
-int main ()
-{
-  return global.c; // break here
-}
Index: lldb/test/API/lang/c/high-mem-global/TestHighMemGlobal.py
===
--- lldb/test/API/lang/c/high-mem-global/TestHighMemGlobal.py
+++ /dev/null
@@ -1,59 +0,0 @@
-"""Look that lldb can display a global loaded in high memory at an addressable address."""
-
-
-import lldb
-from lldbsuite.test.lldbtest import *
-import lldbsuite.test.lldbutil as lldbutil
-from lldbsuite.test.decorators import *
-
-class TestHighMemGlobal(TestBase):
-
-NO_DEBUG_INFO_TESTCASE = True
-
-@skipUnlessDarwin  # hardcoding of __DATA segment name
-def test_command_line(self):
-"""Test that we can display a global variable loaded in high memory."""
-self.build()
-
-exe = self.getBuildArtifact("a.out")
-err = lldb.SBError()
-
-target = self.dbg.CreateTarget(exe, '', '', False, err)
-self.assertTrue(target.IsValid())
-module = target.GetModuleAtIndex(0)
-self.assertTrue(module.IsValid())
-data_segment = module.FindSection("__DATA")
-self.assertTrue(data_segment.IsValid())
-err.Clear()
-
-self.expect("expr -- global.c", substrs=[' = 1'])
-self.expect("expr -- global.d", substrs=[' = 2'])
-self.expect("expr -- global.e", substrs=[' = 3'])
-
-err = target.SetSectionLoadAddress(data_segment, 0x)
-self.assertTrue(err.Success())
-self.expect("expr -- global.c", substrs=[' = 1'])
-self.expect("expr -- global.d", substrs=[' = 2'])
-self.expect("expr -- global.e", substrs=[' = 3'])
-
-err = target.SetSectionLoadAddress(data_segment, 0x08814000)
-self.assertTrue(err.Success())
-self.expect("expr -- global.c", substrs=[' = 1'])
-self.expect("expr -- global.d", substrs=[' = 2'])
-self.expect("expr -- global.e", substrs=[' = 3'])
-
-# This is an address in IRMemoryMap::FindSpace where it has an 
-# lldb-side buffer of memory that's used in IR interpreters when
-# memory cannot be allocated in the inferior / functions cannot
-# be jitted.
-err = target.SetSectionLoadAddress(data_segment, 0xdead0fff)
-self.assertTrue(err.Success())
-
-# The global variable `global` is now overlayed by this 
-# IRMemoryMap special buffer, and now we cannot see the variable.
-# Testing that we get the incorrect values at this address ensures 
-# that IRMemoryMap::FindSpace and this test stay in sync.
-self.runCmd("expr -- int $global_c = global.c")
-self.runCmd("expr -- int $global_d = global.d")
-self.runCmd("expr -- int $global_e = global.e")
-self.expect("expr -- $global_c != 1 || $global_d != 2 || $global_e != 3", substrs=[' = true'])
Index: lldb/test/API/lang/c/high-mem-global/Makefile
===
--- lldb/test/API/lang/c/high-mem-global/Makefile
+++ /dev/null
@@ -1,3 +0,0 @@
-C_SOURCES := main.c
-
-include Makefile.rules
Index: lldb/source/Expression/DWARFExpression.cpp
===
--- lldb/source/Expression/DWARFExpression.cpp
+++ lldb/source/Expression/DWARFExpression.cpp
@@ -925,10 +925,6 @@
 stack.back().SetValueType(Value::ValueType::LoadAddress);
   } else {
 stack.back().SetValueType(Value::ValueType::FileAddress);
-// Convert the file address to a load address, so subsequent
-// DWARF operators can operate on it.
-if (target)
-  stack.back().ConvertToLoadAddress(module_sp.get(), target);
   }
   break;
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 0c2b7fa - Leave DW_OP_addr addresses as load addresses in DWARFExpression

[Jason Molenda via lldb-commits]

Author: Jason Molenda
Date: 2022-12-02T14:45:02-08:00
New Revision: 0c2b7fa8691e9d1f6c7dd656e44321ef8c84ae87

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

LOG: Leave DW_OP_addr addresses as load addresses in DWARFExpression

DWARFExpression::Evaluate will convert DW_OP_addr addresses in
a DWARF expression into load addresses on the expression stack
when there is a StackFrame in the ExecutionContext, this from
a change in 2018 in https://reviews.llvm.org/D46362.  At the
time this was handling a case that came up in swift programs,
and is no longer necessary.  I generalized this conversion to
a load address when a Target is available in
https://reviews.llvm.org/D137682 to make a test case possible;
this change broke a use case that Ted reported.

This change removes my test case, and removes this conversion
of a DW_OP_addr into a load address in some instances.

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

Added: 


Modified: 
lldb/source/Expression/DWARFExpression.cpp

Removed: 
lldb/test/API/lang/c/high-mem-global/Makefile
lldb/test/API/lang/c/high-mem-global/TestHighMemGlobal.py
lldb/test/API/lang/c/high-mem-global/main.c



diff  --git a/lldb/source/Expression/DWARFExpression.cpp 
b/lldb/source/Expression/DWARFExpression.cpp
index 9e7df2d3b82ef..e92216bb5e4d5 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -925,10 +925,6 @@ bool DWARFExpression::Evaluate(
 stack.back().SetValueType(Value::ValueType::LoadAddress);
   } else {
 stack.back().SetValueType(Value::ValueType::FileAddress);
-// Convert the file address to a load address, so subsequent
-// DWARF operators can operate on it.
-if (target)
-  stack.back().ConvertToLoadAddress(module_sp.get(), target);
   }
   break;
 

diff  --git a/lldb/test/API/lang/c/high-mem-global/Makefile 
b/lldb/test/API/lang/c/high-mem-global/Makefile
deleted file mode 100644
index 10495940055b6..0
--- a/lldb/test/API/lang/c/high-mem-global/Makefile
+++ /dev/null
@@ -1,3 +0,0 @@
-C_SOURCES := main.c
-
-include Makefile.rules

diff  --git a/lldb/test/API/lang/c/high-mem-global/TestHighMemGlobal.py 
b/lldb/test/API/lang/c/high-mem-global/TestHighMemGlobal.py
deleted file mode 100644
index b0df19ac690c7..0
--- a/lldb/test/API/lang/c/high-mem-global/TestHighMemGlobal.py
+++ /dev/null
@@ -1,59 +0,0 @@
-"""Look that lldb can display a global loaded in high memory at an addressable 
address."""
-
-
-import lldb
-from lldbsuite.test.lldbtest import *
-import lldbsuite.test.lldbutil as lldbutil
-from lldbsuite.test.decorators import *
-
-class TestHighMemGlobal(TestBase):
-
-NO_DEBUG_INFO_TESTCASE = True
-
-@skipUnlessDarwin  # hardcoding of __DATA segment name
-def test_command_line(self):
-"""Test that we can display a global variable loaded in high memory."""
-self.build()
-
-exe = self.getBuildArtifact("a.out")
-err = lldb.SBError()
-
-target = self.dbg.CreateTarget(exe, '', '', False, err)
-self.assertTrue(target.IsValid())
-module = target.GetModuleAtIndex(0)
-self.assertTrue(module.IsValid())
-data_segment = module.FindSection("__DATA")
-self.assertTrue(data_segment.IsValid())
-err.Clear()
-
-self.expect("expr -- global.c", substrs=[' = 1'])
-self.expect("expr -- global.d", substrs=[' = 2'])
-self.expect("expr -- global.e", substrs=[' = 3'])
-
-err = target.SetSectionLoadAddress(data_segment, 0x)
-self.assertTrue(err.Success())
-self.expect("expr -- global.c", substrs=[' = 1'])
-self.expect("expr -- global.d", substrs=[' = 2'])
-self.expect("expr -- global.e", substrs=[' = 3'])
-
-err = target.SetSectionLoadAddress(data_segment, 0x08814000)
-self.assertTrue(err.Success())
-self.expect("expr -- global.c", substrs=[' = 1'])
-self.expect("expr -- global.d", substrs=[' = 2'])
-self.expect("expr -- global.e", substrs=[' = 3'])
-
-# This is an address in IRMemoryMap::FindSpace where it has an 
-# lldb-side buffer of memory that's used in IR interpreters when
-# memory cannot be allocated in the inferior / functions cannot
-# be jitted.
-err = target.SetSectionLoadAddress(data_segment, 0xdead0fff)
-self.assertTrue(err.Success())
-
-# The global variable `global` is now overlayed by this 
-# IRMemoryMap special buffer, and now we cannot see the variable.
-# Testing that we get the incorrect values at this address ensures 
-# that IRMemoryMap::FindSpace and this test stay in sync.

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

[David Blaikie via Phabricator via lldb-commits]

dblaikie added a comment.

Sorry if this is derailing, but I wonder/worry about a few things here:

1. Compounding structured output with phraseology - it seems like it might be 
worthwhile for these to be separate issues (doesn't mean the terminal output 
has to say exactly the same thing - as you've mentioned, we could have some 
fields that aren't rendered in some modes (eg: maybe a terminal only gets the 
summary, and not the reason - or perhaps you can ask for reasons to be provided 
if you don't mind the verbosity)). In the example case include here - 
describing the template parameter kind mismatch seems OK for Clang's current 
textual diagnostic experience & I don't think that would need to be limited to 
only the SARIF output?
2. Could the new phraseology be included in a separate/parallel diagnostic 
file, essentially akin to a translation? Clang never did get support for 
multiple languages in its diagnostics, but maybe this is the time to do that 
work? And then have this new phrasing as the first "translation" of sorts?
3. I'm not sure I'd commit to calling the current diagnostic experience 
"legacy" just yet & maybe this is my broadest feedback: It'd be great to avoid 
a two-system situation with the intent to merge things back in later.

I think what I'd like to see (though I'm far from the decider in any of this) 
would be that the existing underlying structured diagnostics system could have 
a new emission system, that produces SARIF instead of text on a terminal - same 
underlying structured representation, but providing that more directly (in 
SARIF/JSON/etc) so that it's not lossy in the conversion to an output format - 
essentially a machine-readable mode for clang's diagnostic output. (to a file, 
to the terminal, beside the existing terminal output or instead of it, whatever 
folks prefer) - this would, ideally, initially require no changes to diagnostic 
API usage across clang.
Diagnostic quality improvements, like the template parameter kind mismatch can 
be committed independently as desired.
Diagnostic infrastructure could be improved/expanded - adding the "reason" 
field, and a way to emit that (perhaps only in SARIF output, but maybe there's 
room for a flag to emit it in terminal diagnostics too).
Multiple diagnostic text files supported, and the ability to choose which 
diagnostic text - initially with the intent to support this newly proposed 
phrasing approach, but equally could be used for whole language translation.

Pretty much all of these features could be implemented somewhat in parallel, 
each feature would be of value independently to varying degrees, and we 
wouldn't end up with a deprecated/legacy/not ready yet situation (the only "not 
ready yet" part might be adding the new diagnostic phrasings - perhaps 
initially there'd be some way to fallback to the traditional diagnostics, so 
that the whole database didn't need to be translated wholesale - and once it's 
all translated and there's a lot of user feedback on the direction, we could 
consider changing the diagnostic database default, perhaps eventually 
deprecating the old database, and eventually removing it - without major API 
upheaval/renaming/addition/removal)




Comment at: clang/lib/Basic/DiagnosticIDs.cpp:524
+
+// FIXME(spaceship): default when C++20 becomes the default
+bool operator==(const DiagDesc ) const {

I don't think LLVM generally uses `(xxx)` in FIXMEs, or if we do it's probably 
only for bug numbers and maybe developer user names (though the latter's not 
ideal - people come and go, bug numbers are forever (kinda)) - not sure what 
the "spaceship" in there is meant to communicate (I'm aware of the C++20 
spaceship operator, but I'd expect the thing in the `()` to be some list of 
known entities that we keep track of somewhere?)

Maybe `// FIXME: Use spaceship operator in C++20`?



Comment at: clang/lib/Basic/DiagnosticIDs.cpp:812-825
 namespace {
-  struct WarningOption {
-uint16_t NameOffset;
-uint16_t Members;
-uint16_t SubGroups;
-StringRef Documentation;
-
-// String is stored with a pascal-style length byte.
-StringRef getName() const {
-  return StringRef(DiagGroupNames + NameOffset + 1,
-   DiagGroupNames[NameOffset]);
-}
-  };
-}
+struct WarningOption {
+  uint16_t NameOffset;
+  uint16_t Members;
+  uint16_t SubGroups;
+  StringRef Documentation;
+

Unrelated/accidental reformatting? (please commit separately if it's being 
committed)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138939

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

[Lldb-commits] [PATCH] D139226: Don't mark DW_OP_addr addresses as file addresses while converting them to load addresses sometimes

[Adrian Prantl via Phabricator via lldb-commits]

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

Yeah, the previous code was a hack that worked under *some* circumstances. 
That's not good enough to keep it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139226

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

[Lldb-commits] [PATCH] D139226: Don't mark DW_OP_addr addresses as file addresses while converting them to load addresses sometimes

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda created this revision.
jasonmolenda added a reviewer: aprantl.
jasonmolenda added a project: LLDB.
Herald added subscribers: Michael137, JDevlieghere, arichardson.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.

I touched this code in DWARFExpression.cpp https://reviews.llvm.org/D137682 
where it would convert a DW_OP_addr to a load address if the ExecutionContext 
had a StackFrame available.  This was a proxy way of telling if there was a 
live process.  I changed it to do this conversion if the ExecutionContext had a 
*Target*, to make it possible to write a test case that would move a section 
around to different load addresses without ever creating a process.  This, it 
turns out, broke a use case on another platform that Ted Woodward relayed to me.

This code to convert the address to a load address was added by Adrian in 2018 
via https://reviews.llvm.org/D46362 for swift reasons.  I'm not sure it was 
entirely the correct way to solve this, but it has since become quite 
unnecessary for Swift -- in fact, on Darwin platforms, global addresses before 
execution are no longer even file addresses, they're part of the dynamic linker 
(dyld) fixup data and lldb does not know how to parse these.  For instance, 
`static char *f = "hi";` in a program, `f` will have the address of the string 
bytes.  Before and after process execution, lldb must read this address out of 
the binary, and it is not in any format that lldb can correctly map to a 
section+offset in the binary.  So this change is very definitely not necessary 
on Darwin today; we can't parse these addresses at all.

Given that this is breaking a use case on another platform, I'm removing 
Adrian's DW_OP_addr conversion, and removing my test case for the high memory 
special address testing.  I can't find a way to fake up an address space well 
enough to make this work the way it needs to, for now.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139226

Files:
  lldb/source/Expression/DWARFExpression.cpp
  lldb/test/API/lang/c/high-mem-global/Makefile
  lldb/test/API/lang/c/high-mem-global/TestHighMemGlobal.py
  lldb/test/API/lang/c/high-mem-global/main.c

Index: lldb/test/API/lang/c/high-mem-global/main.c
===
--- lldb/test/API/lang/c/high-mem-global/main.c
+++ /dev/null
@@ -1,9 +0,0 @@
-
-struct mystruct {
-  int c, d, e;
-} global = {1, 2, 3};
-
-int main ()
-{
-  return global.c; // break here
-}
Index: lldb/test/API/lang/c/high-mem-global/TestHighMemGlobal.py
===
--- lldb/test/API/lang/c/high-mem-global/TestHighMemGlobal.py
+++ /dev/null
@@ -1,59 +0,0 @@
-"""Look that lldb can display a global loaded in high memory at an addressable address."""
-
-
-import lldb
-from lldbsuite.test.lldbtest import *
-import lldbsuite.test.lldbutil as lldbutil
-from lldbsuite.test.decorators import *
-
-class TestHighMemGlobal(TestBase):
-
-NO_DEBUG_INFO_TESTCASE = True
-
-@skipUnlessDarwin  # hardcoding of __DATA segment name
-def test_command_line(self):
-"""Test that we can display a global variable loaded in high memory."""
-self.build()
-
-exe = self.getBuildArtifact("a.out")
-err = lldb.SBError()
-
-target = self.dbg.CreateTarget(exe, '', '', False, err)
-self.assertTrue(target.IsValid())
-module = target.GetModuleAtIndex(0)
-self.assertTrue(module.IsValid())
-data_segment = module.FindSection("__DATA")
-self.assertTrue(data_segment.IsValid())
-err.Clear()
-
-self.expect("expr -- global.c", substrs=[' = 1'])
-self.expect("expr -- global.d", substrs=[' = 2'])
-self.expect("expr -- global.e", substrs=[' = 3'])
-
-err = target.SetSectionLoadAddress(data_segment, 0x)
-self.assertTrue(err.Success())
-self.expect("expr -- global.c", substrs=[' = 1'])
-self.expect("expr -- global.d", substrs=[' = 2'])
-self.expect("expr -- global.e", substrs=[' = 3'])
-
-err = target.SetSectionLoadAddress(data_segment, 0x08814000)
-self.assertTrue(err.Success())
-self.expect("expr -- global.c", substrs=[' = 1'])
-self.expect("expr -- global.d", substrs=[' = 2'])
-self.expect("expr -- global.e", substrs=[' = 3'])
-
-# This is an address in IRMemoryMap::FindSpace where it has an 
-# lldb-side buffer of memory that's used in IR interpreters when
-# memory cannot be allocated in the inferior / functions cannot
-# be jitted.
-err = target.SetSectionLoadAddress(data_segment, 0xdead0fff)
-self.assertTrue(err.Success())
-
-# The global variable `global` is now overlayed by this 
-# IRMemoryMap special buffer, and now we cannot see the variable.
-# Testing 

[Lldb-commits] [PATCH] D138724: [lldb][Target] Flush the scratch TypeSystem when process gets deleted

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

(or skip the test on Windows)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138724

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

[Lldb-commits] [PATCH] D137900: Make only one function that needs to be implemented when searching for types.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

I have only two concerns left:

1. IMHO it would be better if `find_one` (which I think of as a search *input*) 
would live in TypeQuery instead of TypeResult. It feels wrong to use TypeResult 
to pass something *in*.
2. It's inconsistent that Module has a FindTypes(TypeQuery) method and 
TypeQuery has a Find(First)Type(Module) method.

Could we just move `find_one` into `TypeQuery` and get rid of the `Find` 
methods in `TypeQuery`.
Let me know if I'm misunderstanding something.




Comment at: lldb/include/lldb/Symbol/CompilerDecl.h:93
+  void GetCompilerContext(
+  llvm::SmallVectorImpl ) const;
+

clayborg wrote:
> clayborg wrote:
> > clayborg wrote:
> > > aprantl wrote:
> > > > aprantl wrote:
> > > > > Why can't this be a return value? The context objects are tiny.
> > > > ping?
> > > Will change!
> > I looked around LLVM code and there are many accessors that use this 
> > technique. Otherwise we need to enforce an exact SmallVector type as the 
> > return code and we can't use 
> > "llvm::SmallVectorImpl". I can change this 
> > to return a "llvm::SmallVector" if needed? But that 
> > locks us into a specific small vector with a specific size of preached 
> > items.
> change "preached" to "preset" above...
Thanks for checking — that makes sense.



Comment at: lldb/source/Symbol/Type.cpp:149
+  module->FindTypes(*this, results);
+  return results.GetTypeMap().FirstType();
+}

IMHO it would be better if `find_one` (which I think of as a search *input*) 
would live in TypeQuery instead of TypeResult. It feels wrong to use TypeResult 
to pass something *in*.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137900

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

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

[Thorsten via Phabricator via lldb-commits]

tschuett added a comment.

I do not ask you to do anything! I just noticed that you add a lot of 
`FormatXXXDiagnostic` functions. An alternativ design is to have one 
`FormatDiagnostic` function with a mode parameter. Then you can decide whether 
to print legacy or user-oriented reasons.

If next year you invent another diagnostic, you can extend the enum and the 
`FormatDiagnostic` function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138939

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

[Lldb-commits] [PATCH] D139058: [lldb/unittests/CMakeLists.txt] Remove extra compiler flag `-include gtest_common.h`, NFC

[Argyrios Kyrtzidis via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa9e24afdc706: [lldb/unittests/CMakeLists.txt] Remove extra 
compiler flag `-include… (authored by akyrtzi).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139058

Files:
  lldb/unittests/CMakeLists.txt


Index: lldb/unittests/CMakeLists.txt
===
--- lldb/unittests/CMakeLists.txt
+++ lldb/unittests/CMakeLists.txt
@@ -10,13 +10,6 @@
   add_compile_options("-Wno-suggest-override")
 endif()
 
-set(LLDB_GTEST_COMMON_INCLUDE ${CMAKE_CURRENT_SOURCE_DIR}/gtest_common.h)
-if (MSVC)
-  list(APPEND LLVM_COMPILE_FLAGS /FI ${LLDB_GTEST_COMMON_INCLUDE})
-else ()
-  list(APPEND LLVM_COMPILE_FLAGS -include ${LLDB_GTEST_COMMON_INCLUDE})
-endif ()
-
 function(add_lldb_unittest test_name)
   cmake_parse_arguments(ARG
 ""


Index: lldb/unittests/CMakeLists.txt
===
--- lldb/unittests/CMakeLists.txt
+++ lldb/unittests/CMakeLists.txt
@@ -10,13 +10,6 @@
   add_compile_options("-Wno-suggest-override")
 endif()
 
-set(LLDB_GTEST_COMMON_INCLUDE ${CMAKE_CURRENT_SOURCE_DIR}/gtest_common.h)
-if (MSVC)
-  list(APPEND LLVM_COMPILE_FLAGS /FI ${LLDB_GTEST_COMMON_INCLUDE})
-else ()
-  list(APPEND LLVM_COMPILE_FLAGS -include ${LLDB_GTEST_COMMON_INCLUDE})
-endif ()
-
 function(add_lldb_unittest test_name)
   cmake_parse_arguments(ARG
 ""
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] a9e24af - [lldb/unittests/CMakeLists.txt] Remove extra compiler flag `-include gtest_common.h`, NFC

[Argyrios Kyrtzidis via lldb-commits]

Author: Argyrios Kyrtzidis
Date: 2022-12-02T10:45:49-08:00
New Revision: a9e24afdc706e6946c4da78188732fc60bdb863b

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

LOG: [lldb/unittests/CMakeLists.txt] Remove extra compiler flag `-include 
gtest_common.h`, NFC

This doesn't seem to be necessary anymore so remove it to be more consistent 
with rest of the LLVM projects
that don't use prefix headers.

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

Added: 


Modified: 
lldb/unittests/CMakeLists.txt

Removed: 




diff  --git a/lldb/unittests/CMakeLists.txt b/lldb/unittests/CMakeLists.txt
index e29bc8da9e9da..c084fa5cca92b 100644
--- a/lldb/unittests/CMakeLists.txt
+++ b/lldb/unittests/CMakeLists.txt
@@ -10,13 +10,6 @@ if (CXX_SUPPORTS_SUGGEST_OVERRIDE_FLAG)
   add_compile_options("-Wno-suggest-override")
 endif()
 
-set(LLDB_GTEST_COMMON_INCLUDE ${CMAKE_CURRENT_SOURCE_DIR}/gtest_common.h)
-if (MSVC)
-  list(APPEND LLVM_COMPILE_FLAGS /FI ${LLDB_GTEST_COMMON_INCLUDE})
-else ()
-  list(APPEND LLVM_COMPILE_FLAGS -include ${LLDB_GTEST_COMMON_INCLUDE})
-endif ()
-
 function(add_lldb_unittest test_name)
   cmake_parse_arguments(ARG
 ""



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

[Lldb-commits] [PATCH] D139061: [lldb] Fix the `dwarf` log descriptions

[Argyrios Kyrtzidis via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG36bea8759d1f: [lldb] Fix the `dwarf` log descriptions 
(authored by akyrtzi).

Changed prior to commit:
  https://reviews.llvm.org/D139061?vs=479099=479672#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139061

Files:
  lldb/source/Plugins/SymbolFile/DWARF/LogChannelDWARF.cpp


Index: lldb/source/Plugins/SymbolFile/DWARF/LogChannelDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/LogChannelDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/LogChannelDWARF.cpp
@@ -12,14 +12,16 @@
 
 static constexpr Log::Category g_categories[] = {
 {{"comp"},
- {"log insertions of object files into DWARF debug maps"},
+ {"log struct/union/class type completions"},
  DWARFLog::TypeCompletion},
 {{"info"}, {"log the parsing of .debug_info"}, DWARFLog::DebugInfo},
 {{"line"}, {"log the parsing of .debug_line"}, DWARFLog::DebugLine},
 {{"lookups"},
  {"log any lookups that happen by name, regex, or address"},
  DWARFLog::Lookups},
-{{"map"}, {"log struct/unions/class type completions"}, 
DWARFLog::DebugMap},
+{{"map"},
+ {"log insertions of object files into DWARF debug maps"},
+ DWARFLog::DebugMap},
 };
 
 static Log::Channel g_channel(g_categories, DWARFLog::DebugInfo);


Index: lldb/source/Plugins/SymbolFile/DWARF/LogChannelDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/LogChannelDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/LogChannelDWARF.cpp
@@ -12,14 +12,16 @@
 
 static constexpr Log::Category g_categories[] = {
 {{"comp"},
- {"log insertions of object files into DWARF debug maps"},
+ {"log struct/union/class type completions"},
  DWARFLog::TypeCompletion},
 {{"info"}, {"log the parsing of .debug_info"}, DWARFLog::DebugInfo},
 {{"line"}, {"log the parsing of .debug_line"}, DWARFLog::DebugLine},
 {{"lookups"},
  {"log any lookups that happen by name, regex, or address"},
  DWARFLog::Lookups},
-{{"map"}, {"log struct/unions/class type completions"}, DWARFLog::DebugMap},
+{{"map"},
+ {"log insertions of object files into DWARF debug maps"},
+ DWARFLog::DebugMap},
 };
 
 static Log::Channel g_channel(g_categories, DWARFLog::DebugInfo);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 36bea87 - [lldb] Fix the `dwarf` log descriptions

[Argyrios Kyrtzidis via lldb-commits]

Author: Argyrios Kyrtzidis
Date: 2022-12-02T10:18:38-08:00
New Revision: 36bea8759d1fa07ac7fd8b16e6c7d15f10b0d145

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

LOG: [lldb] Fix the `dwarf` log descriptions

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

Added: 


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

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/LogChannelDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/LogChannelDWARF.cpp
index d2b8fe19db530..6b063f3bd88d8 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/LogChannelDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/LogChannelDWARF.cpp
@@ -12,14 +12,16 @@ using namespace lldb_private;
 
 static constexpr Log::Category g_categories[] = {
 {{"comp"},
- {"log insertions of object files into DWARF debug maps"},
+ {"log struct/union/class type completions"},
  DWARFLog::TypeCompletion},
 {{"info"}, {"log the parsing of .debug_info"}, DWARFLog::DebugInfo},
 {{"line"}, {"log the parsing of .debug_line"}, DWARFLog::DebugLine},
 {{"lookups"},
  {"log any lookups that happen by name, regex, or address"},
  DWARFLog::Lookups},
-{{"map"}, {"log struct/unions/class type completions"}, 
DWARFLog::DebugMap},
+{{"map"},
+ {"log insertions of object files into DWARF debug maps"},
+ DWARFLog::DebugMap},
 };
 
 static Log::Channel g_channel(g_categories, DWARFLog::DebugInfo);



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

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

[Christopher Di Bella via Phabricator via lldb-commits]

cjdb added a comment.

In D138939#3966938 , @tschuett wrote:

> Maybe the kind/amount of information printed ( `DiagnosticMode` ) and the 
> output device (console/sarif) are orthogonal issues.
>
> Still it would nice to be able to toggle the diagnostic mode for 
> users/testing / A/B.

Hmm, are you asking for it to be possible for there to be legacy diagnostics 
with `-fdiagnostics-format=sarif` as well as user-oriented diagnostics? That 
should be doable, at least for now (I don't want to commit to this long-term in 
case it isn't viable at some point down the road, but it's hardly effort for 
what we currently support).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138939

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

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

[Thorsten via Phabricator via lldb-commits]

tschuett added a comment.

Maybe the kind/amount of information printed ( `DiagnosticMode` ) and the 
output device (console/sarif) are orthogonal issues.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138939

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

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

[Christopher Di Bella via Phabricator via lldb-commits]

cjdb added a comment.

In D138939#3965985 , @tschuett wrote:

> Then Sarif was a distraction. Still to reduce boilerplate and for A/B testing:
>
>   enum class DiagnosticMode {
> Legacy,
> UserOriented,
> Default = Legacy
>   }

It took a fair bit of squinting and coffee, but I think I get it now. Having 
SARIF will be good for option 2: I hadn't realised this was at the Clang API 
level and not an enum for users to toggle on the command line! Thanks for your 
patience, I'll implement this now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138939

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

[Lldb-commits] [PATCH] D138271: Fix license header in TraceHTR.h

[Augie Fackler via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa11d53e30d3d: [lldb] fix license header in TraceHTR.h 
(authored by pietroalbini, committed by durin42).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138271

Files:
  lldb/source/Plugins/TraceExporter/common/TraceHTR.h


Index: lldb/source/Plugins/TraceExporter/common/TraceHTR.h
===
--- lldb/source/Plugins/TraceExporter/common/TraceHTR.h
+++ lldb/source/Plugins/TraceExporter/common/TraceHTR.h
@@ -1,9 +1,8 @@
 //===-- TraceHTR.h 
===//
 //
-// Part of the LLVM Project, under the Apache
-// License v2.0 with LLVM Exceptions.// See https://llvm.org/LICENSE.txt for
-// license information.// SPDX-License-Identifier: Apache-2.0 WITH
-// LLVM-exception
+// 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
 //
 
//===--===//
 


Index: lldb/source/Plugins/TraceExporter/common/TraceHTR.h
===
--- lldb/source/Plugins/TraceExporter/common/TraceHTR.h
+++ lldb/source/Plugins/TraceExporter/common/TraceHTR.h
@@ -1,9 +1,8 @@
 //===-- TraceHTR.h ===//
 //
-// Part of the LLVM Project, under the Apache
-// License v2.0 with LLVM Exceptions.// See https://llvm.org/LICENSE.txt for
-// license information.// SPDX-License-Identifier: Apache-2.0 WITH
-// LLVM-exception
+// 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
 //
 //===--===//
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] a11d53e - [lldb] fix license header in TraceHTR.h

[Augie Fackler via lldb-commits]

Author: Pietro Albini
Date: 2022-12-02T10:59:50-05:00
New Revision: a11d53e30d3d19cf6594cf4bd4cf39ba0271899d

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

LOG: [lldb] fix license header in TraceHTR.h

It seems like the license header got mangled somehow, joining multiple
lines together and splitting some lines across multiple ones. That is
causing errors in a license checker I'm using (called REUSE).

This commit restores the license header to the format used by the rest
of the files in the project.

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

Added: 


Modified: 
lldb/source/Plugins/TraceExporter/common/TraceHTR.h

Removed: 




diff  --git a/lldb/source/Plugins/TraceExporter/common/TraceHTR.h 
b/lldb/source/Plugins/TraceExporter/common/TraceHTR.h
index 03babc5a36ab2..3f5d6f237afba 100644
--- a/lldb/source/Plugins/TraceExporter/common/TraceHTR.h
+++ b/lldb/source/Plugins/TraceExporter/common/TraceHTR.h
@@ -1,9 +1,8 @@
 //===-- TraceHTR.h 
===//
 //
-// Part of the LLVM Project, under the Apache
-// License v2.0 with LLVM Exceptions.// See https://llvm.org/LICENSE.txt for
-// license information.// SPDX-License-Identifier: Apache-2.0 WITH
-// LLVM-exception
+// 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
 //
 
//===--===//
 



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

[Lldb-commits] [PATCH] D138724: [lldb][Target] Flush the scratch TypeSystem when process gets deleted

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

A simple `USE_LIBDL := 1` could fix the linux test.

As for windows, we have some dlopen "portability wrappers" in 
`packages/Python/lldbsuite/test/make/dylib.h`, but windows has the additional 
problem (well, maybe it's kind of a feature) of not being able to modify a 
binary while it's open). It will probably be necessary to kill the process and 
remove the module from its module list (if that doesn't defeat the purpose of 
the test). Force unloading the module (SBDebugger::MemoryPressureDetected) 
might be necessary as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138724

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

[Lldb-commits] [PATCH] D138618: [LLDB] Enable 64 bit debug/type offset

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I don't believe there's no way to split this patch up. I mean, just half of it 
is dedicated to changing PRIx32 into PRIx64. Surely that can be a patch of it's 
own, even if it meant that DWARFDIE::GetOffset temporarily returns something 
other than dw_offset_t. And if we first changed that to use the llvm::formatv 
function (which does not require width specifiers), then changing the size of 
dw_offset_t would be a no-op there. And the fact that the patch description can 
be summed up into one sentence (enable 64-bit offsets) means that I as a 
reviewer have to reverse-engineer from scratch what all of these (very subtle) 
changes do and how they interact.

For example, I only now realized that this patch makes the DIERef class 
essentially interchangable with the `user_id_t` type (in has an (implicit!) 
constructor and a get_id() function). That's not necessarily bad, if every 
DIERef can really be converted to a user_id_t. However, if that's the case, 
then why does `SymbolFileDWARF::GetUID` still exist?

Previously the DIERef class did not encode information about which "OSO" DWARF 
file in the debug map it is referring to, and they way that was enforced was by 
having all conversions go through that function (which was the only way to 
convert from one to the other). If every DIERef really does carry the OSO 
information then this function (and it's counterpart, `DecodeUID`) should not 
exist. If it doesn't carry that information, then we're losing some type safety 
because we now have two ways to do the conversion, and one of them is 
(sometimes?) incorrect. Maybe there's no way to avoid that, it's definitely 
worth discussing, and it that would be a lot easier without the other changes 
in the way.

As for the discussion, I am still undecided about whether encoding the OSO 
information into the DIERef is a good thing. In some ways, it is very similar 
to dwo files (whose information is encoded there), but OTOH, they are also very 
different. An OSO is essentially a completely self-contained dwarf file, and we 
represent it in lldb as such (with its own Module, SymbolFile objects, etc.). A 
DWO file is only syntactically independent (e.g. its DIE tree can be parsed 
independently), but there's no way to interpret the information inside it 
without accessing the parent object file (as that contains all the address 
information). This is also reflected in how they're represented in LLDB. The 
don't have their own Module objects, and the SymbolFileDWARFDwo class is just a 
very thin wrapper that forwards everything to the real symbol file. Therefore, 
it does not seem *un*reasonable to have one way/object to reference a DIE 
inside a single SymbolFileDWARF (and all the DWO files it references), and 
another to reference *any* DIE in the set of all SymbolFileDWARFs (either a 
single object, or multiple objects managed by a SymbolFileDWARFDebugMap) which 
provide the information for this module.




Comment at: lldb/source/Plugins/SymbolFile/DWARF/DIERef.h:52
+
+  DIERef(lldb::user_id_t uid) {
+m_die_offset = uid & k_die_offset_mask;

At least make this explicit so it can't be constructed from any random integer. 
I'd consider even making this a named (static) function (e.g. `DIERef 
fromUID(user_id_t)`), as one should be extra careful around these conversions.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:16
 
+#include "DIERef.h"
 #include "lldb/Core/Module.h"

This would look much better in the block on line 60, next to the other includes 
from this directory.
Or, even better, if you just delete all the empty lines between the includes, 
then clang-format will automatically sort the whole thing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138618

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

[Lldb-commits] [PATCH] D138724: [lldb][Target] Flush the scratch TypeSystem when process gets deleted

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

Reverted for now while figuring out how to best deal with the tests on Linux


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138724

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

[Lldb-commits] [lldb] 71f3cac - Revert "[lldb][Target] Flush the scratch TypeSystem when owning lldb_private::Module gets unloaded"

[Michael Buch via lldb-commits]

Author: Michael Buch
Date: 2022-12-02T14:12:41Z
New Revision: 71f3cac7895ad516ec25438f803ed3c9916c215a

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

LOG: Revert "[lldb][Target] Flush the scratch TypeSystem when owning 
lldb_private::Module gets unloaded"

This reverts commit 4df11394a10b3b15d2fb9bde8b831cf68785aa45.

Added: 


Modified: 
lldb/source/Target/Target.cpp

Removed: 
lldb/test/API/functionalities/rerun_and_expr/Makefile
lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
lldb/test/API/functionalities/rerun_and_expr/main.cpp
lldb/test/API/functionalities/rerun_and_expr/rebuild.cpp
lldb/test/API/functionalities/rerun_and_expr_dylib/Makefile
lldb/test/API/functionalities/rerun_and_expr_dylib/TestRerunAndExprDylib.py
lldb/test/API/functionalities/rerun_and_expr_dylib/lib.cpp
lldb/test/API/functionalities/rerun_and_expr_dylib/main.cpp
lldb/test/API/functionalities/rerun_and_expr_dylib/rebuild.cpp



diff  --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 8a197bf6e502a..22071b76a805b 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -1682,30 +1682,6 @@ void Target::ModulesDidUnload(ModuleList _list, 
bool delete_locations) {
 m_breakpoint_list.UpdateBreakpoints(module_list, false, delete_locations);
 m_internal_breakpoint_list.UpdateBreakpoints(module_list, false,
  delete_locations);
-
-// If a module was torn down it will have torn down the 'TypeSystemClang's
-// that we used as source 'ASTContext's for the persistent variables in
-// the current target. Those would now be unsafe to access because the
-// 'DeclOrigin' are now possibly stale. Thus clear all persistent
-// variables. We only want to flush 'TypeSystem's if the module being
-// unloaded was capable of describing a source type. JITted module unloads
-// happen frequently for Objective-C utility functions or the REPL and rely
-// on the persistent variables to stick around.
-const bool should_flush_type_systems =
-module_list.AnyOf([](lldb_private::Module ) {
-  auto *object_file = module.GetObjectFile();
-
-  if (!object_file)
-return true;
-
-  auto type = object_file->GetType();
-
-  return type == ObjectFile::eTypeObjectFile ||
- type == ObjectFile::eTypeExecutable;
-});
-
-if (should_flush_type_systems)
-  m_scratch_type_system_map.Clear();
   }
 }
 

diff  --git a/lldb/test/API/functionalities/rerun_and_expr/Makefile 
b/lldb/test/API/functionalities/rerun_and_expr/Makefile
deleted file mode 100644
index 22f1051530f87..0
--- a/lldb/test/API/functionalities/rerun_and_expr/Makefile
+++ /dev/null
@@ -1 +0,0 @@
-include Makefile.rules

diff  --git a/lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py 
b/lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
deleted file mode 100644
index 19907ce62de69..0
--- a/lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
+++ /dev/null
@@ -1,57 +0,0 @@
-"""
-Test that re-running a process from within the same target
-after rebuilding the executable flushes the scratch TypeSystems
-tied to that process.
-"""
-
-import lldb
-from lldbsuite.test.lldbtest import *
-from lldbsuite.test import lldbutil
-
-class TestRerun(TestBase):
-def test(self):
-"""
-Tests whether re-launching a process without destroying
-the owning target keeps invalid ASTContexts in the
-scratch AST's importer.
-
-We test this by:
-1. Evaluating an expression to import 'struct Foo' into
-   the scratch AST
-2. Change the definition of 'struct Foo' and rebuild the executable
-3. Re-launch the process
-4. Evaluate the same expression in (1). We expect to have only
-   the latest definition of 'struct Foo' in the scratch AST.
-"""
-self.build(dictionary={'CXX_SOURCES':'main.cpp', 'EXE':'a.out'})
-(target, _, _, bkpt) = \
-lldbutil.run_to_source_breakpoint(self, 'return', 
lldb.SBFileSpec('main.cpp'))
-
-target.BreakpointCreateBySourceRegex('return', 
lldb.SBFileSpec('rebuild.cpp', False))
-
-self.expect_expr('foo', result_type='Foo', result_children=[
-ValueCheck(name='m_val', value='42')
-])
-
-self.build(dictionary={'CXX_SOURCES':'rebuild.cpp', 'EXE':'a.out'})
-
-self.runCmd('process launch')
-
-self.expect_expr('foo', result_type='Foo', result_children=[
-ValueCheck(name='Base', children=[
-ValueCheck(name='m_base_val', value='42')
- 

[Lldb-commits] [PATCH] D138834: [lldb] Fix simple template names interaction with debug info declarations

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:765
+m_ast.CreateClassTemplateSpecializationType(class_specialization_decl);
+return clang_type.GetTypeName(/*BaseOnly*/ true);
+  }

Michael137 wrote:
> Ok so what we're doing is:
> 1. Create a `ClassTemplateSpecializationDecl` with an empty basename
> 2. Return the type-name and since the basename is empty we end up with just 
> the template arguments
> 
> Why can't we just call `GetTemplateParametersString(die)` instead of creating 
> this function?
Can confirm that this works locally. Though required moving that function out 
of the DWARFASTParserClang, which seems OK


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138834

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

[Lldb-commits] [PATCH] D138834: [lldb] Fix simple template names interaction with debug info declarations

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:765
+m_ast.CreateClassTemplateSpecializationType(class_specialization_decl);
+return clang_type.GetTypeName(/*BaseOnly*/ true);
+  }

Ok so what we're doing is:
1. Create a `ClassTemplateSpecializationDecl` with an empty basename
2. Return the type-name and since the basename is empty we end up with just the 
template arguments

Why can't we just call `GetTemplateParametersString(die)` instead of creating 
this function?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138834

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

[Lldb-commits] [PATCH] D139102: [AArch64] Inline AArch64TargetParser.def

[Tomas Matheson via Phabricator via lldb-commits]

tmatheson added a comment.
Herald added a subscriber: JDevlieghere.

> And this is the file that was being generated: 
> https://github.com/ziglang/zig/blob/c86589a738e5053a26b2be7d156bcfee1565f00b/lib/std/target/aarch64.zig

Thanks for the heads up. It looks like that file hasn't been updated for a 
couple of years, and doesn't include any recent features, so unless we hear 
otherwise from @andrewrk I think we can go ahead. If some script _is_ manually 
parsing the .def, it ought to be easy enough to update it to read the .cpp 
instead (it probably shouldn't do either).

> Please update the comment in 
> lldb/test/Shell/Commands/command-disassemble-aarch64-extensions.s as well.

I have reordered the list to be alphabetical and updated the comment, added 
tests for recent features that were missed (CSSC, D128 
, RCPC3, THE, and a few others) and used 
CHECK-NEXT.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139102

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

[Lldb-commits] [PATCH] D139102: [AArch64] Inline AArch64TargetParser.def

[Tomas Matheson via Phabricator via lldb-commits]

tmatheson updated this revision to Diff 479598.
tmatheson added a comment.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Update lldb test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139102

Files:
  lldb/test/Shell/Commands/command-disassemble-aarch64-extensions.s
  llvm/include/llvm/Support/AArch64TargetParser.def
  llvm/include/llvm/Support/AArch64TargetParser.h
  llvm/lib/Support/AArch64TargetParser.cpp

Index: llvm/lib/Support/AArch64TargetParser.cpp
===
--- llvm/lib/Support/AArch64TargetParser.cpp
+++ llvm/lib/Support/AArch64TargetParser.cpp
@@ -30,21 +30,18 @@
   if (CPU == "generic")
 return AI.DefaultExts;
 
-  return StringSwitch(CPU)
-#define AARCH64_CPU_NAME(NAME, ARCH_ID, DEFAULT_EXT)   \
-  .Case(NAME, ARCH_ID.DefaultExts | DEFAULT_EXT)
-#include "../../include/llvm/Support/AArch64TargetParser.def"
-  .Default(AArch64::AEK_INVALID);
+  // Note: this now takes cpu aliases into account
+  const CpuInfo  = parseCpu(CPU);
+  return Cpu.Arch.DefaultExts | Cpu.DefaultExtensions;
 }
 
 const AArch64::ArchInfo ::getArchForCpu(StringRef CPU) {
   if (CPU == "generic")
 return ARMV8A;
 
-  return *StringSwitch(CPU)
-#define AARCH64_CPU_NAME(NAME, ARCH_ID, DEFAULT_EXT) .Case(NAME, _ID)
-#include "../../include/llvm/Support/AArch64TargetParser.def"
-  .Default();
+  // Note: this now takes cpu aliases into account
+  const CpuInfo  = parseCpu(CPU);
+  return Cpu.Arch;
 }
 
 const AArch64::ArchInfo ::ArchInfo::findBySubArch(StringRef SubArch) {
@@ -54,28 +51,24 @@
   return AArch64::INVALID;
 }
 
-bool AArch64::getExtensionFeatures(uint64_t Extensions,
+bool AArch64::getExtensionFeatures(uint64_t InputExts,
std::vector ) {
-  if (Extensions == AArch64::AEK_INVALID)
+  if (InputExts == AArch64::AEK_INVALID)
 return false;
 
-#define AARCH64_ARCH_EXT_NAME(NAME, ID, FEATURE, NEGFEATURE)   \
-  if (Extensions & ID) {   \
-const char *feature = FEATURE; \
-/* INVALID and NONE have no feature name. */   \
-if (feature)   \
-  Features.push_back(feature); \
-  }
-#include "../../include/llvm/Support/AArch64TargetParser.def"
+  for (const auto  : Extensions)
+/* INVALID and NONE have no feature name. */
+if ((InputExts & E.ID) && !E.Feature.empty())
+  Features.push_back(E.Feature);
 
   return true;
 }
 
-StringRef AArch64::resolveCPUAlias(StringRef CPU) {
-  return StringSwitch(CPU)
-#define AARCH64_CPU_ALIAS(ALIAS, NAME) .Case(ALIAS, NAME)
-#include "../../include/llvm/Support/AArch64TargetParser.def"
-  .Default(CPU);
+StringRef AArch64::resolveCPUAlias(StringRef Name) {
+  for (const auto  : CpuAliases)
+if (A.Alias == Name)
+  return A.Name;
+  return Name;
 }
 
 StringRef AArch64::getArchExtFeature(StringRef ArchExt) {
Index: llvm/include/llvm/Support/AArch64TargetParser.h
===
--- llvm/include/llvm/Support/AArch64TargetParser.h
+++ llvm/include/llvm/Support/AArch64TargetParser.h
@@ -99,9 +99,62 @@
 };
 
 inline constexpr ExtensionInfo Extensions[] = {
-#define AARCH64_ARCH_EXT_NAME(NAME, ID, FEATURE, NEGFEATURE)   \
-  {NAME, ID, FEATURE, NEGFEATURE},
-#include "AArch64TargetParser.def"
+{"aes", AArch64::AEK_AES, "+aes", "-aes"},
+{"b16b16", AArch64::AEK_B16B16, "+b16b16", "-b16b16"},
+{"bf16", AArch64::AEK_BF16, "+bf16", "-bf16"},
+{"brbe", AArch64::AEK_BRBE, "+brbe", "-brbe"},
+{"crc", AArch64::AEK_CRC, "+crc", "-crc"},
+{"crypto", AArch64::AEK_CRYPTO, "+crypto", "-crypto"},
+{"cssc", AArch64::AEK_CSSC, "+cssc", "-cssc"},
+{"d128", AArch64::AEK_D128, "+d128", "-d128"},
+{"dotprod", AArch64::AEK_DOTPROD, "+dotprod", "-dotprod"},
+{"f32mm", AArch64::AEK_F32MM, "+f32mm", "-f32mm"},
+{"f64mm", AArch64::AEK_F64MM, "+f64mm", "-f64mm"},
+{"flagm", AArch64::AEK_FLAGM, "+flagm", "-flagm"},
+{"fp", AArch64::AEK_FP, "+fp-armv8", "-fp-armv8"},
+{"fp16", AArch64::AEK_FP16, "+fullfp16", "-fullfp16"},
+{"fp16fml", AArch64::AEK_FP16FML, "+fp16fml", "-fp16fml"},
+{"hbc", AArch64::AEK_HBC, "+hbc", "-hbc"},
+{"i8mm", AArch64::AEK_I8MM, "+i8mm", "-i8mm"},
+{"ls64", AArch64::AEK_LS64, "+ls64", "-ls64"},
+{"lse", AArch64::AEK_LSE, "+lse", "-lse"},
+{"lse128", AArch64::AEK_LSE128, "+lse128", "-lse128"},
+{"memtag", AArch64::AEK_MTE, "+mte", "-mte"},
+{"mops", AArch64::AEK_MOPS, "+mops", "-mops"},
+{"pauth", AArch64::AEK_PAUTH, "+pauth", "-pauth"},
+{"pmuv3", AArch64::AEK_PERFMON, "+perfmon", "-perfmon"},
+

[Lldb-commits] [PATCH] D138724: [lldb][Target] Flush the scratch TypeSystem when process gets deleted

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

Broke windows and linux build bots. Checking


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138724

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

[Lldb-commits] [PATCH] D138558: [lldb][DataFormatter] Add std::ranges::ref_view formatter

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

In D138558#3963732 , @labath wrote:

> You may want to check that this kind of automatic dereferencing does not send 
> lldb into a tailspin if the printed data structure is recursive. I know we 
> had problems like that with smart pointer pretty printers.
>
> I'd try some code like:
>
>   #include 
>   #include 
>   
>   struct A {
> std::ranges::ref_view> a;
>   };
>   
>   int main() {
>   std::vector v;
>   v.push_back(A{v});
>   v[0].a = v;
>   // print v ?
>   }

@labath good point

E.g., the following would cause such unbounded recursion (didn't find a good 
way of triggering it using the actual ref_view since it's not default 
constructible, but it's probably doable):

  #include 
  
  namespace std {
  inline namespace __1 {
  namespace ranges {
  template
  struct ref_view {
  T* __range_;
  };
  }
  }
  }
  
  struct Foo {
  std::ranges::ref_view> a;
  };
  
  int main() {
  Foo f;
  std::vector v;
  v.push_back(f);
  
  std::ranges::ref_view> r{.__range_ = };
  f.a = r;
  
  return 0;
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138558

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

[Lldb-commits] [PATCH] D138724: [lldb][Target] Flush the scratch TypeSystem when process gets deleted

[Michael Buch via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4df11394a10b: [lldb][Target] Flush the scratch TypeSystem 
when owning lldb_private::Module… (authored by Michael137).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138724

Files:
  lldb/source/Target/Target.cpp
  lldb/test/API/functionalities/rerun_and_expr/Makefile
  lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
  lldb/test/API/functionalities/rerun_and_expr/main.cpp
  lldb/test/API/functionalities/rerun_and_expr/rebuild.cpp
  lldb/test/API/functionalities/rerun_and_expr_dylib/Makefile
  lldb/test/API/functionalities/rerun_and_expr_dylib/TestRerunAndExprDylib.py
  lldb/test/API/functionalities/rerun_and_expr_dylib/lib.cpp
  lldb/test/API/functionalities/rerun_and_expr_dylib/main.cpp
  lldb/test/API/functionalities/rerun_and_expr_dylib/rebuild.cpp

Index: lldb/test/API/functionalities/rerun_and_expr_dylib/rebuild.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/rerun_and_expr_dylib/rebuild.cpp
@@ -0,0 +1,7 @@
+struct Base {
+  int m_base_val = 42;
+};
+
+LLDB_DYLIB_EXPORT struct Foo : public Base {
+  int m_derived_val = 137;
+} global_foo;
Index: lldb/test/API/functionalities/rerun_and_expr_dylib/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/rerun_and_expr_dylib/main.cpp
@@ -0,0 +1,12 @@
+#include 
+#include 
+
+extern struct Foo imported;
+
+int main() {
+  void *handle = dlopen("libfoo.dylib", RTLD_NOW);
+  struct Foo *foo = (struct Foo *)dlsym(handle, "global_foo");
+  assert(foo != nullptr);
+
+  return 0;
+}
Index: lldb/test/API/functionalities/rerun_and_expr_dylib/lib.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/rerun_and_expr_dylib/lib.cpp
@@ -0,0 +1 @@
+LLDB_DYLIB_EXPORT struct Foo { int m_val = 42; } global_foo;
Index: lldb/test/API/functionalities/rerun_and_expr_dylib/TestRerunAndExprDylib.py
===
--- /dev/null
+++ lldb/test/API/functionalities/rerun_and_expr_dylib/TestRerunAndExprDylib.py
@@ -0,0 +1,72 @@
+"""
+Test that re-running a process from within the same target
+after rebuilding the a dynamic library flushes the scratch
+TypeSystems tied to that process.
+"""
+
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestRerun(TestBase):
+def test(self):
+"""
+Tests whether re-launching a process without destroying
+the owning target keeps invalid ASTContexts in the
+scratch AST's importer.
+
+We test this by:
+1. Evaluating an expression to import 'struct Foo' into
+   the scratch AST
+2. Change the definition of 'struct Foo' and rebuild the dylib
+3. Re-launch the process
+4. Evaluate the same expression in (1). We expect to have only
+   the latest definition of 'struct Foo' in the scratch AST.
+"""
+
+# Build a.out
+self.build(dictionary={'EXE':'a.out',
+   'CXX_SOURCES':'main.cpp'})
+
+# Build libfoo.dylib
+self.build(dictionary={'DYLIB_CXX_SOURCES':'lib.cpp',
+   'DYLIB_ONLY':'YES',
+   'DYLIB_NAME':'foo',
+   'USE_LIBDL':'1',
+   'LD_EXTRAS':'-L.'})
+
+(target, _, _, bkpt) = \
+lldbutil.run_to_source_breakpoint(self, 'return', lldb.SBFileSpec('main.cpp'))
+
+self.expect_expr('*foo', result_type='Foo', result_children=[
+ValueCheck(name='m_val', value='42')
+])
+
+# Re-build libfoo.dylib
+self.build(dictionary={'DYLIB_CXX_SOURCES':'rebuild.cpp',
+   'DYLIB_ONLY':'YES',
+   'DYLIB_NAME':'foo',
+   'USE_LIBDL':'1',
+   'LD_EXTRAS':'-L.'})
+
+self.runCmd('process launch')
+(target, _, _, bkpt) = \
+lldbutil.run_to_source_breakpoint(self, 'return', lldb.SBFileSpec('main.cpp'))
+
+self.expect_expr('*foo', result_type='Foo', result_children=[
+ValueCheck(name='Base', children=[
+ValueCheck(name='m_base_val', value='42')
+]),
+ValueCheck(name='m_derived_val', value='137')
+])
+
+self.filecheck("target module dump ast", __file__)
+
+# The new definition 'struct Foo' is in the scratch AST
+# CHECK:  |-CXXRecordDecl {{.*}} struct Foo definition
+# CHECK:  | |-public 'Base'
+# CHECK-NEXT: | `-FieldDecl {{.*}} m_derived_val 'int'
+# CHECK-NEXT: `-CXXRecordDecl {{.*}} 

[Lldb-commits] [PATCH] D139083: [lldb][Module][NFC] Add ModuleList::AnyOf

[Michael Buch via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5941858efdca: [lldb][Module][NFC] Add ModuleList::AnyOf 
(authored by Michael137).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139083

Files:
  lldb/include/lldb/Core/ModuleList.h
  lldb/source/Core/ModuleList.cpp


Index: lldb/source/Core/ModuleList.cpp
===
--- lldb/source/Core/ModuleList.cpp
+++ lldb/source/Core/ModuleList.cpp
@@ -1074,3 +1074,16 @@
   break;
   }
 }
+
+bool ModuleList::AnyOf(
+std::function const )
+const {
+  std::lock_guard guard(m_modules_mutex);
+  for (const auto _sp : m_modules) {
+assert(module_sp != nullptr);
+if (callback(*module_sp))
+  return true;
+  }
+
+  return false;
+}
Index: lldb/include/lldb/Core/ModuleList.h
===
--- lldb/include/lldb/Core/ModuleList.h
+++ lldb/include/lldb/Core/ModuleList.h
@@ -473,6 +473,13 @@
   void ForEach(std::function const
) const;
 
+  /// Returns true if 'callback' returns true for one of the modules
+  /// in this ModuleList.
+  ///
+  /// This function is thread-safe.
+  bool AnyOf(
+  std::function const ) const;
+
 protected:
   // Class typedefs.
   typedef std::vector


Index: lldb/source/Core/ModuleList.cpp
===
--- lldb/source/Core/ModuleList.cpp
+++ lldb/source/Core/ModuleList.cpp
@@ -1074,3 +1074,16 @@
   break;
   }
 }
+
+bool ModuleList::AnyOf(
+std::function const )
+const {
+  std::lock_guard guard(m_modules_mutex);
+  for (const auto _sp : m_modules) {
+assert(module_sp != nullptr);
+if (callback(*module_sp))
+  return true;
+  }
+
+  return false;
+}
Index: lldb/include/lldb/Core/ModuleList.h
===
--- lldb/include/lldb/Core/ModuleList.h
+++ lldb/include/lldb/Core/ModuleList.h
@@ -473,6 +473,13 @@
   void ForEach(std::function const
) const;
 
+  /// Returns true if 'callback' returns true for one of the modules
+  /// in this ModuleList.
+  ///
+  /// This function is thread-safe.
+  bool AnyOf(
+  std::function const ) const;
+
 protected:
   // Class typedefs.
   typedef std::vector
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 5941858 - [lldb][Module][NFC] Add ModuleList::AnyOf

[Michael Buch via lldb-commits]

Author: Michael Buch
Date: 2022-12-02T10:52:40Z
New Revision: 5941858efdca72425c29bf043d3ec84e7fec6f62

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

LOG: [lldb][Module][NFC] Add ModuleList::AnyOf

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

Added: 


Modified: 
lldb/include/lldb/Core/ModuleList.h
lldb/source/Core/ModuleList.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/ModuleList.h 
b/lldb/include/lldb/Core/ModuleList.h
index 3ae56f17db744..631d7889f3679 100644
--- a/lldb/include/lldb/Core/ModuleList.h
+++ b/lldb/include/lldb/Core/ModuleList.h
@@ -473,6 +473,13 @@ class ModuleList {
   void ForEach(std::function const
) const;
 
+  /// Returns true if 'callback' returns true for one of the modules
+  /// in this ModuleList.
+  ///
+  /// This function is thread-safe.
+  bool AnyOf(
+  std::function const ) const;
+
 protected:
   // Class typedefs.
   typedef std::vector

diff  --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp
index 991163c63dc50..5aa58d9bba6bd 100644
--- a/lldb/source/Core/ModuleList.cpp
+++ b/lldb/source/Core/ModuleList.cpp
@@ -1074,3 +1074,16 @@ void ModuleList::ForEach(
   break;
   }
 }
+
+bool ModuleList::AnyOf(
+std::function const )
+const {
+  std::lock_guard guard(m_modules_mutex);
+  for (const auto _sp : m_modules) {
+assert(module_sp != nullptr);
+if (callback(*module_sp))
+  return true;
+  }
+
+  return false;
+}



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

[Lldb-commits] [PATCH] D139082: [lldb][Module] Document ModuleList::ForEach and assert nullness

[Michael Buch via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG83599000e1f4: [lldb][Module] Document ModuleList::ForEach 
and assert nullness (authored by Michael137).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139082

Files:
  lldb/include/lldb/Core/ModuleList.h
  lldb/source/Core/ModuleList.cpp


Index: lldb/source/Core/ModuleList.cpp
===
--- lldb/source/Core/ModuleList.cpp
+++ lldb/source/Core/ModuleList.cpp
@@ -1067,9 +1067,10 @@
 void ModuleList::ForEach(
 std::function const ) const {
   std::lock_guard guard(m_modules_mutex);
-  for (const auto  : m_modules) {
+  for (const auto _sp : m_modules) {
+assert(module_sp != nullptr);
 // If the callback returns false, then stop iterating and break out
-if (!callback(module))
+if (!callback(module_sp))
   break;
   }
 }
Index: lldb/include/lldb/Core/ModuleList.h
===
--- lldb/include/lldb/Core/ModuleList.h
+++ lldb/include/lldb/Core/ModuleList.h
@@ -464,6 +464,12 @@
 
   static bool RemoveSharedModuleIfOrphaned(const Module *module_ptr);
 
+  /// Applies 'callback' to each module in this ModuleList.
+  /// If 'callback' returns false, iteration terminates.
+  /// The 'module_sp' passed to 'callback' is guaranteed to
+  /// be non-null.
+  ///
+  /// This function is thread-safe.
   void ForEach(std::function const
) const;
 


Index: lldb/source/Core/ModuleList.cpp
===
--- lldb/source/Core/ModuleList.cpp
+++ lldb/source/Core/ModuleList.cpp
@@ -1067,9 +1067,10 @@
 void ModuleList::ForEach(
 std::function const ) const {
   std::lock_guard guard(m_modules_mutex);
-  for (const auto  : m_modules) {
+  for (const auto _sp : m_modules) {
+assert(module_sp != nullptr);
 // If the callback returns false, then stop iterating and break out
-if (!callback(module))
+if (!callback(module_sp))
   break;
   }
 }
Index: lldb/include/lldb/Core/ModuleList.h
===
--- lldb/include/lldb/Core/ModuleList.h
+++ lldb/include/lldb/Core/ModuleList.h
@@ -464,6 +464,12 @@
 
   static bool RemoveSharedModuleIfOrphaned(const Module *module_ptr);
 
+  /// Applies 'callback' to each module in this ModuleList.
+  /// If 'callback' returns false, iteration terminates.
+  /// The 'module_sp' passed to 'callback' is guaranteed to
+  /// be non-null.
+  ///
+  /// This function is thread-safe.
   void ForEach(std::function const
) const;
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 4df1139 - [lldb][Target] Flush the scratch TypeSystem when owning lldb_private::Module gets unloaded

[Michael Buch via lldb-commits]

Author: Michael Buch
Date: 2022-12-02T10:52:41Z
New Revision: 4df11394a10b3b15d2fb9bde8b831cf68785aa45

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

LOG: [lldb][Target] Flush the scratch TypeSystem when owning 
lldb_private::Module gets unloaded

**Summary**

This patch addresses #59128, where LLDB would crash when evaluating
importing a type that has been imported before into the same target.
The proposed solution is to clear the scratch AST (and associated
persistent variables, `ClangASTImporter`, etc.) whenever a module that
could've owned one of the stale `TypeSystem`s gets unloaded/destroyed.

Details:
1. The first time we evaluate the expression we import the decl for Foo into 
the Targets scratch AST
   context (lives in m_scratch_type_system_map). During this process we also 
create a ClangASTImporter
   that lives in the ClangPersistentVariables::m_ast_importer_sp. This importer 
has decl tracking
   structures which reference the source AST that the decl got imported from. 
This importer also gets
   re-used for all calls to DeportType (which we use to copy the final decl 
into the Targets scratch AST).
2. Rebuilding the executable triggers a tear-down of the Module that was 
backing the ASTContext that
   we originally got the Foo decl from (which lived in the 
Module::m_type_system_map). However, the Target’s scratch AST lives on.
3. Re-running the same expression will now create a new ASTImporterDelegate 
where the destination TranslationUnitDecl is
   the same as the one from step (1).
4. When importing the new Foo decl we first try to find it in the destination 
DeclContext, which happens to be
   the scratch destination TranslationUnitDecl. The `Foo` decl exists in this 
context since we copied it into
   the scratch AST in the first run. The ASTImporter then queries LLDB for the 
origin of that decl. Using the
   same persistent variable ClangASTImporter we claim the decl has an origin in 
the AST context that got torn
   down with the Module. This faulty origin leads to a use-after-free.

**Testing**

- Added API test

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

Added: 
lldb/test/API/functionalities/rerun_and_expr/Makefile
lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
lldb/test/API/functionalities/rerun_and_expr/main.cpp
lldb/test/API/functionalities/rerun_and_expr/rebuild.cpp
lldb/test/API/functionalities/rerun_and_expr_dylib/Makefile
lldb/test/API/functionalities/rerun_and_expr_dylib/TestRerunAndExprDylib.py
lldb/test/API/functionalities/rerun_and_expr_dylib/lib.cpp
lldb/test/API/functionalities/rerun_and_expr_dylib/main.cpp
lldb/test/API/functionalities/rerun_and_expr_dylib/rebuild.cpp

Modified: 
lldb/source/Target/Target.cpp

Removed: 




diff  --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 22071b76a805b..8a197bf6e502a 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -1682,6 +1682,30 @@ void Target::ModulesDidUnload(ModuleList _list, 
bool delete_locations) {
 m_breakpoint_list.UpdateBreakpoints(module_list, false, delete_locations);
 m_internal_breakpoint_list.UpdateBreakpoints(module_list, false,
  delete_locations);
+
+// If a module was torn down it will have torn down the 'TypeSystemClang's
+// that we used as source 'ASTContext's for the persistent variables in
+// the current target. Those would now be unsafe to access because the
+// 'DeclOrigin' are now possibly stale. Thus clear all persistent
+// variables. We only want to flush 'TypeSystem's if the module being
+// unloaded was capable of describing a source type. JITted module unloads
+// happen frequently for Objective-C utility functions or the REPL and rely
+// on the persistent variables to stick around.
+const bool should_flush_type_systems =
+module_list.AnyOf([](lldb_private::Module ) {
+  auto *object_file = module.GetObjectFile();
+
+  if (!object_file)
+return true;
+
+  auto type = object_file->GetType();
+
+  return type == ObjectFile::eTypeObjectFile ||
+ type == ObjectFile::eTypeExecutable;
+});
+
+if (should_flush_type_systems)
+  m_scratch_type_system_map.Clear();
   }
 }
 

diff  --git a/lldb/test/API/functionalities/rerun_and_expr/Makefile 
b/lldb/test/API/functionalities/rerun_and_expr/Makefile
new file mode 100644
index 0..22f1051530f87
--- /dev/null
+++ b/lldb/test/API/functionalities/rerun_and_expr/Makefile
@@ -0,0 +1 @@
+include Makefile.rules

diff  --git a/lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py 

[Lldb-commits] [lldb] 8359900 - [lldb][Module] Document ModuleList::ForEach and assert nullness

[Michael Buch via lldb-commits]

Author: Michael Buch
Date: 2022-12-02T10:52:39Z
New Revision: 83599000e1f4b30d93b8f4509011b9b68d722835

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

LOG: [lldb][Module] Document ModuleList::ForEach and assert nullness

Currently all callsites already assume the pointer is non-null.
This patch just asserts this assumption.

This is practically enforced by `ModuleList::Append`
which won't add `nullptr`s to `m_modules`.

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

Added: 


Modified: 
lldb/include/lldb/Core/ModuleList.h
lldb/source/Core/ModuleList.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/ModuleList.h 
b/lldb/include/lldb/Core/ModuleList.h
index 1630a6d58c570..3ae56f17db744 100644
--- a/lldb/include/lldb/Core/ModuleList.h
+++ b/lldb/include/lldb/Core/ModuleList.h
@@ -464,6 +464,12 @@ class ModuleList {
 
   static bool RemoveSharedModuleIfOrphaned(const Module *module_ptr);
 
+  /// Applies 'callback' to each module in this ModuleList.
+  /// If 'callback' returns false, iteration terminates.
+  /// The 'module_sp' passed to 'callback' is guaranteed to
+  /// be non-null.
+  ///
+  /// This function is thread-safe.
   void ForEach(std::function const
) const;
 

diff  --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp
index f87a2856ab16a..991163c63dc50 100644
--- a/lldb/source/Core/ModuleList.cpp
+++ b/lldb/source/Core/ModuleList.cpp
@@ -1067,9 +1067,10 @@ bool ModuleList::LoadScriptingResourcesInTarget(Target 
*target,
 void ModuleList::ForEach(
 std::function const ) const {
   std::lock_guard guard(m_modules_mutex);
-  for (const auto  : m_modules) {
+  for (const auto _sp : m_modules) {
+assert(module_sp != nullptr);
 // If the callback returns false, then stop iterating and break out
-if (!callback(module))
+if (!callback(module_sp))
   break;
   }
 }



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

[Lldb-commits] [PATCH] D138834: [lldb] Fix simple template names interaction with debug info declarations

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h:91
 
+  lldb_private::ConstString
+  GetForwardDeclarationDIETemplateParams(const DWARFDIE ) override;

Can we add a docstring?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2988
+template_params =
+dwarf_ast->GetForwardDeclarationDIETemplateParams(die);
+}

So after this call we'll have an unnamed `ClassTemplateDecl` hanging off the 
AST? Doing this inside `FindDefinitionTypeForDWARFDeclContext` instead of 
`ParseStructureLikeDIE` feels a bit off. But will have to understand this 
change a bit deeper to know whether there is something we can do about that.

As a side-note, we seem to be accumulating a lot of blocks around LLDB that 
look like:
```
// Do X because -gsimple-template-names
if (name.contains('<')) {
   // Do something
}
```
Still wonder if there isn't some flag that can be set on, e.g., a CU, that 
would tell us this.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3063
+  // With -gsimple-template-names, the DIE name may not contain the 
template
+  // parameters. If we've the declaration has template parameters but
+  // doesn't contain '<', check that the child template parameters match.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138834

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

[Lldb-commits] [PATCH] D139158: [LLDB][LoongArch] Make software single stepping work

[Hui Li via Phabricator via lldb-commits]

lh03061238 added a comment.

In D139158#3965478 , @jingham wrote:

> Note that the two most common uses of single step in lldb are 1) stepping 
> over the instruction under a breakpoint (which may be a branch) and 2) 
> stepping to the next instruction from a branch instruction.  So stepping 
> won't work correctly till you get single step past a branch instruction 
> working.  But you probably knew that...

Yes, you're right.
This patch just make the single stepping to work, only the non-jump 
instructions are supported, the effect is just like pc + 4. Support for jump 
instructions will be added in next patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139158

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

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

[Thorsten via Phabricator via lldb-commits]

tschuett added a comment.

Then Sarif was a distraction. Still to reduce boilerplate and for A/B testing:

  enum class DiagnosticMode {
Legacy,
UserOriented,
Default = Legacy
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138939

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