[Lldb-commits] [lldb] dbc34e2 - [LLDB] Discard register flags where the size doesn't match the register

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2023-04-20T08:36:49Z
New Revision: dbc34e2bed10c18c78100a9e8517d9a0b2f6639d

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

LOG: [LLDB] Discard register flags where the size doesn't match the register

In the particular case I was looking at I autogenerated a 128 bit set
of flags that is only 64 bit. This doesn't crash lldb but it was certainly
not expected.

I suspect that we would have crashed if the top 64 bits weren't
marked as unused (or at least invoked some very undefined behaviour).

When this happens, log the details and ignore the flags. Like this:
```
Size of register flags TTBR0_EL1_flags (16 bytes) for register TTBR0_EL1 does 
not match the register size (8 bytes). Ignoring this set of flags.
```

Turns out a few of the tests relied on this bug so I have updated
them and added a specific test for this case.

Reviewed By: jasonmolenda

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

Added: 


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

Removed: 




diff  --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp 
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index e468c82382d1b..087344198e226 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -4341,8 +4341,19 @@ bool ParseRegisters(
   // gdb_type could reference some flags type defined in XML.
   llvm::StringMap>::iterator it =
   registers_flags_types.find(gdb_type);
-  if (it != registers_flags_types.end())
-reg_info.flags_type = it->second.get();
+  if (it != registers_flags_types.end()) {
+auto flags_type = it->second.get();
+if (reg_info.byte_size == flags_type->GetSize())
+  reg_info.flags_type = flags_type;
+else
+  LLDB_LOGF(log,
+"ProcessGDBRemote::ParseRegisters Size of register "
+"flags %s (%d bytes) for "
+"register %s does not match the register size (%d "
+"bytes). Ignoring this set of flags.",
+flags_type->GetID().c_str(), flags_type->GetSize(),
+reg_info.name.AsCString(), reg_info.byte_size);
+  }
 
   // There's a slim chance that the gdb_type name is both a flags type
   // and a simple type. Just in case, look for that too (setting both

diff  --git 
a/lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py 
b/lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
index 2c930d6773c5d..3ccf3ccae6e9b 100644
--- a/lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
@@ -319,6 +319,20 @@ def test_flags_requried_attributes(self):
 
 self.expect("register read cpsr", substrs=["(C = 1)"])
 
+@skipIfXmlSupportMissing
+@skipIfRemote
+def test_flags_register_size_mismatch(self):
+# If the size of the flag set found does not match the size of the
+# register, we discard the flags.
+self.setup_register_test("""\
+  
+
+  
+  
+  """)
+
+self.expect("register read cpsr", substrs=["(C = 1)"], matching=False)
+
 @skipIfXmlSupportMissing
 @skipIfRemote
 def test_flags_set_even_if_format_set(self):
@@ -439,7 +453,7 @@ def test_xml_includes_multiple(self):
 'core.xml' : dedent("""\
 
 
-  
+  
 
   
   
@@ -475,23 +489,23 @@ def test_xml_includes_flags_redefined(self):
 'core.xml' : dedent("""\
 
 
-  
+  
 
   
   
   
 """),
-# The my_flags here is ignored, so cpsr will use the my_flags from 
above.
+# The my_flags here is ignored, so x1 will use the my_flags from 
above.
 'core-2.xml' : dedent("""\
 
 
-  
+  
 
   
-  
+  
 
 """),
 })
 
 self.expect("register read x0", substrs=["(correct = 1)"])
-self.expect("register read cpsr", substrs=["(correct = 1)"])
+self.expect("register read x1", substrs=["(correct = 1)"])

[Lldb-commits] [PATCH] D148715: [LLDB] Discard register flags where the size doesn't match the register

[David Spickett via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGdbc34e2bed10: [LLDB] Discard register flags where the size 
doesn't match the register (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148715

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


Index: lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
@@ -319,6 +319,20 @@
 
 self.expect("register read cpsr", substrs=["(C = 1)"])
 
+@skipIfXmlSupportMissing
+@skipIfRemote
+def test_flags_register_size_mismatch(self):
+# If the size of the flag set found does not match the size of the
+# register, we discard the flags.
+self.setup_register_test("""\
+  
+
+  
+  
+  """)
+
+self.expect("register read cpsr", substrs=["(C = 1)"], matching=False)
+
 @skipIfXmlSupportMissing
 @skipIfRemote
 def test_flags_set_even_if_format_set(self):
@@ -439,7 +453,7 @@
 'core.xml' : dedent("""\
 
 
-  
+  
 
   
   
@@ -475,23 +489,23 @@
 'core.xml' : dedent("""\
 
 
-  
+  
 
   
   
   
 """),
-# The my_flags here is ignored, so cpsr will use the my_flags from 
above.
+# The my_flags here is ignored, so x1 will use the my_flags from 
above.
 'core-2.xml' : dedent("""\
 
 
-  
+  
 
   
-  
+  
 
 """),
 })
 
 self.expect("register read x0", substrs=["(correct = 1)"])
-self.expect("register read cpsr", substrs=["(correct = 1)"])
+self.expect("register read x1", substrs=["(correct = 1)"])
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
@@ -4341,8 +4341,19 @@
   // gdb_type could reference some flags type defined in XML.
   llvm::StringMap>::iterator it =
   registers_flags_types.find(gdb_type);
-  if (it != registers_flags_types.end())
-reg_info.flags_type = it->second.get();
+  if (it != registers_flags_types.end()) {
+auto flags_type = it->second.get();
+if (reg_info.byte_size == flags_type->GetSize())
+  reg_info.flags_type = flags_type;
+else
+  LLDB_LOGF(log,
+"ProcessGDBRemote::ParseRegisters Size of register "
+"flags %s (%d bytes) for "
+"register %s does not match the register size (%d "
+"bytes). Ignoring this set of flags.",
+flags_type->GetID().c_str(), flags_type->GetSize(),
+reg_info.name.AsCString(), reg_info.byte_size);
+  }
 
   // There's a slim chance that the gdb_type name is both a flags type
   // and a simple type. Just in case, look for that too (setting both


Index: lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
@@ -319,6 +319,20 @@
 
 self.expect("register read cpsr", substrs=["(C = 1)"])
 
+@skipIfXmlSupportMissing
+@skipIfRemote
+def test_flags_register_size_mismatch(self):
+# If the size of the flag set found does not match the size of the
+# register, we discard the flags.
+self.setup_register_test("""\
+  
+
+  
+  
+  """)
+
+self.expect("register read cpsr", substrs=["(C = 1)"], matching=False)
+
 @skipIfXmlSupportMissing
 @skipIfRemote
 def test_flags_set_even_if_format_set(self):
@@ -439,7 +453,7 @@
 'core.xml' : dedent("""\
 
 
-  
+  
 
   
   
@@ -475,23 +489,23 @@
 'core.

[Lldb-commits] [PATCH] D148790: [LLDB] Don't print register fields when asked for a specific format

[David Spickett via Phabricator via lldb-commits]

DavidSpickett created this revision.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Previously if a register had fields we would always print them after the
value if the register was asked for by name.

  (lldb) register read MDCR_EL3
  MDCR_EL3 = 0x
   = {
   ETBAD = 0
  <...>
   RLTE = 0
 }

This can be quite annoying if there are a whole lot of fields but you
want to see the register in a specific format.

  (lldb) register read MDCR_EL3 -f i
  MDCR_EL3 = 0x   unknown udf#0x0
   = {
   ETBAD = 0
  <...lots of fields...>

Since it pushes the interesting bit far up the terminal. To solve this,
don't print fields if the user passes --format. If they're doing that
then I think it's reasonable to assume they know what they want and only
want to see that output.

This also gives users a way to silence fields, but not change the format.
By doing `register read foo -f x`. In case they are not useful or perhaps
they are trying to work around a crash.

I have customised the help text for --format for register read to explain this:

  -f  ( --format  )
   Specify a format to be used for display. If this is set, register fields 
will not be dispayed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148790

Files:
  lldb/include/lldb/Interpreter/OptionGroupFormat.h
  lldb/source/Commands/CommandObjectRegister.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py


Index: lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
@@ -410,6 +410,15 @@
 
 self.expect("register read cpsr", substrs=["= (field_2 = 1, field_1 = 
1, ...)"])
 
+@skipIfXmlSupportMissing
+@skipIfRemote
+def test_format_disables_flags(self):
+# If asked for a specific format, don't print flags after it.
+self.setup_flags_test('')
+
+self.expect("register read cpsr --format X", substrs=["cpsr = 
0x"])
+self.expect("register read cpsr --format X", substrs=["field_0"], 
matching=False)
+
 @skipIfXmlSupportMissing
 @skipIfRemote
 def test_xml_includes(self):
Index: lldb/source/Commands/CommandObjectRegister.cpp
===
--- lldb/source/Commands/CommandObjectRegister.cpp
+++ lldb/source/Commands/CommandObjectRegister.cpp
@@ -44,7 +44,10 @@
 nullptr,
 eCommandRequiresFrame | eCommandRequiresRegContext |
 eCommandProcessMustBeLaunched | eCommandProcessMustBePaused),
-m_format_options(eFormatDefault) {
+m_format_options(eFormatDefault, UINT64_MAX, UINT64_MAX,
+ {{CommandArgumentType::eArgTypeFormat,
+   "Specify a format to be used for display. If this "
+   "is set, register fields will not be dispayed."}}) {
 CommandArgumentEntry arg;
 CommandArgumentData register_arg;
 
@@ -215,8 +218,11 @@
 
   if (const RegisterInfo *reg_info =
   reg_ctx->GetRegisterInfoByName(arg_str)) {
+// If they have asked for a specific format don't obscure that by
+// printing flags afterwards.
+bool print_flags = !m_format_options.FormatWasSet();
 if (!DumpRegister(m_exe_ctx, strm, *reg_ctx, *reg_info,
-  /*print_flags=*/true))
+  print_flags))
   strm.Printf("%-12s = error: unavailable\n", reg_info->name);
   } else {
 result.AppendErrorWithFormat("Invalid register name '%s'.\n",
Index: lldb/include/lldb/Interpreter/OptionGroupFormat.h
===
--- lldb/include/lldb/Interpreter/OptionGroupFormat.h
+++ lldb/include/lldb/Interpreter/OptionGroupFormat.h
@@ -53,6 +53,8 @@
 
   const OptionValueFormat &GetFormatValue() const { return m_format; }
 
+  bool FormatWasSet() const { return m_format.OptionWasSet(); }
+
   OptionValueUInt64 &GetByteSizeValue() { return m_byte_size; }
 
   const OptionValueUInt64 &GetByteSizeValue() const { return m_byte_size; }


Index: lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
@@ -410,6 +410,15 @@
 
 self.expect("register read cpsr", substrs=["= (field_2 = 1, field_1 = 1, ...)"])
 
+@skipIfXmlSupportMissing
+@skipIfRemote
+def test_format_disable

[Lldb-commits] [PATCH] D148790: [LLDB] Don't print register fields when asked for a specific format

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 515271.
DavidSpickett added a comment.
Herald added a subscriber: JDevlieghere.

Use the getter for the format value instead of adding a WasSet to the group 
class.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148790

Files:
  lldb/source/Commands/CommandObjectRegister.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py


Index: lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
@@ -410,6 +410,15 @@
 
 self.expect("register read cpsr", substrs=["= (field_2 = 1, field_1 = 
1, ...)"])
 
+@skipIfXmlSupportMissing
+@skipIfRemote
+def test_format_disables_flags(self):
+# If asked for a specific format, don't print flags after it.
+self.setup_flags_test('')
+
+self.expect("register read cpsr --format X", substrs=["cpsr = 
0x"])
+self.expect("register read cpsr --format X", substrs=["field_0"], 
matching=False)
+
 @skipIfXmlSupportMissing
 @skipIfRemote
 def test_xml_includes(self):
Index: lldb/source/Commands/CommandObjectRegister.cpp
===
--- lldb/source/Commands/CommandObjectRegister.cpp
+++ lldb/source/Commands/CommandObjectRegister.cpp
@@ -44,7 +44,10 @@
 nullptr,
 eCommandRequiresFrame | eCommandRequiresRegContext |
 eCommandProcessMustBeLaunched | eCommandProcessMustBePaused),
-m_format_options(eFormatDefault) {
+m_format_options(eFormatDefault, UINT64_MAX, UINT64_MAX,
+ {{CommandArgumentType::eArgTypeFormat,
+   "Specify a format to be used for display. If this "
+   "is set, register fields will not be dispayed."}}) {
 CommandArgumentEntry arg;
 CommandArgumentData register_arg;
 
@@ -215,8 +218,12 @@
 
   if (const RegisterInfo *reg_info =
   reg_ctx->GetRegisterInfoByName(arg_str)) {
+// If they have asked for a specific format don't obscure that by
+// printing flags afterwards.
+bool print_flags =
+!m_format_options.GetFormatValue().OptionWasSet();
 if (!DumpRegister(m_exe_ctx, strm, *reg_ctx, *reg_info,
-  /*print_flags=*/true))
+  print_flags))
   strm.Printf("%-12s = error: unavailable\n", reg_info->name);
   } else {
 result.AppendErrorWithFormat("Invalid register name '%s'.\n",


Index: lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
@@ -410,6 +410,15 @@
 
 self.expect("register read cpsr", substrs=["= (field_2 = 1, field_1 = 1, ...)"])
 
+@skipIfXmlSupportMissing
+@skipIfRemote
+def test_format_disables_flags(self):
+# If asked for a specific format, don't print flags after it.
+self.setup_flags_test('')
+
+self.expect("register read cpsr --format X", substrs=["cpsr = 0x"])
+self.expect("register read cpsr --format X", substrs=["field_0"], matching=False)
+
 @skipIfXmlSupportMissing
 @skipIfRemote
 def test_xml_includes(self):
Index: lldb/source/Commands/CommandObjectRegister.cpp
===
--- lldb/source/Commands/CommandObjectRegister.cpp
+++ lldb/source/Commands/CommandObjectRegister.cpp
@@ -44,7 +44,10 @@
 nullptr,
 eCommandRequiresFrame | eCommandRequiresRegContext |
 eCommandProcessMustBeLaunched | eCommandProcessMustBePaused),
-m_format_options(eFormatDefault) {
+m_format_options(eFormatDefault, UINT64_MAX, UINT64_MAX,
+ {{CommandArgumentType::eArgTypeFormat,
+   "Specify a format to be used for display. If this "
+   "is set, register fields will not be dispayed."}}) {
 CommandArgumentEntry arg;
 CommandArgumentData register_arg;
 
@@ -215,8 +218,12 @@
 
   if (const RegisterInfo *reg_info =
   reg_ctx->GetRegisterInfoByName(arg_str)) {
+// If they have asked for a specific format don't obscure that by
+// printing flags afterwards.
+bool print_flags =
+!m_format_options.GetFormatValue().OptionWasSet();
 if (!DumpRegister(m_exe_ctx, strm, *reg_ctx, *reg_info,
-  

[Lldb-commits] [PATCH] D148790: [LLDB] Don't print register fields when asked for a specific format

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

One alternative was to come up with a list of formats where printing fields 
afterwards makes sense. Hex makes sense, instruction does not. However it would 
be hard to judge and it doesn't solve the main issue where you want to see what 
you asked for without extras.

This does mean that you will only be able to get the default format if you want 
fields, but I think that will cover the majority of use cases for now. If it 
doesn't we could add an option that enables fields which defaults to on when no 
format is set, and off when one is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148790

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

[Lldb-commits] [PATCH] D144390: [lldb] Send QPassSignals packet on attach, and at start for remote platforms

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144390

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

[Lldb-commits] [PATCH] D144392: [lldb] Skip signal handlers for ignored signals on lldb-server's side when stepping

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 added a comment.
Herald added a subscriber: asb.

@jingham could you please take a look?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144392

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

[Lldb-commits] [PATCH] D144390: [lldb] Send QPassSignals packet on attach, and at start for remote platforms

[David Spickett via Phabricator via lldb-commits]

DavidSpickett resigned from this revision.
DavidSpickett added a comment.

This is probably an obvious change, but I have no experience with this area so 
I don't feel comfortable approving.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144390

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

[Lldb-commits] [PATCH] D148752: lldb: Fix usage of sve functions on arm64

[Manoj Gupta via Phabricator via lldb-commits]

manojgupta added a comment.

I am building on ChromeOS. We only have headers from linux kernel 4.14 
available in our build system (The actual running kernel could be a higher 
version).
But given these functions/struct definitions (sve::) are already available 
and used, why are they not used consistently?

e.g. 
https://source.chromium.org/chromium/external/github.com/llvm/llvm-project/+/main:lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp;l=99
already uses sve::vl_valid and there are uses of sve::user_sve_header as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148752

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

[Lldb-commits] [PATCH] D148546: Reland: [Demangle] replace use of llvm::StringView w/ std::string_view

[Alan Zhao via Phabricator via lldb-commits]

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

This still fails to build on Windows with MSVC's `cl.exe` compiler:

  C:\src\llvm-project\build-ninja>ninja all
  [49/4908] Building CXX object 
lib\Demangle\CMakeFiles\LLVMDemangle.dir\ItaniumDemangle.cpp.obj
  FAILED: lib/Demangle/CMakeFiles/LLVMDemangle.dir/ItaniumDemangle.cpp.obj
  
C:\PROGRA~1\MICROS~2\2022\PROFES~1\VC\Tools\MSVC\1435~1.322\bin\Hostx86\x86\cl.exe
  /nologo /TP -DGTEST_HAS_RTTI=0 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE 
-D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS 
-D_FILE_OFFSET_BITS=64 -D_GLIBCXX_ASSERTIONS -D_HAS_EXCEPTIONS=0 
-D_LARGEFILE_SOURCE -D_LIBCPP_ENABLE_ASSERTIONS -D_SCL_SECURE_NO_DEPRECATE 
-D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS 
-D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS 
-IC:\src\llvm-project\build-ninja\lib\Demangle 
-IC:\src\llvm-project\llvm\lib\Demangle 
-IC:\src\llvm-project\build-ninja\include -IC:\src\llvm-project\llvm\include 
/DWIN32 /D_WINDOWS   /Zc:inline /Zc:preprocessor /Zc:__cplusplus /Oi /bigobj 
/permissive- /W4 -wd4141 -wd4146 -wd4244 -wd4267 -wd4291 -wd4351 -wd4456 
-wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4100 -wd4127 -wd4512 -wd4505 
-wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 
-wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4709 -wd5105 -wd4324 -w14062 
-we4238 /Gw /MD /O2 /Ob2  /EHs-c- /GR- -UNDEBUG -std:c++17 /showIncludes 
/Folib\Demangle\CMakeFiles\LLVMDemangle.dir\ItaniumDemangle.cpp.obj 
/Fdlib\Demangle\CMakeFiles\LLVMDemangle.dir\LLVMDemangle.pdb /FS -c 
C:\src\llvm-project\llvm\lib\Demangle\ItaniumDemangle.cpp
  C:\src\llvm-project\llvm\include\llvm/Demangle/ItaniumDemangle.h(3718): error 
C2665: 'llvm::itanium_demangle::ScopedOverride::ScopedOverride': 
no overloaded function could convert all the argument types
  C:\src\llvm-project\llvm\include\llvm\Demangle\Utility.h(194): note: could be 
'llvm::itanium_demangle::ScopedOverride::ScopedOverride(T &,T)'
  with
  [
  T=const char *
  ]
  C:\src\llvm-project\llvm\include\llvm/Demangle/ItaniumDemangle.h(3718): note: 
'llvm::itanium_demangle::ScopedOverride::ScopedOverride(T &,T)': 
cannot convert argument 2 from 'std::_String_view_iterator<_Traits>' to 'T'
  with
  [
  T=const char *
  ]
  and
  [
  _Traits=std::char_traits
  ]
  and
  [
  T=const char *
  ]
  C:\src\llvm-project\llvm\include\llvm/Demangle/ItaniumDemangle.h(3718): note: 
No user-defined-conversion operator available that can perform this conversion, 
or the operator cannot be called
  C:\src\llvm-project\llvm\include\llvm/Demangle/ItaniumDemangle.h(3718): note: 
while trying to match the argument list '(const char *, 
std::_String_view_iterator<_Traits>)'
  with
  [
  _Traits=std::char_traits
  ]
  C:\src\llvm-project\llvm\include\llvm/Demangle/ItaniumDemangle.h(3707): note: 
while compiling class template member function 'llvm::itanium_demangle::Node 
*llvm::itanium_demangle::AbstractManglingParser,Alloc>::parseQualifiedType(void)'
  with
  [
  Alloc=`anonymous-namespace'::DefaultAllocator
  ]
  C:\src\llvm-project\llvm\include\llvm/Demangle/ItaniumDemangle.h(3796): note: 
see reference to function template instantiation 'llvm::itanium_demangle::Node 
*llvm::itanium_demangle::AbstractManglingParser,Alloc>::parseQualifiedType(void)'
 being compiled
  with
  [
  Alloc=`anonymous-namespace'::DefaultAllocator
  ]
  C:\src\llvm-project\llvm\include\llvm/Demangle/ItaniumDemangle.h(3472): note: 
while compiling class template member function 'std::string_view 
llvm::itanium_demangle::AbstractManglingParser,Alloc>::parseNumber(bool)'
  with
  [
  Alloc=`anonymous-namespace'::DefaultAllocator
  ]
  C:\src\llvm-project\llvm\include\llvm/Demangle/ItaniumDemangle.h(5489): note: 
see reference to function template instantiation 'std::string_view 
llvm::itanium_demangle::AbstractManglingParser,Alloc>::parseNumber(bool)'
 being compiled
  with
  [
  Alloc=`anonymous-namespace'::DefaultAllocator
  ]
  C:\src\llvm-project\llvm\include\llvm/Demangle/ItaniumDemangle.h(5469): note: 
while compiling class template member function 'llvm::itanium_demangle::Node 
*llvm::itanium_demangle::AbstractManglingParser,Alloc>::parse(void)'
  with
  [
  Alloc=`anonymous-namespace'::DefaultAllocator
  ]
  C:\src\llvm-project\llvm\lib\Demangle\ItaniumDemangle.cpp(378): note: see 
reference to function template instantiation 'llvm::itanium_demangle::Node 
*llvm::itanium_demangle::AbstractManglingParser,Alloc>::parse(void)'
 being compiled
  with
  [
  Alloc=`ano

[Lldb-commits] [PATCH] D148546: Reland: [Demangle] replace use of llvm::StringView w/ std::string_view

[Nick Desaulniers via Phabricator via lldb-commits]

nickdesaulniers updated this revision to Diff 515387.
nickdesaulniers marked 3 inline comments as done.
nickdesaulniers added a comment.

- git clang-format HEAD~, fix 2 more conversion issues


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148546

Files:
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  llvm/include/llvm/Demangle/ItaniumDemangle.h
  llvm/include/llvm/Demangle/MicrosoftDemangle.h
  llvm/include/llvm/Demangle/MicrosoftDemangleNodes.h
  llvm/include/llvm/Demangle/Utility.h
  llvm/lib/Demangle/DLangDemangle.cpp
  llvm/lib/Demangle/ItaniumDemangle.cpp
  llvm/lib/Demangle/MicrosoftDemangle.cpp
  llvm/lib/Demangle/MicrosoftDemangleNodes.cpp
  llvm/lib/Demangle/RustDemangle.cpp
  llvm/lib/ProfileData/ItaniumManglingCanonicalizer.cpp
  llvm/unittests/Demangle/ItaniumDemangleTest.cpp
  llvm/unittests/Demangle/OutputBufferTest.cpp

Index: llvm/unittests/Demangle/OutputBufferTest.cpp
===
--- llvm/unittests/Demangle/OutputBufferTest.cpp
+++ llvm/unittests/Demangle/OutputBufferTest.cpp
@@ -10,12 +10,13 @@
 #include "llvm/Demangle/Utility.h"
 #include "gtest/gtest.h"
 #include 
+#include 
 
 using namespace llvm;
 using llvm::itanium_demangle::OutputBuffer;
 
 static std::string toString(OutputBuffer &OB) {
-  StringView SV = OB;
+  std::string_view SV = OB;
   return {SV.begin(), SV.end()};
 }
 
Index: llvm/unittests/Demangle/ItaniumDemangleTest.cpp
===
--- llvm/unittests/Demangle/ItaniumDemangleTest.cpp
+++ llvm/unittests/Demangle/ItaniumDemangleTest.cpp
@@ -11,6 +11,7 @@
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
+#include 
 #include 
 
 using namespace llvm;
@@ -83,7 +84,7 @@
 }
 
 static std::string toString(OutputBuffer &OB) {
-  StringView SV = OB;
+  std::string_view SV = OB;
   return {SV.begin(), SV.end()};
 }
 
@@ -98,7 +99,7 @@
   OutputBuffer OB;
   Node *N = AbstractManglingParser::parseType();
   N->printLeft(OB);
-  StringView Name = N->getBaseName();
+  std::string_view Name = N->getBaseName();
   if (!Name.empty())
 Types.push_back(std::string(Name.begin(), Name.end()));
   else
Index: llvm/lib/ProfileData/ItaniumManglingCanonicalizer.cpp
===
--- llvm/lib/ProfileData/ItaniumManglingCanonicalizer.cpp
+++ llvm/lib/ProfileData/ItaniumManglingCanonicalizer.cpp
@@ -17,13 +17,12 @@
 using llvm::itanium_demangle::ForwardTemplateReference;
 using llvm::itanium_demangle::Node;
 using llvm::itanium_demangle::NodeKind;
-using llvm::itanium_demangle::StringView;
 
 namespace {
 struct FoldingSetNodeIDBuilder {
   llvm::FoldingSetNodeID &ID;
   void operator()(const Node *P) { ID.AddPointer(P); }
-  void operator()(StringView Str) {
+  void operator()(std::string_view Str) {
 ID.AddString(llvm::StringRef(Str.begin(), Str.size()));
   }
   template 
@@ -292,7 +291,7 @@
 N = Demangler.parse();
   else
 N = Demangler.make(
-StringView(Mangling.data(), Mangling.size()));
+std::string_view(Mangling.data(), Mangling.size()));
   return reinterpret_cast(N);
 }
 
Index: llvm/lib/Demangle/RustDemangle.cpp
===
--- llvm/lib/Demangle/RustDemangle.cpp
+++ llvm/lib/Demangle/RustDemangle.cpp
@@ -12,7 +12,7 @@
 //===--===//
 
 #include "llvm/Demangle/Demangle.h"
-#include "llvm/Demangle/StringView.h"
+#include "llvm/Demangle/StringViewExtras.h"
 #include "llvm/Demangle/Utility.h"
 
 #include 
@@ -25,12 +25,11 @@
 
 using llvm::itanium_demangle::OutputBuffer;
 using llvm::itanium_demangle::ScopedOverride;
-using llvm::itanium_demangle::StringView;
 
 namespace {
 
 struct Identifier {
-  StringView Name;
+  std::string_view Name;
   bool Punycode;
 
   bool empty() const { return Name.empty(); }
@@ -77,7 +76,7 @@
   size_t RecursionLevel;
   size_t BoundLifetimes;
   // Input string that is being demangled with "_R" prefix removed.
-  StringView Input;
+  std::string_view Input;
   // Position in the input string.
   size_t Position;
   // When true, print methods append the output to the stream.
@@ -92,7 +91,7 @@
 
   Demangler(size_t MaxRecursionLevel = 500);
 
-  bool demangle(StringView MangledName);
+  bool demangle(std::string_view MangledName);
 
 private:
   bool demanglePath(IsInType Type,
@@ -128,10 +127,10 @@
   uint64_t parseOptionalBase62Number(char Tag);
   uint64_t parseBase62Number();
   uint64_t parseDecimalNumber();
-  uint64_t parseHexNumber(StringView &HexDigits);
+  uint64_t parseHexNumber(std::string_view &HexDigits);
 
   void print(char C);
-  void print(StringView S);
+  void print(std::string_view S);
   void printDecimalNumbe

[Lldb-commits] [PATCH] D148546: Reland: [Demangle] replace use of llvm::StringView w/ std::string_view

[Alan Zhao via Phabricator via lldb-commits]

ayzhao added a comment.

A new conversion issue popped up:

  C:\src\llvm-project\build-ninja>ninja all
  [376/3206] Building CXX object 
lib\ProfileData\CMakeFiles\LLVMProfileData.dir\ItaniumManglingCanonicalizer.cpp.obj
  FAILED: 
lib/ProfileData/CMakeFiles/LLVMProfileData.dir/ItaniumManglingCanonicalizer.cpp.obj
  
C:\PROGRA~1\MICROS~2\2022\PROFES~1\VC\Tools\MSVC\1435~1.322\bin\Hostx86\x86\cl.exe
  /nologo /TP -DGTEST_HAS_RTTI=0 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE 
-D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS 
-D_FILE_OFFSET_BITS=64 -D_GLIBCXX_ASSERTIONS -D_HAS_EXCEPTIONS=0 
-D_LARGEFILE_SOURCE -D_LIBCPP_ENABLE_ASSERTIONS -D_SCL_SECURE_NO_DEPRECATE 
-D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS 
-D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS 
-IC:\src\llvm-project\build-ninja\lib\ProfileData 
-IC:\src\llvm-project\llvm\lib\ProfileData 
-IC:\src\llvm-project\build-ninja\include -IC:\src\llvm-project\llvm\include 
/DWIN32 /D_WINDOWS   /Zc:inline /Zc:preprocessor /Zc:__cplusplus /Oi /bigobj 
/permissive- /W4 -wd4141 -wd4146 -wd4244 -wd4267 -wd4291 -wd4351 -wd4456 
-wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4100 -wd4127 -wd4512 -wd4505 
-wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 
-wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4709 -wd5105 -wd4324 -w14062 
-we4238 /Gw /MD /O2 /Ob2  /EHs-c- /GR- -UNDEBUG -std:c++17 /showIncludes 
/Folib\ProfileData\CMakeFiles\LLVMProfileData.dir\ItaniumManglingCanonicalizer.cpp.obj
 /Fdlib\ProfileData\CMakeFiles\LLVMProfileData.dir\LLVMProfileData.pdb /FS -c 
C:\src\llvm-project\llvm\lib\ProfileData\ItaniumManglingCanonicalizer.cpp
  
C:\src\llvm-project\llvm\lib\ProfileData\ItaniumManglingCanonicalizer.cpp(26): 
error C2440: '': cannot convert from 'initializer list' to 
'llvm::StringRef'
  
C:\src\llvm-project\llvm\lib\ProfileData\ItaniumManglingCanonicalizer.cpp(26): 
note: No constructor could take the source type, or constructor overload 
resolution was ambiguous
  [490/3022] Building CXX object 
tools\clang\lib\CodeGen\CMakeFiles\obj.clangCodeGen.dir\CGExpr.cpp.obj
  C:\src\llvm-project\clang\lib\CodeGen\CGExpr.cpp(3583): warning C4018: '<=': 
signed/unsigned mismatch
  [505/2927] Building CXX object 
tools\clang\lib\ASTMatchers\Dynamic\CMakeFiles\obj.clangDynamicASTMatchers.dir\Registry.cpp.obj
  ninja: build stopped: subcommand failed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148546

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

[Lldb-commits] [PATCH] D148546: Reland: [Demangle] replace use of llvm::StringView w/ std::string_view

[Nick Desaulniers via Phabricator via lldb-commits]

nickdesaulniers updated this revision to Diff 515391.
nickdesaulniers added a comment.

- fix another conversion issue for msvc


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148546

Files:
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  llvm/include/llvm/Demangle/ItaniumDemangle.h
  llvm/include/llvm/Demangle/MicrosoftDemangle.h
  llvm/include/llvm/Demangle/MicrosoftDemangleNodes.h
  llvm/include/llvm/Demangle/Utility.h
  llvm/lib/Demangle/DLangDemangle.cpp
  llvm/lib/Demangle/ItaniumDemangle.cpp
  llvm/lib/Demangle/MicrosoftDemangle.cpp
  llvm/lib/Demangle/MicrosoftDemangleNodes.cpp
  llvm/lib/Demangle/RustDemangle.cpp
  llvm/lib/ProfileData/ItaniumManglingCanonicalizer.cpp
  llvm/unittests/Demangle/ItaniumDemangleTest.cpp
  llvm/unittests/Demangle/OutputBufferTest.cpp

Index: llvm/unittests/Demangle/OutputBufferTest.cpp
===
--- llvm/unittests/Demangle/OutputBufferTest.cpp
+++ llvm/unittests/Demangle/OutputBufferTest.cpp
@@ -10,12 +10,13 @@
 #include "llvm/Demangle/Utility.h"
 #include "gtest/gtest.h"
 #include 
+#include 
 
 using namespace llvm;
 using llvm::itanium_demangle::OutputBuffer;
 
 static std::string toString(OutputBuffer &OB) {
-  StringView SV = OB;
+  std::string_view SV = OB;
   return {SV.begin(), SV.end()};
 }
 
Index: llvm/unittests/Demangle/ItaniumDemangleTest.cpp
===
--- llvm/unittests/Demangle/ItaniumDemangleTest.cpp
+++ llvm/unittests/Demangle/ItaniumDemangleTest.cpp
@@ -11,6 +11,7 @@
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
+#include 
 #include 
 
 using namespace llvm;
@@ -83,7 +84,7 @@
 }
 
 static std::string toString(OutputBuffer &OB) {
-  StringView SV = OB;
+  std::string_view SV = OB;
   return {SV.begin(), SV.end()};
 }
 
@@ -98,7 +99,7 @@
   OutputBuffer OB;
   Node *N = AbstractManglingParser::parseType();
   N->printLeft(OB);
-  StringView Name = N->getBaseName();
+  std::string_view Name = N->getBaseName();
   if (!Name.empty())
 Types.push_back(std::string(Name.begin(), Name.end()));
   else
Index: llvm/lib/ProfileData/ItaniumManglingCanonicalizer.cpp
===
--- llvm/lib/ProfileData/ItaniumManglingCanonicalizer.cpp
+++ llvm/lib/ProfileData/ItaniumManglingCanonicalizer.cpp
@@ -17,14 +17,13 @@
 using llvm::itanium_demangle::ForwardTemplateReference;
 using llvm::itanium_demangle::Node;
 using llvm::itanium_demangle::NodeKind;
-using llvm::itanium_demangle::StringView;
 
 namespace {
 struct FoldingSetNodeIDBuilder {
   llvm::FoldingSetNodeID &ID;
   void operator()(const Node *P) { ID.AddPointer(P); }
-  void operator()(StringView Str) {
-ID.AddString(llvm::StringRef(Str.begin(), Str.size()));
+  void operator()(std::string_view Str) {
+ID.AddString(llvm::StringRef(&*Str.begin(), Str.size()));
   }
   template 
   std::enable_if_t || std::is_enum_v> operator()(T V) {
@@ -292,7 +291,7 @@
 N = Demangler.parse();
   else
 N = Demangler.make(
-StringView(Mangling.data(), Mangling.size()));
+std::string_view(Mangling.data(), Mangling.size()));
   return reinterpret_cast(N);
 }
 
Index: llvm/lib/Demangle/RustDemangle.cpp
===
--- llvm/lib/Demangle/RustDemangle.cpp
+++ llvm/lib/Demangle/RustDemangle.cpp
@@ -12,7 +12,7 @@
 //===--===//
 
 #include "llvm/Demangle/Demangle.h"
-#include "llvm/Demangle/StringView.h"
+#include "llvm/Demangle/StringViewExtras.h"
 #include "llvm/Demangle/Utility.h"
 
 #include 
@@ -25,12 +25,11 @@
 
 using llvm::itanium_demangle::OutputBuffer;
 using llvm::itanium_demangle::ScopedOverride;
-using llvm::itanium_demangle::StringView;
 
 namespace {
 
 struct Identifier {
-  StringView Name;
+  std::string_view Name;
   bool Punycode;
 
   bool empty() const { return Name.empty(); }
@@ -77,7 +76,7 @@
   size_t RecursionLevel;
   size_t BoundLifetimes;
   // Input string that is being demangled with "_R" prefix removed.
-  StringView Input;
+  std::string_view Input;
   // Position in the input string.
   size_t Position;
   // When true, print methods append the output to the stream.
@@ -92,7 +91,7 @@
 
   Demangler(size_t MaxRecursionLevel = 500);
 
-  bool demangle(StringView MangledName);
+  bool demangle(std::string_view MangledName);
 
 private:
   bool demanglePath(IsInType Type,
@@ -128,10 +127,10 @@
   uint64_t parseOptionalBase62Number(char Tag);
   uint64_t parseBase62Number();
   uint64_t parseDecimalNumber();
-  uint64_t parseHexNumber(StringView &HexDigits);
+  uint64_t parseHexNumber(std::string_view &HexDigits);
 
   void print(char C);
-  void print(StringView S);
+  vo

[Lldb-commits] [PATCH] D148546: Reland: [Demangle] replace use of llvm::StringView w/ std::string_view

[Alan Zhao via Phabricator via lldb-commits]

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

LGTM, looks like this builds on Windows now:

  C:\src\llvm-project\build-ninja>ninja all
  [3498/6597] Building CXX object 
tools\clang\lib\AST\CMakeFiles\obj.clangAST.dir\AttrDocTable.cpp.obj
  C:\src\llvm-project\clang\lib\AST\AttrDocTable.cpp(24): warning C4018: '<': 
signed/unsigned mismatch
  [3846/6597] Building CXX object 
tools\clang\lib\CodeGen\CMakeFiles\obj.clangCodeGen.dir\CGExpr.cpp.obj
  C:\src\llvm-project\clang\lib\CodeGen\CGExpr.cpp(3583): warning C4018: '<=': 
signed/unsigned mismatch
  [4655/6597] Building CXX object 
tools\lld\COFF\CMakeFiles\lldCOFF.dir\PDB.cpp.obj
  C:\src\llvm-project\lld\COFF\PDB.cpp(839): warning C4018: '>=': 
signed/unsigned mismatch
  [4919/6597] Building CXX object 
tools\lldb\source\Core\CMakeFiles\lldbCore.dir\ModuleList.cpp.obj
  C:\src\llvm-project\lldb\source\Core\ModuleList.cpp(336): warning C4996: 
'std::shared_ptr::unique': warning STL4016: 
std::shared_ptr::unique() is deprecated in C++17. You can define 
_SILENCE_CXX17_SHARED_PTR_UNIQUE_DEPRECATION_WARNING or 
_SILENCE_ALL_CXX17_DEPRECATION_WARNINGS to suppress this warning.
  C:\src\llvm-project\lldb\source\Core\ModuleList.cpp(367): warning C4996: 
'std::shared_ptr::unique': warning STL4016: 
std::shared_ptr::unique() is deprecated in C++17. You can define 
_SILENCE_CXX17_SHARED_PTR_UNIQUE_DEPRECATION_WARNING or 
_SILENCE_ALL_CXX17_DEPRECATION_WARNINGS to suppress this warning.
  [5175/6597] Building CXX object 
tools\lldb\source\Plugins\Process\Utility\CMakeFiles\lldbPluginProcessUtility.dir\NativeRegisterContextDBReg_x86.cpp.obj
  
C:\src\llvm-project\lldb\source\Plugins\Process\Utility\NativeRegisterContextDBReg_x86.h(23):
 warning C4589: Constructor of abstract class 
'lldb_private::NativeRegisterContextDBReg_x86' ignores initializer for virtual 
base class 'lldb_private::NativeRegisterContextRegisterInfo'
  
C:\src\llvm-project\lldb\source\Plugins\Process\Utility\NativeRegisterContextDBReg_x86.h(23):
 note: virtual base classes are only initialized by the most-derived type
  [5205/6597] Building CXX object 
tools\lldb\source\Plugins\LanguageRuntime\ObjC\AppleObjCRuntime\CMakeFiles\lldbPluginAppleObjCRuntime.dir\AppleObjCRuntimeV2.cpp.obj
  
C:\src\llvm-project\lldb\source\Plugins\LanguageRuntime\ObjC\AppleObjCRuntime\AppleObjCRuntimeV2.cpp(1744):
 warning C5030: attribute 'clang::fallthrough' is not recognized
  [5263/6597] Building CXX object 
tools\lldb\source\Plugins\Process\Utility\CMakeFiles\lldbPluginProcessUtility.dir\RegisterContextPOSIX_mips64.cpp.obj
  
C:\src\llvm-project\lldb\source\Plugins\Process\Utility\RegisterContextPOSIX_mips64.cpp(109):
 warning C4065: switch statement contains 'default' but no 'case' labels
  
C:\src\llvm-project\lldb\source\Plugins\Process\Utility\RegisterContextPOSIX_mips64.cpp(120):
 warning C4065: switch statement contains 'default' but no 'case' labels
  [5593/6597] Building CXX object 
tools\lldb\source\API\CMakeFiles\liblldb.dir\SBTypeFilter.cpp.obj
  C:\src\llvm-project\lldb\source\API\SBTypeFilter.cpp(169): warning C4996: 
'std::shared_ptr::unique': warning STL4016: 
std::shared_ptr::unique() is deprecated in C++17. You can define 
_SILENCE_CXX17_SHARED_PTR_UNIQUE_DEPRECATION_WARNING or 
_SILENCE_ALL_CXX17_DEPRECATION_WARNINGS to suppress this warning.
  [5652/6597] Building CXX object 
tools\lldb\source\API\CMakeFiles\liblldb.dir\SBTypeFormat.cpp.obj
  C:\src\llvm-project\lldb\source\API\SBTypeFormat.cpp(160): warning C4996: 
'std::shared_ptr::unique': warning STL4016: 
std::shared_ptr::unique() is deprecated in C++17. You can define 
_SILENCE_CXX17_SHARED_PTR_UNIQUE_DEPRECATION_WARNING or 
_SILENCE_ALL_CXX17_DEPRECATION_WARNINGS to suppress this warning.
  [5665/6597] Building CXX object 
tools\lldb\source\API\CMakeFiles\liblldb.dir\SBTypeSynthetic.cpp.obj
  C:\src\llvm-project\lldb\source\API\SBTypeSynthetic.cpp(187): warning C4996: 
'std::shared_ptr::unique': warning 
STL4016: std::shared_ptr::unique() is deprecated in C++17. You can define 
_SILENCE_CXX17_SHARED_PTR_UNIQUE_DEPRECATION_WARNING or 
_SILENCE_ALL_CXX17_DEPRECATION_WARNINGS to suppress this warning.
  [5675/6597] Building CXX object 
tools\lldb\source\API\CMakeFiles\liblldb.dir\SBTypeSummary.cpp.obj
  C:\src\llvm-project\lldb\source\API\SBTypeSummary.cpp(384): warning C4996: 
'std::shared_ptr::unique': warning STL4016: 
std::shared_ptr::unique() is deprecated in C++17. You can define 
_SILENCE_CXX17_SHARED_PTR_UNIQUE_DEPRECATION_WARNING or 
_SILENCE_ALL_CXX17_DEPRECATION_WARNINGS to suppress this warning.
  [6585/6597] Building CXX object 
tools\clang\tools\extra\clangd\CMakeFiles\obj.clangDaemon.dir\InlayHints.cpp.obj
  C:\src\llvm-project\clang-tools-extra\clangd\InlayHints.cpp(594): warning 
C4018: '<': signed/unsigned mismatch
  [6597/6597] Linking CXX executable bin\clangd.exe


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D14854

[Lldb-commits] [PATCH] D148546: Reland: [Demangle] replace use of llvm::StringView w/ std::string_view

[Nick Desaulniers via Phabricator via lldb-commits]

nickdesaulniers added a comment.

Thanks for the help testing on Windows @ayzhao . Those warnings look 
potentially interesting to follow up; none appear to be introduced from this 
diff. Landing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148546

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

[Lldb-commits] [PATCH] D148823: Make diagnostics API safer to use

[Adrian Prantl via Phabricator via lldb-commits]

aprantl created this revision.
aprantl added reviewers: kastiglione, JDevlieghere, Michael137.
Herald added a project: All.
aprantl requested review of this revision.

I received a crash report in DiagnosticManager that was causes by a nullptr 
diagnostic having been added. The API allows passing in a null unique_ptr, but 
all the methods are written assuming that all pointers a dereferencable. This 
patch makes it impossible to add a null diagnostic.

rdar://107633615


https://reviews.llvm.org/D148823

Files:
  lldb/include/lldb/Expression/DiagnosticManager.h
  lldb/unittests/Expression/DiagnosticManagerTest.cpp


Index: lldb/unittests/Expression/DiagnosticManagerTest.cpp
===
--- lldb/unittests/Expression/DiagnosticManagerTest.cpp
+++ lldb/unittests/Expression/DiagnosticManagerTest.cpp
@@ -75,6 +75,9 @@
 TEST(DiagnosticManagerTest, GetStringNoDiags) {
   DiagnosticManager mgr;
   EXPECT_EQ("", mgr.GetString());
+  std::unique_ptr empty;
+  mgr.AddDiagnostic(std::move(empty));
+  EXPECT_EQ("", mgr.GetString());
 }
 
 TEST(DiagnosticManagerTest, GetStringBasic) {
Index: lldb/include/lldb/Expression/DiagnosticManager.h
===
--- lldb/include/lldb/Expression/DiagnosticManager.h
+++ lldb/include/lldb/Expression/DiagnosticManager.h
@@ -114,7 +114,8 @@
   }
 
   void AddDiagnostic(std::unique_ptr diagnostic) {
-m_diagnostics.push_back(std::move(diagnostic));
+if (diagnostic)
+  m_diagnostics.push_back(std::move(diagnostic));
   }
 
   size_t Printf(DiagnosticSeverity severity, const char *format, ...)


Index: lldb/unittests/Expression/DiagnosticManagerTest.cpp
===
--- lldb/unittests/Expression/DiagnosticManagerTest.cpp
+++ lldb/unittests/Expression/DiagnosticManagerTest.cpp
@@ -75,6 +75,9 @@
 TEST(DiagnosticManagerTest, GetStringNoDiags) {
   DiagnosticManager mgr;
   EXPECT_EQ("", mgr.GetString());
+  std::unique_ptr empty;
+  mgr.AddDiagnostic(std::move(empty));
+  EXPECT_EQ("", mgr.GetString());
 }
 
 TEST(DiagnosticManagerTest, GetStringBasic) {
Index: lldb/include/lldb/Expression/DiagnosticManager.h
===
--- lldb/include/lldb/Expression/DiagnosticManager.h
+++ lldb/include/lldb/Expression/DiagnosticManager.h
@@ -114,7 +114,8 @@
   }
 
   void AddDiagnostic(std::unique_ptr diagnostic) {
-m_diagnostics.push_back(std::move(diagnostic));
+if (diagnostic)
+  m_diagnostics.push_back(std::move(diagnostic));
   }
 
   size_t Printf(DiagnosticSeverity severity, const char *format, ...)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 7c59e80 - Reland: [Demangle] replace use of llvm::StringView w/ std::string_view

[Nick Desaulniers via lldb-commits]

Author: Nick Desaulniers
Date: 2023-04-20T11:22:20-07:00
New Revision: 7c59e8001a3b66be545a890a26ad8a24a6c9fe4b

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

LOG: Reland: [Demangle] replace use of llvm::StringView w/ std::string_view

This reverts commit d81cdb49d74064e88843733e7da92db865943509.

This refactoring was waiting on converting LLVM to C++17.

Leave StringView.h and cleanup around for subsequent cleanup.

Additional fixes for missing std::string_view conversions for MSVC.

Reviewed By: MaskRay, DavidSpickett, ayzhao

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

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
llvm/include/llvm/Demangle/ItaniumDemangle.h
llvm/include/llvm/Demangle/MicrosoftDemangle.h
llvm/include/llvm/Demangle/MicrosoftDemangleNodes.h
llvm/include/llvm/Demangle/Utility.h
llvm/lib/Demangle/DLangDemangle.cpp
llvm/lib/Demangle/ItaniumDemangle.cpp
llvm/lib/Demangle/MicrosoftDemangle.cpp
llvm/lib/Demangle/MicrosoftDemangleNodes.cpp
llvm/lib/Demangle/RustDemangle.cpp
llvm/lib/ProfileData/ItaniumManglingCanonicalizer.cpp
llvm/unittests/Demangle/ItaniumDemangleTest.cpp
llvm/unittests/Demangle/OutputBufferTest.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp 
b/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
index 1e9e1be62e3b5..c7ff25e904abe 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
@@ -14,17 +14,18 @@
 #include "llvm/DebugInfo/PDB/Native/TpiStream.h"
 #include "llvm/Demangle/MicrosoftDemangle.h"
 
+#include "PdbUtil.h"
 #include "Plugins/ExpressionParser/Clang/ClangASTMetadata.h"
 #include "Plugins/ExpressionParser/Clang/ClangUtil.h"
 #include "Plugins/Language/CPlusPlus/MSVCUndecoratedNameParser.h"
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
+#include "SymbolFileNativePDB.h"
+#include "UdtRecordCompleter.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Utility/LLDBAssert.h"
-#include "PdbUtil.h"
-#include "UdtRecordCompleter.h"
-#include "SymbolFileNativePDB.h"
 #include 
+#include 
 
 using namespace lldb_private;
 using namespace lldb_private::npdb;
@@ -174,7 +175,7 @@ PdbAstBuilder::CreateDeclInfoForType(const TagRecord 
&record, TypeIndex ti) {
 return CreateDeclInfoForUndecoratedName(record.Name);
 
   llvm::ms_demangle::Demangler demangler;
-  StringView sv(record.UniqueName.begin(), record.UniqueName.size());
+  std::string_view sv(record.UniqueName.begin(), record.UniqueName.size());
   llvm::ms_demangle::TagTypeNode *ttn = demangler.parseTagUniqueName(sv);
   if (demangler.Error)
 return {m_clang.GetTranslationUnitDecl(), std::string(record.UniqueName)};

diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp 
b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
index 01756e47e917f..8c91033ba2604 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -57,6 +57,7 @@
 #include "PdbUtil.h"
 #include "UdtRecordCompleter.h"
 #include 
+#include 
 
 using namespace lldb;
 using namespace lldb_private;
@@ -631,7 +632,7 @@ static std::string GetUnqualifiedTypeName(const TagRecord 
&record) {
   }
 
   llvm::ms_demangle::Demangler demangler;
-  StringView sv(record.UniqueName.begin(), record.UniqueName.size());
+  std::string_view sv(record.UniqueName.begin(), record.UniqueName.size());
   llvm::ms_demangle::TagTypeNode *ttn = demangler.parseTagUniqueName(sv);
   if (demangler.Error)
 return std::string(record.Name);

diff  --git a/llvm/include/llvm/Demangle/ItaniumDemangle.h 
b/llvm/include/llvm/Demangle/ItaniumDemangle.h
index a0029b94815dc..07b1097d4b5b0 100644
--- a/llvm/include/llvm/Demangle/ItaniumDemangle.h
+++ b/llvm/include/llvm/Demangle/ItaniumDemangle.h
@@ -17,7 +17,7 @@
 #define DEMANGLE_ITANIUMDEMANGLE_H
 
 #include "DemangleConfig.h"
-#include "StringView.h"
+#include "StringViewExtras.h"
 #include "Utility.h"
 #include 
 #include 
@@ -27,6 +27,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -287,7 +288,7 @@ class Node {
   // implementation.
   virtual void printRight(OutputBuffer &) const {}
 
-  virtual StringView getBaseName() const { return StringView(); }
+  virtual std::string_view getBaseName() const { return {}; }
 
   // Silence compiler warnings, this dtor will never be called.
   virtual ~Node() = default;
@@ -346,10 +347,10 @@ struct NodeArrayNode : Node {
 
 class DotSuffix final : public Node {
   cons

[Lldb-commits] [PATCH] D148546: Reland: [Demangle] replace use of llvm::StringView w/ std::string_view

[Nick Desaulniers via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7c59e8001a3b: Reland: [Demangle] replace use of 
llvm::StringView w/ std::string_view (authored by nickdesaulniers).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148546

Files:
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  llvm/include/llvm/Demangle/ItaniumDemangle.h
  llvm/include/llvm/Demangle/MicrosoftDemangle.h
  llvm/include/llvm/Demangle/MicrosoftDemangleNodes.h
  llvm/include/llvm/Demangle/Utility.h
  llvm/lib/Demangle/DLangDemangle.cpp
  llvm/lib/Demangle/ItaniumDemangle.cpp
  llvm/lib/Demangle/MicrosoftDemangle.cpp
  llvm/lib/Demangle/MicrosoftDemangleNodes.cpp
  llvm/lib/Demangle/RustDemangle.cpp
  llvm/lib/ProfileData/ItaniumManglingCanonicalizer.cpp
  llvm/unittests/Demangle/ItaniumDemangleTest.cpp
  llvm/unittests/Demangle/OutputBufferTest.cpp

Index: llvm/unittests/Demangle/OutputBufferTest.cpp
===
--- llvm/unittests/Demangle/OutputBufferTest.cpp
+++ llvm/unittests/Demangle/OutputBufferTest.cpp
@@ -10,12 +10,13 @@
 #include "llvm/Demangle/Utility.h"
 #include "gtest/gtest.h"
 #include 
+#include 
 
 using namespace llvm;
 using llvm::itanium_demangle::OutputBuffer;
 
 static std::string toString(OutputBuffer &OB) {
-  StringView SV = OB;
+  std::string_view SV = OB;
   return {SV.begin(), SV.end()};
 }
 
Index: llvm/unittests/Demangle/ItaniumDemangleTest.cpp
===
--- llvm/unittests/Demangle/ItaniumDemangleTest.cpp
+++ llvm/unittests/Demangle/ItaniumDemangleTest.cpp
@@ -11,6 +11,7 @@
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
+#include 
 #include 
 
 using namespace llvm;
@@ -83,7 +84,7 @@
 }
 
 static std::string toString(OutputBuffer &OB) {
-  StringView SV = OB;
+  std::string_view SV = OB;
   return {SV.begin(), SV.end()};
 }
 
@@ -98,7 +99,7 @@
   OutputBuffer OB;
   Node *N = AbstractManglingParser::parseType();
   N->printLeft(OB);
-  StringView Name = N->getBaseName();
+  std::string_view Name = N->getBaseName();
   if (!Name.empty())
 Types.push_back(std::string(Name.begin(), Name.end()));
   else
Index: llvm/lib/ProfileData/ItaniumManglingCanonicalizer.cpp
===
--- llvm/lib/ProfileData/ItaniumManglingCanonicalizer.cpp
+++ llvm/lib/ProfileData/ItaniumManglingCanonicalizer.cpp
@@ -17,14 +17,13 @@
 using llvm::itanium_demangle::ForwardTemplateReference;
 using llvm::itanium_demangle::Node;
 using llvm::itanium_demangle::NodeKind;
-using llvm::itanium_demangle::StringView;
 
 namespace {
 struct FoldingSetNodeIDBuilder {
   llvm::FoldingSetNodeID &ID;
   void operator()(const Node *P) { ID.AddPointer(P); }
-  void operator()(StringView Str) {
-ID.AddString(llvm::StringRef(Str.begin(), Str.size()));
+  void operator()(std::string_view Str) {
+ID.AddString(llvm::StringRef(&*Str.begin(), Str.size()));
   }
   template 
   std::enable_if_t || std::is_enum_v> operator()(T V) {
@@ -292,7 +291,7 @@
 N = Demangler.parse();
   else
 N = Demangler.make(
-StringView(Mangling.data(), Mangling.size()));
+std::string_view(Mangling.data(), Mangling.size()));
   return reinterpret_cast(N);
 }
 
Index: llvm/lib/Demangle/RustDemangle.cpp
===
--- llvm/lib/Demangle/RustDemangle.cpp
+++ llvm/lib/Demangle/RustDemangle.cpp
@@ -12,7 +12,7 @@
 //===--===//
 
 #include "llvm/Demangle/Demangle.h"
-#include "llvm/Demangle/StringView.h"
+#include "llvm/Demangle/StringViewExtras.h"
 #include "llvm/Demangle/Utility.h"
 
 #include 
@@ -25,12 +25,11 @@
 
 using llvm::itanium_demangle::OutputBuffer;
 using llvm::itanium_demangle::ScopedOverride;
-using llvm::itanium_demangle::StringView;
 
 namespace {
 
 struct Identifier {
-  StringView Name;
+  std::string_view Name;
   bool Punycode;
 
   bool empty() const { return Name.empty(); }
@@ -77,7 +76,7 @@
   size_t RecursionLevel;
   size_t BoundLifetimes;
   // Input string that is being demangled with "_R" prefix removed.
-  StringView Input;
+  std::string_view Input;
   // Position in the input string.
   size_t Position;
   // When true, print methods append the output to the stream.
@@ -92,7 +91,7 @@
 
   Demangler(size_t MaxRecursionLevel = 500);
 
-  bool demangle(StringView MangledName);
+  bool demangle(std::string_view MangledName);
 
 private:
   bool demanglePath(IsInType Type,
@@ -128,10 +127,10 @@
   uint64_t parseOptionalBase62Number(char Tag);
   uint64_t parseBase62Number();
   uint64_t parseDecimalNumber();
-  uint64_t parseHexNumber(Str

[Lldb-commits] [PATCH] D148823: Make diagnostics API safer to use

[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/D148823/new/

https://reviews.llvm.org/D148823

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

[Lldb-commits] [PATCH] D148823: Make diagnostics API safer to use

[Med Ismail Bennani via Phabricator via lldb-commits]

mib accepted this revision.
mib added a comment.

LGTM!


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

https://reviews.llvm.org/D148823

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

[Lldb-commits] [PATCH] D148399: [lldb] Improve logging for process state change (NFC)

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.

In D148399#4274470 , @JDevlieghere 
wrote:

> I think a lot of this can be simplified by using `LLDB_LOG` instead of 
> `LLDB_LOGF`.



In D148399#4275219 , @bulbazord wrote:

> +1 to what Jonas said. `LLDB_LOG` would greatly simplify this since it puts 
> `__FILE__` and `__func__` in each message, which is what these are doing 
> manually.

@bulbazord @JDevlieghere I don't think using using `LLDB_LOG` instead of 
`LLDB_LOGF` would be of any use here, because we're interested because that 
will just say we're in `Process::func`. What we care about here is to know 
which process plugin calling that function, so I still have to pass 
`GetPluginName` as an argument.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148399

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

[Lldb-commits] [PATCH] D148399: [lldb] Improve logging for process state change (NFC)

[Alex Langford via Phabricator via lldb-commits]

bulbazord added a comment.

We'd go from:

  LLDB_LOGF(log,
"Process::SetPrivateState (plugin = %s, state = %s) state didn't "
"change. Ignoring...",
GetPluginName().data(), StateAsCString(new_state));

to

  LLDB_LOG(log, "(plugin = %s, state = %s) state didn't "
"change. Ignoring...",
GetPluginName().data(), StateAsCString(new_state));

`LLDB_LOG` automatically inserts the file and func into the log. There are 
plenty of places where we copy/paste logs like these into other files without 
remembering to update this piece of information, so removing it from the log 
line entirely helps prevent those kinds of issues in the future. It's a minor 
thing, but if the point of this patch is to improve the logging mechanism 
entirely, then I think we should also think about these kinds of things. See 
0a74fbb7b1d3e04ac03389f1fc455ac593c2e5ee 
 for an 
example of what I mean.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148399

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

[Lldb-commits] [PATCH] D148399: [lldb] Improve logging for process state change (NFC)

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.

In D148399#4284809 , @bulbazord wrote:

> We'd go from:
>
>   LLDB_LOGF(log,
> "Process::SetPrivateState (plugin = %s, state = %s) state didn't "
> "change. Ignoring...",
> GetPluginName().data(), StateAsCString(new_state));
>
> to
>
>   LLDB_LOG(log, "(plugin = %s, state = %s) state didn't "
> "change. Ignoring...",
> GetPluginName().data(), StateAsCString(new_state));
>
> `LLDB_LOG` automatically inserts the file and func into the log. There are 
> plenty of places where we copy/paste logs like these into other files without 
> remembering to update this piece of information, so removing it from the log 
> line entirely helps prevent those kinds of issues in the future. It's a minor 
> thing, but if the point of this patch is to improve the logging mechanism 
> entirely, then I think we should also think about these kinds of things. See 
> 0a74fbb7b1d3e04ac03389f1fc455ac593c2e5ee 
>  for an 
> example of what I mean.

I didn't understand that originally (since what you guys are suggesting doesn't 
have anything to do with my change), but I'll update it in this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148399

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

[Lldb-commits] [PATCH] D148399: [lldb] Improve logging for process state change (NFC)

[Med Ismail Bennani via Phabricator via lldb-commits]

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

Address @JDevlieghere @bulbazord comments


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

https://reviews.llvm.org/D148399

Files:
  lldb/source/Target/Process.cpp

Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -1053,14 +1053,16 @@
   std::lock_guard guard(m_exit_status_mutex);
 
   Log *log(GetLog(LLDBLog::State | LLDBLog::Process));
-  LLDB_LOGF(
-  log, "Process::SetExitStatus (status=%i (0x%8.8x), description=%s%s%s)",
-  status, status, cstr ? "\"" : "", cstr ? cstr : "NULL", cstr ? "\"" : "");
+  LLDB_LOG(log, "(plugin = %s status=%i (0x%8.8x), description=%s%s%s)",
+   GetPluginName().data(), status, status, cstr ? "\"" : "",
+   cstr ? cstr : "NULL", cstr ? "\"" : "");
 
   // We were already in the exited state
   if (m_private_state.GetValue() == eStateExited) {
-LLDB_LOGF(log, "Process::SetExitStatus () ignoring exit status because "
-   "state was already set to eStateExited");
+LLDB_LOG(log,
+ "(plugin = %s) ignoring exit status because state was already set "
+ "to eStateExited",
+ GetPluginName().data());
 return false;
   }
 
@@ -1312,8 +1314,8 @@
   }
 
   Log *log(GetLog(LLDBLog::State | LLDBLog::Process));
-  LLDB_LOGF(log, "Process::SetPublicState (state = %s, restarted = %i)",
-StateAsCString(new_state), restarted);
+  LLDB_LOG(log, "(plugin = %s, state = %s, restarted = %i)",
+   GetPluginName().data(), StateAsCString(new_state), restarted);
   const StateType old_state = m_public_state.GetValue();
   m_public_state.SetValue(new_state);
 
@@ -1322,16 +1324,16 @@
   // program to run.
   if (!StateChangedIsExternallyHijacked()) {
 if (new_state == eStateDetached) {
-  LLDB_LOGF(log,
-"Process::SetPublicState (%s) -- unlocking run lock for detach",
-StateAsCString(new_state));
+  LLDB_LOG(log,
+   "(plugin = %s, state = %s) -- unlocking run lock for detach",
+   GetPluginName().data(), StateAsCString(new_state));
   m_public_run_lock.SetStopped();
 } else {
   const bool old_state_is_stopped = StateIsStoppedState(old_state, false);
   if ((old_state_is_stopped != new_state_is_stopped)) {
 if (new_state_is_stopped && !restarted) {
-  LLDB_LOGF(log, "Process::SetPublicState (%s) -- unlocking run lock",
-StateAsCString(new_state));
+  LLDB_LOG(log, "(plugin = %s, state = %s) -- unlocking run lock",
+   GetPluginName().data(), StateAsCString(new_state));
   m_public_run_lock.SetStopped();
 }
   }
@@ -1341,10 +1343,11 @@
 
 Status Process::Resume() {
   Log *log(GetLog(LLDBLog::State | LLDBLog::Process));
-  LLDB_LOGF(log, "Process::Resume -- locking run lock");
+  LLDB_LOG(log, "(plugin = %s) -- locking run lock", GetPluginName().data());
   if (!m_public_run_lock.TrySetRunning()) {
 Status error("Resume request failed - process still running.");
-LLDB_LOGF(log, "Process::Resume: -- TrySetRunning failed, not resuming.");
+LLDB_LOG(log, "(plugin = %s) -- TrySetRunning failed, not resuming.",
+ GetPluginName().data());
 return error;
   }
   Status error = PrivateResume();
@@ -1416,7 +1419,8 @@
   Log *log(GetLog(LLDBLog::State | LLDBLog::Process | LLDBLog::Unwind));
   bool state_changed = false;
 
-  LLDB_LOGF(log, "Process::SetPrivateState (%s)", StateAsCString(new_state));
+  LLDB_LOG(log, "(plugin = %s, state = %s)", GetPluginName().data(),
+   StateAsCString(new_state));
 
   std::lock_guard thread_guard(m_thread_list.GetMutex());
   std::lock_guard guard(m_private_state.GetMutex());
@@ -1457,15 +1461,15 @@
   if (!m_mod_id.IsLastResumeForUserExpression())
 m_mod_id.SetStopEventForLastNaturalStopID(event_sp);
   m_memory_cache.Clear();
-  LLDB_LOGF(log, "Process::SetPrivateState (%s) stop_id = %u",
-StateAsCString(new_state), m_mod_id.GetStopID());
+  LLDB_LOG(log, "(plugin = %s, state = %s, stop_id = %u",
+   GetPluginName().data(), StateAsCString(new_state),
+   m_mod_id.GetStopID());
 }
 
 m_private_state_broadcaster.BroadcastEvent(event_sp);
   } else {
-LLDB_LOGF(log,
-  "Process::SetPrivateState (%s) state didn't change. Ignoring...",
-  StateAsCString(new_state));
+LLDB_LOG(log, "(plugin = %s, state = %s) state didn't change. Ignoring...",
+ GetPluginName().data(), StateAsCString(new_state));
   }
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D148395: [lldb] Unify default/hijack listener between Process{Attach, Launch}Info (NFC)

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.

In D148395#4270508 , @bulbazord wrote:

> Creating a ProcessAttachInfo from a ProcessLaunchInfo with this change means 
> they'll have different listeners. Is that ProcessLaunchInfo kept around for 
> any other reason? Or is it just made to create the ProcessAttachInfo? This 
> seems like a reasonable move to me, but I'm not sure how LaunchInfo and 
> AttachInfo might interact.

Not sure what you mean ... We need to convert to the `ProcessLaunchInfo` into a 
`ProcessAttachInfo` when the user ask use to launch a process but we end up 
asking the platform to do the launch after which we attach to the process. In 
both cases, we use the default listener (the debugger listener if the user 
didn't provide a custom listener).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148395

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

[Lldb-commits] [PATCH] D148399: [lldb] Improve logging for process state change (NFC)

[Alex Langford via Phabricator via lldb-commits]

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

LGTM


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

https://reviews.llvm.org/D148399

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

[Lldb-commits] [PATCH] D148546: Reland: [Demangle] replace use of llvm::StringView w/ std::string_view

[Nick Desaulniers via Phabricator via lldb-commits]

nickdesaulniers added a comment.

llvm/test/Demangle/ms-cxx14.test is now crashing on windows, working on a fix 
forward for llvm::microsoftDemangle which I think is missing a nullptr check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148546

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

[Lldb-commits] [PATCH] D148846: [lldb] Let Mangled decide whether a name is mangled or not

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: jingham, labath, jasonmolenda, aprantl.
Herald added a subscriber: kristof.beyls.
Herald added a reviewer: shafik.
Herald added a project: All.
JDevlieghere requested review of this revision.

We have a handful of places in LLDB where we try to outsmart the logic in 
Mangled to determine whether a string is mangled or not. There's at least one 
place (*) where we were getting this wrong and this causes a subtle bug. The 
`cstring_is_mangled` is cheap enough that we should always rely on it to 
determine whether a string is mangled or not.

(*) ObjectFileMachO assumes that a symbol that starts with a double underscore 
(such as `__pthread_kill`)  is mangled. That's mostly harmless, until you want 
to use `function.name-without-args` in your frame format. For this formatter, 
we call `Symbol::GetNameNoArguments()` which is a wrapper around 
`Mangled::GetName(Mangled::ePreferDemangledWithoutArguments)`. The latter will 
first try using the appropriate language plugin to get the demangled name 
without arguments, but if that fails, it falls back to returning the demangled 
name. Because we forced Mangled to treat the symbol as a mangled name while 
it's not, there's no demangled name, and your frames don't show symbols at all. 
Alternatively, I could've worked around the issue by checking if we have a 
demangeld at all, but that's just working around the bug.


https://reviews.llvm.org/D148846

Files:
  lldb/include/lldb/Core/Mangled.h
  lldb/source/Core/Mangled.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp

Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
===
--- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -1980,7 +1980,7 @@
   } else if (!func_undecorated_name.empty()) {
 mangled.SetDemangledName(ConstString(func_undecorated_name));
   } else if (!func_name.empty())
-mangled.SetValue(ConstString(func_name), false);
+mangled.SetValue(ConstString(func_name));
 
   return mangled;
 }
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2417,7 +2417,7 @@
&frame_base)) {
 Mangled func_name;
 if (mangled)
-  func_name.SetValue(ConstString(mangled), true);
+  func_name.SetValue(ConstString(mangled));
 else if ((die.GetParent().Tag() == DW_TAG_compile_unit ||
   die.GetParent().Tag() == DW_TAG_partial_unit) &&
  Language::LanguageIsCPlusPlus(
@@ -2428,9 +2428,9 @@
   // If the mangled name is not present in the DWARF, generate the
   // demangled name using the decl context. We skip if the function is
   // "main" as its name is never mangled.
-  func_name.SetValue(ConstructDemangledNameFromDWARF(die), false);
+  func_name.SetValue(ConstructDemangledNameFromDWARF(die));
 } else
-  func_name.SetValue(ConstString(name), false);
+  func_name.SetValue(ConstString(name));
 
 FunctionSP func_sp;
 std::unique_ptr decl_up;
Index: lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
===
--- lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
+++ lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
@@ -246,7 +246,7 @@
 
   if (auto record = FuncRecord::parse(*It)) {
 Mangled func_name;
-func_name.SetValue(ConstString(record->Name), false);
+func_name.SetValue(ConstString(record->Name));
 addr_t address = record->Address + base;
 SectionSP section_sp = list->FindSectionContainingFileAddress(address);
 if (section_sp) {
Index: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
===
--- lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -3030,7 +3030,7 @@
   // the first contains a directory and the
   // second contains a full path.
   sym[sym_idx - 1].GetMangled().SetValue(
-  ConstString(symbol_name), false);
+  ConstString(symbol_name));
   m_nlist_idx_to_sym_idx[nlist_idx] = sym_idx - 1;
   add_nlist = false;
 } else {
@@ -3076,7 +3076,7 @@
 full_so_path += '/';
 

[Lldb-commits] [PATCH] D148397: [lldb/Utility] Add opt-in passthrough mode to event listeners

[Med Ismail Bennani via Phabricator via lldb-commits]

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

In D148397#4275792 , @JDevlieghere 
wrote:

> I find "passthrough" somewhat confusing in this context. When using a 
> listener in this new mode, where does the event go after it has pass trough 
> this event listener? Maybe my understanding of how the event listeners is 
> incomplete, but I was under the impression that normally there's a single 
> listener for a certain kind of event. IIUC, in this new mode, you can have 
> multiple listeners, where you can set one (or as Alex pointed out, possibly 
> more) listeners that can observe events but not necessary deal with them. If 
> that's an accurate description of the interactions, then it seems like 
> there's more of a one-to-many relationship between events and listeners 
> rather than what passthrough implies. I think a better name would be 
> something like "passive listener" or "non-handling" listener or something.

@JDevlieghere You could also have multiple listeners for a certain type of 
event, but events would be considered "handled" only when they're popped from 
the listener passed during the process creation (or the debugger listener if no 
listener is provided in the {launch,attach}info). The idea behind passthrough 
listener was to have a side channel where all the events would be funneled to, 
that would never interfere with the default listener and therefore not change 
the process execution behavior. If you really feel strongly about the 
"passthrough" name, I can rename it to whatever you like but I feel like it's 
pretty self explanatory.




Comment at: lldb/include/lldb/Utility/Broadcaster.h:520-522
+/// A optional listener that all private events get also broadcasted to,
+/// on top the hijacked / default listeners.
+lldb::ListenerSP m_passthrough_listener = nullptr;

bulbazord wrote:
> It looks like Broadcaster can have multiple listeners. Do we want multiple 
> passthrough listeners as well? If not, why only one?
That's a very good point. When I implemented this, I was thinking that the 
passthrough listener would act as a funnel for all the events. But having 
multiple passthrough listener could potentially allow the user to do some 
filtering on the event. @jingham what do you think ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148397

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

[Lldb-commits] [PATCH] D148397: [lldb/Utility] Add opt-in passthrough mode to event listeners

[Med Ismail Bennani via Phabricator via lldb-commits]

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

Fix issue when returning a non-initialized passthrough listener from the SBAPI


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

https://reviews.llvm.org/D148397

Files:
  lldb/include/lldb/API/SBAttachInfo.h
  lldb/include/lldb/API/SBLaunchInfo.h
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Utility/Broadcaster.h
  lldb/include/lldb/Utility/Listener.h
  lldb/include/lldb/Utility/ProcessInfo.h
  lldb/source/API/SBAttachInfo.cpp
  lldb/source/API/SBLaunchInfo.cpp
  lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
  lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Utility/Broadcaster.cpp
  lldb/source/Utility/Listener.cpp

Index: lldb/source/Utility/Listener.cpp
===
--- lldb/source/Utility/Listener.cpp
+++ lldb/source/Utility/Listener.cpp
@@ -35,7 +35,7 @@
 
 Listener::Listener(const char *name)
 : m_name(name), m_broadcasters(), m_broadcasters_mutex(), m_events(),
-  m_events_mutex() {
+  m_events_mutex(), m_is_passthrough() {
   Log *log = GetLog(LLDBLog::Object);
   if (log != nullptr)
 LLDB_LOGF(log, "%p Listener::Listener('%s')", static_cast(this),
@@ -302,7 +302,8 @@
   // to return it so it should be okay to get the next event off the queue
   // here - and it might be useful to do that in the "DoOnRemoval".
   lock.unlock();
-  event_sp->DoOnRemoval();
+  if (!m_is_passthrough)
+event_sp->DoOnRemoval();
 }
 return true;
   }
Index: lldb/source/Utility/Broadcaster.cpp
===
--- lldb/source/Utility/Broadcaster.cpp
+++ lldb/source/Utility/Broadcaster.cpp
@@ -228,6 +228,8 @@
   &m_broadcaster, event_type))
   return;
 hijacking_listener_sp->AddEvent(event_sp);
+if (m_passthrough_listener)
+  m_passthrough_listener->AddEvent(event_sp);
   } else {
 for (auto &pair : GetListeners()) {
   if (!(pair.second & event_type))
@@ -237,6 +239,8 @@
 continue;
 
   pair.first->AddEvent(event_sp);
+  if (m_passthrough_listener)
+m_passthrough_listener->AddEvent(event_sp);
 }
   }
 }
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3193,6 +3193,8 @@
 // Since we didn't have a platform launch the process, launch it here.
 if (m_process_sp) {
   m_process_sp->HijackProcessEvents(launch_info.GetHijackListener());
+  m_process_sp->SetPassthroughListener(
+  launch_info.GetPassthroughListener());
   error = m_process_sp->Launch(launch_info);
 }
   }
Index: lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
===
--- lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
+++ lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
@@ -423,6 +423,8 @@
 
 if (process_sp) {
   process_sp->HijackProcessEvents(launch_info.GetHijackListener());
+  process_sp->SetPassthroughListener(
+  launch_info.GetPassthroughListener());
 
   error = process_sp->ConnectRemote(connect_url.c_str());
   // Retry the connect remote one time...
@@ -515,6 +517,8 @@
   ListenerSP listener_sp = attach_info.GetHijackListener();
   if (listener_sp)
 process_sp->HijackProcessEvents(listener_sp);
+  process_sp->SetPassthroughListener(
+  attach_info.GetPassthroughListener());
   error = process_sp->Attach(attach_info);
 }
 
Index: lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
===
--- lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
+++ lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
@@ -401,6 +401,8 @@
   attach_info.SetHijackListener(listener_sp);
 }
 process_sp->HijackProcessEvents(listener_sp);
+process_sp->SetPassthroughListener(
+attach_info.GetPassthroughListener());
 error = process_sp->Attach(attach_info);
   }
 }
@@ -458,6 +460,7 @@
   LLDB_LOG(log, "successfully created process");
 
   process_sp->HijackProcessEvents(launch_info.GetHijackListener());
+  process_sp->SetPassthroughListener(launch_info.GetPassthroughListener());
 
   // Log file actions.
   if (log) {
Index: lldb/source/API/SBLaunchInfo.cpp
===
--- lldb/source/API/SBLaunchInfo.cpp
+++ lldb/source/API/SBLaunchInfo.cpp
@@ -17,6 +17,7 @@
 #include "lldb/API/SBStructuredData.h"
 #include "lldb/Core/StructuredDataImpl.h"
 #include "lldb/Host/ProcessLaunchInfo.h"
+#include "lldb/Util

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

[Christopher Di Bella via Phabricator via lldb-commits]

cjdb abandoned this revision.
cjdb added a comment.
Herald added a subscriber: PiotrZSL.
Herald added a project: clang-format.
Herald added reviewers: rymiel, HazardyKnusperkeks, owenpan, MyDeveloperDay.

Abandoning since we're heading down a different design path now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138939

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

[Lldb-commits] [PATCH] D145297: [lldb] Add an example of interactive scripted process debugging

[Med Ismail Bennani via Phabricator via lldb-commits]

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



Comment at: 
lldb/test/API/functionalities/interactive_scripted_process/TestInteractiveScriptedProcess.py:102-103
+self.assertState(lldb.SBProcess.GetStateFromEvent(event), 
lldb.eStateRunning)
+event = lldbutil.fetch_next_event(self, mux_process_listener, 
mux_process.GetBroadcaster())
+self.assertState(lldb.SBProcess.GetStateFromEvent(event), 
lldb.eStateStopped)
+

bulbazord wrote:
> Did you mean to get info from the `mux_process_listener` twice? Was one of 
> these supposed to be for the real process?
We're only listening to the `mux_process` events so this is how its supposed to 
be.



Comment at: 
lldb/test/API/functionalities/interactive_scripted_process/interactive_scripted_process.py:9
+
+import os,json,struct,signal
+import time

bulbazord wrote:
> nit: import these all on different lines
why :p ?


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

https://reviews.llvm.org/D145297

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

[Lldb-commits] [PATCH] D145297: [lldb] Add an example of interactive scripted process debugging

[Med Ismail Bennani via Phabricator via lldb-commits]

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

Address @bulbazord comments:

- f-string everything
- remove logs
- add type annotations


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

https://reviews.llvm.org/D145297

Files:
  lldb/test/API/functionalities/interactive_scripted_process/Makefile
  
lldb/test/API/functionalities/interactive_scripted_process/TestInteractiveScriptedProcess.py
  
lldb/test/API/functionalities/interactive_scripted_process/interactive_scripted_process.py
  lldb/test/API/functionalities/interactive_scripted_process/main.cpp

Index: lldb/test/API/functionalities/interactive_scripted_process/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/interactive_scripted_process/main.cpp
@@ -0,0 +1,35 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+
+void spawn_thread(int index) {
+  std::string name = "I'm thread " + std::to_string(index) + " !";
+  bool done = false;
+  std::string state = "Started execution!";
+  while (true) {
+if (done) // also break here
+  break;
+  }
+
+  state = "Stopped execution!";
+}
+
+int main() {
+  constexpr size_t num_threads = 10;
+  std::vector threads;
+
+  for (size_t i = 0; i < num_threads; i++) {
+threads.push_back(std::thread(spawn_thread, i));
+  }
+
+  std::cout << "Spawned " << threads.size() << " threads!" << std::endl; // Break here
+
+  for (auto &t : threads) {
+if (t.joinable())
+  t.join();
+  }
+
+  return 0;
+}
Index: lldb/test/API/functionalities/interactive_scripted_process/interactive_scripted_process.py
===
--- /dev/null
+++ lldb/test/API/functionalities/interactive_scripted_process/interactive_scripted_process.py
@@ -0,0 +1,417 @@
+# Usage:
+# ./bin/lldb $LLVM/lldb/test/API/functionalities/interactive_scripted_process/main \
+#   -o "br set -p 'Break here'" \
+#   -o "command script import $LLVM/lldb/test/API/functionalities/interactive_scripted_process/interactive_scripted_process.py" \
+#   -o "create_mux" \
+#   -o "create_sub" \
+#   -o "br set -p 'also break here'" -o 'continue'
+
+import os,json,struct,signal
+
+from threading import Thread
+from typing import Any, Dict
+
+import lldb
+from lldb.plugins.scripted_process import ScriptedProcess
+from lldb.plugins.scripted_process import ScriptedThread
+
+class PassthruScriptedProcess(ScriptedProcess):
+driving_target = None
+driving_process = None
+
+def __init__(self, exe_ctx: lldb.SBExecutionContext, args :
+ lldb.SBStructuredData, launched_driving_process: bool =True):
+super().__init__(exe_ctx, args)
+
+self.driving_target = None
+self.driving_process = None
+
+self.driving_target_idx = args.GetValueForKey("driving_target_idx")
+if (self.driving_target_idx and self.driving_target_idx.IsValid()):
+if self.driving_target_idx.GetType() == lldb.eStructuredDataTypeInteger:
+idx = self.driving_target_idx.GetIntegerValue(42)
+if self.driving_target_idx.GetType() == lldb.eStructuredDataTypeString:
+idx = int(self.driving_target_idx.GetStringValue(100))
+self.driving_target = self.target.GetDebugger().GetTargetAtIndex(idx)
+
+if launched_driving_process:
+self.driving_process = self.driving_target.GetProcess()
+for driving_thread in self.driving_process:
+structured_data = lldb.SBStructuredData()
+structured_data.SetFromJSON(json.dumps({
+"driving_target_idx" : idx,
+"thread_idx" : driving_thread.GetIndexID()
+}))
+
+self.threads[driving_thread.GetThreadID()] = PassthruScriptedThread(self, structured_data)
+
+for module in self.driving_target.modules:
+path = module.file.fullpath
+load_addr = module.GetObjectFileHeaderAddress().GetLoadAddress(self.driving_target)
+self.loaded_images.append({"path": path, "load_addr": load_addr})
+
+def get_memory_region_containing_address(self, addr: int) -> lldb.SBMemoryRegionInfo:
+mem_region = lldb.SBMemoryRegionInfo()
+error = self.driving_process.GetMemoryRegionInfo(addr, mem_region)
+if error.Fail():
+return None
+return mem_region
+
+def read_memory_at_address(self, addr: int, size: int, error: lldb.SBError) -> lldb.SBData:
+data = lldb.SBData()
+bytes_read = self.driving_process.ReadMemory(addr, size, error)
+
+if error.Fail():
+return data
+
+data.SetDataWithOwnership(error, bytes_read,
+  self.driving_target.GetByteOrder(),
+  self.driving_target.GetAddressByteSize())
+

[Lldb-commits] [PATCH] D145297: [lldb] Add an example of interactive scripted process debugging

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.

ping @jingham @JDevlieghere


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

https://reviews.llvm.org/D145297

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

[Lldb-commits] [PATCH] D148846: [lldb] Let Mangled decide whether a name is mangled or not

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 515522.
JDevlieghere added a comment.

Add simple test


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

https://reviews.llvm.org/D148846

Files:
  lldb/include/lldb/Core/Mangled.h
  lldb/source/Core/Mangled.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/test/API/macosx/format/Makefile
  lldb/test/API/macosx/format/TestFunctionNameWithoutArgs.py
  lldb/test/API/macosx/format/main.c

Index: lldb/test/API/macosx/format/main.c
===
--- /dev/null
+++ lldb/test/API/macosx/format/main.c
@@ -0,0 +1,7 @@
+#include 
+#include 
+
+int main(int argc, char **argv) {
+  assert(argc != 1);
+  return 0;
+}
Index: lldb/test/API/macosx/format/TestFunctionNameWithoutArgs.py
===
--- /dev/null
+++ lldb/test/API/macosx/format/TestFunctionNameWithoutArgs.py
@@ -0,0 +1,29 @@
+import os
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestFunctionNameWithoutArgs(TestBase):
+@skipUnlessDarwin
+def test_function_name_without_args(self):
+self.build()
+target = self.createTestTarget()
+target.LaunchSimple(None, None, self.get_process_working_directory())
+
+self.runCmd("run", RUN_SUCCEEDED)
+self.expect(
+"bt",
+substrs=[
+"stop reason = hit program assert",
+"libsystem_kernel.dylib`__pthread_kill",
+],
+)
+self.runCmd(
+'settings set frame-format "frame #${frame.index}: ${function.name-without-args}\n"'
+)
+self.expect(
+"bt",
+substrs=["stop reason = hit program assert", "frame #0: __pthread_kill"],
+)
Index: lldb/test/API/macosx/format/Makefile
===
--- /dev/null
+++ lldb/test/API/macosx/format/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules
Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
===
--- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -1980,7 +1980,7 @@
   } else if (!func_undecorated_name.empty()) {
 mangled.SetDemangledName(ConstString(func_undecorated_name));
   } else if (!func_name.empty())
-mangled.SetValue(ConstString(func_name), false);
+mangled.SetValue(ConstString(func_name));
 
   return mangled;
 }
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2417,7 +2417,7 @@
&frame_base)) {
 Mangled func_name;
 if (mangled)
-  func_name.SetValue(ConstString(mangled), true);
+  func_name.SetValue(ConstString(mangled));
 else if ((die.GetParent().Tag() == DW_TAG_compile_unit ||
   die.GetParent().Tag() == DW_TAG_partial_unit) &&
  Language::LanguageIsCPlusPlus(
@@ -2428,9 +2428,9 @@
   // If the mangled name is not present in the DWARF, generate the
   // demangled name using the decl context. We skip if the function is
   // "main" as its name is never mangled.
-  func_name.SetValue(ConstructDemangledNameFromDWARF(die), false);
+  func_name.SetValue(ConstructDemangledNameFromDWARF(die));
 } else
-  func_name.SetValue(ConstString(name), false);
+  func_name.SetValue(ConstString(name));
 
 FunctionSP func_sp;
 std::unique_ptr decl_up;
Index: lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
===
--- lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
+++ lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
@@ -246,7 +246,7 @@
 
   if (auto record = FuncRecord::parse(*It)) {
 Mangled func_name;
-func_name.SetValue(ConstString(record->Name), false);
+func_name.SetValue(ConstString(record->Name));
 addr_t address = record->Address + base;
 SectionSP section_sp = list->FindSectionContainingFileAddress(address);
 if (section_sp) {
Index: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
===
--- lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -3030,7 +3030,7 @@
   // the first contains a directory and the

[Lldb-commits] [PATCH] D145297: [lldb] Add an example of interactive scripted process debugging

[Med Ismail Bennani via Phabricator via lldb-commits]

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

Fix typo in one of the type annotation


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

https://reviews.llvm.org/D145297

Files:
  lldb/test/API/functionalities/interactive_scripted_process/Makefile
  
lldb/test/API/functionalities/interactive_scripted_process/TestInteractiveScriptedProcess.py
  
lldb/test/API/functionalities/interactive_scripted_process/interactive_scripted_process.py
  lldb/test/API/functionalities/interactive_scripted_process/main.cpp

Index: lldb/test/API/functionalities/interactive_scripted_process/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/interactive_scripted_process/main.cpp
@@ -0,0 +1,35 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+
+void spawn_thread(int index) {
+  std::string name = "I'm thread " + std::to_string(index) + " !";
+  bool done = false;
+  std::string state = "Started execution!";
+  while (true) {
+if (done) // also break here
+  break;
+  }
+
+  state = "Stopped execution!";
+}
+
+int main() {
+  constexpr size_t num_threads = 10;
+  std::vector threads;
+
+  for (size_t i = 0; i < num_threads; i++) {
+threads.push_back(std::thread(spawn_thread, i));
+  }
+
+  std::cout << "Spawned " << threads.size() << " threads!" << std::endl; // Break here
+
+  for (auto &t : threads) {
+if (t.joinable())
+  t.join();
+  }
+
+  return 0;
+}
Index: lldb/test/API/functionalities/interactive_scripted_process/interactive_scripted_process.py
===
--- /dev/null
+++ lldb/test/API/functionalities/interactive_scripted_process/interactive_scripted_process.py
@@ -0,0 +1,417 @@
+# Usage:
+# ./bin/lldb $LLVM/lldb/test/API/functionalities/interactive_scripted_process/main \
+#   -o "br set -p 'Break here'" \
+#   -o "command script import $LLVM/lldb/test/API/functionalities/interactive_scripted_process/interactive_scripted_process.py" \
+#   -o "create_mux" \
+#   -o "create_sub" \
+#   -o "br set -p 'also break here'" -o 'continue'
+
+import os,json,struct,signal
+
+from threading import Thread
+from typing import Any, Dict
+
+import lldb
+from lldb.plugins.scripted_process import ScriptedProcess
+from lldb.plugins.scripted_process import ScriptedThread
+
+class PassthruScriptedProcess(ScriptedProcess):
+driving_target = None
+driving_process = None
+
+def __init__(self, exe_ctx: lldb.SBExecutionContext, args :
+ lldb.SBStructuredData, launched_driving_process: bool =True):
+super().__init__(exe_ctx, args)
+
+self.driving_target = None
+self.driving_process = None
+
+self.driving_target_idx = args.GetValueForKey("driving_target_idx")
+if (self.driving_target_idx and self.driving_target_idx.IsValid()):
+if self.driving_target_idx.GetType() == lldb.eStructuredDataTypeInteger:
+idx = self.driving_target_idx.GetIntegerValue(42)
+if self.driving_target_idx.GetType() == lldb.eStructuredDataTypeString:
+idx = int(self.driving_target_idx.GetStringValue(100))
+self.driving_target = self.target.GetDebugger().GetTargetAtIndex(idx)
+
+if launched_driving_process:
+self.driving_process = self.driving_target.GetProcess()
+for driving_thread in self.driving_process:
+structured_data = lldb.SBStructuredData()
+structured_data.SetFromJSON(json.dumps({
+"driving_target_idx" : idx,
+"thread_idx" : driving_thread.GetIndexID()
+}))
+
+self.threads[driving_thread.GetThreadID()] = PassthruScriptedThread(self, structured_data)
+
+for module in self.driving_target.modules:
+path = module.file.fullpath
+load_addr = module.GetObjectFileHeaderAddress().GetLoadAddress(self.driving_target)
+self.loaded_images.append({"path": path, "load_addr": load_addr})
+
+def get_memory_region_containing_address(self, addr: int) -> lldb.SBMemoryRegionInfo:
+mem_region = lldb.SBMemoryRegionInfo()
+error = self.driving_process.GetMemoryRegionInfo(addr, mem_region)
+if error.Fail():
+return None
+return mem_region
+
+def read_memory_at_address(self, addr: int, size: int, error: lldb.SBError) -> lldb.SBData:
+data = lldb.SBData()
+bytes_read = self.driving_process.ReadMemory(addr, size, error)
+
+if error.Fail():
+return data
+
+data.SetDataWithOwnership(error, bytes_read,
+  self.driving_target.GetByteOrder(),
+  self.driving_target.GetAddressByteSize())
+
+return data
+
+def write_memory_at_address(self, addr: int, data: lldb.SBDa

[Lldb-commits] [PATCH] D148790: [LLDB] Don't print register fields when asked for a specific format

[Jason Molenda via Phabricator via lldb-commits]

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

This seems fine to me; it's nice to have a way to quiet these when they get in 
the way, even if it's maybe not the first thing you'd look for, it's an 
uncommon enough use case that I'm not worried about that.




Comment at: lldb/source/Commands/CommandObjectRegister.cpp:50
+   "Specify a format to be used for display. If this "
+   "is set, register fields will not be dispayed."}}) {
 CommandArgumentEntry arg;

displayed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148790

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

[Lldb-commits] [PATCH] D145297: [lldb] Add an example of interactive scripted process debugging

[Med Ismail Bennani via Phabricator via lldb-commits]

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

More f-string


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

https://reviews.llvm.org/D145297

Files:
  lldb/test/API/functionalities/interactive_scripted_process/Makefile
  
lldb/test/API/functionalities/interactive_scripted_process/TestInteractiveScriptedProcess.py
  
lldb/test/API/functionalities/interactive_scripted_process/interactive_scripted_process.py
  lldb/test/API/functionalities/interactive_scripted_process/main.cpp

Index: lldb/test/API/functionalities/interactive_scripted_process/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/interactive_scripted_process/main.cpp
@@ -0,0 +1,35 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+
+void spawn_thread(int index) {
+  std::string name = "I'm thread " + std::to_string(index) + " !";
+  bool done = false;
+  std::string state = "Started execution!";
+  while (true) {
+if (done) // also break here
+  break;
+  }
+
+  state = "Stopped execution!";
+}
+
+int main() {
+  constexpr size_t num_threads = 10;
+  std::vector threads;
+
+  for (size_t i = 0; i < num_threads; i++) {
+threads.push_back(std::thread(spawn_thread, i));
+  }
+
+  std::cout << "Spawned " << threads.size() << " threads!" << std::endl; // Break here
+
+  for (auto &t : threads) {
+if (t.joinable())
+  t.join();
+  }
+
+  return 0;
+}
Index: lldb/test/API/functionalities/interactive_scripted_process/interactive_scripted_process.py
===
--- /dev/null
+++ lldb/test/API/functionalities/interactive_scripted_process/interactive_scripted_process.py
@@ -0,0 +1,417 @@
+# Usage:
+# ./bin/lldb $LLVM/lldb/test/API/functionalities/interactive_scripted_process/main \
+#   -o "br set -p 'Break here'" \
+#   -o "command script import $LLVM/lldb/test/API/functionalities/interactive_scripted_process/interactive_scripted_process.py" \
+#   -o "create_mux" \
+#   -o "create_sub" \
+#   -o "br set -p 'also break here'" -o 'continue'
+
+import os,json,struct,signal
+
+from threading import Thread
+from typing import Any, Dict
+
+import lldb
+from lldb.plugins.scripted_process import ScriptedProcess
+from lldb.plugins.scripted_process import ScriptedThread
+
+class PassthruScriptedProcess(ScriptedProcess):
+driving_target = None
+driving_process = None
+
+def __init__(self, exe_ctx: lldb.SBExecutionContext, args :
+ lldb.SBStructuredData, launched_driving_process: bool =True):
+super().__init__(exe_ctx, args)
+
+self.driving_target = None
+self.driving_process = None
+
+self.driving_target_idx = args.GetValueForKey("driving_target_idx")
+if (self.driving_target_idx and self.driving_target_idx.IsValid()):
+if self.driving_target_idx.GetType() == lldb.eStructuredDataTypeInteger:
+idx = self.driving_target_idx.GetIntegerValue(42)
+if self.driving_target_idx.GetType() == lldb.eStructuredDataTypeString:
+idx = int(self.driving_target_idx.GetStringValue(100))
+self.driving_target = self.target.GetDebugger().GetTargetAtIndex(idx)
+
+if launched_driving_process:
+self.driving_process = self.driving_target.GetProcess()
+for driving_thread in self.driving_process:
+structured_data = lldb.SBStructuredData()
+structured_data.SetFromJSON(json.dumps({
+"driving_target_idx" : idx,
+"thread_idx" : driving_thread.GetIndexID()
+}))
+
+self.threads[driving_thread.GetThreadID()] = PassthruScriptedThread(self, structured_data)
+
+for module in self.driving_target.modules:
+path = module.file.fullpath
+load_addr = module.GetObjectFileHeaderAddress().GetLoadAddress(self.driving_target)
+self.loaded_images.append({"path": path, "load_addr": load_addr})
+
+def get_memory_region_containing_address(self, addr: int) -> lldb.SBMemoryRegionInfo:
+mem_region = lldb.SBMemoryRegionInfo()
+error = self.driving_process.GetMemoryRegionInfo(addr, mem_region)
+if error.Fail():
+return None
+return mem_region
+
+def read_memory_at_address(self, addr: int, size: int, error: lldb.SBError) -> lldb.SBData:
+data = lldb.SBData()
+bytes_read = self.driving_process.ReadMemory(addr, size, error)
+
+if error.Fail():
+return data
+
+data.SetDataWithOwnership(error, bytes_read,
+  self.driving_target.GetByteOrder(),
+  self.driving_target.GetAddressByteSize())
+
+return data
+
+def write_memory_at_address(self, addr: int, data: lldb.SBData, error: lldb.SBError) 

[Lldb-commits] [PATCH] D148397: [lldb/Utility] Add opt-in passthrough mode to event listeners

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D148397#4285143 , @mib wrote:

> In D148397#4275792 , @JDevlieghere 
> wrote:
>
>> I find "passthrough" somewhat confusing in this context. When using a 
>> listener in this new mode, where does the event go after it has pass trough 
>> this event listener? Maybe my understanding of how the event listeners is 
>> incomplete, but I was under the impression that normally there's a single 
>> listener for a certain kind of event. IIUC, in this new mode, you can have 
>> multiple listeners, where you can set one (or as Alex pointed out, possibly 
>> more) listeners that can observe events but not necessary deal with them. If 
>> that's an accurate description of the interactions, then it seems like 
>> there's more of a one-to-many relationship between events and listeners 
>> rather than what passthrough implies. I think a better name would be 
>> something like "passive listener" or "non-handling" listener or something.
>
> @JDevlieghere You could also have multiple listeners for a certain type of 
> event, but events would be considered "handled" only when they're popped from 
> the listener passed during the process creation (or the debugger listener if 
> no listener is provided in the {launch,attach}info). The idea behind 
> passthrough listener was to have a side channel where all the events would be 
> funneled to, that would never interfere with the default listener and 
> therefore not change the process execution behavior. If you really feel 
> strongly about the "passthrough" name, I can rename it to whatever you like 
> but I feel like it's pretty self explanatory.

To me, passthrough means something sitting in-between something else. It's 
still not clear to me what the passthrough listener is sitting in between. It 
sounds like it's meant to be a side channel, so maybe shadow listener would be 
a better name? I don't care so much about the name per se but rather about 
making it obvious what it does.


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

https://reviews.llvm.org/D148397

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

[Lldb-commits] [PATCH] D148548: [lldb] Improve breakpoint management for interactive scripted process

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

LGTM but I'll hold off on accepting to give Alex a change to take another look 
as he had more significant comments.




Comment at: lldb/source/Interpreter/ScriptInterpreter.cpp:78-91
 lldb::DataExtractorSP
 ScriptInterpreter::GetDataExtractorFromSBData(const lldb::SBData &data) const {
   return data.m_opaque_sp;
 }
 
+lldb::BreakpointSP ScriptInterpreter::GetOpaqueTypeFromSBBreakpoint(
+const lldb::SBBreakpoint &breakpoint) const {

NTBF for this patch but we should really be using templates here to avoid code 
duplication. 



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp:124
+  if (!CheckStructuredDataObject(LLVM_PRETTY_FUNCTION, obj, error))
+return {};
+

return false


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

https://reviews.llvm.org/D148548

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

[Lldb-commits] [PATCH] D148863: Make sure SelectMostRelevantFrame happens only when returning to the user

[Jim Ingham via Phabricator via lldb-commits]

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

This is a user facing action, it is meant to focus the user's attention on
something other than the 0th frame when you stop somewhere where that's
helpful. For instance, stopping in pthread_kill after an assert will select
the assert frame.

  

This is not something you want to have happen internally in lldb, both
because internally you really don't want the selected frame changing out
from under you, and because the recognizers can do arbitrary work, and that
can cause deadlocks or other unexpected behavior.

  

However, it's not something that the current code does
explicitly after a stop has been delivered, it's expected to happen implicitly
as part of stopping.  I changing this to call SMRF explicitly after a user
stop, but that got pretty ugly quickly.

  

So I added a bool to control whether to run this and audited all the current
uses to determine whether we're returning to the user or not.

  

This is currently a little ad hoc because there isn't a formal way to
determine why we're fetching events.  It would certainly be cleaner to
pass around a "process control baton" that would hold the history of
who is currently running the target, and on whose behalf, so we know:
"we're stopping because we completed a user request", or "we're continuing
because we're running an expression for the user" vrs. "we're continuing
because we're running an expression to allocate memory for another expression",
etc.

  

But that's a much bigger project...


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148863

Files:
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Target/StackFrameList.h
  lldb/include/lldb/Target/Thread.h
  lldb/source/API/SBThread.cpp
  lldb/source/Commands/CommandObjectFrame.cpp
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/CommandObjectType.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/source/Core/ValueObject.cpp
  lldb/source/Expression/REPL.cpp
  lldb/source/Plugins/InstrumentationRuntime/ASan/InstrumentationRuntimeASan.cpp
  
lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp
  lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
  
lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
  lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp
  lldb/source/Target/ExecutionContext.cpp
  lldb/source/Target/Platform.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/StackFrameList.cpp
  lldb/source/Target/StopInfo.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/Thread.cpp

Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -263,10 +263,10 @@
new ThreadEventData(this->shared_from_this(), new_frame_id));
 }
 
-lldb::StackFrameSP Thread::GetSelectedFrame() {
+lldb::StackFrameSP Thread::GetSelectedFrame(bool select_most_relevant) {
   StackFrameListSP stack_frame_list_sp(GetStackFrameList());
   StackFrameSP frame_sp = stack_frame_list_sp->GetFrameAtIndex(
-  stack_frame_list_sp->GetSelectedFrameIndex());
+  stack_frame_list_sp->GetSelectedFrameIndex(select_most_relevant));
   FrameSelectedCallback(frame_sp.get());
   return frame_sp;
 }
@@ -297,7 +297,7 @@
   const bool broadcast = true;
   bool success = SetSelectedFrameByIndex(frame_idx, broadcast);
   if (success) {
-StackFrameSP frame_sp = GetSelectedFrame();
+StackFrameSP frame_sp = GetSelectedFrame(false);
 if (frame_sp) {
   bool already_shown = false;
   SymbolContext frame_sc(
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3358,9 +3358,10 @@
 if (async) {
   process_sp->RestoreProcessEvents();
 } else {
+  // We are stopping all the way out to the user, so update selected frames.
   state = process_sp->WaitForProcessToStop(std::nullopt, nullptr, false,
attach_info.GetHijackListener(),
-   stream);
+   stream, true, true);
   process_sp->RestoreProcessEvents();
 
   if (state != eStateStopped) {
Index: lldb/source/Target/StopInfo.cpp
===
--- lldb/source/Target/StopInfo.cpp
+++ lldb/source/Target/StopInfo.cpp
@@ -1464,7 +1464,7 @@
 return ValueObjectSP();
   }
 
-  StackFram

[Lldb-commits] [PATCH] D148863: Make sure SelectMostRelevantFrame happens only when returning to the user

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a subscriber: dexonsmith.
JDevlieghere added a comment.

I looked at all the call sites and they all seemed reasonable in terms of doing 
work on behalf of the user or not and selecting the most relevant frame 
respectively. My only concern is that we now have a bunch of places where we're 
passing a blind `true` or `false`. While it's pretty obvious in this patch, it 
won't be so obvious when you're reading the same code in the future, let alone 
when someone inevitably copy-pastes it. I can think of two easy ways to improve 
readability:

1. Add an inline comment:

  StackFrameSP frame_sp = GetSelectedFrame(/*select_most_relevant=*/false);



2. Add an enum to `Frame`:

  enum SelectMostRelevant : bool {
SelectMostRelevantFrame = true,
DoNoSelectMostRelevantFrame = false,
  };



  StackFrameSP frame_sp = GetSelectedFrame(SelectMostRelevantFrame);

This is a trick that @dexonsmith thought me a long time ago. I personally like 
it the best because it makes things very explicit without the hassle of having 
to actually treat the value like an enum.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148863

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

[Lldb-commits] [lldb] c5fc780 - [lldb] Fix -Wctad-maybe-unsupported in PathMappingList.cpp (NFC)

[Jie Fu via lldb-commits]

Author: Jie Fu
Date: 2023-04-21T11:59:38+08:00
New Revision: c5fc7809e05940674424aaed7dd06c6be0639864

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

LOG: [lldb] Fix -Wctad-maybe-unsupported in PathMappingList.cpp (NFC)

/home/jiefu/llvm-project/lldb/source/Target/PathMappingList.cpp:51:5: error: 
'scoped_lock' may not intend to support class template argument deduction 
[-Werror,-Wctad-maybe-unsupported]
std::scoped_lock locks(m_mutex, rhs.m_mutex);
^
/usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/mutex:692:11: note: 
add a deduction guide to suppress this warning
class scoped_lock
  ^
/home/jiefu/llvm-project/lldb/source/Target/PathMappingList.cpp:72:3: error: 
'scoped_lock' may not intend to support class template argument deduction 
[-Werror,-Wctad-maybe-unsupported]
  std::scoped_lock locks(m_mutex, rhs.m_mutex);
  ^
/usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/mutex:692:11: note: 
add a deduction guide to suppress this warning
class scoped_lock
  ^
2 errors generated.

Added: 


Modified: 
lldb/source/Target/PathMappingList.cpp

Removed: 




diff  --git a/lldb/source/Target/PathMappingList.cpp 
b/lldb/source/Target/PathMappingList.cpp
index 4efc0bf16c6d2..c369018122a59 100644
--- a/lldb/source/Target/PathMappingList.cpp
+++ b/lldb/source/Target/PathMappingList.cpp
@@ -48,7 +48,7 @@ PathMappingList::PathMappingList(const PathMappingList &rhs)
 
 const PathMappingList &PathMappingList::operator=(const PathMappingList &rhs) {
   if (this != &rhs) {
-std::scoped_lock locks(m_mutex, rhs.m_mutex);
+std::scoped_lock 
locks(m_mutex, rhs.m_mutex);
 m_pairs = rhs.m_pairs;
 m_callback = nullptr;
 m_callback_baton = nullptr;
@@ -69,7 +69,7 @@ void PathMappingList::Append(llvm::StringRef path, 
llvm::StringRef replacement,
 }
 
 void PathMappingList::Append(const PathMappingList &rhs, bool notify) {
-  std::scoped_lock locks(m_mutex, rhs.m_mutex);
+  std::scoped_lock locks(m_mutex, 
rhs.m_mutex);
   ++m_mod_id;
   if (!rhs.m_pairs.empty()) {
 const_iterator pos, end = rhs.m_pairs.end();



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