[Lldb-commits] [PATCH] D110981: [lldb] Improve help for platform put-file

[Pavel Labath via Phabricator via lldb-commits]

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

looks good. thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110981

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

[Lldb-commits] [PATCH] D110997: [lldb] Restore tty attributes on disconnect

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D110997#3039575 , @mgorny wrote:

> In D110997#3039540 , @labath wrote:
>
>> Why is it important to restore the tty settings? (just asking, I have no 
>> clue about how this is supposed to work)
>
> Well, maybe my logic is wrong but the idea is that since LLDB overrides 
> serial port parameters, it should restore them to the original state when 
> it's done with the serial port. Roughly the principle that the program cleans 
> up after itself.

That does not sound unreasonable, though I'm not sure how much is it actually 
useful. Do you know if its like standard practice when working with serial 
ports, ttys and stuff?

, I'm not very happy that we have _any_ tty code inside the 
ConnectionFileDescriptorPosix class, so I'm not enthusiastic about expanding 
it. Let's see how the RFC goes first...


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

https://reviews.llvm.org/D110997

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

[Lldb-commits] [PATCH] D109876: [lldb] [ABI/AArch64] Add pseudo-regs if missing

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/include/lldb/Target/ABI.h:131-134
   virtual void AugmentRegisterInfo(RegisterInfo &info) = 0;
+  // Routine called before DynamicRegisterInfo::Finalize().
+  virtual void
+  PreFinalizeDynamicRegisterInfo(lldb_private::DynamicRegisterInfo &info) {}

Ideally, I would like to subsume the `AugmentRegisterInfo` functionality into 
this function, so that one would just call 
`abi->Augment(Dynamic)RegisterInfo(dyn_reg_info)` and it would automatically 
fill it in with all the necessary information. The old `AugmentRegisterInfo` 
function could become a private/protected implementation detail.

Would such a thing be possible? Perhaps with some preparatory refactoring patch?


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

https://reviews.llvm.org/D109876

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

[Lldb-commits] [lldb] ca5be06 - Revert "[lldb] Refactor variable parsing"

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2021-10-05T10:46:30+02:00
New Revision: ca5be065c4c612554acdcae3ead01a1474eff296

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

LOG: Revert "[lldb] Refactor variable parsing"

This commit has introduced test failures in internal google tests.
Working theory is they are caused by a genuine problem in the patch
which gets tripped by some debug info from system libraries.

Reverting while we try to reproduce the problem in a self-contained
fashion.

This reverts commit 601168e42037ac4433e74b920bb22f76d59ba420.

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index c111ff6567364..f89fb1655d3bf 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2135,7 +2135,7 @@ void SymbolFileDWARF::FindGlobalVariables(
   }
 }
 
-ParseAndAppendGlobalVariable(sc, die, variables);
+ParseVariables(sc, die, LLDB_INVALID_ADDRESS, false, false, &variables);
 while (pruned_idx < variables.GetSize()) {
   VariableSP var_sp = variables.GetVariableAtIndex(pruned_idx);
   if (name_is_mangled ||
@@ -2188,7 +2188,7 @@ void SymbolFileDWARF::FindGlobalVariables(const 
RegularExpression ®ex,
   return true;
 sc.comp_unit = GetCompUnitForDWARFCompUnit(*dwarf_cu);
 
-ParseAndAppendGlobalVariable(sc, die, variables);
+ParseVariables(sc, die, LLDB_INVALID_ADDRESS, false, false, &variables);
 
 return variables.GetSize() - original_size < max_matches;
   });
@@ -3049,8 +3049,8 @@ size_t SymbolFileDWARF::ParseVariablesForContext(const 
SymbolContext &sc) {
   /*check_hi_lo_pc=*/true))
 func_lo_pc = ranges.GetMinRangeBase(0);
   if (func_lo_pc != LLDB_INVALID_ADDRESS) {
-const size_t num_variables =
-ParseVariablesInFunctionContext(sc, function_die, func_lo_pc);
+const size_t num_variables = ParseVariables(
+sc, function_die.GetFirstChild(), func_lo_pc, true, true);
 
 // Let all blocks know they have parse all their variables
 sc.function->GetBlock(false).SetDidParseVariables(true, true);
@@ -3479,137 +3479,117 @@ SymbolFileDWARF::FindBlockContainingSpecification(
   return DWARFDIE();
 }
 
-void SymbolFileDWARF::ParseAndAppendGlobalVariable(
-const SymbolContext &sc, const DWARFDIE &die,
-VariableList &cc_variable_list) {
-  if (!die)
-return;
-
-  dw_tag_t tag = die.Tag();
-  if (tag != DW_TAG_variable && tag != DW_TAG_constant)
-return;
-
-  // Check to see if we have already parsed this variable or constant?
-  VariableSP var_sp = GetDIEToVariable()[die.GetDIE()];
-  if (var_sp) {
-cc_variable_list.AddVariableIfUnique(var_sp);
-return;
-  }
-
-  // We haven't already parsed it, lets do that now.
-  VariableListSP variable_list_sp;
-  DWARFDIE sc_parent_die = GetParentSymbolContextDIE(die);
-  dw_tag_t parent_tag = sc_parent_die.Tag();
-  switch (parent_tag) {
-  case DW_TAG_compile_unit:
-  case DW_TAG_partial_unit:
-if (sc.comp_unit != nullptr) {
-  variable_list_sp = sc.comp_unit->GetVariableList(false);
-} else {
-  GetObjectFile()->GetModule()->ReportError(
-  "parent 0x%8.8" PRIx64 " %s with no valid compile unit in "
-  "symbol context for 0x%8.8" PRIx64 " %s.\n",
-  sc_parent_die.GetID(), sc_parent_die.GetTagAsCString(), die.GetID(),
-  die.GetTagAsCString());
-  return;
-}
-break;
-
-  default:
-GetObjectFile()->GetModule()->ReportError(
-"didn't find appropriate parent DIE for variable list for "
-"0x%8.8" PRIx64 " %s.\n",
-die.GetID(), die.GetTagAsCString());
-return;
-  }
-
-  var_sp = ParseVariableDIE(sc, die, LLDB_INVALID_ADDRESS);
-  if (!var_sp)
-return;
-
-  cc_variable_list.AddVariableIfUnique(var_sp);
-  if (variable_list_sp)
-variable_list_sp->AddVariableIfUnique(var_sp);
-}
-
-size_t SymbolFileDWARF::ParseVariablesInFunctionContext(
-const SymbolContext &sc, const DWARFDIE &die,
-const lldb::addr_t func_low_pc) {
-  if (!die || !sc.function)
+size_t SymbolFileDWARF::ParseVariables(const SymbolContext &sc,
+   const DWARFDIE &orig_die,
+   const lldb::addr_t func_low_pc,
+   bool parse_siblings, bool 
parse_children,
+   VariableList *cc_variable_list) {
+  if (!orig_die)
 return 0;
 
-  Block *block =
-  sc.function->GetBlock(/*can_create=*/true).

[Lldb-commits] [PATCH] D111052: [lldb] [gdb-remote] Correct st_dev values in vFile:fstat packet

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

mgorny added a comment.

@labath has pointed out that this seems not to apply to the `vFile:fstat` that 
we're using — gdbserver seems to pass (truncated) `st_dev` there.


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

https://reviews.llvm.org/D111052

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

[Lldb-commits] [PATCH] D111030: [lldb] [Host] Add setters for common teletype properties to Terminal

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

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

Remove support for 1.5 stop bits — it's valid only for 5-bit transmission 
(which we don't support). Simplify the API to use integer instead of enum.


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

https://reviews.llvm.org/D111030

Files:
  lldb/include/lldb/Host/Terminal.h
  lldb/source/Host/common/Terminal.cpp
  lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
  lldb/unittests/Host/posix/TerminalTest.cpp

Index: lldb/unittests/Host/posix/TerminalTest.cpp
===
--- lldb/unittests/Host/posix/TerminalTest.cpp
+++ lldb/unittests/Host/posix/TerminalTest.cpp
@@ -68,6 +68,132 @@
   EXPECT_EQ(terminfo.c_lflag & ICANON, 0U);
 }
 
+TEST_F(TerminalTest, SetRaw) {
+  struct termios terminfo;
+  Terminal term{m_pty.GetPrimaryFileDescriptor()};
+
+  ASSERT_EQ(term.SetRaw(), true);
+  ASSERT_EQ(tcgetattr(m_pty.GetPrimaryFileDescriptor(), &terminfo), 0);
+  // NB: cfmakeraw() on glibc disables IGNBRK, on FreeBSD sets it
+  EXPECT_EQ(terminfo.c_iflag &
+(BRKINT | PARMRK | ISTRIP | INLCR | IGNCR | ICRNL | IXON),
+0U);
+  EXPECT_EQ(terminfo.c_oflag & OPOST, 0U);
+  EXPECT_EQ(terminfo.c_lflag & (ICANON | ECHO | ISIG | IEXTEN), 0U);
+  EXPECT_EQ(terminfo.c_cflag & (CSIZE | PARENB), 0U | CS8);
+  EXPECT_EQ(terminfo.c_cc[VMIN], 1);
+  EXPECT_EQ(terminfo.c_cc[VTIME], 0);
+}
+
+TEST_F(TerminalTest, SetBaudRate) {
+  struct termios terminfo;
+  Terminal term{m_pty.GetPrimaryFileDescriptor()};
+
+  ASSERT_EQ(term.SetBaudRate(38400), true);
+  ASSERT_EQ(tcgetattr(m_pty.GetPrimaryFileDescriptor(), &terminfo), 0);
+  EXPECT_EQ(cfgetispeed(&terminfo), static_cast(B38400));
+  EXPECT_EQ(cfgetospeed(&terminfo), static_cast(B38400));
+
+  ASSERT_EQ(term.SetBaudRate(115200), true);
+  ASSERT_EQ(tcgetattr(m_pty.GetPrimaryFileDescriptor(), &terminfo), 0);
+  EXPECT_EQ(cfgetispeed(&terminfo), static_cast(B115200));
+  EXPECT_EQ(cfgetospeed(&terminfo), static_cast(B115200));
+
+  // uncommon value
+#if defined(B153600)
+  ASSERT_EQ(term.SetBaudRate(153600), true);
+  ASSERT_EQ(tcgetattr(m_pty.GetPrimaryFileDescriptor(), &terminfo), 0);
+  EXPECT_EQ(cfgetispeed(&terminfo), static_cast(B153600));
+  EXPECT_EQ(cfgetospeed(&terminfo), static_cast(B153600));
+#else
+  EXPECT_EQ(term.SetBaudRate(153600), false);
+#endif
+}
+
+TEST_F(TerminalTest, SetStopBits) {
+  struct termios terminfo;
+  Terminal term{m_pty.GetPrimaryFileDescriptor()};
+
+  ASSERT_EQ(term.SetStopBits(1), true);
+  ASSERT_EQ(tcgetattr(m_pty.GetPrimaryFileDescriptor(), &terminfo), 0);
+  EXPECT_EQ(terminfo.c_cflag & CSTOPB, 0U);
+
+  ASSERT_EQ(term.SetStopBits(2), true);
+  ASSERT_EQ(tcgetattr(m_pty.GetPrimaryFileDescriptor(), &terminfo), 0);
+  EXPECT_NE(terminfo.c_cflag & CSTOPB, 0U);
+
+  ASSERT_EQ(term.SetStopBits(0), false);
+  ASSERT_EQ(term.SetStopBits(3), false);
+}
+
+TEST_F(TerminalTest, SetParity) {
+  struct termios terminfo;
+  Terminal term{m_pty.GetPrimaryFileDescriptor()};
+
+  ASSERT_EQ(term.SetParity(TerminalParity::No), true);
+  ASSERT_EQ(tcgetattr(m_pty.GetPrimaryFileDescriptor(), &terminfo), 0);
+  EXPECT_EQ(terminfo.c_cflag & PARENB, 0U);
+
+  ASSERT_EQ(term.SetParity(TerminalParity::Even), true);
+  ASSERT_EQ(tcgetattr(m_pty.GetPrimaryFileDescriptor(), &terminfo), 0);
+#if !defined(__linux__) // Linux pty devices strip PARENB
+  EXPECT_NE(terminfo.c_cflag & PARENB, 0U);
+#endif
+  EXPECT_EQ(terminfo.c_cflag & PARODD, 0U);
+#if defined(CMSPAR)
+  EXPECT_EQ(terminfo.c_cflag & CMSPAR, 0U);
+#endif
+
+  ASSERT_EQ(term.SetParity(TerminalParity::Odd), true);
+  ASSERT_EQ(tcgetattr(m_pty.GetPrimaryFileDescriptor(), &terminfo), 0);
+#if !defined(__linux__) // Linux pty devices strip PARENB
+  EXPECT_NE(terminfo.c_cflag & PARENB, 0U);
+#endif
+  EXPECT_NE(terminfo.c_cflag & PARODD, 0U);
+#if defined(CMSPAR)
+  EXPECT_EQ(terminfo.c_cflag & CMSPAR, 0U);
+#endif
+
+#if defined(CMSPAR)
+  ASSERT_EQ(term.SetParity(TerminalParity::Space), true);
+  ASSERT_EQ(tcgetattr(m_pty.GetPrimaryFileDescriptor(), &terminfo), 0);
+#if !defined(__linux__) // Linux pty devices strip PARENB
+  EXPECT_NE(terminfo.c_cflag & PARENB, 0U);
+#endif
+  EXPECT_EQ(terminfo.c_cflag & PARODD, 0U);
+  EXPECT_NE(terminfo.c_cflag & CMSPAR, 0U);
+
+  ASSERT_EQ(term.SetParity(TerminalParity::Mark), true);
+  ASSERT_EQ(tcgetattr(m_pty.GetPrimaryFileDescriptor(), &terminfo), 0);
+#if !defined(__linux__) // Linux pty devices strip PARENB
+  EXPECT_NE(terminfo.c_cflag & PARENB, 0U);
+#endif
+  EXPECT_NE(terminfo.c_cflag & PARODD, 0U);
+  EXPECT_NE(terminfo.c_cflag & CMSPAR, 0U);
+#else
+  EXPECT_EQ(term.SetParity(TerminalParity::Space), false);
+  EXPECT_EQ(term.SetParity(TerminalParity::Mark), false);
+#endif
+}
+
+TEST_F(TerminalTest, SetHardwareFlowControl) {
+  struct termios terminfo;
+  Terminal term{m_pty.GetPrimaryFileDescriptor()};
+
+#if defined(CRTSCTS)
+  ASSERT_EQ(term.SetHardwareFlowControl(true), true);
+ 

[Lldb-commits] [PATCH] D110997: [lldb] Restore tty attributes on disconnect

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

mgorny added a comment.

In D110997#3041802 , @labath wrote:

> That does not sound unreasonable, though I'm not sure how much is it actually 
> useful. Do you know if its like standard practice when working with serial 
> ports, ttys and stuff?

To be honest, I don't really know. GDB doesn't seem to do it. minicom does 
restore the initial state.

> , I'm not very happy that we have _any_ tty code inside the 
> ConnectionFileDescriptorPosix class, so I'm not enthusiastic about expanding 
> it. Let's see how the RFC goes first...

Do you think we should move it somewhere else? With the parameters being 
configurable via settings, it will probably make sense to move it into 
gdb-remote anyway.


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

https://reviews.llvm.org/D110997

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

[Lldb-commits] [PATCH] D111131: [LLDB] Round XML register bitsize to byte boundary

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid created this revision.
omjavaid added reviewers: labath, mgorny.
omjavaid requested review of this revision.

This patch allows LLDB to accept register sizes which are not aligned
to 8 bits bitsize boundary. This fixes a crash in LLDB when connecting
to OpenOCD stub. GDB xml description allows for non-aligned bit lengths
but they are rounded off to nearest byte during transfer. In case of
OpenOCD some of SOC specific system registers were less than a single
byte in length and were causing LLDB to crash.


https://reviews.llvm.org/D31

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


Index: 
lldb/test/API/functionalities/gdb_remote_client/TestArmRegisterDefinition.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestArmRegisterDefinition.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestArmRegisterDefinition.py
@@ -38,6 +38,10 @@
 
 
 
+
+
+
+
 
 
 
@@ -87,7 +91,7 @@
 return "E01"
 
 def readRegisters(self):
-return 
"2000f836002000102fcb0008f8360020a0360020200c0020b87f0120b7d100082ed200080001b87f0120"
+return 
"2000f836002000102fcb0008f8360020a0360020200c0020b87f0120b7d100082ed20008addebeafbc0001b87f0120"
 
 def haltReason(self):
 return "S05"
@@ -129,3 +133,15 @@
 
 pc_valobj = 
process.GetThreadAtIndex(0).GetFrameAtIndex(0).FindRegister("pc")
 self.assertEqual(pc_valobj.GetValueAsUnsigned(), 0x0800d22e)
+
+sys_valobj = 
process.GetThreadAtIndex(0).GetFrameAtIndex(0).FindRegister("SYS0")
+self.assertEqual(sys_valobj.GetValueAsUnsigned(), 0xdead)
+
+sys_valobj = 
process.GetThreadAtIndex(0).GetFrameAtIndex(0).FindRegister("SYS1")
+self.assertEqual(sys_valobj.GetValueAsUnsigned(), 0xbe)
+
+sys_valobj = 
process.GetThreadAtIndex(0).GetFrameAtIndex(0).FindRegister("SYS2")
+self.assertEqual(sys_valobj.GetValueAsUnsigned(), 0xaf)
+
+sys_valobj = 
process.GetThreadAtIndex(0).GetFrameAtIndex(0).FindRegister("SYS3")
+self.assertEqual(sys_valobj.GetValueAsUnsigned(), 0xbc)
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
@@ -4257,7 +4257,8 @@
 reg_info.name.SetString(value);
   } else if (name == "bitsize") {
 if (llvm::to_integer(value, reg_info.byte_size))
-  reg_info.byte_size /= CHAR_BIT;
+  reg_info.byte_size =
+  (reg_info.byte_size + CHAR_BIT - 1) / CHAR_BIT;
   } else if (name == "type") {
 gdb_type = value.str();
   } else if (name == "group") {


Index: lldb/test/API/functionalities/gdb_remote_client/TestArmRegisterDefinition.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestArmRegisterDefinition.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestArmRegisterDefinition.py
@@ -38,6 +38,10 @@
 
 
 
+
+
+
+
 
 
 
@@ -87,7 +91,7 @@
 return "E01"
 
 def readRegisters(self):
-return "2000f836002000102fcb0008f8360020a0360020200c0020b87f0120b7d100082ed200080001b87f012000

[Lldb-commits] [PATCH] D111131: [LLDB] Round XML register bitsize to byte boundary

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

mgorny added inline comments.



Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4261
+  reg_info.byte_size =
+  (reg_info.byte_size + CHAR_BIT - 1) / CHAR_BIT;
   } else if (name == "type") {

I suppose LLDB crashed because `byte_size` was zero? Maybe add an assert for 
that then.


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

https://reviews.llvm.org/D31

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

[Lldb-commits] [lldb] 214054f - [lldb] Move DynamicRegisterInfo to public Target library

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-10-05T12:40:55+02:00
New Revision: 214054f78a4e40656b17838300dff2f136032172

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

LOG: [lldb] Move DynamicRegisterInfo to public Target library

Move DynamicRegisterInfo from the internal lldbPluginProcessUtility
library to the public lldbTarget library.  This is a prerequisite
towards ABI plugin changes that are going to pass DynamicRegisterInfo
parameters.

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

Added: 
lldb/include/lldb/Target/DynamicRegisterInfo.h
lldb/source/Target/DynamicRegisterInfo.cpp
lldb/unittests/Target/DynamicRegisterInfoTest.cpp

Modified: 
lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.h
lldb/source/Plugins/Process/Utility/CMakeLists.txt
lldb/source/Plugins/Process/Utility/RegisterContextMemory.cpp
lldb/source/Plugins/Process/Utility/RegisterContextMemory.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
lldb/source/Target/CMakeLists.txt
lldb/unittests/Process/Utility/CMakeLists.txt
lldb/unittests/Target/CMakeLists.txt

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



diff  --git a/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h 
b/lldb/include/lldb/Target/DynamicRegisterInfo.h
similarity index 95%
rename from lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
rename to lldb/include/lldb/Target/DynamicRegisterInfo.h
index 286c8bc1020d7..ac8d6a7781c3f 100644
--- a/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
+++ b/lldb/include/lldb/Target/DynamicRegisterInfo.h
@@ -6,8 +6,8 @@
 //
 
//===--===//
 
-#ifndef LLDB_SOURCE_PLUGINS_PROCESS_UTILITY_DYNAMICREGISTERINFO_H
-#define LLDB_SOURCE_PLUGINS_PROCESS_UTILITY_DYNAMICREGISTERINFO_H
+#ifndef LLDB_TARGET_DYNAMICREGISTERINFO_H
+#define LLDB_TARGET_DYNAMICREGISTERINFO_H
 
 #include 
 #include 
@@ -16,6 +16,8 @@
 #include "lldb/Utility/StructuredData.h"
 #include "lldb/lldb-private.h"
 
+namespace lldb_private {
+
 class DynamicRegisterInfo {
 protected:
   DynamicRegisterInfo(DynamicRegisterInfo &) = default;
@@ -113,4 +115,6 @@ class DynamicRegisterInfo {
   bool m_is_reconfigurable = false;
 };
 
-#endif // LLDB_SOURCE_PLUGINS_PROCESS_UTILITY_DYNAMICREGISTERINFO_H
+} // namespace lldb_private
+
+#endif // LLDB_TARGET_DYNAMICREGISTERINFO_H

diff  --git 
a/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp 
b/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
index 8c9e31dadff5e..40cf1d9816789 100644
--- a/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
+++ b/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
@@ -12,7 +12,6 @@
 
 #include "OperatingSystemPython.h"
 
-#include "Plugins/Process/Utility/DynamicRegisterInfo.h"
 #include "Plugins/Process/Utility/RegisterContextDummy.h"
 #include "Plugins/Process/Utility/RegisterContextMemory.h"
 #include "Plugins/Process/Utility/ThreadMemory.h"

diff  --git 
a/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.h 
b/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.h
index 4bdd38fc81007..743fb545e3403 100644
--- a/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.h
+++ b/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.h
@@ -13,11 +13,10 @@
 
 #if LLDB_ENABLE_PYTHON
 
+#include "lldb/Target/DynamicRegisterInfo.h"
 #include "lldb/Target/OperatingSystem.h"
 #include "lldb/Utility/StructuredData.h"
 
-class DynamicRegisterInfo;
-
 namespace lldb_private {
 class ScriptInterpreter;
 }
@@ -72,14 +71,14 @@ class OperatingSystemPython : public 
lldb_private::OperatingSystem {
   lldb_private::ThreadList &old_thread_list,
   std::vector &core_used_map, bool *did_create_ptr);
 
-  DynamicRegisterInfo *GetDynamicRegisterInfo();
+  lldb_private::DynamicRegisterInfo *GetDynamicRegisterInfo();
 
   lldb::ValueObjectSP m_thread_list_valobj_sp;
-  std::unique_ptr m_register_info_up;
+  std::unique_ptr m_register_info_up;
   lldb_private::ScriptInterpreter *m_interpreter;
   lldb_private::StructuredData::ObjectSP m_python_object_sp;
 };
 
-#endif
+#endif // LLDB_ENABLE_PYTHON
 
 #endif // liblldb_OperatingSystemPython_h_

diff  --git a/lldb/source/Plugins/Process/Utility/CMakeLists.txt 
b/lldb/source/Plugins/Process/Utility/CMakeLists.txt
index 14318763f6a06..2a06af008dcec 100644
--- a/lldb/source/Plugins/Process/Utility/CMakeLists.txt
+++ b/lldb/source/Plugins/Process/Utility

[Lldb-commits] [PATCH] D110942: [lldb] Move DynamicRegisterInfo to public Target library

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rG214054f78a4e: [lldb] Move DynamicRegisterInfo to public 
Target library (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110942

Files:
  lldb/include/lldb/Target/DynamicRegisterInfo.h
  lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
  lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.h
  lldb/source/Plugins/Process/Utility/CMakeLists.txt
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
  lldb/source/Plugins/Process/Utility/RegisterContextMemory.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextMemory.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
  lldb/source/Target/CMakeLists.txt
  lldb/source/Target/DynamicRegisterInfo.cpp
  lldb/unittests/Process/Utility/CMakeLists.txt
  lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
  lldb/unittests/Target/CMakeLists.txt
  lldb/unittests/Target/DynamicRegisterInfoTest.cpp

Index: lldb/unittests/Target/DynamicRegisterInfoTest.cpp
===
--- lldb/unittests/Target/DynamicRegisterInfoTest.cpp
+++ lldb/unittests/Target/DynamicRegisterInfoTest.cpp
@@ -9,8 +9,7 @@
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
-#include "Plugins/Process/Utility/DynamicRegisterInfo.h"
-
+#include "lldb/Target/DynamicRegisterInfo.h"
 #include "lldb/Utility/ArchSpec.h"
 
 using namespace lldb_private;
Index: lldb/unittests/Target/CMakeLists.txt
===
--- lldb/unittests/Target/CMakeLists.txt
+++ lldb/unittests/Target/CMakeLists.txt
@@ -1,5 +1,6 @@
 add_lldb_unittest(TargetTests
   ABITest.cpp
+  DynamicRegisterInfoTest.cpp
   ExecutionContextTest.cpp
   MemoryRegionInfoTest.cpp
   ModuleCacheTest.cpp
Index: lldb/unittests/Process/Utility/CMakeLists.txt
===
--- lldb/unittests/Process/Utility/CMakeLists.txt
+++ lldb/unittests/Process/Utility/CMakeLists.txt
@@ -15,7 +15,6 @@
   ${NETBSD_SOURCES})
 
 add_lldb_unittest(ProcessUtilityTests
-  DynamicRegisterInfoTest.cpp
   LinuxProcMapsTest.cpp
   MemoryTagManagerAArch64MTETest.cpp
   RegisterContextTest.cpp
Index: lldb/source/Target/DynamicRegisterInfo.cpp
===
--- lldb/source/Target/DynamicRegisterInfo.cpp
+++ lldb/source/Target/DynamicRegisterInfo.cpp
@@ -6,8 +6,7 @@
 //
 //===--===//
 
-#include "DynamicRegisterInfo.h"
-
+#include "lldb/Target/DynamicRegisterInfo.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/DataFormatters/FormatManager.h"
 #include "lldb/Interpreter/OptionArgParser.h"
Index: lldb/source/Target/CMakeLists.txt
===
--- lldb/source/Target/CMakeLists.txt
+++ lldb/source/Target/CMakeLists.txt
@@ -9,6 +9,7 @@
 add_lldb_library(lldbTarget
   ABI.cpp
   AssertFrameRecognizer.cpp
+  DynamicRegisterInfo.cpp
   ExecutionContext.cpp
   InstrumentationRuntime.cpp
   InstrumentationRuntimeStopInfo.cpp
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
@@ -11,7 +11,7 @@
 
 #include 
 
-#include "Plugins/Process/Utility/DynamicRegisterInfo.h"
+#include "lldb/Target/DynamicRegisterInfo.h"
 #include "lldb/Target/RegisterContext.h"
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/DataExtractor.h"
Index: lldb/source/Plugins/Process/Utility/RegisterContextMemory.h
===
--- lldb/source/Plugins/Process/Utility/RegisterContextMemory.h
+++ lldb/source/Plugins/Process/Utility/RegisterContextMemory.h
@@ -11,17 +11,16 @@
 
 #include 
 
+#include "lldb/Target/DynamicRegisterInfo.h"
 #include "lldb/Target/RegisterContext.h"
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/lldb-private.h"
 
-class DynamicRegisterInfo;
-
 class RegisterContextMemory : public lldb_private::RegisterContext {
 public:
   RegisterContextMemory(lldb_private::Thread &thread,
 uint32_t concrete_frame_idx,
-DynamicRegisterInfo ®_info,
+lldb_private::DynamicRegisterInfo ®_info,
 lldb::addr_t reg_data_addr);
 
   ~RegisterContextMemory() override;
@@ -60,7 +59,7 @@
 protected:
   void SetAllRegisterValid(bool b);
 
-  DynamicRegisterInfo &m_reg_infos;
+  lldb_private::DynamicRegisterInfo &m_reg_infos;
   std::vector m_reg

[Lldb-commits] [PATCH] D111131: [LLDB] Round XML register bitsize to byte boundary

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added inline comments.



Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4357
 
 assert(reg_info.byte_size != 0);
 registers.push_back(reg_info);

@mgorny the assert already exists but then we also want to allow bit sized 
registers although they ll be viewed as byte length for now.


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

https://reviews.llvm.org/D31

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

[Lldb-commits] [PATCH] D111131: [LLDB] Round XML register bitsize to byte boundary

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

mgorny added inline comments.



Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4357
 
 assert(reg_info.byte_size != 0);
 registers.push_back(reg_info);

omjavaid wrote:
> @mgorny the assert already exists but then we also want to allow bit sized 
> registers although they ll be viewed as byte length for now.
Ah, right. I suppose you could skip zero-byte registers though. That should 
amend the assert with better release behavior.


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

https://reviews.llvm.org/D31

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

[Lldb-commits] [PATCH] D111052: [lldb] [gdb-remote] Correct st_dev values in vFile:fstat packet

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

So I guess all that's left to do is to add some cast to placate compilers ?


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

https://reviews.llvm.org/D111052

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

[Lldb-commits] [PATCH] D111131: [LLDB] Round XML register bitsize to byte boundary

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added inline comments.



Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4357
 
 assert(reg_info.byte_size != 0);
 registers.push_back(reg_info);

mgorny wrote:
> omjavaid wrote:
> > @mgorny the assert already exists but then we also want to allow bit sized 
> > registers although they ll be viewed as byte length for now.
> Ah, right. I suppose you could skip zero-byte registers though. That should 
> amend the assert with better release behavior.
on a second thought, I dont see a zero sized register being sent by stub as a 
big enough reason to abort the whole debug session unless its one of GPRs. May 
be we skip the assert altogether and replace it with an error message. 
What do you think?


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

https://reviews.llvm.org/D31

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

[Lldb-commits] [PATCH] D111052: [lldb] [gdb-remote] Correct st_dev values in vFile:fstat packet

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

mgorny added a comment.

In D111052#3042181 , @labath wrote:

> So I guess all that's left to do is to add some cast to placate compilers ?

Nah, my original logic checks for overflow and replaces the value with `0` if 
one occurs (which IMO is more correct than truncating the value).


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

https://reviews.llvm.org/D111052

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

[Lldb-commits] [PATCH] D111131: [LLDB] Round XML register bitsize to byte boundary

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

mgorny added inline comments.



Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4357
 
 assert(reg_info.byte_size != 0);
 registers.push_back(reg_info);

omjavaid wrote:
> mgorny wrote:
> > omjavaid wrote:
> > > @mgorny the assert already exists but then we also want to allow bit 
> > > sized registers although they ll be viewed as byte length for now.
> > Ah, right. I suppose you could skip zero-byte registers though. That should 
> > amend the assert with better release behavior.
> on a second thought, I dont see a zero sized register being sent by stub as a 
> big enough reason to abort the whole debug session unless its one of GPRs. 
> May be we skip the assert altogether and replace it with an error message. 
> What do you think?
Yes, you are correct. Probably `LLDB_LOG` would go in line with how we handle 
these things.


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

https://reviews.llvm.org/D31

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

[Lldb-commits] [PATCH] D110962: [lldb] Add unit tests for Terminal API

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki added a comment.

This is failing on my Fedora 34 (x86-64) box. Is this expected?

  FAIL: lldb-unit :: Host/./HostTests/TerminalTest.SaveRestoreRAII (78548 of 
79402)
   TEST 'lldb-unit :: 
Host/./HostTests/TerminalTest.SaveRestoreRAII' FAILED 
  Script:
  --
  /tmp/_update_lc/r/tools/lldb/unittests/Host/./HostTests 
--gtest_filter=TerminalTest.SaveRestoreRAII
  --
  Note: Google Test filter = TerminalTest.SaveRestoreRAII
  [==] Running 1 test from 1 test suite.
  [--] Global test environment set-up.
  [--] 1 test from TerminalTest
  [ RUN  ] TerminalTest.SaveRestoreRAII
  /home/dave/ro_s/lp/lldb/unittests/Host/posix/TerminalTest.cpp:93: Failure
  Expected equality of these values:
tcsetattr(m_pty.GetPrimaryFileDescriptor(), 0, &terminfo)
  Which is: -1
0
  [  FAILED  ] TerminalTest.SaveRestoreRAII (4 ms)
  [--] 1 test from TerminalTest (4 ms total)
  
  [--] Global test environment tear-down
  [==] 1 test from 1 test suite ran. (4 ms total)
  [  PASSED  ] 0 tests.
  [  FAILED  ] 1 test, listed below:
  [  FAILED  ] TerminalTest.SaveRestoreRAII
  
   1 FAILED TEST
  
  
  Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..
  FAIL: lldb-unit :: Host/./HostTests/TerminalTest.SaveRestore (78561 of 79402)
   TEST 'lldb-unit :: 
Host/./HostTests/TerminalTest.SaveRestore' FAILED 
  Script:
  --
  /tmp/_update_lc/r/tools/lldb/unittests/Host/./HostTests 
--gtest_filter=TerminalTest.SaveRestore
  --
  Note: Google Test filter = TerminalTest.SaveRestore
  [==] Running 1 test from 1 test suite.
  [--] Global test environment set-up.
  [--] 1 test from TerminalTest
  [ RUN  ] TerminalTest.SaveRestore
  /home/dave/ro_s/lp/lldb/unittests/Host/posix/TerminalTest.cpp:121: Failure
  Expected equality of these values:
tcsetattr(m_pty.GetPrimaryFileDescriptor(), 0, &terminfo)
  Which is: -1
0
  [  FAILED  ] TerminalTest.SaveRestore (4 ms)
  [--] 1 test from TerminalTest (4 ms total)
  
  [--] Global test environment tear-down
  [==] 1 test from 1 test suite ran. (4 ms total)
  [  PASSED  ] 0 tests.
  [  FAILED  ] 1 test, listed below:
  [  FAILED  ] TerminalTest.SaveRestore
  
   1 FAILED TEST


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110962

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

[Lldb-commits] [PATCH] D111131: [LLDB] Round XML register bitsize to byte boundary

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added inline comments.



Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4357
 
 assert(reg_info.byte_size != 0);
 registers.push_back(reg_info);

mgorny wrote:
> omjavaid wrote:
> > mgorny wrote:
> > > omjavaid wrote:
> > > > @mgorny the assert already exists but then we also want to allow bit 
> > > > sized registers although they ll be viewed as byte length for now.
> > > Ah, right. I suppose you could skip zero-byte registers though. That 
> > > should amend the assert with better release behavior.
> > on a second thought, I dont see a zero sized register being sent by stub as 
> > a big enough reason to abort the whole debug session unless its one of 
> > GPRs. May be we skip the assert altogether and replace it with an error 
> > message. 
> > What do you think?
> Yes, you are correct. Probably `LLDB_LOG` would go in line with how we handle 
> these things.
LLDB_LOG will hide message from user unless log is enabled. I think user must 
be notified that register is  zero sized and thats why you wont be able to see 
it in register read. Similar to the way we notify user about unhandled register 
attibutes like "save-restore".


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

https://reviews.llvm.org/D31

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

[Lldb-commits] [PATCH] D111131: [LLDB] Round XML register bitsize to byte boundary

[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/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4260-4261
 if (llvm::to_integer(value, reg_info.byte_size))
-  reg_info.byte_size /= CHAR_BIT;
+  reg_info.byte_size =
+  (reg_info.byte_size + CHAR_BIT - 1) / CHAR_BIT;
   } else if (name == "type") {

`llvm::divideCeil(reg_info.byte_size, CHAR_BIT)`, perhaps



Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4357
 
 assert(reg_info.byte_size != 0);
 registers.push_back(reg_info);

mgorny wrote:
> omjavaid wrote:
> > mgorny wrote:
> > > omjavaid wrote:
> > > > @mgorny the assert already exists but then we also want to allow bit 
> > > > sized registers although they ll be viewed as byte length for now.
> > > Ah, right. I suppose you could skip zero-byte registers though. That 
> > > should amend the assert with better release behavior.
> > on a second thought, I dont see a zero sized register being sent by stub as 
> > a big enough reason to abort the whole debug session unless its one of 
> > GPRs. May be we skip the assert altogether and replace it with an error 
> > message. 
> > What do you think?
> Yes, you are correct. Probably `LLDB_LOG` would go in line with how we handle 
> these things.
Yeah, I don't think crashing is a good response to the stub sending us 
nonsensical register definitions. Though that seems like a separate issue..


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

https://reviews.llvm.org/D31

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

[Lldb-commits] [PATCH] D111131: [LLDB] Round XML register bitsize to byte boundary

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4357
 
 assert(reg_info.byte_size != 0);
 registers.push_back(reg_info);

omjavaid wrote:
> labath wrote:
> > mgorny wrote:
> > > omjavaid wrote:
> > > > mgorny wrote:
> > > > > omjavaid wrote:
> > > > > > @mgorny the assert already exists but then we also want to allow 
> > > > > > bit sized registers although they ll be viewed as byte length for 
> > > > > > now.
> > > > > Ah, right. I suppose you could skip zero-byte registers though. That 
> > > > > should amend the assert with better release behavior.
> > > > on a second thought, I dont see a zero sized register being sent by 
> > > > stub as a big enough reason to abort the whole debug session unless its 
> > > > one of GPRs. May be we skip the assert altogether and replace it with 
> > > > an error message. 
> > > > What do you think?
> > > Yes, you are correct. Probably `LLDB_LOG` would go in line with how we 
> > > handle these things.
> > Yeah, I don't think crashing is a good response to the stub sending us 
> > nonsensical register definitions. Though that seems like a separate issue..
> LLDB_LOG will hide message from user unless log is enabled. I think user must 
> be notified that register is  zero sized and thats why you wont be able to 
> see it in register read. Similar to the way we notify user about unhandled 
> register attibutes like "save-restore".
BTW, I came very close to deleting that printf when I was touching this code 
last month.
FWIW, my hierarchy is:
user-facing warning (though I don't know how would that be implemented here) >> 
log entry >> nothing >> raw printf


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

https://reviews.llvm.org/D31

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

[Lldb-commits] [PATCH] D110962: [lldb] Add unit tests for Terminal API

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

mgorny added a comment.

No, it's not. Could you add a quick `printf()` there to see what the value of 
`errno` is?

I need to check if gtest has a better method of checking for failure with errno.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110962

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

[Lldb-commits] [PATCH] D110962: [lldb] Add unit tests for Terminal API

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

mgorny added a comment.

…and/or try removing lines from the `// make some arbitrary changes` block to 
see which one causes the error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110962

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

[Lldb-commits] [PATCH] D111052: [lldb] [gdb-remote] Correct st_dev values in vFile:fstat packet

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D111052#3042184 , @mgorny wrote:

> In D111052#3042181 , @labath wrote:
>
>> So I guess all that's left to do is to add some cast to placate compilers ?
>
> Nah, my original logic checks for overflow and replaces the value with `0` if 
> one occurs (which IMO is more correct than truncating the value).

I was talking about the warning Raphael ran into.

We already discussed truncation vs 0 on the initial patch. I don't think we 
need to strictly copy gdb behavior here, though I would also be fine with 
changing it.


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

https://reviews.llvm.org/D111052

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

[Lldb-commits] [PATCH] D110962: [lldb] Add unit tests for Terminal API

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki added a comment.

errno == 22 (EINVAL)

The FD in question is 3. Here is the output from `lsof`: HostTests 80108 dave   
 3u   CHR5,2  0t0   231 /dev/ptmx

Can we/I please revert this for now?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110962

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

[Lldb-commits] [PATCH] D111136: [lldb] [DynamicRegisterInfo] Support iterating over Registers()

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

mgorny created this revision.
mgorny added reviewers: labath, krytarowski, teemperor, emaste.
mgorny requested review of this revision.

Add DynamicRegisterInfo::Registers() method that returns
llvm::iterator_range<> over RegisterInfos.  This is a convenient
replacement for GetNumRegisters() + GetRegisterInfoAtIndex().


https://reviews.llvm.org/D36

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


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
@@ -210,16 +210,11 @@
   SetAllRegisterValid(true);
   return true;
 } else if (buffer_sp->GetByteSize() > 0) {
-  const int regcount = m_reg_info_sp->GetNumRegisters();
-  for (int i = 0; i < regcount; i++) {
-struct RegisterInfo *reginfo =
-m_reg_info_sp->GetRegisterInfoAtIndex(i);
-if (reginfo->byte_offset + reginfo->byte_size <=
-buffer_sp->GetByteSize()) {
-  m_reg_valid[i] = true;
-} else {
-  m_reg_valid[i] = false;
-}
+  for (auto x : llvm::enumerate(m_reg_info_sp->Registers())) {
+const struct RegisterInfo ®info = x.value();
+m_reg_valid[x.index()] =
+(reginfo.byte_offset + reginfo.byte_size <=
+ buffer_sp->GetByteSize());
   }
 
   m_gpacket_cached = true;
Index: lldb/include/lldb/Target/DynamicRegisterInfo.h
===
--- lldb/include/lldb/Target/DynamicRegisterInfo.h
+++ lldb/include/lldb/Target/DynamicRegisterInfo.h
@@ -77,9 +77,13 @@
   const lldb_private::RegisterInfo *
   GetRegisterInfo(llvm::StringRef reg_name) const;
 
+  typedef std::vector reg_collection;
+  llvm::iterator_range Registers() {
+return llvm::iterator_range(m_regs);
+  }
+
 protected:
   // Classes that inherit from DynamicRegisterInfo can see and modify these
-  typedef std::vector reg_collection;
   typedef std::vector set_collection;
   typedef std::vector reg_num_collection;
   typedef std::vector set_reg_num_collection;


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
@@ -210,16 +210,11 @@
   SetAllRegisterValid(true);
   return true;
 } else if (buffer_sp->GetByteSize() > 0) {
-  const int regcount = m_reg_info_sp->GetNumRegisters();
-  for (int i = 0; i < regcount; i++) {
-struct RegisterInfo *reginfo =
-m_reg_info_sp->GetRegisterInfoAtIndex(i);
-if (reginfo->byte_offset + reginfo->byte_size <=
-buffer_sp->GetByteSize()) {
-  m_reg_valid[i] = true;
-} else {
-  m_reg_valid[i] = false;
-}
+  for (auto x : llvm::enumerate(m_reg_info_sp->Registers())) {
+const struct RegisterInfo ®info = x.value();
+m_reg_valid[x.index()] =
+(reginfo.byte_offset + reginfo.byte_size <=
+ buffer_sp->GetByteSize());
   }
 
   m_gpacket_cached = true;
Index: lldb/include/lldb/Target/DynamicRegisterInfo.h
===
--- lldb/include/lldb/Target/DynamicRegisterInfo.h
+++ lldb/include/lldb/Target/DynamicRegisterInfo.h
@@ -77,9 +77,13 @@
   const lldb_private::RegisterInfo *
   GetRegisterInfo(llvm::StringRef reg_name) const;
 
+  typedef std::vector reg_collection;
+  llvm::iterator_range Registers() {
+return llvm::iterator_range(m_regs);
+  }
+
 protected:
   // Classes that inherit from DynamicRegisterInfo can see and modify these
-  typedef std::vector reg_collection;
   typedef std::vector set_collection;
   typedef std::vector reg_num_collection;
   typedef std::vector set_reg_num_collection;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D111136: [lldb] [DynamicRegisterInfo] Support iterating over Registers()

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/include/lldb/Target/DynamicRegisterInfo.h:81
+  typedef std::vector reg_collection;
+  llvm::iterator_range Registers() {
+return llvm::iterator_range(m_regs);

llvm (and, surprisingly, lldb -- I guess because its a new feature) follows c++ 
naming conventions for methods like these.



Comment at: lldb/include/lldb/Target/DynamicRegisterInfo.h:82
+  llvm::iterator_range Registers() {
+return llvm::iterator_range(m_regs);
+  }

Could this return const iterators? It seems we already have some non-const 
accessors, but I'd rather not propagate that..


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

https://reviews.llvm.org/D36

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

[Lldb-commits] [lldb] cf818b5 - [lldb][NFC] Remove unnecessary include in cpp/const_this test

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2021-10-05T14:39:10+02:00
New Revision: cf818b55e79ee637c72f1f94a183eec26b4fa3b9

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

LOG: [lldb][NFC] Remove unnecessary include in cpp/const_this test

Added: 


Modified: 
lldb/test/API/lang/cpp/const_this/main.cpp

Removed: 




diff  --git a/lldb/test/API/lang/cpp/const_this/main.cpp 
b/lldb/test/API/lang/cpp/const_this/main.cpp
index 8520077528613..2db14e649918d 100644
--- a/lldb/test/API/lang/cpp/const_this/main.cpp
+++ b/lldb/test/API/lang/cpp/const_this/main.cpp
@@ -1,5 +1,3 @@
-#include 
-
 class foo {
 public:
   template  T func(T x) const {



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

[Lldb-commits] [PATCH] D111142: [lldb] [ABI] Make AugmentRegisterInfo() take whole DynamicRegisterInfo

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

mgorny created this revision.
mgorny added reviewers: labath, teemperor, emaste, krytarowski.
mgorny requested review of this revision.

Modify ABI::AugmentRegisterInfo() to take the whole DynamicRegisterInfo
rather than being claled per-register.


https://reviews.llvm.org/D42

Files:
  lldb/include/lldb/Target/ABI.h
  lldb/include/lldb/Target/DynamicRegisterInfo.h
  lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp
  lldb/source/Plugins/ABI/AArch64/ABIAArch64.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Target/ABI.cpp
  lldb/source/Target/DynamicRegisterInfo.cpp

Index: lldb/source/Target/DynamicRegisterInfo.cpp
===
--- lldb/source/Target/DynamicRegisterInfo.cpp
+++ lldb/source/Target/DynamicRegisterInfo.cpp
@@ -837,3 +837,11 @@
   return ®_info;
   return nullptr;
 }
+
+lldb_private::RegisterInfo *
+DynamicRegisterInfo::GetRegisterInfo(llvm::StringRef reg_name) {
+  for (auto ®_info : m_regs)
+if (reg_info.name == reg_name)
+  return ®_info;
+  return nullptr;
+}
Index: lldb/source/Target/ABI.cpp
===
--- lldb/source/Target/ABI.cpp
+++ lldb/source/Target/ABI.cpp
@@ -214,33 +214,37 @@
   return info_up;
 }
 
-void RegInfoBasedABI::AugmentRegisterInfo(RegisterInfo &info) {
-  if (info.kinds[eRegisterKindEHFrame] != LLDB_INVALID_REGNUM &&
-  info.kinds[eRegisterKindDWARF] != LLDB_INVALID_REGNUM)
-return;
-
-  RegisterInfo abi_info;
-  if (!GetRegisterInfoByName(info.name, abi_info))
-return;
-
-  if (info.kinds[eRegisterKindEHFrame] == LLDB_INVALID_REGNUM)
-info.kinds[eRegisterKindEHFrame] = abi_info.kinds[eRegisterKindEHFrame];
-  if (info.kinds[eRegisterKindDWARF] == LLDB_INVALID_REGNUM)
-info.kinds[eRegisterKindDWARF] = abi_info.kinds[eRegisterKindDWARF];
-  if (info.kinds[eRegisterKindGeneric] == LLDB_INVALID_REGNUM)
-info.kinds[eRegisterKindGeneric] = abi_info.kinds[eRegisterKindGeneric];
+void RegInfoBasedABI::AugmentRegisterInfo(DynamicRegisterInfo &dyn_info) {
+  for (RegisterInfo &info : dyn_info.Registers()) {
+if (info.kinds[eRegisterKindEHFrame] != LLDB_INVALID_REGNUM &&
+info.kinds[eRegisterKindDWARF] != LLDB_INVALID_REGNUM)
+  continue;
+
+RegisterInfo abi_info;
+if (!GetRegisterInfoByName(info.name, abi_info))
+  continue;
+
+if (info.kinds[eRegisterKindEHFrame] == LLDB_INVALID_REGNUM)
+  info.kinds[eRegisterKindEHFrame] = abi_info.kinds[eRegisterKindEHFrame];
+if (info.kinds[eRegisterKindDWARF] == LLDB_INVALID_REGNUM)
+  info.kinds[eRegisterKindDWARF] = abi_info.kinds[eRegisterKindDWARF];
+if (info.kinds[eRegisterKindGeneric] == LLDB_INVALID_REGNUM)
+  info.kinds[eRegisterKindGeneric] = abi_info.kinds[eRegisterKindGeneric];
+  }
 }
 
-void MCBasedABI::AugmentRegisterInfo(RegisterInfo &info) {
+void MCBasedABI::AugmentRegisterInfo(DynamicRegisterInfo &dyn_info) {
   uint32_t eh, dwarf;
-  std::tie(eh, dwarf) = GetEHAndDWARFNums(info.name);
-
-  if (info.kinds[eRegisterKindEHFrame] == LLDB_INVALID_REGNUM)
-info.kinds[eRegisterKindEHFrame] = eh;
-  if (info.kinds[eRegisterKindDWARF] == LLDB_INVALID_REGNUM)
-info.kinds[eRegisterKindDWARF] = dwarf;
-  if (info.kinds[eRegisterKindGeneric] == LLDB_INVALID_REGNUM)
-info.kinds[eRegisterKindGeneric] = GetGenericNum(info.name);
+  for (RegisterInfo &info : dyn_info.Registers()) {
+std::tie(eh, dwarf) = GetEHAndDWARFNums(info.name);
+
+if (info.kinds[eRegisterKindEHFrame] == LLDB_INVALID_REGNUM)
+  info.kinds[eRegisterKindEHFrame] = eh;
+if (info.kinds[eRegisterKindDWARF] == LLDB_INVALID_REGNUM)
+  info.kinds[eRegisterKindDWARF] = dwarf;
+if (info.kinds[eRegisterKindGeneric] == LLDB_INVALID_REGNUM)
+  info.kinds[eRegisterKindGeneric] = GetGenericNum(info.name);
+  }
 }
 
 std::pair
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
@@ -4482,11 +4482,6 @@
 
 void ProcessGDBRemote::AddRemoteRegisters(
 std::vector ®isters, const ArchSpec &arch_to_use) {
-  // Don't use Process::GetABI, this code gets called from DidAttach, and
-  // in that context we haven't set the Target's architecture yet, so the
-  // 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)) {
@@ -4539,12 +4534,14 @@
   : nullptr,
   remote_reg_info.dwarf_opcode_bytes.size(),
 };
-
-if (abi_sp)
-  abi_sp->AugmentRegisterInfo(reg_info);
 m_register_info_sp->AddRegister(reg_info, remote_reg_info.set_name);
   };
 
+  // Don't use Process::GetABI, this code gets called from DidAttach, and
+  // in tha

[Lldb-commits] [PATCH] D111142: [lldb] [ABI] Make AugmentRegisterInfo() take whole DynamicRegisterInfo

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

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

Add one missing call to `AugmentRegisterInfo()` before `Finalize()`.


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

https://reviews.llvm.org/D42

Files:
  lldb/include/lldb/Target/ABI.h
  lldb/include/lldb/Target/DynamicRegisterInfo.h
  lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp
  lldb/source/Plugins/ABI/AArch64/ABIAArch64.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Target/ABI.cpp
  lldb/source/Target/DynamicRegisterInfo.cpp

Index: lldb/source/Target/DynamicRegisterInfo.cpp
===
--- lldb/source/Target/DynamicRegisterInfo.cpp
+++ lldb/source/Target/DynamicRegisterInfo.cpp
@@ -837,3 +837,11 @@
   return ®_info;
   return nullptr;
 }
+
+lldb_private::RegisterInfo *
+DynamicRegisterInfo::GetRegisterInfo(llvm::StringRef reg_name) {
+  for (auto ®_info : m_regs)
+if (reg_info.name == reg_name)
+  return ®_info;
+  return nullptr;
+}
Index: lldb/source/Target/ABI.cpp
===
--- lldb/source/Target/ABI.cpp
+++ lldb/source/Target/ABI.cpp
@@ -214,33 +214,37 @@
   return info_up;
 }
 
-void RegInfoBasedABI::AugmentRegisterInfo(RegisterInfo &info) {
-  if (info.kinds[eRegisterKindEHFrame] != LLDB_INVALID_REGNUM &&
-  info.kinds[eRegisterKindDWARF] != LLDB_INVALID_REGNUM)
-return;
-
-  RegisterInfo abi_info;
-  if (!GetRegisterInfoByName(info.name, abi_info))
-return;
-
-  if (info.kinds[eRegisterKindEHFrame] == LLDB_INVALID_REGNUM)
-info.kinds[eRegisterKindEHFrame] = abi_info.kinds[eRegisterKindEHFrame];
-  if (info.kinds[eRegisterKindDWARF] == LLDB_INVALID_REGNUM)
-info.kinds[eRegisterKindDWARF] = abi_info.kinds[eRegisterKindDWARF];
-  if (info.kinds[eRegisterKindGeneric] == LLDB_INVALID_REGNUM)
-info.kinds[eRegisterKindGeneric] = abi_info.kinds[eRegisterKindGeneric];
+void RegInfoBasedABI::AugmentRegisterInfo(DynamicRegisterInfo &dyn_info) {
+  for (RegisterInfo &info : dyn_info.Registers()) {
+if (info.kinds[eRegisterKindEHFrame] != LLDB_INVALID_REGNUM &&
+info.kinds[eRegisterKindDWARF] != LLDB_INVALID_REGNUM)
+  continue;
+
+RegisterInfo abi_info;
+if (!GetRegisterInfoByName(info.name, abi_info))
+  continue;
+
+if (info.kinds[eRegisterKindEHFrame] == LLDB_INVALID_REGNUM)
+  info.kinds[eRegisterKindEHFrame] = abi_info.kinds[eRegisterKindEHFrame];
+if (info.kinds[eRegisterKindDWARF] == LLDB_INVALID_REGNUM)
+  info.kinds[eRegisterKindDWARF] = abi_info.kinds[eRegisterKindDWARF];
+if (info.kinds[eRegisterKindGeneric] == LLDB_INVALID_REGNUM)
+  info.kinds[eRegisterKindGeneric] = abi_info.kinds[eRegisterKindGeneric];
+  }
 }
 
-void MCBasedABI::AugmentRegisterInfo(RegisterInfo &info) {
+void MCBasedABI::AugmentRegisterInfo(DynamicRegisterInfo &dyn_info) {
   uint32_t eh, dwarf;
-  std::tie(eh, dwarf) = GetEHAndDWARFNums(info.name);
-
-  if (info.kinds[eRegisterKindEHFrame] == LLDB_INVALID_REGNUM)
-info.kinds[eRegisterKindEHFrame] = eh;
-  if (info.kinds[eRegisterKindDWARF] == LLDB_INVALID_REGNUM)
-info.kinds[eRegisterKindDWARF] = dwarf;
-  if (info.kinds[eRegisterKindGeneric] == LLDB_INVALID_REGNUM)
-info.kinds[eRegisterKindGeneric] = GetGenericNum(info.name);
+  for (RegisterInfo &info : dyn_info.Registers()) {
+std::tie(eh, dwarf) = GetEHAndDWARFNums(info.name);
+
+if (info.kinds[eRegisterKindEHFrame] == LLDB_INVALID_REGNUM)
+  info.kinds[eRegisterKindEHFrame] = eh;
+if (info.kinds[eRegisterKindDWARF] == LLDB_INVALID_REGNUM)
+  info.kinds[eRegisterKindDWARF] = dwarf;
+if (info.kinds[eRegisterKindGeneric] == LLDB_INVALID_REGNUM)
+  info.kinds[eRegisterKindGeneric] = GetGenericNum(info.name);
+  }
 }
 
 std::pair
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
@@ -557,6 +557,8 @@
   }
 
   // At this point, we can finalize our register info.
+  if (ABISP abi_sp = ABI::FindPlugin(shared_from_this(), arch_to_use))
+abi_sp->AugmentRegisterInfo(*m_register_info_sp);
   m_register_info_sp->Finalize(GetTarget().GetArchitecture());
 }
 
@@ -4482,11 +4484,6 @@
 
 void ProcessGDBRemote::AddRemoteRegisters(
 std::vector ®isters, const ArchSpec &arch_to_use) {
-  // Don't use Process::GetABI, this code gets called from DidAttach, and
-  // in that context we haven't set the Target's architecture yet, so the
-  // 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)) {
@@ -4539,12 +4536,14 @@
   : nullptr,
   remote_reg_info.dwarf_opcode_bytes.size(),
 

[Lldb-commits] [PATCH] D109876: [lldb] [ABI/AArch64] Add pseudo-regs if missing

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

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

Update to use the new `AugmentRegisterInfo()` API.


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

https://reviews.llvm.org/D109876

Files:
  lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp
  lldb/source/Target/DynamicRegisterInfo.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
@@ -380,6 +380,7 @@
 return "".join(reg_data)
 
 def writeRegisters(self, reg_hex):
+reg_data[:] = [reg_hex]
 return "OK"
 
 def haltReason(self):
@@ -429,3 +430,43 @@
["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}"])
+
+# test partial registers
+self.match("register read w0",
+   ["w0 = 0x04030201"])
+self.runCmd("register write w0 0xfffefdfc")
+self.match("register read x0",
+   ["x0 = 0x08070605fffefdfc"])
+
+self.match("register read w1",
+   ["w1 = 0x14131211"])
+self.runCmd("register write w1 0xefeeedec")
+self.match("register read x1",
+   ["x1 = 0x18171615efeeedec"])
+
+self.match("register read w30",
+   ["w30 = 0x44434241"])
+self.runCmd("register write w30 0xdfdedddc")
+self.match("register read x30",
+   ["x30 = 0x48474645dfdedddc"])
+
+self.match("register read w31",
+   ["w31 = 0x54535251"])
+self.runCmd("register write w31 0xcfcecdcc")
+self.match("register read x31",
+   ["sp = 0x58575655cfcecdcc"])
+
+# test FPU registers (overlapping with vector registers)
+self.runCmd("register write d0 16")
+self.match("register read v0",
+   ["v0 = {0x00 0x00 0x00 0x00 0x00 0x00 0x30 0x40 0x89 0x8a 0x8b 0x8c 0x8d 0x8e 0x8f 0x90}"])
+self.runCmd("register write v31 '{0x00 0x00 0x00 0x00 0x00 0x00 0x50 0x40 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff}'")
+self.match("register read d31",
+   ["d31 = 64"])
+
+self.runCmd("register write s0 32")
+self.match("register read v0",
+   ["v0 = {0x00 0x00 0x00 0x42 0x00 0x00 0x30 0x40 0x89 0x8a 0x8b 0x8c 0x8d 0x8e 0x8f 0x90}"])
+self.runCmd("register write v31 '{0x00 0x00 0x00 0x43 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff}'")
+self.match("register read s31",
+   ["s31 = 128"])
Index: lldb/source/Target/DynamicRegisterInfo.cpp
===
--- lldb/source/Target/DynamicRegisterInfo.cpp
+++ lldb/source/Target/DynamicRegisterInfo.cpp
@@ -832,16 +832,18 @@
 
 const lldb_private::RegisterInfo *
 DynamicRegisterInfo::GetRegisterInfo(llvm::StringRef reg_name) const {
+  assert(!reg_name.empty());
   for (auto ®_info : m_regs)
-if (reg_info.name == reg_name)
+if (reg_info.name == reg_name || reg_info.alt_name == reg_name)
   return ®_info;
   return nullptr;
 }
 
 lldb_private::RegisterInfo *
 DynamicRegisterInfo::GetRegisterInfo(llvm::StringRef reg_name) {
+  assert(!reg_name.empty());
   for (auto ®_info : m_regs)
-if (reg_info.name == reg_name)
+if (reg_info.name == reg_name || reg_info.alt_name == reg_name)
   return ®_info;
   return nullptr;
 }
Index: lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp
===
--- lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp
+++ lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp
@@ -71,6 +71,39 @@
   .Default(LLDB_INVALID_REGNUM);
 }
 
+static void AddPartialRegister(lldb_private::DynamicRegisterInfo &info,
+   const lldb_private::RegisterInfo *full_reg,
+   uint32_t full_reg_size,
+   const std::string &partial_reg_name,
+   uint32_t partial_reg_size,
+   lldb::Encoding encoding, lldb::Format format,
+   uint32_t &next_regnum_lldb) {
+  if (!full_reg || full_reg->byte_size != full_reg_size)
+return;
+  if (info.GetRegisterInfo(partial_reg_name))
+return;
+
+  lldb_private::ConstString group{"partial registers"};
+
+  lldb_private::ConstString partial_reg_name_const{partial_reg_name};
+  uint32_t value_regs[] = {
+  full_reg->kinds[lldb::eRegisterKindProcessPlugin],
+

[Lldb-commits] [PATCH] D111136: [lldb] [DynamicRegisterInfo] Support iterating over Registers()

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

mgorny added inline comments.



Comment at: lldb/include/lldb/Target/DynamicRegisterInfo.h:81
+  typedef std::vector reg_collection;
+  llvm::iterator_range Registers() {
+return llvm::iterator_range(m_regs);

labath wrote:
> llvm (and, surprisingly, lldb -- I guess because its a new feature) follows 
> c++ naming conventions for methods like these.
/me not understand.



Comment at: lldb/include/lldb/Target/DynamicRegisterInfo.h:82
+  llvm::iterator_range Registers() {
+return llvm::iterator_range(m_regs);
+  }

labath wrote:
> Could this return const iterators? It seems we already have some non-const 
> accessors, but I'd rather not propagate that..
It can't — we're using these iterators to augment register infos ;-).


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

https://reviews.llvm.org/D36

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

[Lldb-commits] [PATCH] D110962: [lldb] Add unit tests for Terminal API

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

mgorny added a comment.

In D110962#3042270 , @davezarzycki 
wrote:

> errno == 22 (EINVAL)
>
> The FD in question is 3. Here is the output from `lsof`: HostTests 80108 dave 
>3u   CHR5,2  0t0   231 /dev/ptmx

I don't think FD is the problem — requests for the same `fd` don't fail above. 
I think it's some `terminfo` flags.

> Can we/I please revert this for now?

Feel free to revert if you're going to help me fix it afterwards. It doesn't 
fail here nor on the buildbot.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110962

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

[Lldb-commits] [PATCH] D110962: [lldb] Add unit tests for Terminal API

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki added a comment.

"the buildbot"? There are many. I'd be surprised if this weren't failing on at 
least one of them. It doesn't seem like subtle terminfo behavior is essential 
to this test. Can we please trim down the adjustments to only changing the 
speed? For example:

  diff --git i/lldb/unittests/Host/posix/TerminalTest.cpp 
w/lldb/unittests/Host/posix/TerminalTest.cpp
  index 
ecdb5480216439903b9fc12c39b3d47cb62f9134..e865d44bf6cb9085f07c11e06be34f33a7bd32b9
 100644
  --- i/lldb/unittests/Host/posix/TerminalTest.cpp
  +++ w/lldb/unittests/Host/posix/TerminalTest.cpp
  @@ -80,14 +80,8 @@ TEST_F(TerminalTest, SaveRestoreRAII) {
   terminfo = orig_terminfo;
  
   // make some arbitrary changes
  -terminfo.c_iflag ^= IGNPAR | INLCR;
  -terminfo.c_oflag ^= OPOST | OCRNL;
  -terminfo.c_cflag ^= PARENB | PARODD;
  -terminfo.c_lflag ^= ICANON | ECHO;
  -terminfo.c_cc[VEOF] ^= 8;
  -terminfo.c_cc[VEOL] ^= 4;
  -cfsetispeed(&terminfo, B9600);
  -cfsetospeed(&terminfo, B9600);
  +cfsetispeed(&terminfo, cfgetispeed(&orig_terminfo) == B9600 ? B4800 : 
B9600);
  +cfsetospeed(&terminfo, cfgetospeed(&orig_terminfo) == B9600 ? B4800 : 
B9600);
  
   ASSERT_EQ(tcsetattr(m_pty.GetPrimaryFileDescriptor(), TCSANOW, 
&terminfo),
 0);
  @@ -109,14 +103,8 @@ TEST_F(TerminalTest, SaveRestore) {
 terminfo = orig_terminfo;
  
 // make some arbitrary changes
  -  terminfo.c_iflag ^= IGNPAR | INLCR;
  -  terminfo.c_oflag ^= OPOST | OCRNL;
  -  terminfo.c_cflag ^= PARENB | PARODD;
  -  terminfo.c_lflag ^= ICANON | ECHO;
  -  terminfo.c_cc[VEOF] ^= 8;
  -  terminfo.c_cc[VEOL] ^= 4;
  -  cfsetispeed(&terminfo, B9600);
  -  cfsetospeed(&terminfo, B9600);
  +  cfsetispeed(&terminfo, cfgetispeed(&orig_terminfo) == B9600 ? B4800 : 
B9600);
  +  cfsetospeed(&terminfo, cfgetospeed(&orig_terminfo) == B9600 ? B4800 : 
B9600);
  
 ASSERT_EQ(tcsetattr(m_pty.GetPrimaryFileDescriptor(), TCSANOW, &terminfo), 
0);


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110962

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

[Lldb-commits] [PATCH] D110827: [LLDB] Provide target specific directories to libclang

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 added a comment.

In D110827#3034767 , @clayborg wrote:

> Should we be testing if these directories exist before trying to use them? 
> Might be nice to avoid compiler warnings if the compiler will emit warnings 
> or errors if these directories don't exist.

I think testing if these directories exists may be a little bit redundant 
because clang ignores non-existent include paths passed with -I

> LLDB also tests with compilers that were built, like when LLDB builds clang 
> and uses that clang and clang++ that it built to run the test suite. If we 
> had settings in LLDB that users could set, then the test suite would be able 
> to use the include files for the compiler that is being used instead of 
> always defaulting to the system headers.

Could you please clarify: "LLDB builds clang" - here you mean clang which was 
build with LLDB? And I would like to mention that starting from 
https://reviews.llvm.org/D89013 libcxx puts __config_site header to target 
specific folder


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110827

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

[Lldb-commits] [lldb] 79bf032 - [lldb testing] Avoid subtle terminfo behavioral differences

[David Zarzycki via lldb-commits]

Author: David Zarzycki
Date: 2021-10-05T10:28:02-04:00
New Revision: 79bf032fe10546fd1d6e14c5ac8905f25c2b

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

LOG: [lldb testing] Avoid subtle terminfo behavioral differences

The original "arbitrary" changes were causing EINVAL on a Fedora 34 box.

Added: 


Modified: 
lldb/unittests/Host/posix/TerminalTest.cpp

Removed: 




diff  --git a/lldb/unittests/Host/posix/TerminalTest.cpp 
b/lldb/unittests/Host/posix/TerminalTest.cpp
index ecdb548021643..37b6b3cf60bff 100644
--- a/lldb/unittests/Host/posix/TerminalTest.cpp
+++ b/lldb/unittests/Host/posix/TerminalTest.cpp
@@ -79,15 +79,9 @@ TEST_F(TerminalTest, SaveRestoreRAII) {
 TerminalState term_state{term};
 terminfo = orig_terminfo;
 
-// make some arbitrary changes
-terminfo.c_iflag ^= IGNPAR | INLCR;
-terminfo.c_oflag ^= OPOST | OCRNL;
-terminfo.c_cflag ^= PARENB | PARODD;
-terminfo.c_lflag ^= ICANON | ECHO;
-terminfo.c_cc[VEOF] ^= 8;
-terminfo.c_cc[VEOL] ^= 4;
-cfsetispeed(&terminfo, B9600);
-cfsetospeed(&terminfo, B9600);
+// make an arbitrary change
+cfsetispeed(&terminfo,cfgetispeed(&orig_terminfo) == B9600 ? B4800 : 
B9600);
+cfsetospeed(&terminfo,cfgetospeed(&orig_terminfo) == B9600 ? B4800 : 
B9600);
 
 ASSERT_EQ(tcsetattr(m_pty.GetPrimaryFileDescriptor(), TCSANOW, &terminfo),
   0);
@@ -108,15 +102,9 @@ TEST_F(TerminalTest, SaveRestore) {
   term_state.Save(term, false);
   terminfo = orig_terminfo;
 
-  // make some arbitrary changes
-  terminfo.c_iflag ^= IGNPAR | INLCR;
-  terminfo.c_oflag ^= OPOST | OCRNL;
-  terminfo.c_cflag ^= PARENB | PARODD;
-  terminfo.c_lflag ^= ICANON | ECHO;
-  terminfo.c_cc[VEOF] ^= 8;
-  terminfo.c_cc[VEOL] ^= 4;
-  cfsetispeed(&terminfo, B9600);
-  cfsetospeed(&terminfo, B9600);
+  // make an arbitrary change
+  cfsetispeed(&terminfo, cfgetispeed(&orig_terminfo) == B9600 ? B4800 : B9600);
+  cfsetospeed(&terminfo, cfgetospeed(&orig_terminfo) == B9600 ? B4800 : B9600);
 
   ASSERT_EQ(tcsetattr(m_pty.GetPrimaryFileDescriptor(), TCSANOW, &terminfo), 
0);
 



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

[Lldb-commits] [PATCH] D110962: [lldb] Add unit tests for Terminal API

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki added a comment.

79bf032fe10546fd1d6e14c5ac8905f25c2b 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110962

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

[Lldb-commits] [PATCH] D111131: [LLDB] Round XML register bitsize to byte boundary

[Ted Woodward via Phabricator via lldb-commits]

ted added inline comments.



Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4357
 
 assert(reg_info.byte_size != 0);
 registers.push_back(reg_info);

labath wrote:
> omjavaid wrote:
> > labath wrote:
> > > mgorny wrote:
> > > > omjavaid wrote:
> > > > > mgorny wrote:
> > > > > > omjavaid wrote:
> > > > > > > @mgorny the assert already exists but then we also want to allow 
> > > > > > > bit sized registers although they ll be viewed as byte length for 
> > > > > > > now.
> > > > > > Ah, right. I suppose you could skip zero-byte registers though. 
> > > > > > That should amend the assert with better release behavior.
> > > > > on a second thought, I dont see a zero sized register being sent by 
> > > > > stub as a big enough reason to abort the whole debug session unless 
> > > > > its one of GPRs. May be we skip the assert altogether and replace it 
> > > > > with an error message. 
> > > > > What do you think?
> > > > Yes, you are correct. Probably `LLDB_LOG` would go in line with how we 
> > > > handle these things.
> > > Yeah, I don't think crashing is a good response to the stub sending us 
> > > nonsensical register definitions. Though that seems like a separate 
> > > issue..
> > LLDB_LOG will hide message from user unless log is enabled. I think user 
> > must be notified that register is  zero sized and thats why you wont be 
> > able to see it in register read. Similar to the way we notify user about 
> > unhandled register attibutes like "save-restore".
> BTW, I came very close to deleting that printf when I was touching this code 
> last month.
> FWIW, my hierarchy is:
> user-facing warning (though I don't know how would that be implemented here) 
> >> log entry >> nothing >> raw printf
+1 on not crashing. The remote sending us garbage shouldn't cause us to crash - 
we should try to make sense of the garbage, and if we can't, error out 
gracefully. Better to print an error and ignore the register than crash or 
abort the session.


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

https://reviews.llvm.org/D31

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

[Lldb-commits] [PATCH] D111136: [lldb] [DynamicRegisterInfo] Support iterating over Registers()

[Ted Woodward via Phabricator via lldb-commits]

ted added inline comments.



Comment at: lldb/include/lldb/Target/DynamicRegisterInfo.h:82
+  llvm::iterator_range Registers() {
+return llvm::iterator_range(m_regs);
+  }

mgorny wrote:
> labath wrote:
> > Could this return const iterators? It seems we already have some non-const 
> > accessors, but I'd rather not propagate that..
> It can't — we're using these iterators to augment register infos ;-).
Maybe make a non-const protected, and a const public, so random plugin #37 
can't modify register info?


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

https://reviews.llvm.org/D36

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

[Lldb-commits] [PATCH] D110962: [lldb] Add unit tests for Terminal API

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

mgorny added a comment.

I suppose that's good enough.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110962

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

[Lldb-commits] [PATCH] D111136: [lldb] [DynamicRegisterInfo] Support iterating over Registers()

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

mgorny added inline comments.



Comment at: lldb/include/lldb/Target/DynamicRegisterInfo.h:82
+  llvm::iterator_range Registers() {
+return llvm::iterator_range(m_regs);
+  }

ted wrote:
> mgorny wrote:
> > labath wrote:
> > > Could this return const iterators? It seems we already have some 
> > > non-const accessors, but I'd rather not propagate that..
> > It can't — we're using these iterators to augment register infos ;-).
> Maybe make a non-const protected, and a const public, so random plugin #37 
> can't modify register info?
That would work for me; however, we'd have to make `ABI` and 
`DynamicRegisterInfo` `friend`s then. @labath, what do you thik?


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

https://reviews.llvm.org/D36

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

[Lldb-commits] [lldb] 0f3254b - [lldb] Improve help for platform put-file

[Keith Smiley via lldb-commits]

Author: Keith Smiley
Date: 2021-10-05T10:29:37-07:00
New Revision: 0f3254b29f375d449e815e91d63bef78d9e81354

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

LOG: [lldb] Improve help for platform put-file

Previously it was not clear what arguments this required, or what it would do 
if you didn't pass the destination argument.

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

Added: 


Modified: 
lldb/source/Commands/CommandObjectPlatform.cpp

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectPlatform.cpp 
b/lldb/source/Commands/CommandObjectPlatform.cpp
index cd4d880e7..6bfb4c8a0689c 100644
--- a/lldb/source/Commands/CommandObjectPlatform.cpp
+++ b/lldb/source/Commands/CommandObjectPlatform.cpp
@@ -1067,7 +1067,18 @@ class CommandObjectPlatformPutFile : public 
CommandObjectParsed {
   CommandObjectPlatformPutFile(CommandInterpreter &interpreter)
   : CommandObjectParsed(
 interpreter, "platform put-file",
-"Transfer a file from this system to the remote end.", nullptr, 0) 
{
+"Transfer a file from this system to the remote end.",
+"platform put-file  []", 0) {
+SetHelpLong(
+R"(Examples:
+
+(lldb) platform put-file /source/foo.txt /destination/bar.txt
+
+(lldb) platform put-file /source/foo.txt
+
+Relative source file paths are resolved against lldb's local working 
directory.
+
+Omitting the destination places the file in the platform working 
directory.)");
   }
 
   ~CommandObjectPlatformPutFile() override = default;



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

[Lldb-commits] [PATCH] D110981: [lldb] Improve help for platform put-file

[Keith Smiley via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0f3254b29f37: [lldb] Improve help for platform put-file 
(authored by keith).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110981

Files:
  lldb/source/Commands/CommandObjectPlatform.cpp


Index: lldb/source/Commands/CommandObjectPlatform.cpp
===
--- lldb/source/Commands/CommandObjectPlatform.cpp
+++ lldb/source/Commands/CommandObjectPlatform.cpp
@@ -1067,7 +1067,18 @@
   CommandObjectPlatformPutFile(CommandInterpreter &interpreter)
   : CommandObjectParsed(
 interpreter, "platform put-file",
-"Transfer a file from this system to the remote end.", nullptr, 0) 
{
+"Transfer a file from this system to the remote end.",
+"platform put-file  []", 0) {
+SetHelpLong(
+R"(Examples:
+
+(lldb) platform put-file /source/foo.txt /destination/bar.txt
+
+(lldb) platform put-file /source/foo.txt
+
+Relative source file paths are resolved against lldb's local working 
directory.
+
+Omitting the destination places the file in the platform working 
directory.)");
   }
 
   ~CommandObjectPlatformPutFile() override = default;


Index: lldb/source/Commands/CommandObjectPlatform.cpp
===
--- lldb/source/Commands/CommandObjectPlatform.cpp
+++ lldb/source/Commands/CommandObjectPlatform.cpp
@@ -1067,7 +1067,18 @@
   CommandObjectPlatformPutFile(CommandInterpreter &interpreter)
   : CommandObjectParsed(
 interpreter, "platform put-file",
-"Transfer a file from this system to the remote end.", nullptr, 0) {
+"Transfer a file from this system to the remote end.",
+"platform put-file  []", 0) {
+SetHelpLong(
+R"(Examples:
+
+(lldb) platform put-file /source/foo.txt /destination/bar.txt
+
+(lldb) platform put-file /source/foo.txt
+
+Relative source file paths are resolved against lldb's local working directory.
+
+Omitting the destination places the file in the platform working directory.)");
   }
 
   ~CommandObjectPlatformPutFile() override = default;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D110962: [lldb] Add unit tests for Terminal API

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki added a comment.

If you want to test more, please let me know and I can test them on my Fedora 
box.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110962

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

[Lldb-commits] [lldb] 5bc32ad - [lldb testing] NFC: run through clang-format

[David Zarzycki via lldb-commits]

Author: David Zarzycki
Date: 2021-10-05T13:40:27-04:00
New Revision: 5bc32ad08d9a25b1a4fc4fe7daa4056d1d1ef67f

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

LOG: [lldb testing] NFC: run through clang-format

Added: 


Modified: 
lldb/unittests/Host/posix/TerminalTest.cpp

Removed: 




diff  --git a/lldb/unittests/Host/posix/TerminalTest.cpp 
b/lldb/unittests/Host/posix/TerminalTest.cpp
index 37b6b3cf60bf..788c2a6ea579 100644
--- a/lldb/unittests/Host/posix/TerminalTest.cpp
+++ b/lldb/unittests/Host/posix/TerminalTest.cpp
@@ -80,8 +80,10 @@ TEST_F(TerminalTest, SaveRestoreRAII) {
 terminfo = orig_terminfo;
 
 // make an arbitrary change
-cfsetispeed(&terminfo,cfgetispeed(&orig_terminfo) == B9600 ? B4800 : 
B9600);
-cfsetospeed(&terminfo,cfgetospeed(&orig_terminfo) == B9600 ? B4800 : 
B9600);
+cfsetispeed(&terminfo,
+cfgetispeed(&orig_terminfo) == B9600 ? B4800 : B9600);
+cfsetospeed(&terminfo,
+cfgetospeed(&orig_terminfo) == B9600 ? B4800 : B9600);
 
 ASSERT_EQ(tcsetattr(m_pty.GetPrimaryFileDescriptor(), TCSANOW, &terminfo),
   0);



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

[Lldb-commits] [PATCH] D107585: [lldb/Plugins] Add support for ScriptedThread in ScriptedProcess

[Med Ismail Bennani via Phabricator via lldb-commits]

mib marked 2 inline comments as done.
mib added inline comments.



Comment at: lldb/source/Plugins/Process/scripted/ScriptedThread.cpp:186
+void ScriptedThread::RefreshStateAfterStop() {
+  // TODO: Implement
+  if (m_reg_context_sp)

JDevlieghere wrote:
> Still relevant?
Yes. We might need to implement this when introducing execution control to 
Scripted Threads.



Comment at: lldb/source/Plugins/Process/scripted/ScriptedThread.cpp:205-206
+*reg_info, m_scripted_process.GetTarget().GetArchitecture());
+assert(m_register_info_sp->GetNumRegisters() > 0);
+assert(m_register_info_sp->GetNumRegisterSets() > 0);
+  }

JDevlieghere wrote:
> Does this assertion depend on "user-input"? In other words, can this be 
> triggered by not returning any registers from the interface? 
Yes, the user has to provide some basic register information to be able to 
parse it and use it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107585

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

[Lldb-commits] [PATCH] D107585: [lldb/Plugins] Add support for ScriptedThread in ScriptedProcess

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 377324.
mib marked 2 inline comments as done.
mib added a comment.

Address @JDevlieghere last comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107585

Files:
  lldb/bindings/python/python-wrapper.swig
  lldb/examples/python/scripted_process/my_scripted_process.py
  lldb/examples/python/scripted_process/scripted_process.py
  lldb/include/lldb/Interpreter/ScriptedInterface.h
  lldb/include/lldb/Interpreter/ScriptedProcessInterface.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/Plugins/Process/scripted/CMakeLists.txt
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.h
  lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
  lldb/source/Plugins/Process/scripted/ScriptedThread.h
  lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
  lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
  
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.h
  lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
  lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
@@ -223,6 +223,12 @@
   return nullptr;
 }
 
+extern "C" void *LLDBSwigPythonCreateScriptedThread(
+const char *python_class_name, const char *session_dictionary_name,
+const lldb::TargetSP &target_sp, std::string &error_string) {
+  return nullptr;
+}
+
 extern "C" void *
 LLDBSWIGPython_CreateFrameRecognizer(const char *python_class_name,
  const char *session_dictionary_name) {
Index: lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
===
--- lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
+++ lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
@@ -72,6 +72,28 @@
 self.assertTrue(error.Success(), "Failed to read memory from scripted process.")
 self.assertEqual(hello_world, memory_read)
 
+self.assertEqual(process.GetNumThreads(), 1)
+
+thread = process.GetSelectedThread()
+self.assertTrue(thread, "Invalid thread.")
+self.assertEqual(thread.GetThreadID(), 0x19)
+self.assertEqual(thread.GetName(), "MyScriptedThread.thread-1")
+self.assertEqual(thread.GetStopReason(), lldb.eStopReasonSignal)
+
+self.assertGreater(thread.GetNumFrames(), 0)
+
+frame = thread.GetFrameAtIndex(0)
+register_set = frame.registers # Returns an SBValueList.
+for regs in register_set:
+if 'GPR' in regs.name:
+registers  = regs
+break
+
+self.assertTrue(registers, "Invalid General Purpose Registers Set")
+self.assertEqual(registers.GetNumChildren(), 21)
+for idx, reg in enumerate(registers, start=1):
+self.assertEqual(idx, int(reg.value, 16))
+
 def test_launch_scripted_process_cli(self):
 """Test that we can launch an lldb scripted process from the command
 line, check its process ID and read string from memory."""
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.h
===
--- /dev/null
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.h
@@ -0,0 +1,48 @@
+//===-- ScriptedThreadPythonInterface.h *- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_SCRIPTEDTHREADPYTHONINTERFACE_H
+#define LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_SCRIPTEDTHREADPYTHONINTERFACE_H
+
+#include "lldb/Host/Config.h"
+
+#if LLDB_ENABLE_PYTHON
+
+#include "ScriptedPythonInterface.h"
+#include "lldb/Interpreter/ScriptedProcessInterface.h"
+
+namespace lldb_private {
+class ScriptedThreadPythonInterface : public ScriptedThreadInterface,
+  public ScriptedPythonInterface {
+public:
+  S

[Lldb-commits] [PATCH] D108953: [lldb/Plugins] Add memory region support in ScriptedProcess

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 377326.
mib edited the summary of this revision.
mib added a comment.

Reverted from `shared_ptr` to `unique_ptr` for SBMemoryRegionInfo opaque_ptr.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108953

Files:
  lldb/bindings/interface/SBMemoryRegionInfo.i
  lldb/bindings/interface/SBMemoryRegionInfoList.i
  lldb/bindings/python/python-wrapper.swig
  lldb/examples/python/scripted_process/main.stack-dump
  lldb/examples/python/scripted_process/my_scripted_process.py
  lldb/include/lldb/API/SBMemoryRegionInfo.h
  lldb/include/lldb/API/SBMemoryRegionInfoList.h
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  lldb/include/lldb/Interpreter/ScriptedProcessInterface.h
  lldb/source/API/SBMemoryRegionInfo.cpp
  lldb/source/API/SBMemoryRegionInfoList.cpp
  lldb/source/Interpreter/ScriptInterpreter.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
  
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
  lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
  lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
  lldb/test/API/functionalities/scripted_process/main.c
  lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
@@ -166,6 +166,10 @@
   return nullptr;
 }
 
+extern "C" void *LLDBSWIGPython_CastPyObjectToSBMemoryRegionInfo(void *data) {
+  return nullptr;
+}
+
 extern lldb::ValueObjectSP
 LLDBSWIGPython_GetValueObjectSPFromSBValue(void *data) {
   return nullptr;
Index: lldb/test/API/functionalities/scripted_process/main.c
===
--- lldb/test/API/functionalities/scripted_process/main.c
+++ lldb/test/API/functionalities/scripted_process/main.c
@@ -1,5 +1,8 @@
-#include 
-
-int main() {
-  return 0; // break here
+int bar(int i) {
+  int j = i * i;
+  return j; // break here
 }
+
+int foo(int i) { return bar(i); }
+
+int main() { return foo(42); }
Index: lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
===
--- /dev/null
+++ lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
@@ -0,0 +1,90 @@
+import os,struct, signal
+
+from typing import Any, Dict
+
+import lldb
+from lldb.plugins.scripted_process import ScriptedProcess
+from lldb.plugins.scripted_process import ScriptedThread
+
+class DummyScriptedProcess(ScriptedProcess):
+def __init__(self, target: lldb.SBTarget, args : lldb.SBStructuredData):
+super().__init__(target, args)
+
+def get_memory_region_containing_address(self, addr: int) -> lldb.SBMemoryRegionInfo:
+return self.memory_regions[0]
+
+def get_thread_with_id(self, tid: int):
+return {}
+
+def get_registers_for_thread(self, tid: int):
+return {}
+
+def read_memory_at_address(self, addr: int, size: int) -> lldb.SBData:
+data = lldb.SBData().CreateDataFromCString(
+self.target.GetByteOrder(),
+self.target.GetCodeByteSize(),
+"Hello, world!")
+return data
+
+def get_loaded_images(self):
+return self.loaded_images
+
+def get_process_id(self) -> int:
+return 42
+
+def should_stop(self) -> bool:
+return True
+
+def is_alive(self) -> bool:
+return True
+
+def get_scripted_thread_plugin(self):
+return DummyScriptedThread.__module__ + "." + DummyScriptedThread.__name__
+
+
+class DummyScriptedThread(ScriptedThread):
+def __init__(self, target):
+super().__init__(target)
+
+def get_thread_id(self) -> int:
+return 0x19
+
+def get_name(self) -> str:
+return DummyScriptedThread.__name__ + ".thread-1"
+
+def get_state(self) -> int:
+return lldb.eStateStopped
+
+def get_stop_reason(self) -> Dict[str, Any]:
+return { "type": lldb.eStopReasonSignal, "data": {
+"signal": signal.SIGINT
+} }
+
+def get_stackframes(self):
+class ScriptedStackFrame:
+def __init__(idx, cfa, pc, symbol_ctx):
+self.idx = idx
+self.cfa = cfa
+self.pc = pc
+self.symbol_ctx = symbol_ctx
+
+
+symbol_ctx = lldb.SBSymbolC

[Lldb-commits] [PATCH] D108953: [lldb/Plugins] Add memory region support in ScriptedProcess

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.

ping @JDevlieghere


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108953

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

[Lldb-commits] [lldb] 730fca4 - [lldb] Improve meta data stripping from JSON crashlogs

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2021-10-05T12:15:54-07:00
New Revision: 730fca46fc87dad09040cb0b27ede10ae2c7c9d7

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

LOG: [lldb] Improve meta data stripping from JSON crashlogs

JSON crashlogs normally start with a single line of meta data that we
strip unconditionally. Some producers started omitting the meta data
which tripped up crashlog. Be more resilient by only removing the first
line when we know it really is meta data.

rdar://82641662

Added: 


Modified: 
lldb/examples/python/crashlog.py
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/patch-crashlog.py

Removed: 




diff  --git a/lldb/examples/python/crashlog.py 
b/lldb/examples/python/crashlog.py
index e6d88a033a232..aec4096585222 100755
--- a/lldb/examples/python/crashlog.py
+++ b/lldb/examples/python/crashlog.py
@@ -415,8 +415,14 @@ def parse(self):
 with open(self.path, 'r') as f:
 buffer = f.read()
 
-# First line is meta-data.
-buffer = buffer[buffer.index('\n') + 1:]
+# Skip the first line if it contains meta data.
+head, _, tail = buffer.partition('\n')
+try:
+metadata = json.loads(head)
+if 'app_name' in metadata and 'app_version' in metadata:
+buffer = tail
+except ValueError:
+pass
 
 try:
 self.data = json.loads(buffer)

diff  --git a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test 
b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test
index a514b07fe9f85..0c522e9d202b7 100644
--- a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test
+++ b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test
@@ -1,8 +1,13 @@
 # RUN: %clang_host -g %S/Inputs/test.c -o %t.out
+
 # RUN: cp %S/Inputs/a.out.ips %t.crash
 # RUN: python %S/patch-crashlog.py --binary %t.out --crashlog %t.crash 
--offsets '{"main":20, "bar":9, "foo":16}' --json
 # RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 
'crashlog %t.crash' 2>&1 | FileCheck %s
 
+# RUN: cp %S/Inputs/a.out.ips %t.nometadata.crash
+# RUN: python %S/patch-crashlog.py --binary %t.out --crashlog 
%t.nometadata.crash --offsets '{"main":20, "bar":9, "foo":16}' --json 
--no-metadata
+# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 
'crashlog %t.nometadata.crash' 2>&1 | FileCheck %s
+
 # CHECK: Thread[0] Crashing Thread Name EXC_BAD_ACCESS (SIGSEGV) 
(KERN_INVALID_ADDRESS at 0x)
 # CHECK: [  0] {{.*}}out`foo + 16 at test.c
 # CHECK: [  1] {{.*}}out`bar + 8 at test.c

diff  --git 
a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/patch-crashlog.py 
b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/patch-crashlog.py
index 28396c530355e..56558bc153ed7 100644
--- a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/patch-crashlog.py
+++ b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/patch-crashlog.py
@@ -49,6 +49,9 @@ def patch_addresses(self):
 self.data = self.data.replace(
 "@{}@".format(symbol), str(representation(patch_addr)))
 
+def remove_metadata(self):
+self.data= self.data[self.data.index('\n') + 1:]
+
 
 if __name__ == '__main__':
 parser = argparse.ArgumentParser(description='Crashlog Patcher')
@@ -56,6 +59,7 @@ def patch_addresses(self):
 parser.add_argument('--crashlog', required=True)
 parser.add_argument('--offsets', required=True)
 parser.add_argument('--json', default=False, action='store_true')
+parser.add_argument('--no-metadata', default=False, action='store_true')
 args = parser.parse_args()
 
 offsets = json.loads(args.offsets)
@@ -68,5 +72,8 @@ def patch_addresses(self):
 p.patch_uuid()
 p.patch_addresses()
 
+if args.no_metadata:
+p.remove_metadata()
+
 with open(args.crashlog, 'w') as file:
 file.write(p.data)



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

[Lldb-commits] [PATCH] D109326: [lldb] [Process/FreeBSD] Support SaveCore() using PT_COREDUMP

[Brooks Davis via Phabricator via lldb-commits]

brooks added a comment.

This needs to be guarded by the presence of PT_COREDUMP since it's a new 
feature. As it stands this breaks the lldb build on all supported FreeBSD 
releases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109326

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

[Lldb-commits] [PATCH] D110885: [NFC][AttributeList] Replace index_begin/end with an iterator

[Reid Kleckner via Phabricator via lldb-commits]

rnk added a comment.

I had another idle thought on this matter: The unsigned overflow here really 
isn't that weird. It's the same thing as iterating over the range [-1, 0, 
`#args+1`]. We could update all these APIs to traffic in plain signed ints, and 
then there would be no wrapping, just normal index math.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110885

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

[Lldb-commits] [PATCH] D109326: [lldb] [Process/FreeBSD] Support SaveCore() using PT_COREDUMP

[Brooks Davis via Phabricator via lldb-commits]

brooks added a comment.

This patch is sufficient to let things build, but doesn't address the tests. If 
there's a plan to MFC PT_COREDUMP that's probably an ok state of affairs. 
https://cgit.freebsd.org/ports/tree/devel/llvm-devel/files/patch-lldb-PT_COREDUMP.diff?id=7e2f156ce907e7785863d49cbd33d36f07df7eae


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109326

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

[Lldb-commits] [PATCH] D111200: Use llvm::VersionTuple to store DWARF producer info (NFC)

[Adrian Prantl via Phabricator via lldb-commits]

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

This has the nice side-effect that it can actually store the quadruple version 
numbers that Apple's tools are using nowadays.

rdar://82982162


https://reviews.llvm.org/D111200

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -195,11 +195,7 @@
 
   DWARFProducer GetProducer();
 
-  uint32_t GetProducerVersionMajor();
-
-  uint32_t GetProducerVersionMinor();
-
-  uint32_t GetProducerVersionUpdate();
+  llvm::VersionTuple GetProducerVersion();
 
   uint64_t GetDWARFLanguageType();
 
@@ -311,9 +307,7 @@
   std::unique_ptr m_func_aranges_up;
   dw_addr_t m_base_addr = 0;
   DWARFProducer m_producer = eProducerInvalid;
-  uint32_t m_producer_version_major = 0;
-  uint32_t m_producer_version_minor = 0;
-  uint32_t m_producer_version_update = 0;
+  llvm::VersionTuple m_producer_version;
   llvm::Optional m_language_type;
   lldb_private::LazyBool m_is_optimized = lldb_private::eLazyBoolCalculate;
   llvm::Optional m_comp_dir;
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -654,20 +654,13 @@
 }
 
 bool DWARFUnit::Supports_unnamed_objc_bitfields() {
-  if (GetProducer() == eProducerClang) {
-const uint32_t major_version = GetProducerVersionMajor();
-return major_version > 425 ||
-   (major_version == 425 && GetProducerVersionUpdate() >= 13);
-  }
-  return true; // Assume all other compilers didn't have incorrect ObjC 
bitfield
-   // info
+  if (GetProducer() == eProducerClang)
+return GetProducerVersion() > llvm::VersionTuple(425, 0, 13);
+  // Assume all other compilers didn't have incorrect ObjC bitfield info.
+  return true;
 }
 
 void DWARFUnit::ParseProducerInfo() {
-  m_producer_version_major = UINT32_MAX;
-  m_producer_version_minor = UINT32_MAX;
-  m_producer_version_update = UINT32_MAX;
-
   const DWARFDebugInfoEntry *die = GetUnitDIEPtrOnly();
   if (die) {
 
@@ -682,14 +675,11 @@
 m_producer = eProducerLLVMGCC;
   } else if (strstr(producer_cstr, "clang")) {
 static RegularExpression g_clang_version_regex(
-llvm::StringRef("clang-([0-9]+)\\.([0-9]+)\\.([0-9]+)"));
+llvm::StringRef(R"(clang-([0-9]+\.[0-9]+\.[0-9]+(.[0-9]+)?)"));
 llvm::SmallVector matches;
 if (g_clang_version_regex.Execute(llvm::StringRef(producer_cstr),
   &matches)) {
-  // FIXME: improve error handling
-  llvm::to_integer(matches[1], m_producer_version_major);
-  llvm::to_integer(matches[2], m_producer_version_minor);
-  llvm::to_integer(matches[3], m_producer_version_update);
+  m_producer_version.tryParse(matches[1]);
 }
 m_producer = eProducerClang;
   } else if (strstr(producer_cstr, "GNU"))
@@ -706,22 +696,10 @@
   return m_producer;
 }
 
-uint32_t DWARFUnit::GetProducerVersionMajor() {
-  if (m_producer_version_major == 0)
-ParseProducerInfo();
-  return m_producer_version_major;
-}
-
-uint32_t DWARFUnit::GetProducerVersionMinor() {
-  if (m_producer_version_minor == 0)
-ParseProducerInfo();
-  return m_producer_version_minor;
-}
-
-uint32_t DWARFUnit::GetProducerVersionUpdate() {
-  if (m_producer_version_update == 0)
+llvm::VersionTuple DWARFUnit::GetProducerVersion() {
+  if (m_producer_version.empty())
 ParseProducerInfo();
-  return m_producer_version_update;
+  return m_producer_version;
 }
 
 uint64_t DWARFUnit::GetDWARFLanguageType() {


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -195,11 +195,7 @@
 
   DWARFProducer GetProducer();
 
-  uint32_t GetProducerVersionMajor();
-
-  uint32_t GetProducerVersionMinor();
-
-  uint32_t GetProducerVersionUpdate();
+  llvm::VersionTuple GetProducerVersion();
 
   uint64_t GetDWARFLanguageType();
 
@@ -311,9 +307,7 @@
   std::unique_ptr m_func_aranges_up;
   dw_addr_t m_base_addr = 0;
   DWARFProducer m_producer = eProducerInvalid;
-  uint32_t m_producer_version_major = 0;
-  uint32_t m_producer_version_minor = 0;
-  uint32_t m_producer_version_update = 0;
+  llvm::VersionTuple m_producer_version;
   llvm::Optional m_language_type;
   lldb_private::LazyBool m_is_optimized = lldb_private::eLazyBoolCalculate;
   llvm::Optional m_comp_dir;
Index: lldb/source/Plugins/SymbolFile/DWARF/

[Lldb-commits] [PATCH] D111200: Use llvm::VersionTuple to store DWARF producer info (NFC)

[Adrian Prantl via Phabricator via lldb-commits]

aprantl updated this revision to Diff 377402.

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

https://reviews.llvm.org/D111200

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -195,11 +195,7 @@
 
   DWARFProducer GetProducer();
 
-  uint32_t GetProducerVersionMajor();
-
-  uint32_t GetProducerVersionMinor();
-
-  uint32_t GetProducerVersionUpdate();
+  llvm::VersionTuple GetProducerVersion();
 
   uint64_t GetDWARFLanguageType();
 
@@ -311,9 +307,7 @@
   std::unique_ptr m_func_aranges_up;
   dw_addr_t m_base_addr = 0;
   DWARFProducer m_producer = eProducerInvalid;
-  uint32_t m_producer_version_major = 0;
-  uint32_t m_producer_version_minor = 0;
-  uint32_t m_producer_version_update = 0;
+  llvm::VersionTuple m_producer_version;
   llvm::Optional m_language_type;
   lldb_private::LazyBool m_is_optimized = lldb_private::eLazyBoolCalculate;
   llvm::Optional m_comp_dir;
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -654,20 +654,13 @@
 }
 
 bool DWARFUnit::Supports_unnamed_objc_bitfields() {
-  if (GetProducer() == eProducerClang) {
-const uint32_t major_version = GetProducerVersionMajor();
-return major_version > 425 ||
-   (major_version == 425 && GetProducerVersionUpdate() >= 13);
-  }
-  return true; // Assume all other compilers didn't have incorrect ObjC 
bitfield
-   // info
+  if (GetProducer() == eProducerClang)
+return GetProducerVersion() > llvm::VersionTuple(425, 0, 13);
+  // Assume all other compilers didn't have incorrect ObjC bitfield info.
+  return true;
 }
 
 void DWARFUnit::ParseProducerInfo() {
-  m_producer_version_major = UINT32_MAX;
-  m_producer_version_minor = UINT32_MAX;
-  m_producer_version_update = UINT32_MAX;
-
   const DWARFDebugInfoEntry *die = GetUnitDIEPtrOnly();
   if (die) {
 
@@ -682,14 +675,11 @@
 m_producer = eProducerLLVMGCC;
   } else if (strstr(producer_cstr, "clang")) {
 static RegularExpression g_clang_version_regex(
-llvm::StringRef("clang-([0-9]+)\\.([0-9]+)\\.([0-9]+)"));
+llvm::StringRef(R"(clang-([0-9]+\.[0-9]+\.[0-9]+(\.[0-9]+)?)"));
 llvm::SmallVector matches;
 if (g_clang_version_regex.Execute(llvm::StringRef(producer_cstr),
   &matches)) {
-  // FIXME: improve error handling
-  llvm::to_integer(matches[1], m_producer_version_major);
-  llvm::to_integer(matches[2], m_producer_version_minor);
-  llvm::to_integer(matches[3], m_producer_version_update);
+  m_producer_version.tryParse(matches[1]);
 }
 m_producer = eProducerClang;
   } else if (strstr(producer_cstr, "GNU"))
@@ -706,22 +696,10 @@
   return m_producer;
 }
 
-uint32_t DWARFUnit::GetProducerVersionMajor() {
-  if (m_producer_version_major == 0)
-ParseProducerInfo();
-  return m_producer_version_major;
-}
-
-uint32_t DWARFUnit::GetProducerVersionMinor() {
-  if (m_producer_version_minor == 0)
-ParseProducerInfo();
-  return m_producer_version_minor;
-}
-
-uint32_t DWARFUnit::GetProducerVersionUpdate() {
-  if (m_producer_version_update == 0)
+llvm::VersionTuple DWARFUnit::GetProducerVersion() {
+  if (m_producer_version.empty())
 ParseProducerInfo();
-  return m_producer_version_update;
+  return m_producer_version;
 }
 
 uint64_t DWARFUnit::GetDWARFLanguageType() {


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -195,11 +195,7 @@
 
   DWARFProducer GetProducer();
 
-  uint32_t GetProducerVersionMajor();
-
-  uint32_t GetProducerVersionMinor();
-
-  uint32_t GetProducerVersionUpdate();
+  llvm::VersionTuple GetProducerVersion();
 
   uint64_t GetDWARFLanguageType();
 
@@ -311,9 +307,7 @@
   std::unique_ptr m_func_aranges_up;
   dw_addr_t m_base_addr = 0;
   DWARFProducer m_producer = eProducerInvalid;
-  uint32_t m_producer_version_major = 0;
-  uint32_t m_producer_version_minor = 0;
-  uint32_t m_producer_version_update = 0;
+  llvm::VersionTuple m_producer_version;
   llvm::Optional m_language_type;
   lldb_private::LazyBool m_is_optimized = lldb_private::eLazyBoolCalculate;
   llvm::Optional m_comp_dir;
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/Symbol

[Lldb-commits] [PATCH] D111200: Use llvm::VersionTuple to store DWARF producer info (NFC)

[Adrian Prantl via Phabricator via lldb-commits]

aprantl updated this revision to Diff 377403.
aprantl added a comment.

Fix typo


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

https://reviews.llvm.org/D111200

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -195,11 +195,7 @@
 
   DWARFProducer GetProducer();
 
-  uint32_t GetProducerVersionMajor();
-
-  uint32_t GetProducerVersionMinor();
-
-  uint32_t GetProducerVersionUpdate();
+  llvm::VersionTuple GetProducerVersion();
 
   uint64_t GetDWARFLanguageType();
 
@@ -311,9 +307,7 @@
   std::unique_ptr m_func_aranges_up;
   dw_addr_t m_base_addr = 0;
   DWARFProducer m_producer = eProducerInvalid;
-  uint32_t m_producer_version_major = 0;
-  uint32_t m_producer_version_minor = 0;
-  uint32_t m_producer_version_update = 0;
+  llvm::VersionTuple m_producer_version;
   llvm::Optional m_language_type;
   lldb_private::LazyBool m_is_optimized = lldb_private::eLazyBoolCalculate;
   llvm::Optional m_comp_dir;
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -654,20 +654,13 @@
 }
 
 bool DWARFUnit::Supports_unnamed_objc_bitfields() {
-  if (GetProducer() == eProducerClang) {
-const uint32_t major_version = GetProducerVersionMajor();
-return major_version > 425 ||
-   (major_version == 425 && GetProducerVersionUpdate() >= 13);
-  }
-  return true; // Assume all other compilers didn't have incorrect ObjC 
bitfield
-   // info
+  if (GetProducer() == eProducerClang)
+return GetProducerVersion() >= llvm::VersionTuple(425, 0, 13);
+  // Assume all other compilers didn't have incorrect ObjC bitfield info.
+  return true;
 }
 
 void DWARFUnit::ParseProducerInfo() {
-  m_producer_version_major = UINT32_MAX;
-  m_producer_version_minor = UINT32_MAX;
-  m_producer_version_update = UINT32_MAX;
-
   const DWARFDebugInfoEntry *die = GetUnitDIEPtrOnly();
   if (die) {
 
@@ -682,14 +675,11 @@
 m_producer = eProducerLLVMGCC;
   } else if (strstr(producer_cstr, "clang")) {
 static RegularExpression g_clang_version_regex(
-llvm::StringRef("clang-([0-9]+)\\.([0-9]+)\\.([0-9]+)"));
+llvm::StringRef(R"(clang-([0-9]+\.[0-9]+\.[0-9]+(\.[0-9]+)?)"));
 llvm::SmallVector matches;
 if (g_clang_version_regex.Execute(llvm::StringRef(producer_cstr),
   &matches)) {
-  // FIXME: improve error handling
-  llvm::to_integer(matches[1], m_producer_version_major);
-  llvm::to_integer(matches[2], m_producer_version_minor);
-  llvm::to_integer(matches[3], m_producer_version_update);
+  m_producer_version.tryParse(matches[1]);
 }
 m_producer = eProducerClang;
   } else if (strstr(producer_cstr, "GNU"))
@@ -706,22 +696,10 @@
   return m_producer;
 }
 
-uint32_t DWARFUnit::GetProducerVersionMajor() {
-  if (m_producer_version_major == 0)
-ParseProducerInfo();
-  return m_producer_version_major;
-}
-
-uint32_t DWARFUnit::GetProducerVersionMinor() {
-  if (m_producer_version_minor == 0)
-ParseProducerInfo();
-  return m_producer_version_minor;
-}
-
-uint32_t DWARFUnit::GetProducerVersionUpdate() {
-  if (m_producer_version_update == 0)
+llvm::VersionTuple DWARFUnit::GetProducerVersion() {
+  if (m_producer_version.empty())
 ParseProducerInfo();
-  return m_producer_version_update;
+  return m_producer_version;
 }
 
 uint64_t DWARFUnit::GetDWARFLanguageType() {


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -195,11 +195,7 @@
 
   DWARFProducer GetProducer();
 
-  uint32_t GetProducerVersionMajor();
-
-  uint32_t GetProducerVersionMinor();
-
-  uint32_t GetProducerVersionUpdate();
+  llvm::VersionTuple GetProducerVersion();
 
   uint64_t GetDWARFLanguageType();
 
@@ -311,9 +307,7 @@
   std::unique_ptr m_func_aranges_up;
   dw_addr_t m_base_addr = 0;
   DWARFProducer m_producer = eProducerInvalid;
-  uint32_t m_producer_version_major = 0;
-  uint32_t m_producer_version_minor = 0;
-  uint32_t m_producer_version_update = 0;
+  llvm::VersionTuple m_producer_version;
   llvm::Optional m_language_type;
   lldb_private::LazyBool m_is_optimized = lldb_private::eLazyBoolCalculate;
   llvm::Optional m_comp_dir;
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUn

[Lldb-commits] [PATCH] D111200: Use llvm::VersionTuple to store DWARF producer info (NFC)

[Jonas Devlieghere via Phabricator via lldb-commits]

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

LGTM


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

https://reviews.llvm.org/D111200

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

[Lldb-commits] [PATCH] D108953: [lldb/Plugins] Add memory region support in ScriptedProcess

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:278
+if (error.Fail())
+  return error;
+

If this is the only way out of this loop, does that mean we always return an 
error here? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108953

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

[Lldb-commits] [PATCH] D109326: [lldb] [Process/FreeBSD] Support SaveCore() using PT_COREDUMP

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

mgorny added a comment.

I'm sorry for the trouble. I'll take a look at it today.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109326

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