[Lldb-commits] [PATCH] D107079: [lldb] Add an option for emitting LLDB logs as JSON

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

Friendly ping!


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

https://reviews.llvm.org/D107079

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

[Lldb-commits] [PATCH] D108510: [lldb] Allow to register frame recognizers applied beyond the first instruction

[Roman Podoliaka via Phabricator via lldb-commits]

malor updated this revision to Diff 368565.
malor added a comment.

Updated the description of the flag to incorporate Jim's feedback.


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

https://reviews.llvm.org/D108510

Files:
  lldb/source/Commands/CommandObjectFrame.cpp
  lldb/source/Commands/Options.td
  lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py

Index: lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
===
--- lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
+++ lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
@@ -205,6 +205,64 @@
 self.expect("frame recognizer info 0",
 substrs=['frame 0 is recognized by recognizer.MyFrameRecognizer'])
 
+@skipUnlessDarwin
+def test_frame_recognizer_not_only_first_instruction(self):
+self.build()
+exe = self.getBuildArtifact("a.out")
+
+# Clear internal & plugins recognizers that get initialized at launch.
+self.runCmd("frame recognizer clear")
+
+self.runCmd("command script import " + os.path.join(self.getSourceDir(), "recognizer.py"))
+
+self.expect("frame recognizer list",
+substrs=['no matching results found.'])
+
+# Create a target.
+target, process, thread, _ = lldbutil.run_to_name_breakpoint(self, "foo",
+ exe_name = exe)
+
+# Move the PC one instruction further.
+self.runCmd("next")
+
+# Add a frame recognizer in that target.
+self.runCmd("frame recognizer add -l recognizer.MyFrameRecognizer -s a.out -n foo -n bar")
+
+# It's not applied to foo(), because frame's PC is not at the first instruction of the function.
+self.expect("frame recognizer info 0",
+substrs=['frame 0 not recognized by any recognizer'])
+
+# Add a frame recognizer with --first-instruction-only=true.
+self.runCmd("frame recognizer clear")
+
+self.runCmd("frame recognizer add -l recognizer.MyFrameRecognizer -s a.out -n foo -n bar --first-instruction-only=true")
+
+# It's not applied to foo(), because frame's PC is not at the first instruction of the function.
+self.expect("frame recognizer info 0",
+substrs=['frame 0 not recognized by any recognizer'])
+
+# Now add a frame recognizer with --first-instruction-only=false.
+self.runCmd("frame recognizer clear")
+
+self.runCmd("frame recognizer add -l recognizer.MyFrameRecognizer -s a.out -n foo -n bar --first-instruction-only=false")
+
+# This time it should recognize the frame.
+self.expect("frame recognizer info 0",
+substrs=['frame 0 is recognized by recognizer.MyFrameRecognizer'])
+
+opts = lldb.SBVariablesOptions()
+opts.SetIncludeRecognizedArguments(True)
+frame = thread.GetSelectedFrame()
+variables = frame.GetVariables(opts)
+
+self.assertEqual(variables.GetSize(), 2)
+self.assertEqual(variables.GetValueAtIndex(0).name, "a")
+self.assertEqual(variables.GetValueAtIndex(0).signed, 42)
+self.assertEqual(variables.GetValueAtIndex(0).GetValueType(), lldb.eValueTypeVariableArgument)
+self.assertEqual(variables.GetValueAtIndex(1).name, "b")
+self.assertEqual(variables.GetValueAtIndex(1).signed, 56)
+self.assertEqual(variables.GetValueAtIndex(1).GetValueType(), lldb.eValueTypeVariableArgument)
+
 @no_debug_info_test
 def test_frame_recognizer_delete_invalid_arg(self):
 self.expect("frame recognizer delete a", error=True,
@@ -226,3 +284,12 @@
 substrs=["error: '-1' is not a valid frame index."])
 self.expect("frame recognizer info 4294967297", error=True,
 substrs=["error: '4294967297' is not a valid frame index."])
+
+@no_debug_info_test
+def test_frame_recognizer_add_invalid_arg(self):
+self.expect("frame recognizer add -f", error=True,
+substrs=["error: last option requires an argument"])
+self.expect("frame recognizer add -f -1", error=True,
+substrs=["error: invalid boolean value '-1' passed for -f option"])
+self.expect("frame recognizer add -f foo", error=True,
+substrs=["error: invalid boolean value 'foo' passed for -f option"])
Index: lldb/source/Commands/Options.td
===
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -404,6 +404,13 @@
 Desc<"Give the name of a Python class to use for this frame recognizer.">;
   def frame_recognizer_regex : Option<"regex", "x">,
 Desc<"Function name and module name are actually regular expressions.">;
+  def frame_recognizer_first_instruction_only : Option<"firs

[Lldb-commits] [PATCH] D108545: [LLDB][GUI] Add initial searcher support

[Omar Emara via Phabricator via lldb-commits]

OmarEmaraDev added a comment.

@clayborg I see a number of issues with using forms directly:

- The user will have to bother with form navigation. If we have a text field 
and a choice field, the user will have to switch between them using Tab and 
Shift+Tab in order to explore, rectify, and select a match. This is 
significantly more work then just typing without worry, selecting using arrow 
keys, and executing using enter.
- Not all users will use the OPT+Enter keys, it is likely that once the user 
sees a button that says "Select", the user will try to navigate to that button 
ignoring the tip in the windows border. Which is more work than just pressing 
enter.
- Updating the choices will require that we materialize all choices into text, 
this can work for the common completion searcher, but other searchers will 
require actual processing that might slow down the update process. The searcher 
window only materialize the match that it draws, so it can draw things much 
faster. We could update the choices field to accept a callback to compute its 
contents, but this will complicate the code, so it is worth considering if this 
is really needed.
- Practically all search dialogs work in a manner similar to the searcher 
window, like browser search bar, editor search dialogs, and the like. I don't 
recall seeing a search dialog that is implemented as a form.
- We want the ability to do custom styling. One thing I was considering is 
coloring the matched substring a different color, if we use a choice field, 
then we give up on custom styling.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108545

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

[Lldb-commits] [PATCH] D108233: WIP: Add minidump save-core functionality to ELF object files

[Andrej Korman via Phabricator via lldb-commits]

Aj0SK marked 9 inline comments as done.
Aj0SK added a comment.

Thanks for the review! Some requested changes need to be clarified for me. I 
have a problem mainly with successfully registering the Plugin when it inherits 
only from PluginInterface.




Comment at: lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp:30-31
+  GetPluginNameStatic(), GetPluginDescriptionStatic(), CreateInstance,
+  /*create_memory_callback=*/nullptr,
+  /*get_module_specifications=*/nullptr, SaveCore);
+}

clayborg wrote:
> We have two options here:
> 1 - if we want to pass NULL for any callbacks, then we need to fix all 
> callsites to that use 
> PluginManager::GetObjectFileCreateMemoryCallbackAtIndex(...) and 
> PluginManager::GetObjectFileGetModuleSpecificationsCallbackAtIndex(...) to 
> have this done inside of new plug-in manager 
> PluginManager::CreateMemoryCallbackAtIndex(...) and 
> PluginManager::GetModuleSpecifications(...) where these new functions iterate 
> over all plug-ins and skip any plug-ins that have NULL callbacks.
> 2 - add valid create_memory_callback and get_module_specifications callbacks 
> that return invalid values
> 
> Otherwise this plug-in might stop the iteration early for existing callers of 
> PluginManager::GetObjectFileCreateMemoryCallbackAtIndex(...) and 
> PluginManager::GetObjectFileGetModuleSpecificationsCallbackAtIndex(...).
I would prefer modifying code outside of this functionality as little as 
possible so I would stick to the second option. However, maybe returning the 
invalid values is also not the best solution. Also, does this option works with 
ObjectFileMinidump inheriting from PluginInterface? I think that I am unable to 
use the same PluginManager::RegisterPlugin method, so not sure how this plugin 
gets to ObjectFilesInstances... Maybe I am missing out something. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108233

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

[Lldb-commits] [PATCH] D108510: [lldb] Allow to register frame recognizers applied beyond the first instruction

[Jim Ingham via Phabricator via lldb-commits]

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

LGTM


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

https://reviews.llvm.org/D108510

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

[Lldb-commits] [PATCH] D108257: Add type to the output for FieldDecl when logging in ClangASTSource::layoutRecordType

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

In D108257#2950994 , @aprantl wrote:

> If getType() doesn't trigger type completion then this LGTM!

AFAICT the call to `getType` end up in `ValueDecl`:

  QualType getType() const { return DeclType; }

and this is just returning a field.


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

https://reviews.llvm.org/D108257

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

[Lldb-commits] [PATCH] D108717: Fix Reference case for TypeSystemClang::GetChildCompilerTypeAtIndex(...) to avoid possible invalid cast

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik created this revision.
shafik added reviewers: aprantl, teemperor, werat.
Herald added a subscriber: arphaman.
shafik requested review of this revision.

D103532  modified this case to preserve type 
sugar but we can end up with cases where the cast is not valid. I modified the 
code to use `GetLValueReferenceType(type)`/`GetRValueReferenceType(type)` 
respectively.

In the case being tested in the test case we end with the following type:

  TypedefType 0x7f8a710202f0 'std::__compressed_pair_elem, class 
std::allocator >::__rep, 0, false>::const_reference' sugar
  |-Typedef 0x7f8a71020280 'const_reference'
  `-LValueReferenceType 0x7f8a71020250 'const struct std::basic_string, class std::allocator >::__rep &'
  ...

which can't be cast to `ReferenceType`.


https://reviews.llvm.org/D108717

Files:
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/test/API/lang/cpp/null_references/Makefile
  lldb/test/API/lang/cpp/null_references/TestNullReferences.py
  lldb/test/API/lang/cpp/null_references/main.cpp


Index: lldb/test/API/lang/cpp/null_references/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/null_references/main.cpp
@@ -0,0 +1,12 @@
+#include 
+
+int f(std::string &instr) {
+  return instr.size(); // break here
+}
+
+int main() {
+  std::string *bad_str = (std::string *)nullptr;
+  // This is undefined behavior. We are purposefully trying to hit
+  // GetCrashingDereference(...)
+  return f(*bad_str);
+}
Index: lldb/test/API/lang/cpp/null_references/TestNullReferences.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/null_references/TestNullReferences.py
@@ -0,0 +1,14 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestNullReferences(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def test(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here", 
lldb.SBFileSpec("main.cpp"))
+
+self.runCmd("continue")
Index: lldb/test/API/lang/cpp/null_references/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/cpp/null_references/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -6502,10 +6502,13 @@
   case clang::Type::LValueReference:
   case clang::Type::RValueReference:
 if (idx_is_valid) {
-  const clang::ReferenceType *reference_type =
-  llvm::cast(GetQualType(type).getTypePtr());
-  CompilerType pointee_clang_type =
-  GetType(reference_type->getPointeeType());
+  CompilerType pointee_clang_type;
+
+  if (parent_type_class == clang::Type::LValueReference)
+pointee_clang_type = GetLValueReferenceType(type).GetPointeeType();
+  else
+pointee_clang_type = GetRValueReferenceType(type).GetPointeeType();
+
   if (transparent_pointers && pointee_clang_type.IsAggregateType()) {
 child_is_deref_of_parent = false;
 bool tmp_child_is_deref_of_parent = false;


Index: lldb/test/API/lang/cpp/null_references/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/null_references/main.cpp
@@ -0,0 +1,12 @@
+#include 
+
+int f(std::string &instr) {
+  return instr.size(); // break here
+}
+
+int main() {
+  std::string *bad_str = (std::string *)nullptr;
+  // This is undefined behavior. We are purposefully trying to hit
+  // GetCrashingDereference(...)
+  return f(*bad_str);
+}
Index: lldb/test/API/lang/cpp/null_references/TestNullReferences.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/null_references/TestNullReferences.py
@@ -0,0 +1,14 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestNullReferences(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def test(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here", lldb.SBFileSpec("main.cpp"))
+
+self.runCmd("continue")
Index: lldb/test/API/lang/cpp/null_references/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/cpp/null_references/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang

[Lldb-commits] [PATCH] D107521: [lldb/Plugins] Introduce Scripted Interface Factory

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 368704.
mib marked 7 inline comments as done.
mib edited the summary of this revision.
mib added a reviewer: jingham.
mib added a comment.

Address @JDevlieghere and implement the generic 
`ScriptedPythonInterface::Dispatch` method that @jingham suggested.

This method uses templated variadic arguments and a templated return type to be 
able to call any python method. This allows removing the type-specific helper 
functions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107521

Files:
  lldb/bindings/python/python-wrapper.swig
  lldb/include/lldb/Interpreter/ScriptedInterface.h
  lldb/include/lldb/Interpreter/ScriptedProcessInterface.h
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
  
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
  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
@@ -12,7 +12,7 @@
 
 #include "Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h"
 #include "Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h"
-#include "lldb/API/SBError.h"
+
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/HostInfo.h"
 
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
===
--- /dev/null
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
@@ -0,0 +1,153 @@
+//===-- ScriptedPythonInterface.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_SCRIPTEDPYTHONINTERFACE_H
+#define LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_SCRIPTEDPYTHONINTERFACE_H
+
+#include "lldb/Host/Config.h"
+
+#if LLDB_ENABLE_PYTHON
+
+#include "lldb/Interpreter/ScriptedInterface.h"
+#include "lldb/Utility/DataBufferHeap.h"
+
+#include "PythonDataObjects.h"
+#include "SWIGPythonBridge.h"
+#include "ScriptInterpreterPythonImpl.h"
+
+namespace lldb_private {
+class ScriptInterpreterPythonImpl;
+class ScriptedPythonInterface : virtual public ScriptedInterface {
+public:
+  ScriptedPythonInterface(ScriptInterpreterPythonImpl &interpreter);
+  virtual ~ScriptedPythonInterface() = default;
+
+protected:
+  template 
+  T ExtractValueFromPythonObject(python::PythonObject &p, Status &error) {
+return p.CreateStructuredObject();
+  }
+
+  template <>
+  Status ExtractValueFromPythonObject(python::PythonObject &p,
+  Status &error) {
+lldb::SBError *sb_error = reinterpret_cast(
+LLDBSWIGPython_CastPyObjectToSBError(p.get()));
+
+if (!sb_error)
+  error.SetErrorString("Couldn't cast lldb::SBError to lldb::Status.");
+else
+  error = m_interpreter.GetStatusFromSBError(*sb_error);
+
+return error;
+  }
+
+  template <>
+  lldb::DataExtractorSP
+  ExtractValueFromPythonObject(python::PythonObject &p,
+  Status &error) {
+lldb::SBData *sb_data = reinterpret_cast(
+LLDBSWIGPython_CastPyObjectToSBData(p.get()));
+
+if (!sb_data) {
+  error.SetErrorString("Couldn't cast lldb::SBError to lldb::Status.");
+  return nullptr;
+}
+
+return m_interpreter.GetDataExtractorFromSBData(*sb_data);
+  }
+
+  template 
+  T Dispatch(llvm::StringRef method_name, Status &error, Args... args) {
+using namespace python;
+using Locker = ScriptInterpreterPythonImpl::Locker;
+
+Locker py_lock(&m_interpreter, Locker::AcquireLock | Locker::NoSTDIN,
+   Locker::FreeLock);
+
+auto error_with_message = [&method_name, &error](llvm::StringRef message) {
+  error.SetErrorStringWithFormatv(
+  "ScriptedPythonInterface::{0} ({1}) ERROR = {2}", __FUNCTION__,
+  method_name, message);
+  return T();
+};
+
+if (!m_object_instance_sp)
+  return error_with_message("Python object ill-formed");
+
+PythonObject implementor(PyRefType::Borrowed,
+ (PyObject *)m_object_instance_sp->Get

[Lldb-commits] [PATCH] D108717: Fix Reference case for TypeSystemClang::GetChildCompilerTypeAtIndex(...) to avoid possible invalid cast

[Adrian Prantl via Phabricator via lldb-commits]

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

That looks obviously correct, thanks!




Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:6506
+  CompilerType pointee_clang_type;
+
+  if (parent_type_class == clang::Type::LValueReference)

maybe use  `= (parent_type_class == clang::Type::LValueReference) ?  
GetLValueReferenceType(type).GetPointeeType() : 
GetRValueReferenceType(type).GetPointeeType()` ?


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

https://reviews.llvm.org/D108717

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

[Lldb-commits] [PATCH] D108717: Fix Reference case for TypeSystemClang::GetChildCompilerTypeAtIndex(...) to avoid possible invalid cast

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: lldb/test/API/lang/cpp/null_references/TestNullReferences.py:14
+
+self.runCmd("continue")

Why is the `continue` necessary and what is it actually that's triggering the 
crash?


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

https://reviews.llvm.org/D108717

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

[Lldb-commits] [PATCH] D108717: Fix Reference case for TypeSystemClang::GetChildCompilerTypeAtIndex(...) to avoid possible invalid cast

[Raphael Isemann via Phabricator via lldb-commits]

teemperor requested changes to this revision.
teemperor added a comment.
This revision now requires changes to proceed.

Thanks for the patch!

So IIUC correctly this fixes a crash when calling `Dereference` on an SBValue 
that is of type `SomeTypedef` with `typedef int& SomeTypedef`? If yes, then I 
think the test here could just be another type+assert in 
`TestCPPDereferencingReferences.py`. The test here is a bit expensive and seems 
to depend on a specific libc++ version (-> it will also always pass on Linux), 
even though this is just a TypeSystemClang patch.




Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:6505
 if (idx_is_valid) {
-  const clang::ReferenceType *reference_type =
-  llvm::cast(GetQualType(type).getTypePtr());
-  CompilerType pointee_clang_type =
-  GetType(reference_type->getPointeeType());
+  CompilerType pointee_clang_type;
+

I really like the idea of reusing the TypeSystemClang functions here (we should 
do this everywhere I think). FWIW, I think this can all just be `CompilerType 
pointee_clang_type = GetNonReferenceType(type);` From the code below 
`GetPointeeType` would also work. We already call the variable here `pointee` 
type so I don't think calling references pointers here will hurt too much, so I 
think both is fine.



Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:6508
+  if (parent_type_class == clang::Type::LValueReference)
+pointee_clang_type = GetLValueReferenceType(type).GetPointeeType();
+  else

I think the logic here is reversed? `type` is a reference type (potentially 
behind a typedef). `GetLValueReferenceType` on `type` returns the reference 
type *to* that type. Not the underlying reference. The fallback for this is 
just that it returns the current type IIRC if you already have a reference, 
that's why this works. So, `GetLValueReferenceType` is just a no-op and 
`GetPointeeType` is doing the actual dereferencing. I think just the 
`GetPointeeType` is needed.


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

https://reviews.llvm.org/D108717

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

[Lldb-commits] [PATCH] D108545: [LLDB][GUI] Add initial searcher support

[Greg Clayton via Phabricator via lldb-commits]

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

Ok, sounds like you have thought this through.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108545

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

[Lldb-commits] [lldb] 3c11e57 - [LLDB][GUI] Add initial searcher support

[Greg Clayton via lldb-commits]

Author: Omar Emara
Date: 2021-08-25T13:55:11-07:00
New Revision: 3c11e5722c302a9dcf01b97ace868e156e376bc6

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

LOG: [LLDB][GUI] Add initial searcher support

This patch adds a new type of reusable UI components. Searcher Windows
contain a text field to enter a search keyword and a list of scrollable
matches are presented. The target match can be selected and executed
which invokes a user callback to do something with the match.

This patch also adds one searcher delegate, which wraps the common
command completion searchers for simple use cases.

Reviewed By: clayborg

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

Added: 


Modified: 
lldb/source/Core/IOHandlerCursesGUI.cpp

Removed: 




diff  --git a/lldb/source/Core/IOHandlerCursesGUI.cpp 
b/lldb/source/Core/IOHandlerCursesGUI.cpp
index ef4f5c3d35f6..71e8365e2511 100644
--- a/lldb/source/Core/IOHandlerCursesGUI.cpp
+++ b/lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -3641,6 +3641,232 @@ class ProcessLaunchFormDelegate : public FormDelegate {
   EnvironmentVariableListFieldDelegate *m_inherited_environment_field;
 };
 
+
+// Searchers
+
+
+class SearcherDelegate {
+public:
+  SearcherDelegate() {}
+
+  virtual ~SearcherDelegate() = default;
+
+  virtual int GetNumberOfMatches() = 0;
+
+  // Get the string that will be displayed for the match at the input index.
+  virtual const std::string &GetMatchTextAtIndex(int index) = 0;
+
+  // Update the matches of the search. This is executed every time the text
+  // field handles an event.
+  virtual void UpdateMatches(const std::string &text) = 0;
+
+  // Execute the user callback given the index of some match. This is executed
+  // once the user selects a match.
+  virtual void ExecuteCallback(int match_index) = 0;
+};
+
+typedef std::shared_ptr SearcherDelegateSP;
+
+class SearcherWindowDelegate : public WindowDelegate {
+public:
+  SearcherWindowDelegate(SearcherDelegateSP &delegate_sp)
+  : m_delegate_sp(delegate_sp), m_text_field("Search", "", false),
+m_selected_match(0), m_first_visible_match(0) {
+;
+  }
+
+  // A completion window is padded by one character from all sides. A text 
field
+  // is first drawn for inputting the searcher request, then a list of matches
+  // are displayed in a scrollable list.
+  //
+  // ___
+  // |   |
+  // | __[Search]___ |
+  // | |   | |
+  // | |___| |
+  // | - Match 1.|
+  // | - Match 2.|
+  // | - ... |
+  // |   |
+  // |[Press Esc to Cancel]__|
+  //
+
+  // Get the index of the last visible match. Assuming at least one match
+  // exists.
+  int GetLastVisibleMatch(int height) {
+int index = m_first_visible_match + height;
+return std::min(index, m_delegate_sp->GetNumberOfMatches()) - 1;
+  }
+
+  int GetNumberOfVisibleMatches(int height) {
+return GetLastVisibleMatch(height) - m_first_visible_match + 1;
+  }
+
+  void UpdateScrolling(Surface &surface) {
+if (m_selected_match < m_first_visible_match) {
+  m_first_visible_match = m_selected_match;
+  return;
+}
+
+int height = surface.GetHeight();
+int last_visible_match = GetLastVisibleMatch(height);
+if (m_selected_match > last_visible_match) {
+  m_first_visible_match = m_selected_match - height + 1;
+}
+  }
+
+  void DrawMatches(Surface &surface) {
+if (m_delegate_sp->GetNumberOfMatches() == 0)
+  return;
+
+UpdateScrolling(surface);
+
+int count = GetNumberOfVisibleMatches(surface.GetHeight());
+for (int i = 0; i < count; i++) {
+  surface.MoveCursor(1, i);
+  int current_match = m_first_visible_match + i;
+  if (current_match == m_selected_match)
+surface.AttributeOn(A_REVERSE);
+  surface.PutCString(
+  m_delegate_sp->GetMatchTextAtIndex(current_match).c_str());
+  if (current_match == m_selected_match)
+surface.AttributeOff(A_REVERSE);
+}
+  }
+
+  void DrawContent(Surface &surface) {
+Rect content_bounds = surface.GetFrame();
+Rect text_field_bounds, matchs_bounds;
+content_bounds.HorizontalSplit(m_text_field.FieldDelegateGetHeight(),
+   text_field_bounds, matchs_bounds);
+Surface text_field_surface = surface.SubSurface(text_field_bounds);
+Surface matches_surface = surface.SubSurface(matchs_bounds);
+
+m_text_field.Fi

[Lldb-commits] [PATCH] D108545: [LLDB][GUI] Add initial searcher support

[Greg Clayton via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3c11e5722c30: [LLDB][GUI] Add initial searcher support 
(authored by OmarEmaraDev, committed by clayborg).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108545

Files:
  lldb/source/Core/IOHandlerCursesGUI.cpp

Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -3641,6 +3641,232 @@
   EnvironmentVariableListFieldDelegate *m_inherited_environment_field;
 };
 
+
+// Searchers
+
+
+class SearcherDelegate {
+public:
+  SearcherDelegate() {}
+
+  virtual ~SearcherDelegate() = default;
+
+  virtual int GetNumberOfMatches() = 0;
+
+  // Get the string that will be displayed for the match at the input index.
+  virtual const std::string &GetMatchTextAtIndex(int index) = 0;
+
+  // Update the matches of the search. This is executed every time the text
+  // field handles an event.
+  virtual void UpdateMatches(const std::string &text) = 0;
+
+  // Execute the user callback given the index of some match. This is executed
+  // once the user selects a match.
+  virtual void ExecuteCallback(int match_index) = 0;
+};
+
+typedef std::shared_ptr SearcherDelegateSP;
+
+class SearcherWindowDelegate : public WindowDelegate {
+public:
+  SearcherWindowDelegate(SearcherDelegateSP &delegate_sp)
+  : m_delegate_sp(delegate_sp), m_text_field("Search", "", false),
+m_selected_match(0), m_first_visible_match(0) {
+;
+  }
+
+  // A completion window is padded by one character from all sides. A text field
+  // is first drawn for inputting the searcher request, then a list of matches
+  // are displayed in a scrollable list.
+  //
+  // ___
+  // |   |
+  // | __[Search]___ |
+  // | |   | |
+  // | |___| |
+  // | - Match 1.|
+  // | - Match 2.|
+  // | - ... |
+  // |   |
+  // |[Press Esc to Cancel]__|
+  //
+
+  // Get the index of the last visible match. Assuming at least one match
+  // exists.
+  int GetLastVisibleMatch(int height) {
+int index = m_first_visible_match + height;
+return std::min(index, m_delegate_sp->GetNumberOfMatches()) - 1;
+  }
+
+  int GetNumberOfVisibleMatches(int height) {
+return GetLastVisibleMatch(height) - m_first_visible_match + 1;
+  }
+
+  void UpdateScrolling(Surface &surface) {
+if (m_selected_match < m_first_visible_match) {
+  m_first_visible_match = m_selected_match;
+  return;
+}
+
+int height = surface.GetHeight();
+int last_visible_match = GetLastVisibleMatch(height);
+if (m_selected_match > last_visible_match) {
+  m_first_visible_match = m_selected_match - height + 1;
+}
+  }
+
+  void DrawMatches(Surface &surface) {
+if (m_delegate_sp->GetNumberOfMatches() == 0)
+  return;
+
+UpdateScrolling(surface);
+
+int count = GetNumberOfVisibleMatches(surface.GetHeight());
+for (int i = 0; i < count; i++) {
+  surface.MoveCursor(1, i);
+  int current_match = m_first_visible_match + i;
+  if (current_match == m_selected_match)
+surface.AttributeOn(A_REVERSE);
+  surface.PutCString(
+  m_delegate_sp->GetMatchTextAtIndex(current_match).c_str());
+  if (current_match == m_selected_match)
+surface.AttributeOff(A_REVERSE);
+}
+  }
+
+  void DrawContent(Surface &surface) {
+Rect content_bounds = surface.GetFrame();
+Rect text_field_bounds, matchs_bounds;
+content_bounds.HorizontalSplit(m_text_field.FieldDelegateGetHeight(),
+   text_field_bounds, matchs_bounds);
+Surface text_field_surface = surface.SubSurface(text_field_bounds);
+Surface matches_surface = surface.SubSurface(matchs_bounds);
+
+m_text_field.FieldDelegateDraw(text_field_surface, true);
+DrawMatches(matches_surface);
+  }
+
+  bool WindowDelegateDraw(Window &window, bool force) override {
+window.Erase();
+
+window.DrawTitleBox(window.GetName(), "Press Esc to Cancel");
+
+Rect content_bounds = window.GetFrame();
+content_bounds.Inset(2, 2);
+Surface content_surface = window.SubSurface(content_bounds);
+
+DrawContent(content_surface);
+return true;
+  }
+
+  void SelectNext() {
+if (m_selected_match != m_delegate_sp->GetNumberOfMatches() - 1)
+  m_selected_match++;
+return;
+  }
+
+  void SelectPrevious() {
+

[Lldb-commits] [PATCH] D108233: WIP: Add minidump save-core functionality to ELF object files

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp:31
+  /*create_memory_callback=*/nullptr,
+  /*get_module_specifications=*/nullptr, SaveCore);
+}

It should be fine to not inherit from ObjectFile and yes I agree, probably best 
to not modify other code. 

All you need to provide is static function callbacks that have the correct 
static function signature, and that doesn't require your class to inherit from 
ObjectFile. You already have a CreateInstance function to see how to do this. 
The CreateInstance(...) function you have can do nothing and just return 
nullptr. 

Then you need to make a create memory callback and a get module specifications:
```
ObjectFile *ObjectFileMinidump::CreateMemoryInstance(
const lldb::ModuleSP &module_sp, DataBufferSP &data_sp,
const ProcessSP &process_sp, lldb::addr_t header_addr) {
  return nullptr;
}

size_t ObjectFileMinidump::GetModuleSpecifications(
const lldb_private::FileSpec &file, lldb::DataBufferSP &data_sp,
lldb::offset_t data_offset, lldb::offset_t file_offset,
lldb::offset_t length, lldb_private::ModuleSpecList &specs) {
  specs.Clear();
  return 0;
}
```

Then your registration call looks like:

```
PluginManager::RegisterPlugin(
  GetPluginNameStatic(), GetPluginDescriptionStatic(), CreateInstance,
  CreateMemoryInstance, GetModuleSpecifications, SaveCore);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108233

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

[Lldb-commits] [PATCH] D108717: Fix Reference case for TypeSystemClang::GetChildCompilerTypeAtIndex(...) to avoid possible invalid cast

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:6505
 if (idx_is_valid) {
-  const clang::ReferenceType *reference_type =
-  llvm::cast(GetQualType(type).getTypePtr());
-  CompilerType pointee_clang_type =
-  GetType(reference_type->getPointeeType());
+  CompilerType pointee_clang_type;
+

teemperor wrote:
> I really like the idea of reusing the TypeSystemClang functions here (we 
> should do this everywhere I think). FWIW, I think this can all just be 
> `CompilerType pointee_clang_type = GetNonReferenceType(type);` From the code 
> below `GetPointeeType` would also work. We already call the variable here 
> `pointee` type so I don't think calling references pointers here will hurt 
> too much, so I think both is fine.
I tried `GetNonReferenceType(type).GetPointeeType()` but I get back an empty 
`CompilerType` it looks like it is doing almost exactly the same thing as the 
previous code:

`(*this)->getAs()`


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

https://reviews.llvm.org/D108717

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

[Lldb-commits] [PATCH] D108717: Fix Reference case for TypeSystemClang::GetChildCompilerTypeAtIndex(...) to avoid possible invalid cast

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:6508
+  if (parent_type_class == clang::Type::LValueReference)
+pointee_clang_type = GetLValueReferenceType(type).GetPointeeType();
+  else

teemperor wrote:
> I think the logic here is reversed? `type` is a reference type (potentially 
> behind a typedef). `GetLValueReferenceType` on `type` returns the reference 
> type *to* that type. Not the underlying reference. The fallback for this is 
> just that it returns the current type IIRC if you already have a reference, 
> that's why this works. So, `GetLValueReferenceType` is just a no-op and 
> `GetPointeeType` is doing the actual dereferencing. I think just the 
> `GetPointeeType` is needed.
Maybe I am confused but I thought given:

```
TypedefType 0x7fb11c841460 'std::__compressed_pair_elem, class 
std::allocator >::__rep, 0, false>::const_reference' sugar
|-Typedef 0x7fb11c8413f0 'const_reference'
`-LValueReferenceType 0x7fb11c8413c0 'const struct std::basic_string, class std::allocator >::__rep &'
  `-QualType 0x7fb11cac3361 'const struct std::basic_string, class std::allocator >::__rep' const
`-RecordType 0x7fb11cac3360 'struct std::basic_string, class std::allocator >::__rep'
  `-CXXRecord 0x7fb11cac32b8 '__rep'
```

that `llvm::cast(GetQualType(type).getTypePtr())` was 
intended to obtain:

```
LValueReferenceType 0x7fb11c829630 'const struct std::basic_string, class std::allocator >::__rep &'
`-QualType 0x7fb11cac3361 'const struct std::basic_string, class std::allocator >::__rep' const
  `-RecordType 0x7fb11cac3360 'struct std::basic_string, class std::allocator >::__rep'
`-CXXRecord 0x7fb11cac32b8 '__rep'
```

which is what `GetLValueReferenceType(type)` does.




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

https://reviews.llvm.org/D108717

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

[Lldb-commits] [PATCH] D108717: Fix Reference case for TypeSystemClang::GetChildCompilerTypeAtIndex(...) to avoid possible invalid cast

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

In D108717#2965815 , @teemperor wrote:

> Thanks for the patch!
>
> So IIUC correctly this fixes a crash when calling `Dereference` on an SBValue 
> that is of type `SomeTypedef` with `typedef int& SomeTypedef`? If yes, then I 
> think the test here could just be another type+assert in 
> `TestCPPDereferencingReferences.py`. The test here is a bit expensive and 
> seems to depend on a specific libc++ version (-> it will also always pass on 
> Linux), even though this is just a TypeSystemClang patch.

Let me see if I can mock up a test using that and if so that would definitely 
be nicer.


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

https://reviews.llvm.org/D108717

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

[Lldb-commits] [PATCH] D108061: [lldb] Add support for shared library load when executable called through ld.

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added inline comments.



Comment at: lldb/test/API/functionalities/dyld-launch-linux/Makefile:1
+CXX_SOURCES := main.cpp
+LD_EXTRAS := -Wl,-rpath "-Wl,$(shell pwd)"

```
CXX_SOURCES := main.cpp
DYLIB_NAME := signal_file
DYLIB_CXX_SOURCES := signal_file.cpp

include Makefile.rules
```

macOS uses `.dylib` dynamic libraries and the hard-coded `.so` can cause build 
failures, even if the test skips execution on macOS.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108061

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

[Lldb-commits] [PATCH] D108061: [lldb] Add support for shared library load when executable called through ld.

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added inline comments.



Comment at: 
lldb/test/API/functionalities/dyld-launch-linux/TestDyldLaunchLinux.py:16
+exe = "/lib64/ld-linux-x86-64.so.2"
+if(os.path.exists(exe)):
+target = self.dbg.CreateTarget(exe)





Comment at: 
lldb/test/API/functionalities/dyld-launch-linux/TestDyldLaunchLinux.py:38
+self.assertEqual(process.GetState(), lldb.eStateStopped)
+
self.assertIn("get_signal_crash",thread.GetFrameAtIndex(0).GetDisplayFunctionName())
+process.Continue()

space after comma



Comment at: 
lldb/test/API/functionalities/dyld-launch-linux/TestDyldLaunchLinux.py:45
+
self.assertIn("get_signal_crash",thread.GetFrameAtIndex(1).GetDisplayFunctionName())
+
+

delete blank lines



Comment at: lldb/test/API/functionalities/dyld-launch-linux/signal_file.h:1
+int get_signal_crash(void);

C++ doesn't typically use `(void)`. Just use `()`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108061

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

[Lldb-commits] [PATCH] D108061: [lldb] Add support for shared library load when executable called through ld.

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added inline comments.



Comment at: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp:293
+  // ld.so saves empty file name for the executable file in the link map.
+  // When executable is run using ld.so, we need to be update executable path.
+  if (name.empty()) {

This sentence may be ambiguous, because all dynamically linked executables are 
interpreted by ld.so

"When argv[0] is ld.so"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108061

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

[Lldb-commits] [PATCH] D108233: WIP: Add minidump save-core functionality to ELF object files

[Andrej Korman via Phabricator via lldb-commits]

Aj0SK updated this revision to Diff 368765.
Aj0SK added a comment.

Change ObjectFileMinidump plugin to inherit from PluginInterface


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108233

Files:
  lldb/include/lldb/Core/PluginManager.h
  lldb/source/API/SBProcess.cpp
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Commands/Options.td
  lldb/source/Core/PluginManager.cpp
  lldb/source/Plugins/ObjectFile/CMakeLists.txt
  lldb/source/Plugins/ObjectFile/Minidump/CMakeLists.txt
  lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
  lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
  lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
  lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h
  lldb/test/API/functionalities/process_save_core_minidump/Makefile
  
lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
  lldb/test/API/functionalities/process_save_core_minidump/main.cpp

Index: lldb/test/API/functionalities/process_save_core_minidump/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/process_save_core_minidump/main.cpp
@@ -0,0 +1,30 @@
+#include 
+#include 
+#include 
+
+using namespace std;
+
+void g() { assert(false); }
+
+void f() { g(); }
+
+size_t h() {
+  size_t sum = 0;
+  for (size_t i = 0; i < 100; ++i)
+for (size_t j = 0; j < 100; ++j)
+  if ((i * j) % 2 == 0) {
+sum += 1;
+  }
+  return sum;
+}
+
+int main() {
+  thread t1(f);
+
+  size_t x = h();
+
+  t1.join();
+
+  cout << "X is " << x << "\n";
+  return 0;
+}
\ No newline at end of file
Index: lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
===
--- /dev/null
+++ lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -0,0 +1,78 @@
+"""
+Test saving a mini dump.
+"""
+
+
+import os
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class ProcessSaveCoreMinidumpTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipUnlessPlatform(['linux'])
+def test_save_linux_mini_dump(self):
+"""Test that we can save a Linux mini dump."""
+self.build()
+exe = self.getBuildArtifact("a.out")
+core = self.getBuildArtifact("core.dmp")
+try:
+target = self.dbg.CreateTarget(exe)
+process = target.LaunchSimple(
+None, None, self.get_process_working_directory())
+self.assertEqual(process.GetState(), lldb.eStateStopped)
+
+# get neccessary data for the verification phase
+process_info = process.GetProcessInfo()
+expected_pid = process_info.GetProcessID() if process_info.IsValid() else -1
+expected_number_of_modules = target.GetNumModules()
+expected_modules = target.modules
+expected_number_of_threads = process.GetNumThreads()
+expected_threads = []
+
+for thread_idx in range(process.GetNumThreads()):
+thread = process.GetThreadAtIndex(thread_idx)
+thread_id = thread.GetThreadID()
+expected_threads.append(thread_id)
+
+# save core and, kill process and verify corefile existence
+self.runCmd("process save-core --plugin-name=minidump " + core)
+self.assertTrue(os.path.isfile(core))
+self.assertTrue(process.Kill().Success())
+
+# To verify, we'll launch with the mini dump
+target = self.dbg.CreateTarget(None)
+process = target.LoadCore(core)
+
+# check if the core is in desired state
+self.assertTrue(process, PROCESS_IS_VALID)
+self.assertTrue(process.GetProcessInfo().IsValid())
+self.assertEqual(process.GetProcessInfo().GetProcessID(), expected_pid)
+self.assertTrue(target.GetTriple().find("linux") != -1)
+self.assertTrue(target.GetNumModules(), expected_number_of_modules)
+self.assertEqual(process.GetNumThreads(), expected_number_of_threads)
+
+for module, expected in zip(target.modules, expected_modules):
+self.assertTrue(module.IsValid())
+module_file_name = module.GetFileSpec().GetFilename()
+expected_file_name = expected.GetFileSpec().GetFilename()
+# skip kernel virtual dynamic shared objects
+if "vdso" in expected_file_name:
+continue
+self.assertEqual(module_file_name, expected_file_name)
+self.assertEqual(module.GetUUIDString(), expected.GetUUIDString())
+
+for thread_idx in range(proc

[Lldb-commits] [PATCH] D108739: [LLDB] AArch64 SVE restore SVE registers after expression

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid created this revision.
omjavaid added reviewers: DavidSpickett, rankov.
omjavaid added a project: LLDB.
Herald added subscribers: ctetreau, JDevlieghere, kristof.beyls, tschuett.
omjavaid requested review of this revision.

This patch fixes register save/restore on expression call to also include SVE 
registers.

This will fix expression calls like:

re re p1

https://reviews.llvm.org/P1> before expression>

p 

re re p1

https://reviews.llvm.org/P1> after expression>

In above example register P1  should remain the 
same before and after the expression evaluation.


https://reviews.llvm.org/D108739

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/main.c

Index: lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/main.c
===
--- lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/main.c
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/main.c
@@ -1,4 +1,6 @@
-int main() {
+#include 
+
+void write_sve_regs () {
   asm volatile("setffr\n\t");
   asm volatile("ptrue p0.b\n\t");
   asm volatile("ptrue p1.h\n\t");
@@ -49,5 +51,20 @@
   asm volatile("cpy  z29.b, p5/z, #30\n\t");
   asm volatile("cpy  z30.b, p10/z, #31\n\t");
   asm volatile("cpy  z31.b, p15/z, #32\n\t");
+}
+
+// This function will be called using jitted expression call. We change vector
+// length and write SVE registers. Our program context should restore to
+// orignal vector length and register values after expression evaluation.
+int expr_eval_func() {
+  prctl(PR_SVE_SET_VL, 8 * 2);
+  write_sve_regs();
+  prctl(PR_SVE_SET_VL, 8 * 4);
+  write_sve_regs();
+  return 1;
+}
+
+int main() {
+  write_sve_regs();
   return 0; // Set a break point here.
 }
Index: lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py
===
--- lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py
@@ -17,6 +17,51 @@
 self.assertEqual(reg_value.GetByteSize(), expected,
  'Verify "%s" == %i' % (name, expected))
 
+def check_sve_regs_read(self, z_reg_size):
+p_reg_size = int(z_reg_size / 8)
+
+for i in range(32):
+z_regs_value = '{' + \
+' '.join('0x{:02x}'.format(i + 1)
+ for _ in range(z_reg_size)) + '}'
+self.expect("register read " + 'z%i' %
+(i), substrs=[z_regs_value])
+
+p_value_bytes = ['0xff', '0x55', '0x11', '0x01', '0x00']
+for i in range(16):
+p_regs_value = '{' + \
+' '.join(p_value_bytes[i % 5] for _ in range(p_reg_size)) + '}'
+self.expect("register read " + 'p%i' % (i), substrs=[p_regs_value])
+
+self.expect("register read ffr", substrs=[p_regs_value])
+
+def check_sve_regs_read_after_write(self, z_reg_size):
+p_reg_size = int(z_reg_size / 8)
+
+z_regs_value = '{' + \
+' '.join(('0x9d' for _ in range(z_reg_size))) + '}'
+
+p_regs_value = '{' + \
+' '.join(('0xee' for _ in range(p_reg_size))) + '}'
+
+for i in range(32):
+self.runCmd('register write ' + 'z%i' %
+(i) + " '" + z_regs_value + "'")
+
+for i in range(32):
+self.expect("register read " + 'z%i' % (i), substrs=[z_regs_value])
+
+for i in range(16):
+self.runCmd('register write ' + 'p%i' %
+(i) + " '" + p_regs_value + "'")
+
+for i in range(16):
+self.expect("register read " + 'p%i' % (i), substrs=[p_regs_value])
+
+self.runCmd('register write ' + 'ffr ' + "'" + p_regs_value + "'")
+
+self.expect("register read " + 'ffr', substrs=[p_regs_value])
+
 mydir = TestBase.compute_mydir(__file__)
 
 @no_debug_info_test
@@ -117,43 +162,17 @@
 
 z_reg_size = vg_reg_value * 8
 
-p_reg_size = int(z_reg_size / 8)
-
-for i in range(32):
-z_regs_value = '{' + \
-' '.join('0x{:02x}'.format(i + 1)
- for _ in range(z_reg_size)) + '}'
-self.expect("register read " + 'z%i' %
-(i), substrs=[z_regs_value])
+self.check_sve_regs_read(z_reg_size)
 
-p_v

[Lldb-commits] [PATCH] D108233: WIP: Add minidump save-core functionality to ELF object files

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Very nice! Structure is good now. I found a few other issue we should fix as 
well.




Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:225
+m.SizeOfImage = static_cast(
+mod->GetObjectFile()->GetByteSize());
+m.Checksum = static_cast(0);

I am not sure if the object file's full size is the right value here. I think 
the right value would be the last contiguous loaded address range for a given 
file. This information is going to be used to set the load addresses of any 
sections in the object file when it is loaded, and if we have a huge ELF file 
that has tons of debug info, this is going to make the loaded address range way 
too big.

So the algorithm I would suggest is:
1 - grab the section that contains base address:
2 - in a loop, grab the next section that starts  at the end of this section, 
and as long as the addresses are contiguous, increase the SizeOfImage:

So something like:
```
lldb::SectionSP sect_sp = mod->GetObjectFile()->GetBaseAddress().GetSection();
uint64_t SizeOfImage = 0;
if (sect_sp) {
  lldb::addr_t sect_addr = sect_sp->GetLoadAddress(&target);
  // Use memory size since zero fill sections, like ".bss", will be smaller on 
disk.
  lldb::addr_t sect_size = sect_sp->MemorySize(); 
  // This will usually be zero, but make sure to calculate the BaseOfImage 
offset.
  const lldb::addr_t base_sect_offset = m.BaseOfImage - sect_addr;
  SizeOfImage = sect_size - base_sect_offset;
  lldb::addr_t next_sect_addr = sect_addr + sect_size;
  Address sect_so_addr;
  target.ResolveLoadAddress(next_sect_addr, sect_so_addr);
  lldb::SectionSP next_sect_sp = sect_so_addr.GetSections();
  while (next_sect_sp && next_sect_sp->GetLoadBaseAddress(&target) == 
next_sect_addr)) {
sect_size = sect_sp->MemorySize(); 
SizeOfImage += sect_size;
next_sect_addr += sect_size;
target.ResolveLoadAddress(next_sect_addr, sect_so_addr);
next_sect_sp = sect_so_addr.GetSections();
  }
  m.SizeOfImage = static_cast(SizeOfImage);
} else {
  m.SizeOfImage = static_cast(sect_size);
}
```




Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:519
+lldb_private::Status
+MinidumpFileBuilder::AddMemoryList(const lldb::ProcessSP &process_sp) {
+  Status error;

This should take in the CoreStyle and only emit what was asked for.

If style is set to "eSaveCoreStackOnly", then grab only the memory regions for 
each thread stack pointer and save only those. 

We can't tell from LLDB APIs if a page is dirty, so if we get 
"eSaveCoreDirtyOnly", then we will need to save all memory regions that have 
write permissions.

If it is either "eSaveCoreFull"  or "eSaveCoreUnspecified" then save everything.



Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:667
+if (memory_buffer) {
+  size_t size = memory_buffer->getBufferSize();
+  AddDirectory(stream, size);

Do we need to make sure size is not zero?



Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h:1-3
+//===-- MinidumpFileBuilder.h -- -*- C++
+//-*-===//
+//

Fix this header comment line to be on one line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108233

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

[Lldb-commits] [PATCH] D108229: [lldb] Refactor Module::LookupInfo constructor

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

FYI (you may have have already noticed) this causes some merge conflicts in 
swift-lldb. It would be great if we could work together to figure out how to 
best resolve them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108229

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

[Lldb-commits] [PATCH] D108229: [lldb] Refactor Module::LookupInfo constructor

[Alex Langford via Phabricator via lldb-commits]

bulbazord added a comment.

In D108229#2966405 , @aprantl wrote:

> FYI (you may have have already noticed) this causes some merge conflicts in 
> swift-lldb. It would be great if we could work together to figure out how to 
> best resolve them.

Yes, I figured that would happen at some point. I have an interest in seeing 
swift-lldb apply this change (and some of my other refactoring patches) so I 
would love to work with you and anyone else to get this change in smoothly. 
Unfortunately I had to revert this patch. It appears my environment had some 
issues that prevented me from running about half of lldb's test suite so I 
didn't catch any of the failures. I was finally able to debug the failing tests 
today and I realized that this change has several issues that need to be 
addressed. Once I have time to work through those issues and resubmit this 
patch, I'll add you as a reviewer and we can go from there. Does this sound 
good to you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108229

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