[Lldb-commits] [PATCH] D110011: [lldb] Add --stack option to `target symbols add` command

[Dave Lee via Phabricator via lldb-commits]

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



Comment at: lldb/source/Commands/CommandObjectTarget.cpp:4300-4301
+  current_frame_flush))
+symbols_found = true;
+  flush |= current_frame_flush;
+}

JDevlieghere wrote:
> kastiglione wrote:
> > JDevlieghere wrote:
> > > kastiglione wrote:
> > > > do you need the separate variable? can it be:
> > > > 
> > > > ```
> > > > flush |= true;
> > > > ```
> > > I want `flush` to be true only if at least one frame requires a flush. 
> > my suggestion still achieves that.
> > 
> > Instead of:
> > 
> > ```
> > bool current_frame_flush = false;
> > if (DownloadObjectAndSymbolFile(module_spec, target, result,
> > current_frame_flush))
> >   symbols_found = true;
> > 
> > flush |= current_frame_flush;
> > ```
> > 
> > Inline the `current_frame_flush` variable:
> > 
> > ```
> > if (DownloadObjectAndSymbolFile(module_spec, target, result,
> > current_frame_flush))
> >   flush |= true;
> > ```
> > 
> > This makes `flush` only true when one frame needs it.
> That relies on the assumption that flush is true iff 
> `DownloadObjectAndSymbolFile`return true (and the same for 
> `AddModuleSymbols`). Looking at the code that does indeed seem to be the 
> case, but then I'd rather see the API refactored to remove the `flush` output 
> variable instead of relying on an implementation-specific assumption here. 
thanks, makes sense. I had overlooked that `current_frame_flush` is also an 
output parameter.


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

https://reviews.llvm.org/D110011

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

[Lldb-commits] [PATCH] D110115: [lldb] Remove Expression's dependency on CPlusPlusLanguagePlugin

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:524-530
+  if (param_and_qual_matches.size())
+return param_and_qual_matches[0]; // It is assumed that there will be only
+  // one!
+  else if (param_matches.size())
+return param_matches[0]; // Return one of them as a best match
+  else
+return ConstString();

bulbazord wrote:
> JDevlieghere wrote:
> > I know you just copied this, but `else` after a return? 
> I think there's a lot of opportunity to refactor this method. Think I should 
> do it in this change or would a follow-up be more reasonable?
If you're planning more changes then a follow-up sounds like the best course of 
action.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110115

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

[Lldb-commits] [PATCH] D110115: [lldb] Remove Expression's dependency on CPlusPlusLanguagePlugin

[Alex Langford via Phabricator via lldb-commits]

bulbazord added inline comments.



Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:524-530
+  if (param_and_qual_matches.size())
+return param_and_qual_matches[0]; // It is assumed that there will be only
+  // one!
+  else if (param_matches.size())
+return param_matches[0]; // Return one of them as a best match
+  else
+return ConstString();

JDevlieghere wrote:
> I know you just copied this, but `else` after a return? 
I think there's a lot of opportunity to refactor this method. Think I should do 
it in this change or would a follow-up be more reasonable?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110115

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

[Lldb-commits] [PATCH] D110115: [lldb] Remove Expression's dependency on CPlusPlusLanguagePlugin

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:524-530
+  if (param_and_qual_matches.size())
+return param_and_qual_matches[0]; // It is assumed that there will be only
+  // one!
+  else if (param_matches.size())
+return param_matches[0]; // Return one of them as a best match
+  else
+return ConstString();

I know you just copied this, but `else` after a return? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110115

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

[Lldb-commits] [PATCH] D110115: [lldb] Remove Expression's dependency on CPlusPlusLanguagePlugin

[Alex Langford via Phabricator via lldb-commits]

bulbazord created this revision.
bulbazord added reviewers: clayborg, jingham, teemperor, JDevlieghere.
Herald added a subscriber: mgorny.
bulbazord requested review of this revision.
Herald added a project: LLDB.

This change accomplishes the following:

- Moves `IRExecutionUnit::FindBestAlternateMangledName` to `Language`.
- Renames `FindBestAlternateMangledName` to 
`FindBestAlternateFunctionMangledName`
- Changes the first parameter of said method from a `ConstString` representing 
a demangled name to a `Mangled`.

This allows us to remove the use of CPlusPlusLanguage from Expression


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110115

Files:
  lldb/include/lldb/Target/Language.h
  lldb/source/Expression/CMakeLists.txt
  lldb/source/Expression/IRExecutionUnit.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h

Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
===
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
@@ -130,6 +130,9 @@
   std::vector
   GenerateAlternateFunctionManglings(const ConstString mangled) const override;
 
+  ConstString FindBestAlternateFunctionMangledName(
+  const Mangled mangled, const SymbolContext _ctx) const override;
+
   // PluginInterface protocol
   ConstString GetPluginName() override;
 };
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -20,12 +20,14 @@
 #include "llvm/Demangle/ItaniumDemangle.h"
 
 #include "lldb/Core/Mangled.h"
+#include "lldb/Core/Module.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/UniqueCStringMap.h"
 #include "lldb/DataFormatters/CXXFunctionPointer.h"
 #include "lldb/DataFormatters/DataVisualization.h"
 #include "lldb/DataFormatters/FormattersHelpers.h"
 #include "lldb/DataFormatters/VectorType.h"
+#include "lldb/Symbol/SymbolFile.h"
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/RegularExpression.h"
@@ -478,6 +480,56 @@
   return alternates;
 }
 
+ConstString CPlusPlusLanguage::FindBestAlternateFunctionMangledName(
+const Mangled mangled, const SymbolContext _ctx) const {
+  ConstString demangled = mangled.GetDemangledName();
+  if (!demangled)
+return ConstString();
+
+  CPlusPlusLanguage::MethodName cpp_name(demangled);
+  std::string scope_qualified_name = cpp_name.GetScopeQualifiedName();
+
+  if (!scope_qualified_name.size())
+return ConstString();
+
+  if (!sym_ctx.module_sp)
+return ConstString();
+
+  lldb_private::SymbolFile *sym_file = sym_ctx.module_sp->GetSymbolFile();
+  if (!sym_file)
+return ConstString();
+
+  std::vector alternates;
+  sym_file->GetMangledNamesForFunction(scope_qualified_name, alternates);
+
+  std::vector param_and_qual_matches;
+  std::vector param_matches;
+  for (size_t i = 0; i < alternates.size(); i++) {
+ConstString alternate_mangled_name = alternates[i];
+Mangled mangled(alternate_mangled_name);
+ConstString demangled = mangled.GetDemangledName();
+
+CPlusPlusLanguage::MethodName alternate_cpp_name(demangled);
+if (!cpp_name.IsValid())
+  continue;
+
+if (alternate_cpp_name.GetArguments() == cpp_name.GetArguments()) {
+  if (alternate_cpp_name.GetQualifiers() == cpp_name.GetQualifiers())
+param_and_qual_matches.push_back(alternate_mangled_name);
+  else
+param_matches.push_back(alternate_mangled_name);
+}
+  }
+
+  if (param_and_qual_matches.size())
+return param_and_qual_matches[0]; // It is assumed that there will be only
+  // one!
+  else if (param_matches.size())
+return param_matches[0]; // Return one of them as a best match
+  else
+return ConstString();
+}
+
 static void LoadLibCxxFormatters(lldb::TypeCategoryImplSP cpp_category_sp) {
   if (!cpp_category_sp)
 return;
Index: lldb/source/Expression/IRExecutionUnit.cpp
===
--- lldb/source/Expression/IRExecutionUnit.cpp
+++ lldb/source/Expression/IRExecutionUnit.cpp
@@ -26,6 +26,7 @@
 #include "lldb/Symbol/SymbolFile.h"
 #include "lldb/Symbol/SymbolVendor.h"
 #include "lldb/Target/ExecutionContext.h"
+#include "lldb/Target/Language.h"
 #include "lldb/Target/LanguageRuntime.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/DataBufferHeap.h"
@@ -33,7 +34,6 @@
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/Log.h"
 
-#include "lldb/../../source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
 #include "lldb/../../source/Plugins/ObjectFile/JIT/ObjectFileJIT.h"
 
 using namespace lldb_private;
@@ -652,52 

[Lldb-commits] [PATCH] D109834: [lldb/win] Improve check-lldb-shell with LLVM_ENABLE_DIA_SDK=NO

[Nico Weber via Phabricator via lldb-commits]

thakis added a comment.

Oh, that's a good idea. But even if I force on the native PDB reader if 
LLVM_ENABLE_DIA_SDK is false [1], I still get 15 failures. That's better than 
27, but it's more than 0. Maybe LLDB_USE_NATIVE_PDB_READER doesn't hit all uses 
of DIA in lldb?

  Failed Tests (15):
lldb-shell :: Driver/TestSingleQuote.test
lldb-shell :: Expr/nodefaultlib.cpp
lldb-shell :: SymbolFile/NativePDB/stack_unwinding01.cpp
lldb-shell :: SymbolFile/PDB/ast-restore.test
lldb-shell :: SymbolFile/PDB/calling-conventions.test
lldb-shell :: SymbolFile/PDB/class-layout.test
lldb-shell :: SymbolFile/PDB/enums-layout.test
lldb-shell :: SymbolFile/PDB/expressions.test
lldb-shell :: SymbolFile/PDB/func-symbols.test
lldb-shell :: SymbolFile/PDB/function-nested-block.test
lldb-shell :: SymbolFile/PDB/pointers.test
lldb-shell :: SymbolFile/PDB/type-quals.test
lldb-shell :: SymbolFile/PDB/typedefs.test
lldb-shell :: SymbolFile/PDB/udt-layout.test
lldb-shell :: SymbolFile/PDB/variables.test
  
  
  Testing Time: 38.91s
Unsupported  : 1113
Passed   :  263
Expectedly Failed:   14
Failed   :   15

1:

  C:\src\llvm-project>git diff
  diff --git a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp 
b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cppindex 
794fab5f7309..51c74c59b137 100644
  --- a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  +++ b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  @@ -52,6 +52,10 @@
   #include "Plugins/Language/CPlusPlus/MSVCUndecoratedNameParser.h"
   #include "Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h"
  
  +#if defined(_WIN32)
  +#include "llvm/Config/config.h"
  +#endif
  +
   using namespace lldb;
   using namespace lldb_private;
   using namespace llvm::pdb;
  @@ -83,6 +87,7 @@ bool ShouldAddLine(uint32_t requested_line, uint32_t 
actual_line,
  
   static bool ShouldUseNativeReader() {
   #if defined(_WIN32)
  +#if LLVM_ENABLE_DIA_SDK
 llvm::StringRef use_native = ::getenv("LLDB_USE_NATIVE_PDB_READER");
 return use_native.equals_insensitive("on") ||
use_native.equals_insensitive("yes") ||
  @@ -91,6 +96,9 @@ static bool ShouldUseNativeReader() {
   #else
 return true;
   #endif
  +#else
  +  return true;
  +#endif
   }
  
   void SymbolFilePDB::Initialize() {


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

https://reviews.llvm.org/D109834

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

[Lldb-commits] [PATCH] D108090: [lldb/lua] Supplement Lua bindings for lldb module

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere requested changes to this revision.
JDevlieghere added inline comments.
This revision now requires changes to proceed.



Comment at: lldb/CMakeLists.txt:55-60
+   # FIXME: Lua 5.3 is hardcoded but it should support 5.3+!
+   find_program(Lua_EXECUTABLE lua5.3)
+   if (NOT Lua_EXECUTABLE)
+  message(FATAL_ERROR "Lua executable not found")
+   else ()
+  execute_process(

`FindLuaAndSwig.cmake` is responsible for finding Lua. If `LLDB_ENABLE_LUA` is 
set, then you can assume Lua is available. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108090

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

[Lldb-commits] [lldb] c4a406b - [lldb][NFC] Remove outdated FIXME

[Alex Langford via lldb-commits]

Author: Alex Langford
Date: 2021-09-20T11:44:20-07:00
New Revision: c4a406bbd0fe3afa8366b72c49b1bc494a168624

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

LOG: [lldb][NFC] Remove outdated FIXME

Added: 


Modified: 
lldb/source/Symbol/DeclVendor.cpp

Removed: 




diff  --git a/lldb/source/Symbol/DeclVendor.cpp 
b/lldb/source/Symbol/DeclVendor.cpp
index cf87f4f879b1d..e99ebfee4cff7 100644
--- a/lldb/source/Symbol/DeclVendor.cpp
+++ b/lldb/source/Symbol/DeclVendor.cpp
@@ -17,8 +17,6 @@ using namespace lldb_private;
 
 std::vector DeclVendor::FindTypes(ConstString name,
 uint32_t max_matches) {
-  // FIXME: This depends on clang, but should be able to support any
-  // TypeSystem.
   std::vector ret;
   std::vector decls;
   if (FindDecls(name, /*append*/ true, max_matches, decls))



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

[Lldb-commits] [PATCH] D110011: [lldb] Add --stack option to `target symbols add` command

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 373673.

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

https://reviews.llvm.org/D110011

Files:
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/test/API/macosx/add-dsym/TestAddDsymDownload.py

Index: lldb/test/API/macosx/add-dsym/TestAddDsymDownload.py
===
--- /dev/null
+++ lldb/test/API/macosx/add-dsym/TestAddDsymDownload.py
@@ -0,0 +1,98 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+@skipUnlessDarwin
+class AddDsymDownload(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+dwarfdump_uuid_regex = re.compile('UUID: ([-0-9a-fA-F]+) \(([^\(]+)\) .*')
+
+def get_uuid(self):
+dwarfdump_cmd_output = subprocess.check_output(
+('/usr/bin/dwarfdump --uuid "%s"' % self.exe),
+shell=True).decode("utf-8")
+for line in dwarfdump_cmd_output.splitlines():
+match = self.dwarfdump_uuid_regex.search(line)
+if match:
+return match.group(1)
+return None
+
+def create_dsym_for_uuid(self):
+shell_cmds = [
+'#! /bin/sh', '# the last argument is the uuid',
+'while [ $# -gt 1 ]', 'do', '  shift', 'done', 'ret=0',
+'echo ""',
+'echo "http://www.apple.com/DTDs/PropertyList-1.0.dtd\\;>"',
+'echo ""', '',
+'if [ "$1" != "%s" ]' % (self.uuid), 'then',
+'  echo "DBGErrornot found"',
+'  echo ""', '  exit 1', 'fi',
+'  uuid=%s' % self.uuid,
+'  bin=%s' % self.exe,
+'  dsym=%s' % self.dsym, 'echo "$uuid"', '',
+'echo "DBGDSYMPath$dsym"',
+'echo "DBGSymbolRichExecutable$bin"',
+'echo ""', 'exit $ret'
+]
+
+with open(self.dsym_for_uuid, "w") as writer:
+for l in shell_cmds:
+writer.write(l + '\n')
+
+os.chmod(self.dsym_for_uuid, 0o755)
+
+def setUp(self):
+TestBase.setUp(self)
+self.source = 'main.c'
+self.exe = self.getBuildArtifact("a.out")
+self.dsym = os.path.join(
+self.getBuildDir(),
+"hide.app/Contents/a.out.dSYM/Contents/Resources/DWARF/",
+os.path.basename(self.exe))
+self.dsym_for_uuid = self.getBuildArtifact("dsym-for-uuid.sh")
+
+self.buildDefault(dictionary={'MAKE_DSYM': 'YES'})
+self.assertTrue(os.path.exists(self.exe))
+self.assertTrue(os.path.exists(self.dsym))
+
+self.uuid = self.get_uuid()
+self.assertNotEqual(self.uuid, None, "Could not get uuid for a.out")
+
+self.create_dsym_for_uuid()
+
+os.environ['LLDB_APPLE_DSYMFORUUID_EXECUTABLE'] = self.dsym_for_uuid
+self.addTearDownHook(
+lambda: os.environ.pop('LLDB_APPLE_DSYMFORUUID_EXECUTABLE', None))
+
+def do_test(self, command):
+self.target = self.dbg.CreateTarget(self.exe)
+self.assertTrue(self.target, VALID_TARGET)
+
+main_bp = self.target.BreakpointCreateByName("main", "a.out")
+self.assertTrue(main_bp, VALID_BREAKPOINT)
+
+self.process = self.target.LaunchSimple(
+None, None, self.get_process_working_directory())
+self.assertTrue(self.process, PROCESS_IS_VALID)
+
+# The stop reason of the thread should be breakpoint.
+self.assertEquals(self.process.GetState(), lldb.eStateStopped,
+  STOPPED_DUE_TO_BREAKPOINT)
+
+self.runCmd(command)
+self.expect("frame select", substrs=['a.out`main at main.c'])
+
+@no_debug_info_test
+def test_frame(self):
+self.do_test("add-dsym --frame")
+
+@no_debug_info_test
+def test_uuid(self):
+self.do_test("add-dsym --uuid {}".format(self.uuid))
+
+@no_debug_info_test
+def test_stack(self):
+self.do_test("add-dsym --stack")
Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -3962,8 +3962,12 @@
 "name."),
 m_current_frame_option(
 LLDB_OPT_SET_2, false, "frame", 'F',
-"Locate the debug symbols for the currently selected frame.",
-false, true)
+"Locate the debug symbols for the currently selected frame.", false,
+true),
+m_current_stack_option(LLDB_OPT_SET_2, false, "stack", 'S',
+   "Locate the debug symbols for every frame in "
+   "the current call stack.",
+   false, true)
 
   {
 m_option_group.Append(_uuid_option_group, LLDB_OPT_SET_ALL,
@@ -3971,6 +3975,8 @@
 m_option_group.Append(_file_option, LLDB_OPT_SET_ALL, 

[Lldb-commits] [PATCH] D110013: [lldb][crashlog] Avoid specifying arch for image when a UUID is present

[Vedant Kumar via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe31b2d7d7be9: [lldb][crashlog] Avoid specifying arch for 
image when a UUID is present (authored by vsk).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110013

Files:
  lldb/examples/python/symbolication.py


Index: lldb/examples/python/symbolication.py
===
--- lldb/examples/python/symbolication.py
+++ lldb/examples/python/symbolication.py
@@ -379,7 +379,7 @@
 return None
 resolved_path = self.get_resolved_path()
 self.module = target.AddModule(
-resolved_path, str(self.arch), uuid_str, self.symfile)
+resolved_path, None, uuid_str, self.symfile)
 if not self.module:
 return 'error: unable to get module for (%s) "%s"' % (
 self.arch, self.get_resolved_path())


Index: lldb/examples/python/symbolication.py
===
--- lldb/examples/python/symbolication.py
+++ lldb/examples/python/symbolication.py
@@ -379,7 +379,7 @@
 return None
 resolved_path = self.get_resolved_path()
 self.module = target.AddModule(
-resolved_path, str(self.arch), uuid_str, self.symfile)
+resolved_path, None, uuid_str, self.symfile)
 if not self.module:
 return 'error: unable to get module for (%s) "%s"' % (
 self.arch, self.get_resolved_path())
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] e31b2d7 - [lldb][crashlog] Avoid specifying arch for image when a UUID is present

[Vedant Kumar via lldb-commits]

Author: Vedant Kumar
Date: 2021-09-20T10:23:35-07:00
New Revision: e31b2d7d7be98cbbaa665b2702cd0ed2975da4cc

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

LOG: [lldb][crashlog] Avoid specifying arch for image when a UUID is present

When adding an image to a target for crashlog purposes, avoid specifying
the architecture of the image.

This has the effect of making SBTarget::AddModule infer the ArchSpec for
the image based on the SBTarget's architecture, which LLDB puts serious
effort into calculating correctly (in TargetList::CreateTargetInternal).

The status quo is that LLDB randomly guesses the ArchSpec for a module
if its architecture is specified, via:

```
  SBTarget::AddModule -> Platform::GetAugmentedArchSpec -> 
Platform::IsCompatibleArchitecture ->
GetSupportedArchitectureAtIndex -> {ARM,x86}GetSupportedArchitectureAtIndex
```

... which means that the same crashlog can fail to load on an Apple
Silicon Mac (due to the random guess of arm64e-apple-macosx for the
module's ArchSpec not being compatible with the SBTarget's (correct)
ArchSpec), while loading just fine on an Intel Mac.

I'm not sure how to add a test for this (it doesn't look like there's
test coverage of this path in-tree). It seems like it would be pretty
complicated to regression test: the host LLDB would need to be built for
arm64e, we'd need a hand-crafted arm64e iOS crashlog, and we'd need a
binary with an iOS deployment target. I'm open to other / simpler
options.

rdar://82679400

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

Added: 


Modified: 
lldb/examples/python/symbolication.py

Removed: 




diff  --git a/lldb/examples/python/symbolication.py 
b/lldb/examples/python/symbolication.py
index 70f2ff3bb27c3..e15f7f4eaa3cf 100755
--- a/lldb/examples/python/symbolication.py
+++ b/lldb/examples/python/symbolication.py
@@ -379,7 +379,7 @@ def add_module(self, target):
 return None
 resolved_path = self.get_resolved_path()
 self.module = target.AddModule(
-resolved_path, str(self.arch), uuid_str, self.symfile)
+resolved_path, None, uuid_str, self.symfile)
 if not self.module:
 return 'error: unable to get module for (%s) "%s"' % (
 self.arch, self.get_resolved_path())



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

[Lldb-commits] [PATCH] D110011: [lldb] Add --stack option to `target symbols add` command

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 373642.
JDevlieghere marked an inline comment as done.
JDevlieghere added a comment.

- Rebase
- Add test


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

https://reviews.llvm.org/D110011

Files:
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/test/API/macosx/add-dsym/TestAddDsymDownload.py

Index: lldb/test/API/macosx/add-dsym/TestAddDsymDownload.py
===
--- /dev/null
+++ lldb/test/API/macosx/add-dsym/TestAddDsymDownload.py
@@ -0,0 +1,98 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+@skipUnlessDarwin
+class AddDsymDownload(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+dwarfdump_uuid_regex = re.compile('UUID: ([-0-9a-fA-F]+) \(([^\(]+)\) .*')
+
+def get_uuid(self):
+dwarfdump_cmd_output = subprocess.check_output(
+('/usr/bin/dwarfdump --uuid "%s"' % self.exe),
+shell=True).decode("utf-8")
+for line in dwarfdump_cmd_output.splitlines():
+match = self.dwarfdump_uuid_regex.search(line)
+if match:
+return match.group(1)
+return None
+
+def create_dsym_for_uuid(self):
+shell_cmds = [
+'#! /bin/sh', '# the last argument is the uuid',
+'while [ $# -gt 1 ]', 'do', '  shift', 'done', 'ret=0',
+'echo ""',
+'echo "http://www.apple.com/DTDs/PropertyList-1.0.dtd\\;>"',
+'echo ""', '',
+'if [ "$1" != "%s" ]' % (self.uuid), 'then',
+'  echo "DBGErrornot found"',
+'  echo ""', '  exit 1', 'fi',
+'  uuid=%s' % self.uuid,
+'  bin=%s' % self.exe,
+'  dsym=%s' % self.dsym, 'echo "$uuid"', '',
+'echo "DBGDSYMPath$dsym"',
+'echo "DBGSymbolRichExecutable$bin"',
+'echo ""', 'exit $ret'
+]
+
+with open(self.dsym_for_uuid, "w") as writer:
+for l in shell_cmds:
+writer.write(l + '\n')
+
+os.chmod(self.dsym_for_uuid, 0o755)
+
+def setUp(self):
+TestBase.setUp(self)
+self.source = 'main.c'
+self.exe = self.getBuildArtifact("a.out")
+self.dsym = os.path.join(
+self.getBuildDir(),
+"hide.app/Contents/a.out.dSYM/Contents/Resources/DWARF/",
+os.path.basename(self.exe))
+self.dsym_for_uuid = self.getBuildArtifact("dsym-for-uuid.sh")
+
+self.buildDefault(dictionary={'MAKE_DSYM': 'YES'})
+self.assertTrue(os.path.exists(self.exe))
+self.assertTrue(os.path.exists(self.dsym))
+
+self.uuid = self.get_uuid()
+self.assertNotEqual(self.uuid, None, "Could not get uuid for a.out")
+
+self.create_dsym_for_uuid()
+
+os.environ['LLDB_APPLE_DSYMFORUUID_EXECUTABLE'] = self.dsym_for_uuid
+self.addTearDownHook(
+lambda: os.environ.pop('LLDB_APPLE_DSYMFORUUID_EXECUTABLE', None))
+
+def do_test(self, command):
+self.target = self.dbg.CreateTarget(self.exe)
+self.assertTrue(self.target, VALID_TARGET)
+
+main_bp = self.target.BreakpointCreateByName("main", "a.out")
+self.assertTrue(main_bp, VALID_BREAKPOINT)
+
+self.process = self.target.LaunchSimple(
+None, None, self.get_process_working_directory())
+self.assertTrue(self.process, PROCESS_IS_VALID)
+
+# The stop reason of the thread should be breakpoint.
+self.assertEquals(self.process.GetState(), lldb.eStateStopped,
+  STOPPED_DUE_TO_BREAKPOINT)
+
+self.runCmd(command)
+self.expect("frame select", substrs=['a.out`main at main.c'])
+
+@no_debug_info_test
+def test_frame(self):
+self.do_test("add-dsym --frame")
+
+@no_debug_info_test
+def test_stack(self):
+self.do_test("add-dsym --uuid {}".format(self.uuid))
+
+@no_debug_info_test
+def test_stack(self):
+self.do_test("add-dsym --stack")
Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -3962,8 +3962,12 @@
 "name."),
 m_current_frame_option(
 LLDB_OPT_SET_2, false, "frame", 'F',
-"Locate the debug symbols for the currently selected frame.",
-false, true)
+"Locate the debug symbols for the currently selected frame.", false,
+true),
+m_current_stack_option(LLDB_OPT_SET_2, false, "stack", 'S',
+   "Locate the debug symbols for every frame in "
+   "the current call stack.",
+   false, true)
 
   {
 m_option_group.Append(_uuid_option_group, 

[Lldb-commits] [PATCH] D104413: Fixed use of -o and -k in LLDB under Windows when statically compiled with vcruntime.

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat added a comment.

Friendly ping =)


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

https://reviews.llvm.org/D104413

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

[Lldb-commits] [PATCH] D110010: [lldb] Extract adding symbols for UUID/File/Frame (NFC)

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
JDevlieghere marked an inline comment as done.
Closed by commit rGa89bfc61203d: [lldb] Extract adding symbols for 
UUID/File/Frame (NFC) (authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D110010?vs=373356=373617#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110010

Files:
  lldb/source/Commands/CommandObjectTarget.cpp

Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -4143,6 +4143,110 @@
 return false;
   }
 
+  bool DownloadObjectAndSymbolFile(ModuleSpec _spec,
+   CommandReturnObject , bool ) {
+if (Symbols::DownloadObjectAndSymbolFile(module_spec)) {
+  if (module_spec.GetSymbolFileSpec())
+return AddModuleSymbols(m_exe_ctx.GetTargetPtr(), module_spec, flush,
+result);
+}
+return false;
+  }
+
+  bool AddSymbolsForUUID(CommandReturnObject , bool ) {
+assert(m_uuid_option_group.GetOptionValue().OptionWasSet());
+
+ModuleSpec module_spec;
+module_spec.GetUUID() =
+m_uuid_option_group.GetOptionValue().GetCurrentValue();
+
+if (!DownloadObjectAndSymbolFile(module_spec, result, flush)) {
+  StreamString error_strm;
+  error_strm.PutCString("unable to find debug symbols for UUID ");
+  module_spec.GetUUID().Dump(_strm);
+  result.AppendError(error_strm.GetString());
+  return false;
+}
+
+return true;
+  }
+
+  bool AddSymbolsForFile(CommandReturnObject , bool ) {
+assert(m_file_option.GetOptionValue().OptionWasSet());
+
+ModuleSpec module_spec;
+module_spec.GetFileSpec() =
+m_file_option.GetOptionValue().GetCurrentValue();
+
+Target *target = m_exe_ctx.GetTargetPtr();
+ModuleSP module_sp(target->GetImages().FindFirstModule(module_spec));
+if (module_sp) {
+  module_spec.GetFileSpec() = module_sp->GetFileSpec();
+  module_spec.GetPlatformFileSpec() = module_sp->GetPlatformFileSpec();
+  module_spec.GetUUID() = module_sp->GetUUID();
+  module_spec.GetArchitecture() = module_sp->GetArchitecture();
+} else {
+  module_spec.GetArchitecture() = target->GetArchitecture();
+}
+
+if (!DownloadObjectAndSymbolFile(module_spec, result, flush)) {
+  StreamString error_strm;
+  error_strm.PutCString(
+  "unable to find debug symbols for the executable file ");
+  error_strm << module_spec.GetFileSpec();
+  result.AppendError(error_strm.GetString());
+  return false;
+}
+
+return true;
+  }
+
+  bool AddSymbolsForFrame(CommandReturnObject , bool ) {
+assert(m_current_frame_option.GetOptionValue().OptionWasSet());
+
+Process *process = m_exe_ctx.GetProcessPtr();
+if (!process) {
+  result.AppendError(
+  "a process must exist in order to use the --frame option");
+  return false;
+}
+
+const StateType process_state = process->GetState();
+if (!StateIsStoppedState(process_state, true)) {
+  result.AppendErrorWithFormat("process is not stopped: %s",
+   StateAsCString(process_state));
+  return false;
+}
+
+StackFrame *frame = m_exe_ctx.GetFramePtr();
+if (!frame) {
+  result.AppendError("invalid current frame");
+  return false;
+}
+
+ModuleSP frame_module_sp(
+frame->GetSymbolContext(eSymbolContextModule).module_sp);
+if (!frame_module_sp) {
+  result.AppendError("frame has no module");
+  return false;
+}
+
+ModuleSpec module_spec;
+module_spec.GetUUID() = frame_module_sp->GetUUID();
+
+if (FileSystem::Instance().Exists(frame_module_sp->GetPlatformFileSpec())) {
+  module_spec.GetArchitecture() = frame_module_sp->GetArchitecture();
+  module_spec.GetFileSpec() = frame_module_sp->GetPlatformFileSpec();
+}
+
+if (!DownloadObjectAndSymbolFile(module_spec, result, flush)) {
+  result.AppendError("unable to find debug symbols for the current frame");
+  return false;
+}
+
+return true;
+  }
+
   bool DoExecute(Args , CommandReturnObject ) override {
 Target *target = m_exe_ctx.GetTargetPtr();
 result.SetStatus(eReturnStatusFailed);
@@ -4156,97 +4260,15 @@
 const size_t argc = args.GetArgumentCount();
 
 if (argc == 0) {
-  if (uuid_option_set || file_option_set || frame_option_set) {
-bool success = false;
-bool error_set = false;
-if (frame_option_set) {
-  Process *process = m_exe_ctx.GetProcessPtr();
-  if (process) {
-const StateType process_state = process->GetState();
-if 

[Lldb-commits] [lldb] a89bfc6 - [lldb] Extract adding symbols for UUID/File/Frame (NFC)

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2021-09-20T09:08:04-07:00
New Revision: a89bfc61203d5c2071cddaff26345771716463ec

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

LOG: [lldb] Extract adding symbols for UUID/File/Frame (NFC)

This moves the logic for adding symbols based on UUID, file and frame
into little helper functions. This is in preparation for D110011.

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

Added: 


Modified: 
lldb/source/Commands/CommandObjectTarget.cpp

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectTarget.cpp 
b/lldb/source/Commands/CommandObjectTarget.cpp
index 9d93d8574a92..92aa8fc70a7b 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -4143,6 +4143,110 @@ class CommandObjectTargetSymbolsAdd : public 
CommandObjectParsed {
 return false;
   }
 
+  bool DownloadObjectAndSymbolFile(ModuleSpec _spec,
+   CommandReturnObject , bool ) {
+if (Symbols::DownloadObjectAndSymbolFile(module_spec)) {
+  if (module_spec.GetSymbolFileSpec())
+return AddModuleSymbols(m_exe_ctx.GetTargetPtr(), module_spec, flush,
+result);
+}
+return false;
+  }
+
+  bool AddSymbolsForUUID(CommandReturnObject , bool ) {
+assert(m_uuid_option_group.GetOptionValue().OptionWasSet());
+
+ModuleSpec module_spec;
+module_spec.GetUUID() =
+m_uuid_option_group.GetOptionValue().GetCurrentValue();
+
+if (!DownloadObjectAndSymbolFile(module_spec, result, flush)) {
+  StreamString error_strm;
+  error_strm.PutCString("unable to find debug symbols for UUID ");
+  module_spec.GetUUID().Dump(_strm);
+  result.AppendError(error_strm.GetString());
+  return false;
+}
+
+return true;
+  }
+
+  bool AddSymbolsForFile(CommandReturnObject , bool ) {
+assert(m_file_option.GetOptionValue().OptionWasSet());
+
+ModuleSpec module_spec;
+module_spec.GetFileSpec() =
+m_file_option.GetOptionValue().GetCurrentValue();
+
+Target *target = m_exe_ctx.GetTargetPtr();
+ModuleSP module_sp(target->GetImages().FindFirstModule(module_spec));
+if (module_sp) {
+  module_spec.GetFileSpec() = module_sp->GetFileSpec();
+  module_spec.GetPlatformFileSpec() = module_sp->GetPlatformFileSpec();
+  module_spec.GetUUID() = module_sp->GetUUID();
+  module_spec.GetArchitecture() = module_sp->GetArchitecture();
+} else {
+  module_spec.GetArchitecture() = target->GetArchitecture();
+}
+
+if (!DownloadObjectAndSymbolFile(module_spec, result, flush)) {
+  StreamString error_strm;
+  error_strm.PutCString(
+  "unable to find debug symbols for the executable file ");
+  error_strm << module_spec.GetFileSpec();
+  result.AppendError(error_strm.GetString());
+  return false;
+}
+
+return true;
+  }
+
+  bool AddSymbolsForFrame(CommandReturnObject , bool ) {
+assert(m_current_frame_option.GetOptionValue().OptionWasSet());
+
+Process *process = m_exe_ctx.GetProcessPtr();
+if (!process) {
+  result.AppendError(
+  "a process must exist in order to use the --frame option");
+  return false;
+}
+
+const StateType process_state = process->GetState();
+if (!StateIsStoppedState(process_state, true)) {
+  result.AppendErrorWithFormat("process is not stopped: %s",
+   StateAsCString(process_state));
+  return false;
+}
+
+StackFrame *frame = m_exe_ctx.GetFramePtr();
+if (!frame) {
+  result.AppendError("invalid current frame");
+  return false;
+}
+
+ModuleSP frame_module_sp(
+frame->GetSymbolContext(eSymbolContextModule).module_sp);
+if (!frame_module_sp) {
+  result.AppendError("frame has no module");
+  return false;
+}
+
+ModuleSpec module_spec;
+module_spec.GetUUID() = frame_module_sp->GetUUID();
+
+if (FileSystem::Instance().Exists(frame_module_sp->GetPlatformFileSpec())) 
{
+  module_spec.GetArchitecture() = frame_module_sp->GetArchitecture();
+  module_spec.GetFileSpec() = frame_module_sp->GetPlatformFileSpec();
+}
+
+if (!DownloadObjectAndSymbolFile(module_spec, result, flush)) {
+  result.AppendError("unable to find debug symbols for the current frame");
+  return false;
+}
+
+return true;
+  }
+
   bool DoExecute(Args , CommandReturnObject ) override {
 Target *target = m_exe_ctx.GetTargetPtr();
 result.SetStatus(eReturnStatusFailed);
@@ -4156,97 +4260,15 @@ class CommandObjectTargetSymbolsAdd : public 
CommandObjectParsed {
 const size_t argc = args.GetArgumentCount();
 
 if (argc == 0) {
-  if (uuid_option_set || 

[Lldb-commits] [lldb] fe4b846 - [lldb] Fix whitespace in CommandObjectTarget (NFC)

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2021-09-20T09:08:03-07:00
New Revision: fe4b8467b5dca564b4859256b08ece5fa1eaa574

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

LOG: [lldb] Fix whitespace in CommandObjectTarget (NFC)

Added: 


Modified: 
lldb/source/Commands/CommandObjectTarget.cpp

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectTarget.cpp 
b/lldb/source/Commands/CommandObjectTarget.cpp
index 614ba5a8e4c8..9d93d8574a92 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -4514,29 +4514,29 @@ class CommandObjectTargetStopHookAdd : public 
CommandObjectParsed,
 Command Based stop-hooks:
 -
   Stop hooks can run a list of lldb commands by providing one or more
-  --one-line-command options.  The commands will get run in the order they are 
+  --one-line-command options.  The commands will get run in the order they are
   added.  Or you can provide no commands, in which case you will enter a
   command editor where you can enter the commands to be run.
-  
+
 Python Based Stop Hooks:
 
   Stop hooks can be implemented with a suitably defined Python class, whose 
name
   is passed in the --python-class option.
-  
+
   When the stop hook is added, the class is initialized by calling:
-  
+
 def __init__(self, target, extra_args, internal_dict):
-
+
 target: The target that the stop hook is being added to.
-extra_args: An SBStructuredData Dictionary filled with the -key -value 
-option pairs passed to the command. 
+extra_args: An SBStructuredData Dictionary filled with the -key -value
+option pairs passed to the command.
 dict: An implementation detail provided by lldb.
 
-  Then when the stop-hook triggers, lldb will run the 'handle_stop' method. 
+  Then when the stop-hook triggers, lldb will run the 'handle_stop' method.
   The method has the signature:
-  
+
 def handle_stop(self, exe_ctx, stream):
-
+
 exe_ctx: An SBExecutionContext for the thread that has stopped.
 stream: An SBStream, anything written to this stream will be printed in the
 the stop message when the process stops.
@@ -4545,12 +4545,12 @@ Python Based Stop Hooks:
   from all the stop hook executions on threads that stopped
   with a reason, then the process will continue.  Note that 
this
   will happen only after all the stop hooks are run.
-
+
 Filter Options:
 ---
   Stop hooks can be set to always run, or to only run when the stopped thread
   matches the filter options passed on the command line.  The available filter
-  options include a shared library or a thread or queue specification, 
+  options include a shared library or a thread or queue specification,
   a line range in a source file, a function name or a class name.
 )");
 m_all_options.Append(_python_class_options,



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

[Lldb-commits] [PATCH] D110011: [lldb] Add --stack option to `target symbols add` command

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

Does it make sense to add a test for this? It looks like we have two basic 
tests for `add-dsym` already.


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

https://reviews.llvm.org/D110011

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

[Lldb-commits] [PATCH] D110010: [lldb] Extract adding symbols for UUID/File/Frame (NFC)

[Jonas Devlieghere via Phabricator via lldb-commits]

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



Comment at: lldb/source/Commands/CommandObjectTarget.cpp:4203
+
+  bool AddSymbolsForFrame(Target *target, CommandReturnObject ,
+  bool ) {

labath wrote:
> Do we need to pass the target around? I would assume that can be retrieved 
> from `m_exe_ctx` as well..
Yup, I'll fix that before landing. 


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

https://reviews.llvm.org/D110010

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

[Lldb-commits] [PATCH] D109879: [lldb] [DynamicRegisterInfo] Unset value_regs/invalidate_regs before Finalize()

[Michał Górny 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 rGec50d351ffdd: [lldb] [DynamicRegisterInfo] Unset 
value_regs/invalidate_regs before Finalize() (authored by mgorny).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D109879?vs=373511=373568#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109879

Files:
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
  lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp


Index: lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
===
--- lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
+++ lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
@@ -100,3 +100,27 @@
   ASSERT_REG(i1, 16, {}, {b1});
   ASSERT_REG(i2, 8, {b2}, {b1, i1});
 }
+
+TEST_F(DynamicRegisterInfoTest, no_finalize_regs) {
+  // Add regular registers
+  uint32_t b1 = AddTestRegister("b1", 8);
+  uint32_t b2 = AddTestRegister("b2", 8);
+
+  // Add a few sub-registers
+  uint32_t s1 = AddTestRegister("s1", 4, {b1});
+  uint32_t s2 = AddTestRegister("s2", 4, {b2});
+
+  // Add a register with invalidate_regs
+  uint32_t i1 = AddTestRegister("i1", 8, {}, {b1});
+
+  // Add a register with indirect invalidate regs to be expanded
+  // TODO: why is it done conditionally to value_regs?
+  uint32_t i2 = AddTestRegister("i2", 4, {b2}, {i1});
+
+  ASSERT_REG(b1, LLDB_INVALID_INDEX32);
+  ASSERT_REG(b2, LLDB_INVALID_INDEX32);
+  ASSERT_REG(s1, LLDB_INVALID_INDEX32);
+  ASSERT_REG(s2, LLDB_INVALID_INDEX32);
+  ASSERT_REG(i1, LLDB_INVALID_INDEX32);
+  ASSERT_REG(i2, LLDB_INVALID_INDEX32);
+}
Index: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
===
--- lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
+++ lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
@@ -35,7 +35,7 @@
   size_t SetRegisterInfo(const lldb_private::StructuredData::Dictionary ,
  const lldb_private::ArchSpec );
 
-  void AddRegister(lldb_private::RegisterInfo _info,
+  void AddRegister(lldb_private::RegisterInfo reg_info,
lldb_private::ConstString _name);
 
   void Finalize(const lldb_private::ArchSpec );
Index: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
===
--- lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
+++ lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
@@ -395,7 +395,7 @@
   return m_regs.size();
 }
 
-void DynamicRegisterInfo::AddRegister(RegisterInfo _info,
+void DynamicRegisterInfo::AddRegister(RegisterInfo reg_info,
   ConstString _name) {
   assert(!m_finalized);
   const uint32_t reg_num = m_regs.size();
@@ -404,10 +404,16 @@
   if (reg_info.value_regs) {
 for (i = 0; reg_info.value_regs[i] != LLDB_INVALID_REGNUM; ++i)
   m_value_regs_map[reg_num].push_back(reg_info.value_regs[i]);
+
+// invalidate until Finalize() is called
+reg_info.value_regs = nullptr;
   }
   if (reg_info.invalidate_regs) {
 for (i = 0; reg_info.invalidate_regs[i] != LLDB_INVALID_REGNUM; ++i)
   m_invalidate_regs_map[reg_num].push_back(reg_info.invalidate_regs[i]);
+
+// invalidate until Finalize() is called
+reg_info.invalidate_regs = nullptr;
   }
   if (reg_info.dynamic_size_dwarf_expr_bytes) {
 for (i = 0; i < reg_info.dynamic_size_dwarf_len; ++i)


Index: lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
===
--- lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
+++ lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
@@ -100,3 +100,27 @@
   ASSERT_REG(i1, 16, {}, {b1});
   ASSERT_REG(i2, 8, {b2}, {b1, i1});
 }
+
+TEST_F(DynamicRegisterInfoTest, no_finalize_regs) {
+  // Add regular registers
+  uint32_t b1 = AddTestRegister("b1", 8);
+  uint32_t b2 = AddTestRegister("b2", 8);
+
+  // Add a few sub-registers
+  uint32_t s1 = AddTestRegister("s1", 4, {b1});
+  uint32_t s2 = AddTestRegister("s2", 4, {b2});
+
+  // Add a register with invalidate_regs
+  uint32_t i1 = AddTestRegister("i1", 8, {}, {b1});
+
+  // Add a register with indirect invalidate regs to be expanded
+  // TODO: why is it done conditionally to value_regs?
+  uint32_t i2 = AddTestRegister("i2", 4, {b2}, {i1});
+
+  ASSERT_REG(b1, LLDB_INVALID_INDEX32);
+  ASSERT_REG(b2, LLDB_INVALID_INDEX32);
+  ASSERT_REG(s1, LLDB_INVALID_INDEX32);
+  ASSERT_REG(s2, LLDB_INVALID_INDEX32);
+  ASSERT_REG(i1, LLDB_INVALID_INDEX32);
+  ASSERT_REG(i2, LLDB_INVALID_INDEX32);
+}
Index: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h

[Lldb-commits] [PATCH] D109906: [lldb] [test] Add unittest for DynamicRegisterInfo::Finalize()

[Michał Górny 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 rG4737dcbc83e0: [lldb] [test] Add unittest for 
DynamicRegisterInfo::Finalize() (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109906

Files:
  lldb/unittests/Process/Utility/CMakeLists.txt
  lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp

Index: lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
===
--- /dev/null
+++ lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
@@ -0,0 +1,102 @@
+//===-- DynamicRegisterInfoTest.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 "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+#include "Plugins/Process/Utility/DynamicRegisterInfo.h"
+
+#include "lldb/Utility/ArchSpec.h"
+
+using namespace lldb_private;
+
+static std::vector regs_to_vector(uint32_t *regs) {
+  std::vector ret;
+  if (regs) {
+while (*regs != LLDB_INVALID_REGNUM)
+  ret.push_back(*regs++);
+  }
+  return ret;
+}
+
+class DynamicRegisterInfoTest : public ::testing::Test {
+protected:
+  DynamicRegisterInfo info;
+  uint32_t next_regnum = 0;
+  ConstString group{"group"};
+
+  uint32_t AddTestRegister(const char *name, uint32_t byte_size,
+   std::vector value_regs = {},
+   std::vector invalidate_regs = {}) {
+struct RegisterInfo new_reg {
+  name, nullptr, byte_size, LLDB_INVALID_INDEX32, lldb::eEncodingUint,
+  lldb::eFormatUnsigned,
+  {LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM,
+   next_regnum, next_regnum},
+  nullptr, nullptr, nullptr, 0
+};
+
+if (!value_regs.empty()) {
+  value_regs.push_back(LLDB_INVALID_REGNUM);
+  new_reg.value_regs = value_regs.data();
+}
+if (!invalidate_regs.empty()) {
+  invalidate_regs.push_back(LLDB_INVALID_REGNUM);
+  new_reg.invalidate_regs = invalidate_regs.data();
+}
+
+info.AddRegister(new_reg, group);
+return next_regnum++;
+  }
+
+  void AssertRegisterInfo(uint32_t reg_num, const char *reg_name,
+  uint32_t byte_offset,
+  std::vector value_regs = {},
+  std::vector invalidate_regs = {}) {
+const RegisterInfo *reg = info.GetRegisterInfoAtIndex(reg_num);
+EXPECT_NE(reg, nullptr);
+if (!reg)
+  return;
+
+EXPECT_STREQ(reg->name, reg_name);
+EXPECT_EQ(reg->byte_offset, byte_offset);
+EXPECT_THAT(regs_to_vector(reg->value_regs), value_regs);
+EXPECT_THAT(regs_to_vector(reg->invalidate_regs), invalidate_regs);
+  }
+};
+
+#define ASSERT_REG(reg, ...) { \
+  SCOPED_TRACE("at register " #reg); \
+  AssertRegisterInfo(reg, #reg, __VA_ARGS__); \
+  }
+
+TEST_F(DynamicRegisterInfoTest, finalize_regs) {
+  // Add regular registers
+  uint32_t b1 = AddTestRegister("b1", 8);
+  uint32_t b2 = AddTestRegister("b2", 8);
+
+  // Add a few sub-registers
+  uint32_t s1 = AddTestRegister("s1", 4, {b1});
+  uint32_t s2 = AddTestRegister("s2", 4, {b2});
+
+  // Add a register with invalidate_regs
+  uint32_t i1 = AddTestRegister("i1", 8, {}, {b1});
+
+  // Add a register with indirect invalidate regs to be expanded
+  // TODO: why is it done conditionally to value_regs?
+  uint32_t i2 = AddTestRegister("i2", 4, {b2}, {i1});
+
+  info.Finalize(lldb_private::ArchSpec());
+
+  ASSERT_REG(b1, 0);
+  ASSERT_REG(b2, 8);
+  ASSERT_REG(s1, 0, {b1});
+  ASSERT_REG(s2, 8, {b2});
+  ASSERT_REG(i1, 16, {}, {b1});
+  ASSERT_REG(i2, 8, {b2}, {b1, i1});
+}
Index: lldb/unittests/Process/Utility/CMakeLists.txt
===
--- lldb/unittests/Process/Utility/CMakeLists.txt
+++ lldb/unittests/Process/Utility/CMakeLists.txt
@@ -15,9 +15,10 @@
   ${NETBSD_SOURCES})
 
 add_lldb_unittest(ProcessUtilityTests
-  RegisterContextTest.cpp
+  DynamicRegisterInfoTest.cpp
   LinuxProcMapsTest.cpp
   MemoryTagManagerAArch64MTETest.cpp
+  RegisterContextTest.cpp
   ${PLATFORM_SOURCES}
 
   LINK_LIBS
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] ec50d35 - [lldb] [DynamicRegisterInfo] Unset value_regs/invalidate_regs before Finalize()

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-09-20T15:02:20+02:00
New Revision: ec50d351ffdd4559ccce6013d3ab4a3f41c42cee

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

LOG: [lldb] [DynamicRegisterInfo] Unset value_regs/invalidate_regs before 
Finalize()

Set value_regs and invalidate_regs in RegisterInfo pushed onto m_regs
to nullptr, to ensure that the temporaries passed there are not
accidentally used.

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

Added: 


Modified: 
lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp 
b/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
index cc79260d0797..2755e5f93d5a 100644
--- a/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
+++ b/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
@@ -395,7 +395,7 @@ DynamicRegisterInfo::SetRegisterInfo(const 
StructuredData::Dictionary ,
   return m_regs.size();
 }
 
-void DynamicRegisterInfo::AddRegister(RegisterInfo _info,
+void DynamicRegisterInfo::AddRegister(RegisterInfo reg_info,
   ConstString _name) {
   assert(!m_finalized);
   const uint32_t reg_num = m_regs.size();
@@ -404,10 +404,16 @@ void DynamicRegisterInfo::AddRegister(RegisterInfo 
_info,
   if (reg_info.value_regs) {
 for (i = 0; reg_info.value_regs[i] != LLDB_INVALID_REGNUM; ++i)
   m_value_regs_map[reg_num].push_back(reg_info.value_regs[i]);
+
+// invalidate until Finalize() is called
+reg_info.value_regs = nullptr;
   }
   if (reg_info.invalidate_regs) {
 for (i = 0; reg_info.invalidate_regs[i] != LLDB_INVALID_REGNUM; ++i)
   m_invalidate_regs_map[reg_num].push_back(reg_info.invalidate_regs[i]);
+
+// invalidate until Finalize() is called
+reg_info.invalidate_regs = nullptr;
   }
   if (reg_info.dynamic_size_dwarf_expr_bytes) {
 for (i = 0; i < reg_info.dynamic_size_dwarf_len; ++i)

diff  --git a/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h 
b/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
index 663fbab57810..a6495dcda608 100644
--- a/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
+++ b/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
@@ -35,7 +35,7 @@ class DynamicRegisterInfo {
   size_t SetRegisterInfo(const lldb_private::StructuredData::Dictionary ,
  const lldb_private::ArchSpec );
 
-  void AddRegister(lldb_private::RegisterInfo _info,
+  void AddRegister(lldb_private::RegisterInfo reg_info,
lldb_private::ConstString _name);
 
   void Finalize(const lldb_private::ArchSpec );

diff  --git a/lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp 
b/lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
index 68c1ee3517c0..5f2b278b9b75 100644
--- a/lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
+++ b/lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
@@ -100,3 +100,27 @@ TEST_F(DynamicRegisterInfoTest, finalize_regs) {
   ASSERT_REG(i1, 16, {}, {b1});
   ASSERT_REG(i2, 8, {b2}, {b1, i1});
 }
+
+TEST_F(DynamicRegisterInfoTest, no_finalize_regs) {
+  // Add regular registers
+  uint32_t b1 = AddTestRegister("b1", 8);
+  uint32_t b2 = AddTestRegister("b2", 8);
+
+  // Add a few sub-registers
+  uint32_t s1 = AddTestRegister("s1", 4, {b1});
+  uint32_t s2 = AddTestRegister("s2", 4, {b2});
+
+  // Add a register with invalidate_regs
+  uint32_t i1 = AddTestRegister("i1", 8, {}, {b1});
+
+  // Add a register with indirect invalidate regs to be expanded
+  // TODO: why is it done conditionally to value_regs?
+  uint32_t i2 = AddTestRegister("i2", 4, {b2}, {i1});
+
+  ASSERT_REG(b1, LLDB_INVALID_INDEX32);
+  ASSERT_REG(b2, LLDB_INVALID_INDEX32);
+  ASSERT_REG(s1, LLDB_INVALID_INDEX32);
+  ASSERT_REG(s2, LLDB_INVALID_INDEX32);
+  ASSERT_REG(i1, LLDB_INVALID_INDEX32);
+  ASSERT_REG(i2, LLDB_INVALID_INDEX32);
+}



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

[Lldb-commits] [lldb] 4737dcb - [lldb] [test] Add unittest for DynamicRegisterInfo::Finalize()

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-09-20T15:02:20+02:00
New Revision: 4737dcbc83e05ac97c8695cf9a19bddb6446d71f

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

LOG: [lldb] [test] Add unittest for DynamicRegisterInfo::Finalize()

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

Added: 
lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp

Modified: 
lldb/unittests/Process/Utility/CMakeLists.txt

Removed: 




diff  --git a/lldb/unittests/Process/Utility/CMakeLists.txt 
b/lldb/unittests/Process/Utility/CMakeLists.txt
index 95b65cef6a42..47abc10f5aa0 100644
--- a/lldb/unittests/Process/Utility/CMakeLists.txt
+++ b/lldb/unittests/Process/Utility/CMakeLists.txt
@@ -15,9 +15,10 @@ set(LLVM_OPTIONAL_SOURCES
   ${NETBSD_SOURCES})
 
 add_lldb_unittest(ProcessUtilityTests
-  RegisterContextTest.cpp
+  DynamicRegisterInfoTest.cpp
   LinuxProcMapsTest.cpp
   MemoryTagManagerAArch64MTETest.cpp
+  RegisterContextTest.cpp
   ${PLATFORM_SOURCES}
 
   LINK_LIBS

diff  --git a/lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp 
b/lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
new file mode 100644
index ..68c1ee3517c0
--- /dev/null
+++ b/lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
@@ -0,0 +1,102 @@
+//===-- DynamicRegisterInfoTest.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 "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+#include "Plugins/Process/Utility/DynamicRegisterInfo.h"
+
+#include "lldb/Utility/ArchSpec.h"
+
+using namespace lldb_private;
+
+static std::vector regs_to_vector(uint32_t *regs) {
+  std::vector ret;
+  if (regs) {
+while (*regs != LLDB_INVALID_REGNUM)
+  ret.push_back(*regs++);
+  }
+  return ret;
+}
+
+class DynamicRegisterInfoTest : public ::testing::Test {
+protected:
+  DynamicRegisterInfo info;
+  uint32_t next_regnum = 0;
+  ConstString group{"group"};
+
+  uint32_t AddTestRegister(const char *name, uint32_t byte_size,
+   std::vector value_regs = {},
+   std::vector invalidate_regs = {}) {
+struct RegisterInfo new_reg {
+  name, nullptr, byte_size, LLDB_INVALID_INDEX32, lldb::eEncodingUint,
+  lldb::eFormatUnsigned,
+  {LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM,
+   next_regnum, next_regnum},
+  nullptr, nullptr, nullptr, 0
+};
+
+if (!value_regs.empty()) {
+  value_regs.push_back(LLDB_INVALID_REGNUM);
+  new_reg.value_regs = value_regs.data();
+}
+if (!invalidate_regs.empty()) {
+  invalidate_regs.push_back(LLDB_INVALID_REGNUM);
+  new_reg.invalidate_regs = invalidate_regs.data();
+}
+
+info.AddRegister(new_reg, group);
+return next_regnum++;
+  }
+
+  void AssertRegisterInfo(uint32_t reg_num, const char *reg_name,
+  uint32_t byte_offset,
+  std::vector value_regs = {},
+  std::vector invalidate_regs = {}) {
+const RegisterInfo *reg = info.GetRegisterInfoAtIndex(reg_num);
+EXPECT_NE(reg, nullptr);
+if (!reg)
+  return;
+
+EXPECT_STREQ(reg->name, reg_name);
+EXPECT_EQ(reg->byte_offset, byte_offset);
+EXPECT_THAT(regs_to_vector(reg->value_regs), value_regs);
+EXPECT_THAT(regs_to_vector(reg->invalidate_regs), invalidate_regs);
+  }
+};
+
+#define ASSERT_REG(reg, ...) { \
+  SCOPED_TRACE("at register " #reg); \
+  AssertRegisterInfo(reg, #reg, __VA_ARGS__); \
+  }
+
+TEST_F(DynamicRegisterInfoTest, finalize_regs) {
+  // Add regular registers
+  uint32_t b1 = AddTestRegister("b1", 8);
+  uint32_t b2 = AddTestRegister("b2", 8);
+
+  // Add a few sub-registers
+  uint32_t s1 = AddTestRegister("s1", 4, {b1});
+  uint32_t s2 = AddTestRegister("s2", 4, {b2});
+
+  // Add a register with invalidate_regs
+  uint32_t i1 = AddTestRegister("i1", 8, {}, {b1});
+
+  // Add a register with indirect invalidate regs to be expanded
+  // TODO: why is it done conditionally to value_regs?
+  uint32_t i2 = AddTestRegister("i2", 4, {b2}, {i1});
+
+  info.Finalize(lldb_private::ArchSpec());
+
+  ASSERT_REG(b1, 0);
+  ASSERT_REG(b2, 8);
+  ASSERT_REG(s1, 0, {b1});
+  ASSERT_REG(s2, 8, {b2});
+  ASSERT_REG(i1, 16, {}, {b1});
+  ASSERT_REG(i2, 8, {b2}, {b1, i1});
+}



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

[Lldb-commits] [PATCH] D109797: Fix rendezvous for rebase_exec=true case

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay added a comment.

In D109797#3005885 , @labath wrote:

> I'm not sure all of this is relevant. I've just tried to debug a very simple 
> (linux) executable that does a dlopens a library. If I attach to the 
> executable before the dlopen call, lldb does not notice the new library. This 
> patch fixes the issue. So I guess the test could be just that. You could do 
> something similar to TestLoadUnload -- either add a new test case which 
> attaches to that binary, or use it as inspiration to write a new test.

PTAL. I added a new `dlopen` test for this. I can only test on Linux at the 
moment, so LMK if there's something I need to do to support more platforms or 
remove them from testing matrix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109797

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

[Lldb-commits] [PATCH] D109797: Fix rendezvous for rebase_exec=true case

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 373560.
emrekultursay added a comment.

Minor touch-ups to the newly added test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109797

Files:
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  lldb/test/API/functionalities/dlopen/Makefile
  lldb/test/API/functionalities/dlopen/TestDlopen.py
  lldb/test/API/functionalities/dlopen/b.cpp
  lldb/test/API/functionalities/dlopen/main.cpp

Index: lldb/test/API/functionalities/dlopen/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/dlopen/main.cpp
@@ -0,0 +1,29 @@
+#include 
+#include 
+#include 
+#include 
+
+int main(int argc, char* argv[]) {
+  const char* solib = "./liblib_b.so";
+  if (argc == 2) {
+solib = argv[1];
+  }
+  printf("Using solib at: %s\n", solib);
+
+  int main_thread_continue = 0;
+  // Wait until debugger is attached.
+  int i = 0;
+  int timeout = 10;
+  for (i = 0; i < timeout; i++) {
+usleep(1000*1000); // break here
+if (main_thread_continue) {
+  break;
+}
+  }
+  assert(i != timeout && "timed out waiting for debugger");
+
+  // dlopen the 'liblib_b.so' shared library.
+  void* h = dlopen(solib, RTLD_LAZY);
+  assert(h && "dlopen failed?");
+  return i; // break after dlopen
+}
Index: lldb/test/API/functionalities/dlopen/b.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/dlopen/b.cpp
@@ -0,0 +1,4 @@
+
+
+int LLDB_DYLIB_EXPORT b_function() { return 500; }
+
Index: lldb/test/API/functionalities/dlopen/TestDlopen.py
===
--- /dev/null
+++ lldb/test/API/functionalities/dlopen/TestDlopen.py
@@ -0,0 +1,60 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipIfRemote
+@skipIfWindows
+def test_dlopen_after_attach(self):
+self.build()
+exe = self.getBuildArtifact("a.out")
+so = self.getBuildArtifact("liblib_b.so")
+
+# Spawn a new process.
+# use realpath to workaround llvm.org/pr48376
+# Pass path to solib for dlopen to properly locate the library.
+popen = self.spawnSubprocess(os.path.realpath(exe), args = [os.path.realpath(so)])
+pid = popen.pid
+
+# Attach to the spawned process.
+self.runCmd("process attach -p " + str(pid))
+
+target = self.dbg.GetSelectedTarget()
+process = target.GetProcess()
+self.assertTrue(process, PROCESS_IS_VALID)
+
+# Continue until first breakpoint.
+breakpoint1 = self.target().BreakpointCreateBySourceRegex(
+"// break here", lldb.SBFileSpec("main.cpp"))
+self.assertEqual(breakpoint1.GetNumResolvedLocations(), 1)
+stopped_threads = lldbutil.continue_to_breakpoint(self.process(), breakpoint1)
+self.assertEqual(len(stopped_threads), 1)
+
+# Check that image list does not contain lib_b.so before dlopen.
+self.match(
+"image list",
+patterns = ["liblib_b.so"],
+matching = False,
+msg = "liblib_b.so should not have been in image list")
+
+# Change a variable to escape the loop
+self.runCmd("expression main_thread_continue = 1")
+
+# Continue so that dlopen is called.
+breakpoint2 = self.target().BreakpointCreateBySourceRegex(
+"// break after dlopen", lldb.SBFileSpec("main.cpp"))
+self.assertEqual(breakpoint2.GetNumResolvedLocations(), 1)
+process.Continue()
+self.assertEqual(len(stopped_threads), 1)
+
+# Check that image list contains lib_b.so after dlopen.
+self.match(
+"image list",
+patterns = ["liblib_b.so"],
+matching = True,
+msg = "Missing liblib_b.so in image list")
+
Index: lldb/test/API/functionalities/dlopen/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/dlopen/Makefile
@@ -0,0 +1,9 @@
+CXX_SOURCES := main.cpp
+USE_LIBDL := 1
+
+lib_b:
+	$(MAKE) -f $(MAKEFILE_RULES) \
+		DYLIB_ONLY=YES DYLIB_CXX_SOURCES=b.cpp DYLIB_NAME=lib_b
+all: lib_b
+
+include Makefile.rules
Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -442,14 +442,18 @@
 if (module_sp->GetObjectFile()->GetBaseAddress().GetLoadAddress(
 _process->GetTarget()) == 

[Lldb-commits] [PATCH] D109797: Fix rendezvous for rebase_exec=true case

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 373559.
emrekultursay added a comment.

Add new dlopen test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109797

Files:
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  lldb/test/API/functionalities/dlopen/Makefile
  lldb/test/API/functionalities/dlopen/TestDlopen.py
  lldb/test/API/functionalities/dlopen/b.cpp
  lldb/test/API/functionalities/dlopen/main.cpp

Index: lldb/test/API/functionalities/dlopen/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/dlopen/main.cpp
@@ -0,0 +1,30 @@
+#include 
+#include 
+#include 
+#include 
+
+int main(int argc, char* argv[]) {
+  const char* solib = "./liblib_b.so";
+  if (argc == 2) {
+solib = argv[1];
+  }
+  printf("Using solib at: %s\n", solib);
+
+  int main_thread_continue = 0;
+  // Wait until debugger is attached.
+  int i = 0;
+  for (i = 0; i < 100; i++) {
+usleep(1000*1000); // break here
+if (main_thread_continue) {
+  break;
+}
+  }
+  assert(i != 100 && "timed out waiting for debugger");
+
+  // dlopen the 'lib_b.so' shared library.
+  void* h = dlopen(solib, RTLD_LAZY);
+  if (h == nullptr) {
+assert(h && "dlopen failed?"); // break on error
+  }
+  return i; // break after dlopen
+}
Index: lldb/test/API/functionalities/dlopen/b.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/dlopen/b.cpp
@@ -0,0 +1,4 @@
+
+
+int LLDB_DYLIB_EXPORT b_function() { return 500; }
+
Index: lldb/test/API/functionalities/dlopen/TestDlopen.py
===
--- /dev/null
+++ lldb/test/API/functionalities/dlopen/TestDlopen.py
@@ -0,0 +1,60 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipIfRemote
+@skipIfWindows
+def test_dlopen_after_attach(self):
+self.build()
+exe = self.getBuildArtifact("a.out")
+so = self.getBuildArtifact("liblib_b.so")
+
+# Spawn a new process.
+# use realpath to workaround llvm.org/pr48376
+# Pass path to solib for dlopen to properly locate the library.
+popen = self.spawnSubprocess(os.path.realpath(exe), args = [os.path.realpath(so)])
+pid = popen.pid
+
+# Attach to the spawned process.
+self.runCmd("process attach -p " + str(pid))
+
+target = self.dbg.GetSelectedTarget()
+process = target.GetProcess()
+self.assertTrue(process, PROCESS_IS_VALID)
+
+# Continue until first breakpoint.
+breakpoint1 = self.target().BreakpointCreateBySourceRegex(
+"// break here", lldb.SBFileSpec("main.cpp"))
+self.assertEqual(breakpoint1.GetNumResolvedLocations(), 1)
+stopped_threads = lldbutil.continue_to_breakpoint(self.process(), breakpoint1)
+self.assertEqual(len(stopped_threads), 1)
+
+# Check that image list does not contain lib_b.so before dlopen.
+self.match(
+"image list",
+patterns = ["liblib_b.so"],
+matching = False,
+msg = "liblib_b.so should not have been in image list")
+
+# Change a variable to escape the loop
+self.runCmd("expression main_thread_continue = 1")
+
+# Continue so that dlopen is called.
+breakpoint2 = self.target().BreakpointCreateBySourceRegex(
+"// break after dlopen", lldb.SBFileSpec("main.cpp"))
+self.assertEqual(breakpoint2.GetNumResolvedLocations(), 1)
+process.Continue()
+self.assertEqual(len(stopped_threads), 1)
+
+# Check that image list contains lib_b.so after dlopen.
+self.match(
+"image list",
+patterns = ["liblib_b.so"],
+matching = True,
+msg = "Missing liblib_b.so in image list")
+
Index: lldb/test/API/functionalities/dlopen/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/dlopen/Makefile
@@ -0,0 +1,9 @@
+CXX_SOURCES := main.cpp
+USE_LIBDL := 1
+
+lib_b:
+	$(MAKE) -f $(MAKEFILE_RULES) \
+		DYLIB_ONLY=YES DYLIB_CXX_SOURCES=b.cpp DYLIB_NAME=lib_b
+all: lib_b
+
+include Makefile.rules
Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -442,14 +442,18 @@
 if (module_sp->GetObjectFile()->GetBaseAddress().GetLoadAddress(
 _process->GetTarget()) == 

[Lldb-commits] [PATCH] D110033: [lldb] [gdb-remote] Always send PID when detaching w/ multiprocess

[Michał Górny via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
mgorny marked an inline comment as done.
Closed by commit rGb1099120ff96: [lldb] [gdb-remote] Always send PID when 
detaching w/ multiprocess (authored by mgorny).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D110033?vs=373545=373551#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110033

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py


Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
@@ -354,3 +354,46 @@
   "QEnvironmentHexEncoded:4e45454453454e43343d6623726f62",
   "QEnvironmentHexEncoded:455155414c533d666f6f3d626172",
 ])
+
+def test_detach_no_multiprocess(self):
+class MyResponder(MockGDBServerResponder):
+def __init__(self):
+super().__init__()
+self.detached = None
+
+def qfThreadInfo(self):
+return "10200"
+
+def D(self, packet):
+self.detached = packet
+return "OK"
+
+self.server.responder = MyResponder()
+target = self.dbg.CreateTarget('')
+process = self.connect(target)
+process.Detach()
+self.assertEqual(self.server.responder.detached, "D")
+
+def test_detach_pid(self):
+class MyResponder(MockGDBServerResponder):
+def __init__(self, test_case):
+super().__init__()
+self.test_case = test_case
+self.detached = None
+
+def qSupported(self, client_supported):
+self.test_case.assertIn("multiprocess+", client_supported)
+return "multiprocess+;" + super().qSupported(client_supported)
+
+def qfThreadInfo(self):
+return "mp400.10200"
+
+def D(self, packet):
+self.detached = packet
+return "OK"
+
+self.server.responder = MyResponder(self)
+target = self.dbg.CreateTarget('')
+process = self.connect(target)
+process.Detach()
+self.assertRegex(self.server.responder.detached, r"D;0*400")
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1543,14 +1543,16 @@
 }
   }
 
-  if (pid != LLDB_INVALID_PROCESS_ID) {
-if (!m_supports_multiprocess) {
-  error.SetErrorString(
-  "Multiprocess extension not supported by the server.");
-  return error;
-}
+  if (m_supports_multiprocess) {
+// Some servers (e.g. qemu) require specifying the PID even if only a 
single
+// process is running.
+if (pid == LLDB_INVALID_PROCESS_ID)
+  pid = GetCurrentProcessID();
 packet.PutChar(';');
 packet.PutHex64(pid);
+  } else if (pid != LLDB_INVALID_PROCESS_ID) {
+error.SetErrorString("Multiprocess extension not supported by the 
server.");
+return error;
   }
 
   StringExtractorGDBRemote response;


Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
@@ -354,3 +354,46 @@
   "QEnvironmentHexEncoded:4e45454453454e43343d6623726f62",
   "QEnvironmentHexEncoded:455155414c533d666f6f3d626172",
 ])
+
+def test_detach_no_multiprocess(self):
+class MyResponder(MockGDBServerResponder):
+def __init__(self):
+super().__init__()
+self.detached = None
+
+def qfThreadInfo(self):
+return "10200"
+
+def D(self, packet):
+self.detached = packet
+return "OK"
+
+self.server.responder = MyResponder()
+target = self.dbg.CreateTarget('')
+process = self.connect(target)
+process.Detach()
+self.assertEqual(self.server.responder.detached, "D")
+
+def test_detach_pid(self):
+class MyResponder(MockGDBServerResponder):
+def __init__(self, test_case):
+super().__init__()
+self.test_case = test_case
+self.detached = None
+
+def 

[Lldb-commits] [lldb] b109912 - [lldb] [gdb-remote] Always send PID when detaching w/ multiprocess

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-09-20T13:29:07+02:00
New Revision: b1099120ff963d0a0f1de12e3315b1ee4e4ed7e7

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

LOG: [lldb] [gdb-remote] Always send PID when detaching w/ multiprocess

Always send PID in the detach packet when multiprocess extensions are
enabled.  This is required by qemu's GDB server, as plain 'D' packet
results in an error and the emulated system is not resumed.

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

Added: 


Modified: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 829fb328a130..d949cfe7a64e 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1543,14 +1543,16 @@ Status GDBRemoteCommunicationClient::Detach(bool 
keep_stopped,
 }
   }
 
-  if (pid != LLDB_INVALID_PROCESS_ID) {
-if (!m_supports_multiprocess) {
-  error.SetErrorString(
-  "Multiprocess extension not supported by the server.");
-  return error;
-}
+  if (m_supports_multiprocess) {
+// Some servers (e.g. qemu) require specifying the PID even if only a 
single
+// process is running.
+if (pid == LLDB_INVALID_PROCESS_ID)
+  pid = GetCurrentProcessID();
 packet.PutChar(';');
 packet.PutHex64(pid);
+  } else if (pid != LLDB_INVALID_PROCESS_ID) {
+error.SetErrorString("Multiprocess extension not supported by the 
server.");
+return error;
   }
 
   StringExtractorGDBRemote response;

diff  --git 
a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py 
b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
index a58de7f19ec6..03ef4a73af50 100644
--- a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
@@ -354,3 +354,46 @@ def QEnvironment(self, packet):
   "QEnvironmentHexEncoded:4e45454453454e43343d6623726f62",
   "QEnvironmentHexEncoded:455155414c533d666f6f3d626172",
 ])
+
+def test_detach_no_multiprocess(self):
+class MyResponder(MockGDBServerResponder):
+def __init__(self):
+super().__init__()
+self.detached = None
+
+def qfThreadInfo(self):
+return "10200"
+
+def D(self, packet):
+self.detached = packet
+return "OK"
+
+self.server.responder = MyResponder()
+target = self.dbg.CreateTarget('')
+process = self.connect(target)
+process.Detach()
+self.assertEqual(self.server.responder.detached, "D")
+
+def test_detach_pid(self):
+class MyResponder(MockGDBServerResponder):
+def __init__(self, test_case):
+super().__init__()
+self.test_case = test_case
+self.detached = None
+
+def qSupported(self, client_supported):
+self.test_case.assertIn("multiprocess+", client_supported)
+return "multiprocess+;" + super().qSupported(client_supported)
+
+def qfThreadInfo(self):
+return "mp400.10200"
+
+def D(self, packet):
+self.detached = packet
+return "OK"
+
+self.server.responder = MyResponder(self)
+target = self.dbg.CreateTarget('')
+process = self.connect(target)
+process.Detach()
+self.assertRegex(self.server.responder.detached, r"D;0*400")



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

[Lldb-commits] [PATCH] D110033: [lldb] [gdb-remote] Always send PID when detaching w/ multiprocess

[Michał Górny via Phabricator via lldb-commits]

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



Comment at: 
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py:384
+def qSupported(self, client_supported):
+assert "multiprocess+" in client_supported
+return "multiprocess+;" + super().qSupported(client_supported)

labath wrote:
> Whether this fires depends on how python was run. Better use regular asserts. 
> You could pass in the test case to MyResponder constructor, or stash 
> client_supported in a member variable for later checks, or ...
Sure, makes sense.


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

https://reviews.llvm.org/D110033

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

[Lldb-commits] [PATCH] D110033: [lldb] [gdb-remote] Always send PID when detaching w/ multiprocess

[Pavel Labath via Phabricator via lldb-commits]

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



Comment at: 
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py:384
+def qSupported(self, client_supported):
+assert "multiprocess+" in client_supported
+return "multiprocess+;" + super().qSupported(client_supported)

Whether this fires depends on how python was run. Better use regular asserts. 
You could pass in the test case to MyResponder constructor, or stash 
client_supported in a member variable for later checks, or ...


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

https://reviews.llvm.org/D110033

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

[Lldb-commits] [PATCH] D110033: [lldb] [gdb-remote] Always send PID when detaching w/ multiprocess

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 373545.
mgorny added a comment.

Add tests.


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

https://reviews.llvm.org/D110033

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py


Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
@@ -354,3 +354,45 @@
   "QEnvironmentHexEncoded:4e45454453454e43343d6623726f62",
   "QEnvironmentHexEncoded:455155414c533d666f6f3d626172",
 ])
+
+def test_detach_no_multiprocess(self):
+class MyResponder(MockGDBServerResponder):
+def __init__(self):
+super().__init__()
+self.detached = None
+
+def qfThreadInfo(self):
+return "10200"
+
+def D(self, packet):
+self.detached = packet
+return "OK"
+
+self.server.responder = MyResponder()
+target = self.dbg.CreateTarget('')
+process = self.connect(target)
+process.Detach()
+self.assertEqual(self.server.responder.detached, "D")
+
+def test_detach_pid(self):
+class MyResponder(MockGDBServerResponder):
+def __init__(self):
+super().__init__()
+self.detached = None
+
+def qSupported(self, client_supported):
+assert "multiprocess+" in client_supported
+return "multiprocess+;" + super().qSupported(client_supported)
+
+def qfThreadInfo(self):
+return "mp400.10200"
+
+def D(self, packet):
+self.detached = packet
+return "OK"
+
+self.server.responder = MyResponder()
+target = self.dbg.CreateTarget('')
+process = self.connect(target)
+process.Detach()
+self.assertRegex(self.server.responder.detached, r"D;0*400")
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1543,14 +1543,16 @@
 }
   }
 
-  if (pid != LLDB_INVALID_PROCESS_ID) {
-if (!m_supports_multiprocess) {
-  error.SetErrorString(
-  "Multiprocess extension not supported by the server.");
-  return error;
-}
+  if (m_supports_multiprocess) {
+// Some servers (e.g. qemu) require specifying the PID even if only a 
single
+// process is running.
+if (pid == LLDB_INVALID_PROCESS_ID)
+  pid = GetCurrentProcessID();
 packet.PutChar(';');
 packet.PutHex64(pid);
+  } else if (pid != LLDB_INVALID_PROCESS_ID) {
+error.SetErrorString("Multiprocess extension not supported by the 
server.");
+return error;
   }
 
   StringExtractorGDBRemote response;


Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
@@ -354,3 +354,45 @@
   "QEnvironmentHexEncoded:4e45454453454e43343d6623726f62",
   "QEnvironmentHexEncoded:455155414c533d666f6f3d626172",
 ])
+
+def test_detach_no_multiprocess(self):
+class MyResponder(MockGDBServerResponder):
+def __init__(self):
+super().__init__()
+self.detached = None
+
+def qfThreadInfo(self):
+return "10200"
+
+def D(self, packet):
+self.detached = packet
+return "OK"
+
+self.server.responder = MyResponder()
+target = self.dbg.CreateTarget('')
+process = self.connect(target)
+process.Detach()
+self.assertEqual(self.server.responder.detached, "D")
+
+def test_detach_pid(self):
+class MyResponder(MockGDBServerResponder):
+def __init__(self):
+super().__init__()
+self.detached = None
+
+def qSupported(self, client_supported):
+assert "multiprocess+" in client_supported
+return "multiprocess+;" + super().qSupported(client_supported)
+
+def qfThreadInfo(self):
+return "mp400.10200"
+
+def D(self, packet):
+self.detached = packet
+return "OK"
+
+self.server.responder = MyResponder()
+target = self.dbg.CreateTarget('')
+process = self.connect(target)
+

[Lldb-commits] [PATCH] D110027: [lldb] [gdb-remote] Use local regnos for value_regs/invalidate_regs

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 373538.
mgorny marked 2 inline comments as done.
mgorny added a comment.

Rebase. Apply suggested changes.


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

https://reviews.llvm.org/D110027

Files:
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -4596,20 +4596,46 @@
   // ABI is also potentially incorrect.
   ABISP abi_sp = ABI::FindPlugin(shared_from_this(), arch_to_use);
 
+  std::map remote_to_local_map;
   uint32_t remote_regnum = 0;
+  for (auto it : llvm::enumerate(registers)) {
+RemoteRegisterInfo _reg_info = it.value();
+
+// Assign successive remote regnums if missing.
+if (remote_reg_info.regnum_remote == LLDB_INVALID_REGNUM)
+  remote_reg_info.regnum_remote = remote_regnum;
+
+// Create a mapping from remote to local regnos.
+remote_to_local_map[remote_reg_info.regnum_remote] = it.index();
+
+remote_regnum = remote_reg_info.regnum_remote + 1;
+  }
+
   for (auto it : llvm::enumerate(registers)) {
 uint32_t local_regnum = it.index();
 RemoteRegisterInfo _reg_info = it.value();
-// Use remote regnum if available, previous remote regnum + 1 when not.
-if (remote_reg_info.regnum_remote != LLDB_INVALID_REGNUM)
-  remote_regnum = remote_reg_info.regnum_remote;
+
+for (uint32_t  : remote_reg_info.value_regs) {
+  std::map::iterator x_lldb =
+  remote_to_local_map.find(x);
+  x = x_lldb != remote_to_local_map.end() ? x_lldb->second
+  : LLDB_INVALID_REGNUM;
+}
+
+for (uint32_t  : remote_reg_info.invalidate_regs) {
+  std::map::iterator x_lldb =
+  remote_to_local_map.find(x);
+  x = x_lldb != remote_to_local_map.end() ? x_lldb->second
+  : LLDB_INVALID_REGNUM;
+}
 
 struct RegisterInfo reg_info {
   remote_reg_info.name.AsCString(), remote_reg_info.alt_name.AsCString(),
   remote_reg_info.byte_size, remote_reg_info.byte_offset,
   remote_reg_info.encoding, remote_reg_info.format,
   {remote_reg_info.regnum_ehframe, remote_reg_info.regnum_dwarf,
-   remote_reg_info.regnum_generic, remote_regnum++, local_regnum},
+   remote_reg_info.regnum_generic, remote_reg_info.regnum_remote,
+   local_regnum},
   remote_reg_info.value_regs.data(),
   remote_reg_info.invalidate_regs.data(),
   remote_reg_info.dwarf_opcode_bytes.data(),
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
@@ -253,7 +253,7 @@
 // We have a valid primordial register as our constituent. Grab the
 // corresponding register info.
 const RegisterInfo *prim_reg_info =
-GetRegisterInfo(eRegisterKindProcessPlugin, prim_reg);
+GetRegisterInfo(eRegisterKindLLDB, prim_reg);
 if (prim_reg_info == nullptr)
   success = false;
 else {
@@ -384,7 +384,7 @@
 // We have a valid primordial register as our constituent. Grab the
 // corresponding register info.
 const RegisterInfo *value_reg_info =
-GetRegisterInfo(eRegisterKindProcessPlugin, reg);
+GetRegisterInfo(eRegisterKindLLDB, reg);
 if (value_reg_info == nullptr)
   success = false;
 else
@@ -405,7 +405,7 @@
reg != LLDB_INVALID_REGNUM;
reg = reg_info->invalidate_regs[++idx])
 SetRegisterIsValid(ConvertRegisterKindToRegisterNumber(
-   eRegisterKindProcessPlugin, reg),
+   eRegisterKindLLDB, reg),
false);
 }
 
Index: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
===
--- lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
+++ lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
@@ -665,9 +665,7 @@
   if (reg.byte_offset == LLDB_INVALID_INDEX32) {
 uint32_t value_regnum = reg.value_regs[0];
 if (value_regnum != LLDB_INVALID_INDEX32)
-  reg.byte_offset =
-  GetRegisterInfoAtIndex(remote_to_local_regnum_map[value_regnum])
-  ->byte_offset;
+  reg.byte_offset = 

[Lldb-commits] [PATCH] D110025: [lldb] [gdb-remote] Refactor getting remote regs to use local vector [WIP]

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 373530.
mgorny marked an inline comment as done.
mgorny added a comment.

Put the struct into the header. Put the common conversion and finalization code 
into a helper method. Use `llvm::enumerate` for local regnums. Convert 
qRegisterInfo getters.


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

https://reviews.llvm.org/D110025

Files:
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h

Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -44,6 +44,23 @@
 }
 namespace process_gdb_remote {
 
+struct RemoteRegisterInfo {
+  ConstString name;
+  ConstString alt_name;
+  ConstString set_name;
+  uint32_t byte_size = LLDB_INVALID_INDEX32;
+  uint32_t byte_offset = LLDB_INVALID_INDEX32;
+  lldb::Encoding encoding = lldb::eEncodingUint;
+  lldb::Format format = lldb::eFormatHex;
+  uint32_t regnum_dwarf = LLDB_INVALID_REGNUM;
+  uint32_t regnum_ehframe = LLDB_INVALID_REGNUM;
+  uint32_t regnum_generic = LLDB_INVALID_REGNUM;
+  uint32_t regnum_remote = LLDB_INVALID_REGNUM;
+  std::vector value_regs;
+  std::vector invalidate_regs;
+  std::vector dwarf_opcode_bytes;
+};
+
 class ThreadGDBRemote;
 
 class ProcessGDBRemote : public Process,
@@ -394,11 +411,14 @@
 
   DynamicLoader *GetDynamicLoader() override;
 
-  bool GetGDBServerRegisterInfoXMLAndProcess(ArchSpec _to_use,
- std::string xml_filename,
- uint32_t _reg_remote,
- uint32_t _reg_local);
+  bool GetGDBServerRegisterInfoXMLAndProcess(
+ArchSpec _to_use, std::string xml_filename,
+std::vector );
 
+  // Convert RemoteRegisterInfos into RegisterInfos and add to the dynamic
+  // register list.
+  void AddRemoteRegisters(std::vector ,
+  const ArchSpec _to_use);
   // Query remote GDBServer for register information
   bool GetGDBServerRegisterInfo(ArchSpec );
 
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -454,7 +454,7 @@
 return;
 
   char packet[128];
-  uint32_t reg_offset = LLDB_INVALID_INDEX32;
+  std::vector registers;
   uint32_t reg_num = 0;
   for (StringExtractorGDBRemote::ResponseType response_type =
StringExtractorGDBRemote::eResponse;
@@ -470,53 +470,26 @@
   if (response_type == StringExtractorGDBRemote::eResponse) {
 llvm::StringRef name;
 llvm::StringRef value;
-ConstString reg_name;
-ConstString alt_name;
-ConstString set_name;
-std::vector value_regs;
-std::vector invalidate_regs;
-std::vector dwarf_opcode_bytes;
-RegisterInfo reg_info = {
-nullptr,   // Name
-nullptr,   // Alt name
-0, // byte size
-reg_offset,// offset
-eEncodingUint, // encoding
-eFormatHex,// format
-{
-LLDB_INVALID_REGNUM, // eh_frame reg num
-LLDB_INVALID_REGNUM, // DWARF reg num
-LLDB_INVALID_REGNUM, // generic reg num
-reg_num, // process plugin reg num
-reg_num  // native register number
-},
-nullptr,
-nullptr,
-nullptr, // Dwarf expression opcode bytes pointer
-0// Dwarf expression opcode bytes length
-};
+RemoteRegisterInfo reg_info;
 
 while (response.GetNameColonValue(name, value)) {
   if (name.equals("name")) {
-reg_name.SetString(value);
+reg_info.name.SetString(value);
   } else if (name.equals("alt-name")) {
-alt_name.SetString(value);
+reg_info.alt_name.SetString(value);
   } else if (name.equals("bitsize")) {
-value.getAsInteger(0, reg_info.byte_size);
-reg_info.byte_size /= CHAR_BIT;
+reg_info.byte_size =
+StringConvert::ToUInt32(value.data(), 0, 0) / CHAR_BIT;
   } else if (name.equals("offset")) {
-if (value.getAsInteger(0, reg_offset))
-  reg_offset = UINT32_MAX;
+reg_info.byte_offset =
+StringConvert::ToUInt32(value.data(), LLDB_INVALID_INDEX32, 0);
   } else if (name.equals("encoding")) {
 const Encoding encoding = Args::StringToEncoding(value);
 if (encoding != eEncodingInvalid)
   reg_info.encoding = encoding;
   } 

[Lldb-commits] [PATCH] D109906: [lldb] [test] Add unittest for DynamicRegisterInfo::Finalize()

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 373524.
mgorny added a comment.

Implemented all the suggested improvements.


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

https://reviews.llvm.org/D109906

Files:
  lldb/unittests/Process/Utility/CMakeLists.txt
  lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp

Index: lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
===
--- /dev/null
+++ lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
@@ -0,0 +1,102 @@
+//===-- DynamicRegisterInfoTest.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 "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+#include "Plugins/Process/Utility/DynamicRegisterInfo.h"
+
+#include "lldb/Utility/ArchSpec.h"
+
+using namespace lldb_private;
+
+static std::vector regs_to_vector(uint32_t *regs) {
+  std::vector ret;
+  if (regs) {
+while (*regs != LLDB_INVALID_REGNUM)
+  ret.push_back(*regs++);
+  }
+  return ret;
+}
+
+class DynamicRegisterInfoTest : public ::testing::Test {
+protected:
+  DynamicRegisterInfo info;
+  uint32_t next_regnum = 0;
+  ConstString group{"group"};
+
+  uint32_t AddTestRegister(const char *name, uint32_t byte_size,
+   std::vector value_regs = {},
+   std::vector invalidate_regs = {}) {
+struct RegisterInfo new_reg {
+  name, nullptr, byte_size, LLDB_INVALID_INDEX32, lldb::eEncodingUint,
+  lldb::eFormatUnsigned,
+  {LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM,
+   next_regnum, next_regnum},
+  nullptr, nullptr, nullptr, 0
+};
+
+if (!value_regs.empty()) {
+  value_regs.push_back(LLDB_INVALID_REGNUM);
+  new_reg.value_regs = value_regs.data();
+}
+if (!invalidate_regs.empty()) {
+  invalidate_regs.push_back(LLDB_INVALID_REGNUM);
+  new_reg.invalidate_regs = invalidate_regs.data();
+}
+
+info.AddRegister(new_reg, group);
+return next_regnum++;
+  }
+
+  void AssertRegisterInfo(uint32_t reg_num, const char *reg_name,
+  uint32_t byte_offset,
+  std::vector value_regs = {},
+  std::vector invalidate_regs = {}) {
+const RegisterInfo *reg = info.GetRegisterInfoAtIndex(reg_num);
+EXPECT_NE(reg, nullptr);
+if (!reg)
+  return;
+
+EXPECT_STREQ(reg->name, reg_name);
+EXPECT_EQ(reg->byte_offset, byte_offset);
+EXPECT_THAT(regs_to_vector(reg->value_regs), value_regs);
+EXPECT_THAT(regs_to_vector(reg->invalidate_regs), invalidate_regs);
+  }
+};
+
+#define ASSERT_REG(reg, ...) { \
+  SCOPED_TRACE("at register " #reg); \
+  AssertRegisterInfo(reg, #reg, __VA_ARGS__); \
+  }
+
+TEST_F(DynamicRegisterInfoTest, finalize_regs) {
+  // Add regular registers
+  uint32_t b1 = AddTestRegister("b1", 8);
+  uint32_t b2 = AddTestRegister("b2", 8);
+
+  // Add a few sub-registers
+  uint32_t s1 = AddTestRegister("s1", 4, {b1});
+  uint32_t s2 = AddTestRegister("s2", 4, {b2});
+
+  // Add a register with invalidate_regs
+  uint32_t i1 = AddTestRegister("i1", 8, {}, {b1});
+
+  // Add a register with indirect invalidate regs to be expanded
+  // TODO: why is it done conditionally to value_regs?
+  uint32_t i2 = AddTestRegister("i2", 4, {b2}, {i1});
+
+  info.Finalize(lldb_private::ArchSpec());
+
+  ASSERT_REG(b1, 0);
+  ASSERT_REG(b2, 8);
+  ASSERT_REG(s1, 0, {b1});
+  ASSERT_REG(s2, 8, {b2});
+  ASSERT_REG(i1, 16, {}, {b1});
+  ASSERT_REG(i2, 8, {b2}, {b1, i1});
+}
Index: lldb/unittests/Process/Utility/CMakeLists.txt
===
--- lldb/unittests/Process/Utility/CMakeLists.txt
+++ lldb/unittests/Process/Utility/CMakeLists.txt
@@ -15,9 +15,10 @@
   ${NETBSD_SOURCES})
 
 add_lldb_unittest(ProcessUtilityTests
-  RegisterContextTest.cpp
+  DynamicRegisterInfoTest.cpp
   LinuxProcMapsTest.cpp
   MemoryTagManagerAArch64MTETest.cpp
+  RegisterContextTest.cpp
   ${PLATFORM_SOURCES}
 
   LINK_LIBS
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D110023: [lldb] [DynamicRegisterInfo] Add a convenience method to add suppl. registers

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 373515.
mgorny added a comment.

Rebased. Updated the code to assume dedupe and cleanup happens in `Finalize()`, 
and to use new test assertions.


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

https://reviews.llvm.org/D110023

Files:
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
  lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp

Index: lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
===
--- lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
+++ lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
@@ -149,3 +149,59 @@
   ASSERT_REG(i1, LLDB_INVALID_INDEX32);
   ASSERT_REG(i2, LLDB_INVALID_INDEX32);
 }
+
+TEST(DynamicRegisterInfoTest, add_supplementary_register) {
+  TestDynamicRegisterInfo info;
+
+  // Add a base register
+  uint32_t rax = info.AddTestRegister("rax", 8);
+
+  // Register numbers
+  uint32_t eax = 1;
+  uint32_t ax = 2;
+  uint32_t ah = 3;
+  uint32_t al = 4;
+
+  ConstString group{"supplementary registers"};
+  uint32_t value_regs[2] = {rax, LLDB_INVALID_REGNUM};
+  struct RegisterInfo eax_reg {
+"eax", nullptr, 4, LLDB_INVALID_INDEX32, lldb::eEncodingUint,
+lldb::eFormatUnsigned,
+{LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, eax,
+ eax},
+value_regs, nullptr, nullptr, 0
+  };
+  info.AddSupplementaryRegister(eax_reg, group);
+
+  struct RegisterInfo ax_reg {
+"ax", nullptr, 2, LLDB_INVALID_INDEX32, lldb::eEncodingUint,
+lldb::eFormatUnsigned,
+{LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, ax, ax},
+value_regs, nullptr, nullptr, 0
+  };
+  info.AddSupplementaryRegister(ax_reg, group);
+
+  struct RegisterInfo ah_reg {
+"ah", nullptr, 1, LLDB_INVALID_INDEX32, lldb::eEncodingUint,
+lldb::eFormatUnsigned,
+{LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, ah, ah},
+value_regs, nullptr, nullptr, 0
+  };
+  info.AddSupplementaryRegister(ah_reg, group);
+
+  struct RegisterInfo al_reg {
+"al", nullptr, 1, LLDB_INVALID_INDEX32, lldb::eEncodingUint,
+lldb::eFormatUnsigned,
+{LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, al, al},
+value_regs, nullptr, nullptr, 0
+  };
+  info.AddSupplementaryRegister(al_reg, group);
+
+  info.Finalize(lldb_private::ArchSpec());
+
+  ASSERT_REG(rax, 0, {}, {eax, ax, ah, al});
+  ASSERT_REG(eax, 0, {rax}, {rax, ax, ah, al});
+  ASSERT_REG(ax, 0, {rax}, {rax, eax, ah, al});
+  ASSERT_REG(ah, 0, {rax}, {rax, eax, ax, al});
+  ASSERT_REG(al, 0, {rax}, {rax, eax, ax, ah});
+}
Index: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
===
--- lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
+++ lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
@@ -38,6 +38,11 @@
   void AddRegister(lldb_private::RegisterInfo reg_info,
lldb_private::ConstString _name);
 
+  // Add a new register and cross-link it via invalidate_regs with other
+  // registers sharing its value_regs.
+  void AddSupplementaryRegister(lldb_private::RegisterInfo reg_info,
+lldb_private::ConstString _name);
+
   void Finalize(const lldb_private::ArchSpec );
 
   size_t GetNumRegisters() const;
Index: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
===
--- lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
+++ lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
@@ -432,6 +432,32 @@
   m_set_reg_nums[set].push_back(reg_num);
 }
 
+void DynamicRegisterInfo::AddSupplementaryRegister(RegisterInfo new_reg_info,
+   ConstString _name) {
+  assert(new_reg_info.value_regs != nullptr);
+  const uint32_t reg_num = m_regs.size();
+  AddRegister(new_reg_info, set_name);
+
+  reg_to_regs_map to_add;
+  for (uint32_t value_reg : m_value_regs_map[reg_num]) {
+if (value_reg == LLDB_INVALID_REGNUM)
+  break;
+
+// copy value_regs to invalidate_regs
+to_add[reg_num].push_back(value_reg);
+
+// copy invalidate_regs from the parent register
+llvm::append_range(to_add[reg_num], m_invalidate_regs_map[value_reg]);
+
+// add reverse invalidate entries
+for (uint32_t x : to_add[reg_num])
+  to_add[x].push_back(new_reg_info.kinds[eRegisterKindLLDB]);
+  }
+
+  for (reg_to_regs_map::value_type  : to_add)
+llvm::append_range(m_invalidate_regs_map[x.first], x.second);
+}
+
 void DynamicRegisterInfo::Finalize(const ArchSpec ) {
   if (m_finalized)
 return;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D109906: [lldb] [test] Add unittest for DynamicRegisterInfo::Finalize()

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In general, I'd prefer to avoid adding new methods to the class under test, and 
save subclassing for the cases where it's impossible to do things otherwise 
(abstract class, visibility restrictions (although that often means you're not 
testing what you should be)). These things could live inside an gtest fixture 
(subclass of testing::Test), which could have a DynamicRegisterInfo member if 
needed.




Comment at: lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp:51-52
+  std::vector invalidate_regs = {}) {
+std::string reg_msg =
+llvm::formatv("at register {0} (num: {1})", reg_name, reg_num);
+const RegisterInfo *reg = GetRegisterInfoAtIndex(reg_num);

Maybe use `SCOPED_TRACE` instead?



Comment at: lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp:61-93
+if (value_regs.empty())
+  EXPECT_EQ(reg->value_regs, nullptr) << reg_msg;
+else {
+  EXPECT_NE(reg->value_regs, nullptr) << reg_msg;
+
+  size_t i;
+  for (i = 0; i < value_regs.size(); i++) {

I'd probably write a helper function to convert the register lists to a vector, 
and then have gtest compare the vectors:
```
std::vector to_vector(uint32_t *regs) {
  std::vector result;
  if (regs) {
while(*regs != LLDB_INVALID_REGNUM)
  result.push_back(*regs++);
  }
  return result;
}
...
ASSERT_THAT(to_vector(info->value_regs), value_regs);
```


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

https://reviews.llvm.org/D109906

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

[Lldb-commits] [PATCH] D109899: [lldb] [gdb-remote] Recognize aarch64v type from gdbserver

[Michał Górny 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 rGf6e0edc23e61: [lldb] [gdb-remote] Recognize aarch64v type 
from gdbserver (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109899

Files:
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py


Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
@@ -423,3 +423,9 @@
["x30 = 0x4847464544434241"])
 self.match("register read flags",
["cpsr = 0x74737271"])
+
+# test vector registers
+self.match("register read v0",
+   ["v0 = {0x81 0x82 0x83 0x84 0x85 0x86 0x87 0x88 0x89 0x8a 
0x8b 0x8c 0x8d 0x8e 0x8f 0x90}"])
+self.match("register read v31",
+   ["v31 = {0xa1 0xa2 0xa3 0xa4 0xa5 0xa6 0xa7 0xa8 0xa9 0xaa 
0xab 0xac 0xad 0xae 0xaf 0xb0}"])
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -4524,6 +4524,9 @@
   } else if (gdb_type == "i387_ext" || gdb_type == "float") {
 reg_info.format = eFormatFloat;
 reg_info.encoding = eEncodingIEEE754;
+  } else if (gdb_type == "aarch64v") {
+reg_info.format = eFormatVectorOfUInt8;
+reg_info.encoding = eEncodingVector;
   }
 }
 


Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
@@ -423,3 +423,9 @@
["x30 = 0x4847464544434241"])
 self.match("register read flags",
["cpsr = 0x74737271"])
+
+# test vector registers
+self.match("register read v0",
+   ["v0 = {0x81 0x82 0x83 0x84 0x85 0x86 0x87 0x88 0x89 0x8a 0x8b 0x8c 0x8d 0x8e 0x8f 0x90}"])
+self.match("register read v31",
+   ["v31 = {0xa1 0xa2 0xa3 0xa4 0xa5 0xa6 0xa7 0xa8 0xa9 0xaa 0xab 0xac 0xad 0xae 0xaf 0xb0}"])
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -4524,6 +4524,9 @@
   } else if (gdb_type == "i387_ext" || gdb_type == "float") {
 reg_info.format = eFormatFloat;
 reg_info.encoding = eEncodingIEEE754;
+  } else if (gdb_type == "aarch64v") {
+reg_info.format = eFormatVectorOfUInt8;
+reg_info.encoding = eEncodingVector;
   }
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] f6e0edc - [lldb] [gdb-remote] Recognize aarch64v type from gdbserver

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-09-20T10:41:38+02:00
New Revision: f6e0edc23e6199bbb5fb4ef3b018b49a5b303183

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

LOG: [lldb] [gdb-remote] Recognize aarch64v type from gdbserver

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

Added: 


Modified: 
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py

Removed: 




diff  --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp 
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 79c54bce13372..7dd1f340cb331 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -4524,6 +4524,9 @@ bool ParseRegisters(XMLNode feature_node, 
GdbServerTargetInfo _info,
   } else if (gdb_type == "i387_ext" || gdb_type == "float") {
 reg_info.format = eFormatFloat;
 reg_info.encoding = eEncodingIEEE754;
+  } else if (gdb_type == "aarch64v") {
+reg_info.format = eFormatVectorOfUInt8;
+reg_info.encoding = eEncodingVector;
   }
 }
 

diff  --git 
a/lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py 
b/lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
index 5d307fc1e3d4c..225b16d8946e1 100644
--- a/lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
@@ -423,3 +423,9 @@ def haltReason(self):
["x30 = 0x4847464544434241"])
 self.match("register read flags",
["cpsr = 0x74737271"])
+
+# test vector registers
+self.match("register read v0",
+   ["v0 = {0x81 0x82 0x83 0x84 0x85 0x86 0x87 0x88 0x89 0x8a 
0x8b 0x8c 0x8d 0x8e 0x8f 0x90}"])
+self.match("register read v31",
+   ["v31 = {0xa1 0xa2 0xa3 0xa4 0xa5 0xa6 0xa7 0xa8 0xa9 0xaa 
0xab 0xac 0xad 0xae 0xaf 0xb0}"])



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

[Lldb-commits] [PATCH] D109879: [lldb] [DynamicRegisterInfo] Unset value_regs/invalidate_regs before Finalize()

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 373511.
mgorny retitled this revision from "[lldb] [DynamicRegisterInfo] Replace 
value_regs/invalidate_regs in AddRegister()" to "[lldb] [DynamicRegisterInfo] 
Unset value_regs/invalidate_regs before Finalize()".
mgorny edited the summary of this revision.
mgorny added a comment.

Unset the pointers instead.


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

https://reviews.llvm.org/D109879

Files:
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
  lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp


Index: lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
===
--- lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
+++ lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
@@ -123,3 +123,29 @@
   ASSERT_REG(i1, 16, {}, {b1});
   ASSERT_REG(i2, 8, {b2}, {b1, i1});
 }
+
+TEST(DynamicRegisterInfoTest, no_finalize_regs) {
+  TestDynamicRegisterInfo info;
+
+  // Add regular registers
+  uint32_t b1 = info.AddTestRegister("b1", 8);
+  uint32_t b2 = info.AddTestRegister("b2", 8);
+
+  // Add a few sub-registers
+  uint32_t s1 = info.AddTestRegister("s1", 4, {b1});
+  uint32_t s2 = info.AddTestRegister("s2", 4, {b2});
+
+  // Add a register with invalidate_regs
+  uint32_t i1 = info.AddTestRegister("i1", 8, {}, {b1});
+
+  // Add a register with indirect invalidate regs to be expanded
+  // TODO: why is it done conditionally to value_regs?
+  uint32_t i2 = info.AddTestRegister("i2", 4, {b2}, {i1});
+
+  ASSERT_REG(b1, LLDB_INVALID_INDEX32);
+  ASSERT_REG(b2, LLDB_INVALID_INDEX32);
+  ASSERT_REG(s1, LLDB_INVALID_INDEX32);
+  ASSERT_REG(s2, LLDB_INVALID_INDEX32);
+  ASSERT_REG(i1, LLDB_INVALID_INDEX32);
+  ASSERT_REG(i2, LLDB_INVALID_INDEX32);
+}
Index: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
===
--- lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
+++ lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
@@ -35,7 +35,7 @@
   size_t SetRegisterInfo(const lldb_private::StructuredData::Dictionary ,
  const lldb_private::ArchSpec );
 
-  void AddRegister(lldb_private::RegisterInfo _info,
+  void AddRegister(lldb_private::RegisterInfo reg_info,
lldb_private::ConstString _name);
 
   void Finalize(const lldb_private::ArchSpec );
Index: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
===
--- lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
+++ lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
@@ -395,7 +395,7 @@
   return m_regs.size();
 }
 
-void DynamicRegisterInfo::AddRegister(RegisterInfo _info,
+void DynamicRegisterInfo::AddRegister(RegisterInfo reg_info,
   ConstString _name) {
   assert(!m_finalized);
   const uint32_t reg_num = m_regs.size();
@@ -404,10 +404,16 @@
   if (reg_info.value_regs) {
 for (i = 0; reg_info.value_regs[i] != LLDB_INVALID_REGNUM; ++i)
   m_value_regs_map[reg_num].push_back(reg_info.value_regs[i]);
+
+// invalidate until Finalize() is called
+reg_info.value_regs = nullptr;
   }
   if (reg_info.invalidate_regs) {
 for (i = 0; reg_info.invalidate_regs[i] != LLDB_INVALID_REGNUM; ++i)
   m_invalidate_regs_map[reg_num].push_back(reg_info.invalidate_regs[i]);
+
+// invalidate until Finalize() is called
+reg_info.invalidate_regs = nullptr;
   }
   if (reg_info.dynamic_size_dwarf_expr_bytes) {
 for (i = 0; i < reg_info.dynamic_size_dwarf_len; ++i)


Index: lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
===
--- lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
+++ lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
@@ -123,3 +123,29 @@
   ASSERT_REG(i1, 16, {}, {b1});
   ASSERT_REG(i2, 8, {b2}, {b1, i1});
 }
+
+TEST(DynamicRegisterInfoTest, no_finalize_regs) {
+  TestDynamicRegisterInfo info;
+
+  // Add regular registers
+  uint32_t b1 = info.AddTestRegister("b1", 8);
+  uint32_t b2 = info.AddTestRegister("b2", 8);
+
+  // Add a few sub-registers
+  uint32_t s1 = info.AddTestRegister("s1", 4, {b1});
+  uint32_t s2 = info.AddTestRegister("s2", 4, {b2});
+
+  // Add a register with invalidate_regs
+  uint32_t i1 = info.AddTestRegister("i1", 8, {}, {b1});
+
+  // Add a register with indirect invalidate regs to be expanded
+  // TODO: why is it done conditionally to value_regs?
+  uint32_t i2 = info.AddTestRegister("i2", 4, {b2}, {i1});
+
+  ASSERT_REG(b1, LLDB_INVALID_INDEX32);
+  ASSERT_REG(b2, LLDB_INVALID_INDEX32);
+  ASSERT_REG(s1, LLDB_INVALID_INDEX32);
+  ASSERT_REG(s2, LLDB_INVALID_INDEX32);
+  ASSERT_REG(i1, LLDB_INVALID_INDEX32);
+  ASSERT_REG(i2, LLDB_INVALID_INDEX32);
+}
Index: 

[Lldb-commits] [PATCH] D109906: [lldb] [test] Add unittest for DynamicRegisterInfo::Finalize()

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 373509.
mgorny added a comment.

Test for register name too.


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

https://reviews.llvm.org/D109906

Files:
  lldb/unittests/Process/Utility/CMakeLists.txt
  lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp

Index: lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
===
--- /dev/null
+++ lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
@@ -0,0 +1,125 @@
+//===-- DynamicRegisterInfoTest.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 "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+#include "Plugins/Process/Utility/DynamicRegisterInfo.h"
+
+#include "lldb/Utility/ArchSpec.h"
+
+using namespace lldb_private;
+
+class TestDynamicRegisterInfo : public DynamicRegisterInfo {
+  uint32_t next_regnum = 0;
+  ConstString group{"group"};
+
+public:
+  uint32_t AddTestRegister(const char *name, uint32_t byte_size,
+   std::vector value_regs = {},
+   std::vector invalidate_regs = {}) {
+struct RegisterInfo new_reg {
+  name, nullptr, byte_size, LLDB_INVALID_INDEX32, lldb::eEncodingUint,
+  lldb::eFormatUnsigned,
+  {LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM,
+   next_regnum, next_regnum},
+  nullptr, nullptr, nullptr, 0
+};
+
+if (!value_regs.empty()) {
+  value_regs.push_back(LLDB_INVALID_REGNUM);
+  new_reg.value_regs = value_regs.data();
+}
+if (!invalidate_regs.empty()) {
+  invalidate_regs.push_back(LLDB_INVALID_REGNUM);
+  new_reg.invalidate_regs = invalidate_regs.data();
+}
+
+AddRegister(new_reg, group);
+return next_regnum++;
+  }
+
+  void AssertRegisterInfo(uint32_t reg_num, const char *reg_name,
+  uint32_t byte_offset,
+  std::vector value_regs = {},
+  std::vector invalidate_regs = {}) {
+std::string reg_msg =
+llvm::formatv("at register {0} (num: {1})", reg_name, reg_num);
+const RegisterInfo *reg = GetRegisterInfoAtIndex(reg_num);
+EXPECT_NE(reg, nullptr) << reg_msg;
+if (!reg)
+  return;
+
+EXPECT_STREQ(reg->name, reg_name) << reg_msg;
+EXPECT_EQ(reg->byte_offset, byte_offset) << reg_msg;
+
+if (value_regs.empty())
+  EXPECT_EQ(reg->value_regs, nullptr) << reg_msg;
+else {
+  EXPECT_NE(reg->value_regs, nullptr) << reg_msg;
+
+  size_t i;
+  for (i = 0; i < value_regs.size(); i++) {
+EXPECT_EQ(reg->value_regs[i], value_regs[i])
+<< reg_msg << " at i = " << i;
+if (reg->value_regs[i] == LLDB_INVALID_REGNUM)
+  break;
+  }
+
+  EXPECT_EQ(reg->value_regs[i], LLDB_INVALID_REGNUM)
+  << reg_msg << " at i = " << i;
+}
+
+if (invalidate_regs.empty())
+  EXPECT_EQ(reg->invalidate_regs, nullptr) << reg_msg;
+else {
+  EXPECT_NE(reg->invalidate_regs, nullptr) << reg_msg;
+
+  size_t i;
+  for (i = 0; i < invalidate_regs.size(); i++) {
+EXPECT_EQ(reg->invalidate_regs[i], invalidate_regs[i])
+<< reg_msg << " at i = " << i;
+if (reg->invalidate_regs[i] == LLDB_INVALID_REGNUM)
+  break;
+  }
+
+  EXPECT_EQ(reg->invalidate_regs[i], LLDB_INVALID_REGNUM)
+  << reg_msg << " at i = " << i;
+}
+  }
+};
+
+#define ASSERT_REG(reg, ...) info.AssertRegisterInfo(reg, #reg, __VA_ARGS__)
+
+TEST(DynamicRegisterInfoTest, finalize_regs) {
+  TestDynamicRegisterInfo info;
+
+  // Add regular registers
+  uint32_t b1 = info.AddTestRegister("b1", 8);
+  uint32_t b2 = info.AddTestRegister("b2", 8);
+
+  // Add a few sub-registers
+  uint32_t s1 = info.AddTestRegister("s1", 4, {b1});
+  uint32_t s2 = info.AddTestRegister("s2", 4, {b2});
+
+  // Add a register with invalidate_regs
+  uint32_t i1 = info.AddTestRegister("i1", 8, {}, {b1});
+
+  // Add a register with indirect invalidate regs to be expanded
+  // TODO: why is it done conditionally to value_regs?
+  uint32_t i2 = info.AddTestRegister("i2", 4, {b2}, {i1});
+
+  info.Finalize(lldb_private::ArchSpec());
+
+  ASSERT_REG(b1, 0);
+  ASSERT_REG(b2, 8);
+  ASSERT_REG(s1, 0, {b1});
+  ASSERT_REG(s2, 8, {b2});
+  ASSERT_REG(i1, 16, {}, {b1});
+  ASSERT_REG(i2, 8, {b2}, {b1, i1});
+}
Index: lldb/unittests/Process/Utility/CMakeLists.txt
===
--- lldb/unittests/Process/Utility/CMakeLists.txt
+++ lldb/unittests/Process/Utility/CMakeLists.txt
@@ -15,9 +15,10 @@
   ${NETBSD_SOURCES})
 
 add_lldb_unittest(ProcessUtilityTests
-  

[Lldb-commits] [PATCH] D110020: [lldb] [gdb-remote] Remove unused arg from GDBRemoteRegisterContext::ReadRegisterBytes()

[Michał Górny 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 rG92904cc68fbc: [lldb] [gdb-remote] Remove unused arg from 
GDBRemoteRegisterContext… (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110020

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h


Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
@@ -83,7 +83,7 @@
 protected:
   friend class ThreadGDBRemote;
 
-  bool ReadRegisterBytes(const RegisterInfo *reg_info, DataExtractor );
+  bool ReadRegisterBytes(const RegisterInfo *reg_info);
 
   bool WriteRegisterBytes(const RegisterInfo *reg_info, DataExtractor ,
   uint32_t data_offset);
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
@@ -90,7 +90,7 @@
 bool GDBRemoteRegisterContext::ReadRegister(const RegisterInfo *reg_info,
 RegisterValue ) {
   // Read the register
-  if (ReadRegisterBytes(reg_info, m_reg_data)) {
+  if (ReadRegisterBytes(reg_info)) {
 const uint32_t reg = reg_info->kinds[eRegisterKindLLDB];
 if (m_reg_valid[reg] == false)
   return false;
@@ -184,8 +184,7 @@
   return false;
 }
 
-bool GDBRemoteRegisterContext::ReadRegisterBytes(const RegisterInfo *reg_info,
- DataExtractor ) {
+bool GDBRemoteRegisterContext::ReadRegisterBytes(const RegisterInfo *reg_info) 
{
   ExecutionContext exe_ctx(CalculateThread());
 
   Process *process = exe_ctx.GetProcessPtr();
@@ -279,22 +278,6 @@
   return false;
   }
 
-  if ( != _reg_data) {
-assert(m_reg_data.GetByteSize() >=
-   reg_info->byte_offset + reg_info->byte_size);
-// If our register context and our register info disagree, which should
-// never happen, don't read past the end of the buffer.
-if (m_reg_data.GetByteSize() < reg_info->byte_offset + reg_info->byte_size)
-  return false;
-
-// If we aren't extracting into our own buffer (which only happens when
-// this function is called from ReadRegisterValue(uint32_t, Scalar&)) then
-// we transfer bytes from our buffer into the data buffer that was passed
-// in
-
-data.SetByteOrder(m_reg_data.GetByteOrder());
-data.SetData(m_reg_data, reg_info->byte_offset, reg_info->byte_size);
-  }
   return true;
 }
 
@@ -526,7 +509,7 @@
   if (reg_info
   ->value_regs) // skip registers that are slices of real registers
 continue;
-  ReadRegisterBytes(reg_info, m_reg_data);
+  ReadRegisterBytes(reg_info);
   // ReadRegisterBytes saves the contents of the register in to the
   // m_reg_data buffer
 }


Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
@@ -83,7 +83,7 @@
 protected:
   friend class ThreadGDBRemote;
 
-  bool ReadRegisterBytes(const RegisterInfo *reg_info, DataExtractor );
+  bool ReadRegisterBytes(const RegisterInfo *reg_info);
 
   bool WriteRegisterBytes(const RegisterInfo *reg_info, DataExtractor ,
   uint32_t data_offset);
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
@@ -90,7 +90,7 @@
 bool GDBRemoteRegisterContext::ReadRegister(const RegisterInfo *reg_info,
 RegisterValue ) {
   // Read the register
-  if (ReadRegisterBytes(reg_info, m_reg_data)) {
+  if (ReadRegisterBytes(reg_info)) {
 const uint32_t reg = reg_info->kinds[eRegisterKindLLDB];
 if (m_reg_valid[reg] == false)
   return false;
@@ -184,8 +184,7 @@
   return false;
 }
 
-bool GDBRemoteRegisterContext::ReadRegisterBytes(const RegisterInfo *reg_info,
- DataExtractor ) {
+bool GDBRemoteRegisterContext::ReadRegisterBytes(const RegisterInfo *reg_info) {
   ExecutionContext exe_ctx(CalculateThread());
 
   Process *process = 

[Lldb-commits] [lldb] 92904cc - [lldb] [gdb-remote] Remove unused arg from GDBRemoteRegisterContext::ReadRegisterBytes()

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-09-20T10:24:01+02:00
New Revision: 92904cc68fbc1d000387b30accc8b05b3fe95daa

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

LOG: [lldb] [gdb-remote] Remove unused arg from 
GDBRemoteRegisterContext::ReadRegisterBytes()

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

Added: 


Modified: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
index c050efbc36545..df5d052d2e33b 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
@@ -90,7 +90,7 @@ const RegisterSet 
*GDBRemoteRegisterContext::GetRegisterSet(size_t reg_set) {
 bool GDBRemoteRegisterContext::ReadRegister(const RegisterInfo *reg_info,
 RegisterValue ) {
   // Read the register
-  if (ReadRegisterBytes(reg_info, m_reg_data)) {
+  if (ReadRegisterBytes(reg_info)) {
 const uint32_t reg = reg_info->kinds[eRegisterKindLLDB];
 if (m_reg_valid[reg] == false)
   return false;
@@ -184,8 +184,7 @@ bool GDBRemoteRegisterContext::GetPrimordialRegister(
   return false;
 }
 
-bool GDBRemoteRegisterContext::ReadRegisterBytes(const RegisterInfo *reg_info,
- DataExtractor ) {
+bool GDBRemoteRegisterContext::ReadRegisterBytes(const RegisterInfo *reg_info) 
{
   ExecutionContext exe_ctx(CalculateThread());
 
   Process *process = exe_ctx.GetProcessPtr();
@@ -279,22 +278,6 @@ bool GDBRemoteRegisterContext::ReadRegisterBytes(const 
RegisterInfo *reg_info,
   return false;
   }
 
-  if ( != _reg_data) {
-assert(m_reg_data.GetByteSize() >=
-   reg_info->byte_offset + reg_info->byte_size);
-// If our register context and our register info disagree, which should
-// never happen, don't read past the end of the buffer.
-if (m_reg_data.GetByteSize() < reg_info->byte_offset + reg_info->byte_size)
-  return false;
-
-// If we aren't extracting into our own buffer (which only happens when
-// this function is called from ReadRegisterValue(uint32_t, Scalar&)) then
-// we transfer bytes from our buffer into the data buffer that was passed
-// in
-
-data.SetByteOrder(m_reg_data.GetByteOrder());
-data.SetData(m_reg_data, reg_info->byte_offset, reg_info->byte_size);
-  }
   return true;
 }
 
@@ -526,7 +509,7 @@ bool GDBRemoteRegisterContext::ReadAllRegisterValues(
   if (reg_info
   ->value_regs) // skip registers that are slices of real registers
 continue;
-  ReadRegisterBytes(reg_info, m_reg_data);
+  ReadRegisterBytes(reg_info);
   // ReadRegisterBytes saves the contents of the register in to the
   // m_reg_data buffer
 }

diff  --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
index 18fcb73b9815b..7aef414465126 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
@@ -83,7 +83,7 @@ class GDBRemoteRegisterContext : public RegisterContext {
 protected:
   friend class ThreadGDBRemote;
 
-  bool ReadRegisterBytes(const RegisterInfo *reg_info, DataExtractor );
+  bool ReadRegisterBytes(const RegisterInfo *reg_info);
 
   bool WriteRegisterBytes(const RegisterInfo *reg_info, DataExtractor ,
   uint32_t data_offset);



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

[Lldb-commits] [PATCH] D109906: [lldb] [test] Add unittest for DynamicRegisterInfo::Finalize()

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 373506.
mgorny added a comment.

Create a helper class and move adding regs there. Add a helper function to 
assert on registers.


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

https://reviews.llvm.org/D109906

Files:
  lldb/unittests/Process/Utility/CMakeLists.txt
  lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp

Index: lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
===
--- /dev/null
+++ lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
@@ -0,0 +1,124 @@
+//===-- DynamicRegisterInfoTest.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 "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+#include "Plugins/Process/Utility/DynamicRegisterInfo.h"
+
+#include "lldb/Utility/ArchSpec.h"
+
+using namespace lldb_private;
+
+class TestDynamicRegisterInfo : public DynamicRegisterInfo {
+  uint32_t next_regnum = 0;
+  ConstString group{"group"};
+
+public:
+  uint32_t AddTestRegister(const char *name, uint32_t byte_size,
+   std::vector value_regs = {},
+   std::vector invalidate_regs = {}) {
+struct RegisterInfo new_reg {
+  name, nullptr, byte_size, LLDB_INVALID_INDEX32, lldb::eEncodingUint,
+  lldb::eFormatUnsigned,
+  {LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM,
+   next_regnum, next_regnum},
+  nullptr, nullptr, nullptr, 0
+};
+
+if (!value_regs.empty()) {
+  value_regs.push_back(LLDB_INVALID_REGNUM);
+  new_reg.value_regs = value_regs.data();
+}
+if (!invalidate_regs.empty()) {
+  invalidate_regs.push_back(LLDB_INVALID_REGNUM);
+  new_reg.invalidate_regs = invalidate_regs.data();
+}
+
+AddRegister(new_reg, group);
+return next_regnum++;
+  }
+
+  void AssertRegisterInfo(uint32_t reg_num, const char *reg_name,
+  uint32_t byte_offset,
+  std::vector value_regs = {},
+  std::vector invalidate_regs = {}) {
+std::string reg_msg =
+llvm::formatv("at register {0} (num: {1})", reg_name, reg_num);
+const RegisterInfo *reg = GetRegisterInfoAtIndex(reg_num);
+EXPECT_NE(reg, nullptr) << reg_msg;
+if (!reg)
+  return;
+
+EXPECT_EQ(reg->byte_offset, byte_offset) << reg_msg;
+
+if (value_regs.empty())
+  EXPECT_EQ(reg->value_regs, nullptr) << reg_msg;
+else {
+  EXPECT_NE(reg->value_regs, nullptr) << reg_msg;
+
+  size_t i;
+  for (i = 0; i < value_regs.size(); i++) {
+EXPECT_EQ(reg->value_regs[i], value_regs[i])
+<< reg_msg << " at i = " << i;
+if (reg->value_regs[i] == LLDB_INVALID_REGNUM)
+  break;
+  }
+
+  EXPECT_EQ(reg->value_regs[i], LLDB_INVALID_REGNUM)
+  << reg_msg << " at i = " << i;
+}
+
+if (invalidate_regs.empty())
+  EXPECT_EQ(reg->invalidate_regs, nullptr) << reg_msg;
+else {
+  EXPECT_NE(reg->invalidate_regs, nullptr) << reg_msg;
+
+  size_t i;
+  for (i = 0; i < invalidate_regs.size(); i++) {
+EXPECT_EQ(reg->invalidate_regs[i], invalidate_regs[i])
+<< reg_msg << " at i = " << i;
+if (reg->invalidate_regs[i] == LLDB_INVALID_REGNUM)
+  break;
+  }
+
+  EXPECT_EQ(reg->invalidate_regs[i], LLDB_INVALID_REGNUM)
+  << reg_msg << " at i = " << i;
+}
+  }
+};
+
+#define ASSERT_REG(reg, ...) info.AssertRegisterInfo(reg, #reg, __VA_ARGS__)
+
+TEST(DynamicRegisterInfoTest, finalize_regs) {
+  TestDynamicRegisterInfo info;
+
+  // Add regular registers
+  uint32_t b1 = info.AddTestRegister("b1", 8);
+  uint32_t b2 = info.AddTestRegister("b2", 8);
+
+  // Add a few sub-registers
+  uint32_t s1 = info.AddTestRegister("s1", 4, {b1});
+  uint32_t s2 = info.AddTestRegister("s2", 4, {b2});
+
+  // Add a register with invalidate_regs
+  uint32_t i1 = info.AddTestRegister("i1", 8, {}, {b1});
+
+  // Add a register with indirect invalidate regs to be expanded
+  // TODO: why is it done conditionally to value_regs?
+  uint32_t i2 = info.AddTestRegister("i2", 4, {b2}, {i1});
+
+  info.Finalize(lldb_private::ArchSpec());
+
+  ASSERT_REG(b1, 0);
+  ASSERT_REG(b2, 8);
+  ASSERT_REG(s1, 0, {b1});
+  ASSERT_REG(s2, 8, {b2});
+  ASSERT_REG(i1, 16, {}, {b1});
+  ASSERT_REG(i2, 8, {b2}, {b1, i1});
+}
Index: lldb/unittests/Process/Utility/CMakeLists.txt
===
--- lldb/unittests/Process/Utility/CMakeLists.txt
+++ lldb/unittests/Process/Utility/CMakeLists.txt
@@ -15,9 +15,10 @@
   ${NETBSD_SOURCES})
 
 

[Lldb-commits] [PATCH] D109906: [lldb] [test] Add unittest for DynamicRegisterInfo::Finalize()

[Michał Górny via Phabricator via lldb-commits]

mgorny marked an inline comment as done.
mgorny added a comment.

Made fixture class. Now you made me think of adding a helper method to test 
register properties too.


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

https://reviews.llvm.org/D109906

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

[Lldb-commits] [PATCH] D109899: [lldb] [gdb-remote] Recognize aarch64v type from gdbserver

[Pavel Labath via Phabricator via lldb-commits]

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

I'm guessing this is what's being sent by gdbserver.


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

https://reviews.llvm.org/D109899

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

[Lldb-commits] [PATCH] D110023: [lldb] [DynamicRegisterInfo] Add a convenience method to add suppl. registers

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp:482
+// copy invalidate_regs from the parent register
+llvm::append_range(to_add[reg_num], m_invalidate_regs_map[value_reg]);
+

mgorny wrote:
> @labath, any suggestion how to do this nicely while not copying the 
> terminating `LLDB_INVALID_REGNUM`?
How about by making sure that `LLDB_INVALID_REGNUM` is not present in the 
firstplace (by adding it only during finalization)?


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

https://reviews.llvm.org/D110023

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

[Lldb-commits] [PATCH] D110033: [lldb] [gdb-remote] Always send PID when detaching w/ multiprocess

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

A test case?


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

https://reviews.llvm.org/D110033

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

[Lldb-commits] [PATCH] D110027: [lldb] [gdb-remote] Use local regnos for value_regs/invalidate_regs

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4684
+std::map remote_to_local_map;
 for (RemoteRegisterInfo& remote_reg_info : registers) {
+  // Assign successive remote regnums if missing.

drop `local_regnum` and use `llvm::enumerate`



Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4699
+  for (uint32_t  : remote_reg_info.value_regs) {
+if (x != LLDB_INVALID_REGNUM)
+  x = remote_to_local_map[x];

Why should we have `LLDB_INVALID_REGNUM` in this list?

I think that a more interesting question is what to do if the value is not 
located in the `remote_to_local_map`. This will map it to zero, which isn't 
very useful. I suppose we could just drop this value -- this is something that 
was probably happening already (if we weren't just crashing).


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

https://reviews.llvm.org/D110027

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

[Lldb-commits] [PATCH] D109879: [lldb] [DynamicRegisterInfo] Replace value_regs/invalidate_regs in AddRegister()

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

Sure, I suppose either way works for me.


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

https://reviews.llvm.org/D109879

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

[Lldb-commits] [lldb] 9669223 - [lldb] Remove two #ifndef linux from Platform.cpp

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2021-09-20T08:30:02+02:00
New Revision: 966922320f09b8bf6e4a69a32f344b3acec36434

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

LOG: [lldb] Remove two #ifndef linux from Platform.cpp

These have been here since r215992, guarding the calls to HostInfo, but
their purpose unclear -- HostInfoLinux provides these functions and they
work fine.

Added: 


Modified: 
lldb/source/Target/Platform.cpp

Removed: 




diff  --git a/lldb/source/Target/Platform.cpp b/lldb/source/Target/Platform.cpp
index a9b97f7957953..7f762a68710e8 100644
--- a/lldb/source/Target/Platform.cpp
+++ b/lldb/source/Target/Platform.cpp
@@ -497,24 +497,14 @@ bool Platform::GetOSBuildString(std::string ) {
   s.clear();
 
   if (IsHost())
-#if !defined(__linux__)
 return HostInfo::GetOSBuildString(s);
-#else
-return false;
-#endif
-  else
-return GetRemoteOSBuildString(s);
+  return GetRemoteOSBuildString(s);
 }
 
 bool Platform::GetOSKernelDescription(std::string ) {
   if (IsHost())
-#if !defined(__linux__)
 return HostInfo::GetOSKernelDescription(s);
-#else
-return false;
-#endif
-  else
-return GetRemoteOSKernelDescription(s);
+  return GetRemoteOSKernelDescription(s);
 }
 
 void Platform::AddClangModuleCompilationOptions(



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

[Lldb-commits] [PATCH] D109879: [lldb] [DynamicRegisterInfo] Replace value_regs/invalidate_regs in AddRegister()

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

As I alluded to in the second comment, I'm not sure this is really that much 
helpful, since it's also nice to have all invalidate_regs handling happen in a 
single function. Maybe AddRegister should just set this field to null (to give 
a more predictable behavior (crash) if anyone accesses it), and have a comment 
saying that it will be filled in in `Finalize` ?


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

https://reviews.llvm.org/D109879

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

[Lldb-commits] [PATCH] D110025: [lldb] [gdb-remote] Refactor getting remote regs to use local vector [WIP]

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I like it.




Comment at: 
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4368-4371
+namespace lldb_private {
+namespace process_gdb_remote {
+
+struct RemoteRegisterInfo {

`struct process_gdb_remote::RemoteRegisterInfo {` should work too. Putting the 
declaration in the header would also be ok.


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

https://reviews.llvm.org/D110025

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

[Lldb-commits] [PATCH] D109906: [lldb] [test] Add unittest for DynamicRegisterInfo::Finalize()

[Pavel Labath via Phabricator via lldb-commits]

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



Comment at: lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp:23
+  ConstString group{"group"};
+  static uint32_t regnum = 0;
+

It would be better to create a fixture class, and make this a member variable 
of that class. If this ends up used from two tests, it will begin returning 
different values depending on the order of tests, whether they are run 
standalone, etc.


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

https://reviews.llvm.org/D109906

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

[Lldb-commits] [PATCH] D110020: [lldb] [gdb-remote] Remove unused arg from GDBRemoteRegisterContext::ReadRegisterBytes()

[Pavel Labath via Phabricator via lldb-commits]

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

yay


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

https://reviews.llvm.org/D110020

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

[Lldb-commits] [PATCH] D110010: [lldb] Extract adding symbols for UUID/File/Frame (NFC)

[Pavel Labath via Phabricator via lldb-commits]

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

seems straight-forward enough




Comment at: lldb/source/Commands/CommandObjectTarget.cpp:4203
+
+  bool AddSymbolsForFrame(Target *target, CommandReturnObject ,
+  bool ) {

Do we need to pass the target around? I would assume that can be retrieved from 
`m_exe_ctx` as well..


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

https://reviews.llvm.org/D110010

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

[Lldb-commits] [PATCH] D109937: [lldb] Handle malformed qfThreadInfo reply

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D109937#3007071 , @ted wrote:

> I've got a test written. It doesn't crash like the debugger in the wild does, 
> but it does give a tid of 0 for each thread I ask about. So I can assert if 
> the threads don't have the correct tid.

I guess you forgot to upload it (?)

> Which leads me to 1 more question - what should I do when we have a malformed 
> response, and there are no threads listed? I'm leaning towards just falling 
> through to the if at line 2931, and returning a pid and tid of 1. That's what 
> we do if there's and empty response.

Sounds reasonable to me. We'd be basically treating it as if the stub did not 
support the packet at all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109937

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