date:20220520

[Lldb-commits] [PATCH] D126057: [lldb] Fix that empty target.run-args are not actually used when launching process

2022-05-20 Thread Raphael Isemann via Phabricator via lldb-commits

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

`GetPropertyAtIndexAsArgs` returns true on success and false on failure. Right
now it returns the converted `size_t` returned from `GetArgs` which describes
the number of arguments in the argument list. So for empty argument lists
(`(size_t)0` -> `(bool)false`) this function always fails.

The only observable effect of this seems to be that empty arguments are never
propagated to the internal LaunchInfo for a process. This causes that once any
argument has been added to `target.run-args`, clearing `target.run-args` doesn't
have any effect.

Fixes issue #55568


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126057

Files:
  lldb/source/Interpreter/OptionValueProperties.cpp
  lldb/test/API/python_api/target/TestTargetAPI.py


Index: lldb/test/API/python_api/target/TestTargetAPI.py
===
--- lldb/test/API/python_api/target/TestTargetAPI.py
+++ lldb/test/API/python_api/target/TestTargetAPI.py
@@ -196,6 +196,15 @@
 self.assertIn('arg: foo', output)
 self.assertIn('env: bar=baz', output)
 
+# Clear all the run args set above.
+self.runCmd("setting clear target.run-args")
+process = target.LaunchSimple(None, None,
+  self.get_process_working_directory())
+process.Continue()
+self.assertEqual(process.GetState(), lldb.eStateExited)
+output = process.GetSTDOUT()
+self.assertNotIn('arg: foo', output)
+
 self.runCmd("settings set target.disable-stdio true")
 process = target.LaunchSimple(
 None, None, self.get_process_working_directory())
Index: lldb/source/Interpreter/OptionValueProperties.cpp
===
--- lldb/source/Interpreter/OptionValueProperties.cpp
+++ lldb/source/Interpreter/OptionValueProperties.cpp
@@ -248,16 +248,22 @@
 return false;
 
   const OptionValueArgs *arguments = value->GetAsArgs();
-  if (arguments)
-return arguments->GetArgs(args);
+  if (arguments) {
+arguments->GetArgs(args);
+return true;
+  }
 
   const OptionValueArray *array = value->GetAsArray();
-  if (array)
-return array->GetArgs(args);
+  if (array) {
+array->GetArgs(args);
+return true;
+  }
 
   const OptionValueDictionary *dict = value->GetAsDictionary();
-  if (dict)
-return dict->GetArgs(args);
+  if (dict) {
+dict->GetArgs(args);
+return true;
+  }
 
   return false;
 }


Index: lldb/test/API/python_api/target/TestTargetAPI.py
===
--- lldb/test/API/python_api/target/TestTargetAPI.py
+++ lldb/test/API/python_api/target/TestTargetAPI.py
@@ -196,6 +196,15 @@
 self.assertIn('arg: foo', output)
 self.assertIn('env: bar=baz', output)
 
+# Clear all the run args set above.
+self.runCmd("setting clear target.run-args")
+process = target.LaunchSimple(None, None,
+  self.get_process_working_directory())
+process.Continue()
+self.assertEqual(process.GetState(), lldb.eStateExited)
+output = process.GetSTDOUT()
+self.assertNotIn('arg: foo', output)
+
 self.runCmd("settings set target.disable-stdio true")
 process = target.LaunchSimple(
 None, None, self.get_process_working_directory())
Index: lldb/source/Interpreter/OptionValueProperties.cpp
===
--- lldb/source/Interpreter/OptionValueProperties.cpp
+++ lldb/source/Interpreter/OptionValueProperties.cpp
@@ -248,16 +248,22 @@
 return false;
 
   const OptionValueArgs *arguments = value->GetAsArgs();
-  if (arguments)
-return arguments->GetArgs(args);
+  if (arguments) {
+arguments->GetArgs(args);
+return true;
+  }
 
   const OptionValueArray *array = value->GetAsArray();
-  if (array)
-return array->GetArgs(args);
+  if (array) {
+array->GetArgs(args);
+return true;
+  }
 
   const OptionValueDictionary *dict = value->GetAsDictionary();
-  if (dict)
-return dict->GetArgs(args);
+  if (dict) {
+dict->GetArgs(args);
+return true;
+  }
 
   return false;
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D126057: [lldb] Fix that empty target.run-args are not actually used when launching process

2022-05-20 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor added a comment.

Don't have access to a macOS machine that can run the tests, so maybe someone 
should give this a spin before landing :)




Comment at: lldb/source/Interpreter/OptionValueProperties.cpp:257
   const OptionValueArray *array = value->GetAsArray();
-  if (array)
-return array->GetArgs(args);
+  if (array) {
+array->GetArgs(args);

The fix for the test is the change a few lines above. This change and the one 
below seem to have the same bug. Not sure if any properties are using this code 
at the moment, so technically this is an untested chance. I can drop if people 
care.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126057

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

[Lldb-commits] [PATCH] D125575: [lldb] [llgs] Implement non-stop style stop notification packets

2022-05-20 Thread Michał Górny via Phabricator via lldb-commits

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

Rebase.


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

https://reviews.llvm.org/D125575

Files:
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/test/API/tools/lldb-server/TestLldbGdbServer.py

Index: lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
===
--- lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
+++ lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
@@ -1410,3 +1410,80 @@
 self.assertEqual(decoded[errno_idx], 0)  # si_errno
 self.assertEqual(decoded[code_idx], SEGV_MAPERR)  # si_code
 self.assertEqual(decoded[addr_idx], 0)  # si_addr
+
+def test_QNonStop(self):
+self.build()
+self.set_inferior_startup_launch()
+thread_num = 3
+procs = self.prep_debug_monitor_and_inferior(
+inferior_args=["thread:segfault"] + thread_num * ["thread:new"])
+self.test_sequence.add_log_lines(
+["read packet: $QNonStop:1#00",
+ "send packet: $OK#00",
+ "read packet: $c#63",
+ ], True)
+self.expect_gdbremote_sequence()
+
+segv_signo = lldbutil.get_signal_number('SIGSEGV')
+all_threads = set()
+all_segv_threads = []
+
+# we should get segfaults from all the threads
+for segv_no in range(thread_num):
+# first wait for the notification event
+self.reset_test_sequence()
+self.test_sequence.add_log_lines(
+[{"direction": "send",
+  "regex": r"^%Stop:T([0-9a-fA-F]{2})thread:([0-9a-fA-F]+);",
+  "capture": {1: "signo", 2: "thread_id"},
+  },
+ ], True)
+threads = [self.expect_gdbremote_sequence()]
+
+# then we may get events for the remaining threads
+# (but note that not all threads may have been started yet)
+while True:
+self.reset_test_sequence()
+self.test_sequence.add_log_lines(
+["read packet: $vStopped#00",
+ {"direction": "send",
+  "regex": r"^\$(OK|T([0-9a-fA-F]{2})thread:([0-9a-fA-F]+);)",
+  "capture": {1: "packet", 2: "signo", 3: "thread_id"},
+  },
+ ], True)
+m = self.expect_gdbremote_sequence()
+if m["packet"] == "OK":
+break
+threads.append(m)
+
+segv_threads = []
+other_threads = []
+for t in threads:
+signo = int(t["signo"], 16)
+if signo == segv_signo:
+segv_threads.append(t["thread_id"])
+else:
+self.assertEqual(signo, 0)
+other_threads.append(t["thread_id"])
+
+# verify that exactly one thread segfaulted
+self.assertEqual(len(segv_threads), 1)
+# we should get only one segv from every thread
+self.assertNotIn(segv_threads[0], all_segv_threads)
+all_segv_threads.extend(segv_threads)
+# segv_threads + other_threads should always be a superset
+# of all_threads, i.e. we should get states for all threads
+# already started
+self.assertFalse(
+all_threads.difference(other_threads + segv_threads))
+all_threads.update(other_threads + segv_threads)
+
+self.reset_test_sequence()
+self.test_sequence.add_log_lines(
+["read packet: $vCont;C{:02x}:{};c#00"
+ .format(segv_signo, segv_threads[0]),
+ ], True)
+self.expect_gdbremote_sequence()
+
+# finally, verify that all threads have started
+self.assertEqual(len(all_threads), thread_num + 1)
Index: lldb/source/Utility/StringExtractorGDBRemote.cpp
===
--- lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ lldb/source/Utility/StringExtractorGDBRemote.cpp
@@ -150,6 +150,11 @@
 return eServerPacketType_QMemTags;
   break;
 
+case 'N':
+  if (PACKET_STARTS_WITH("QNonStop:"))
+return eServerPacketType_QNonStop;
+  break;
+
 case 'R':
   if (PACKET_STARTS_WITH("QRestoreRegisterSta

[Lldb-commits] [lldb] 80c836e - [lldb] Disable scripted_crashlog_json.test on Apple Silicon

2022-05-20 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2022-05-20T08:24:46-07:00
New Revision: 80c836ec557acd8ffb9fd6b3483dc75c4bcf0b11

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

LOG: [lldb] Disable scripted_crashlog_json.test on Apple Silicon

Disable scripted_crashlog_json.test on Apple Silicon until Ismail has
bandwidth to investigate.

rdar://93655633

Added: 


Modified: 

lldb/test/Shell/ScriptInterpreter/Python/Crashlog/scripted_crashlog_json.test

Removed: 




diff  --git 
a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/scripted_crashlog_json.test 
b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/scripted_crashlog_json.test
index 4cf61599dd877..2bf8d3e022baf 100644
--- 
a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/scripted_crashlog_json.test
+++ 
b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/scripted_crashlog_json.test
@@ -1,5 +1,8 @@
 # REQUIRES: python, native && target-aarch64 && system-darwin
 
+# rdar://93655633
+# UNSUPPORTED: arm64
+
 # RUN: %clangxx_host -std=c++17 -g %S/Inputs/multithread-test.cc -o %t.out
 
 # RUN: cp %S/Inputs/scripted_crashlog.ips %t.crash



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

[Lldb-commits] [PATCH] D126057: [lldb] Fix that empty target.run-args are not actually used when launching process

2022-05-20 Thread 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. Passes the test suite on macOS.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126057

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

[Lldb-commits] [PATCH] D124731: [lldb] Consider binary as module of last resort

2022-05-20 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere accepted this revision.
JDevlieghere added a comment.

No worries, thank you for your patience. This LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124731

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

[Lldb-commits] [PATCH] D126076: [lldb] Set correct register number for cpsr (GENERIC_REGNUM_FLAGS)

2022-05-20 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere created this revision.
JDevlieghere added a reviewer: jasonmolenda.
Herald added a project: All.
JDevlieghere requested review of this revision.

Report the correct register number (GENERIC_REGNUM_FLAGS) for cpsr. This fixes 
TestLldbGdbServer.py on Apple Silicon.


https://reviews.llvm.org/D126076

Files:
  lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp


Index: lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
===
--- lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
+++ lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
@@ -1643,7 +1643,7 @@
 // used for
 // userland debugging.
 {e_regSetGPR, gpr_cpsr, "cpsr", "flags", Uint, Hex, 4,
- GPR_OFFSET_NAME(cpsr), dwarf_elr_mode, dwarf_elr_mode, INVALID_NUB_REGNUM,
+ GPR_OFFSET_NAME(cpsr), dwarf_elr_mode, dwarf_elr_mode, 
GENERIC_REGNUM_FLAGS,
  debugserver_gpr_cpsr, NULL, NULL},
 
 DEFINE_PSEUDO_GPR_IDX(0, w0),


Index: lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
===
--- lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
+++ lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
@@ -1643,7 +1643,7 @@
 // used for
 // userland debugging.
 {e_regSetGPR, gpr_cpsr, "cpsr", "flags", Uint, Hex, 4,
- GPR_OFFSET_NAME(cpsr), dwarf_elr_mode, dwarf_elr_mode, INVALID_NUB_REGNUM,
+ GPR_OFFSET_NAME(cpsr), dwarf_elr_mode, dwarf_elr_mode, GENERIC_REGNUM_FLAGS,
  debugserver_gpr_cpsr, NULL, NULL},
 
 DEFINE_PSEUDO_GPR_IDX(0, w0),
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D126076: [lldb] Set correct register number for cpsr (GENERIC_REGNUM_FLAGS)

2022-05-20 Thread Jason Molenda via Phabricator via lldb-commits

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

Ah, good catch!


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

https://reviews.llvm.org/D126076

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

[Lldb-commits] [PATCH] D126057: [lldb] Fix that empty target.run-args are not actually used when launching process

2022-05-20 Thread Jim Ingham via Phabricator via lldb-commits

jingham accepted this revision.
jingham added a comment.

GetPropertyAtIndexAsArg isn't actually documented, so we don't know what the 
original intent for the return value was.  But in almost all the cases where we 
look up a property using this function, when we use the enum directly we 
discard the result.  In the only other case (GetUserSpecifiedTrapHandlerNames) 
were we do return the result of the GetProperty... function, the return result 
of the GetUser... function is always discarded.  So it seems pretty clear that 
the result was just supposed to be "couldn't find the property".  It's also 
weird to conflate "couldn't find property" with "property has an empty value".

But then that leads one to wonder why TargetProperties::GetRunArguments is 
returning a bool not a void.  Does that relay any useful information?  It's not 
like we're going to remove the property at ePropertyRunArgs but leave the 
GetRunArguments function around, thus rendering useful the check on return 
value?  And if the intent was to return info about elements in the array, 
returning the number of elements makes some sense, but an "empty" bool is odd 
and not in line with the way we usually do things.

The other question is - given that this function used to mean "true if there 
was more than one arg, false if there were none" - whether there was any reason 
why when someone reset the target run-args to an empty args set we wouldn't 
want to update the launch info.  That really seemed the intent of the original 
code.  Was there some instance where we would inadvertently empty the run args 
property w/o wanting to update the next run of that target?  Anyway, I couldn't 
think of any good reason for that, and it clearly has this undesirable side 
effect.  So I think that was just a case of "it has a return value I should 
check it" rather than intending to do something special for empty run args.

TL;DR This looks okay to me.  I think it would be good to also change 
GetRunArguments to remove the now useless bool return value, but the patch is 
also fine without that.  I think you should keep the other changes, it would be 
confusing for this to be inconsistent and it's not like those changes are going 
to have subtle bugs in the change.

I'd keep the other two changes.  We should be consistent, and it's not like 
this change itself is likely to have subtle bugs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126057

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

[Lldb-commits] [lldb] a1cf154 - [lldb] Set correct register number for cpsr (GENERIC_REGNUM_FLAGS)

2022-05-20 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2022-05-20T09:36:58-07:00
New Revision: a1cf154dd476e021a4c7924e49fac85da01cecf8

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

LOG: [lldb] Set correct register number for cpsr (GENERIC_REGNUM_FLAGS)

Report the correct register number (GENERIC_REGNUM_FLAGS) for cpsr. This
fixes TestLldbGdbServer.py on Apple Silicon.

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

Added: 


Modified: 
lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp

Removed: 




diff  --git a/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp 
b/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
index e065718c7df94..fa4ba5f67263a 100644
--- a/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
+++ b/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
@@ -1643,7 +1643,7 @@ const DNBRegisterInfo DNBArchMachARM64::g_gpr_registers[] 
= {
 // used for
 // userland debugging.
 {e_regSetGPR, gpr_cpsr, "cpsr", "flags", Uint, Hex, 4,
- GPR_OFFSET_NAME(cpsr), dwarf_elr_mode, dwarf_elr_mode, INVALID_NUB_REGNUM,
+ GPR_OFFSET_NAME(cpsr), dwarf_elr_mode, dwarf_elr_mode, 
GENERIC_REGNUM_FLAGS,
  debugserver_gpr_cpsr, NULL, NULL},
 
 DEFINE_PSEUDO_GPR_IDX(0, w0),



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

[Lldb-commits] [PATCH] D126076: [lldb] Set correct register number for cpsr (GENERIC_REGNUM_FLAGS)

2022-05-20 Thread Jonas Devlieghere via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa1cf154dd476: [lldb] Set correct register number for cpsr 
(GENERIC_REGNUM_FLAGS) (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126076

Files:
  lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp


Index: lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
===
--- lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
+++ lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
@@ -1643,7 +1643,7 @@
 // used for
 // userland debugging.
 {e_regSetGPR, gpr_cpsr, "cpsr", "flags", Uint, Hex, 4,
- GPR_OFFSET_NAME(cpsr), dwarf_elr_mode, dwarf_elr_mode, INVALID_NUB_REGNUM,
+ GPR_OFFSET_NAME(cpsr), dwarf_elr_mode, dwarf_elr_mode, 
GENERIC_REGNUM_FLAGS,
  debugserver_gpr_cpsr, NULL, NULL},
 
 DEFINE_PSEUDO_GPR_IDX(0, w0),


Index: lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
===
--- lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
+++ lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
@@ -1643,7 +1643,7 @@
 // used for
 // userland debugging.
 {e_regSetGPR, gpr_cpsr, "cpsr", "flags", Uint, Hex, 4,
- GPR_OFFSET_NAME(cpsr), dwarf_elr_mode, dwarf_elr_mode, INVALID_NUB_REGNUM,
+ GPR_OFFSET_NAME(cpsr), dwarf_elr_mode, dwarf_elr_mode, GENERIC_REGNUM_FLAGS,
  debugserver_gpr_cpsr, NULL, NULL},
 
 DEFINE_PSEUDO_GPR_IDX(0, w0),
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D126013: Add [opt] suffix to optimized stack frame in lldb-vscode

2022-05-20 Thread jeffrey tan via Phabricator via lldb-commits

yinghuitan added inline comments.



Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:762
+  } else {
+EmplaceSafeString(object, "name", frame.GetFunctionName());
+  }

clayborg wrote:
> We should probably get the function name with "const char *func_name = 
> frame.GetFunctionName();" before the entire "if (is_optimized)" and use it 
> here, and above
Thanks, seems we have this potential crash in existing code already. I will fix 
it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126013

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

[Lldb-commits] [PATCH] D126078: [lldb] Improve formatting of dlopen error messages (NFC)

2022-05-20 Thread Dave Lee via Phabricator via lldb-commits

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

Ensure there's a space between "utility" and "function", and also makes
it easier to grep/search for "utility function".

While making this change, I also re-formatted the other dlopen error messages
(with clang-format). This fix other instances of spaces missing between words,
and makes each of these strings fit a single line, making them greppable.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126078

Files:
  lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp

Index: lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
===
--- lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
+++ lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
@@ -613,9 +613,9 @@
   std::move(expr), dlopen_wrapper_name, eLanguageTypeC_plus_plus, exe_ctx);
   if (!utility_fn_or_error) {
 std::string error_str = llvm::toString(utility_fn_or_error.takeError());
-error.SetErrorStringWithFormat("dlopen error: could not create utility"
-   "function: %s",
-   error_str.c_str());
+error.SetErrorStringWithFormat(
+"dlopen error: could not create utility function: %s",
+error_str.c_str());
 return nullptr;
   }
   std::unique_ptr dlopen_utility_func_up =
@@ -650,8 +650,9 @@
   do_dlopen_function = dlopen_utility_func_up->MakeFunctionCaller(
   clang_void_pointer_type, arguments, exe_ctx.GetThreadSP(), utility_error);
   if (utility_error.Fail()) {
-error.SetErrorStringWithFormat("dlopen error: could not make function"
-   "caller: %s", utility_error.AsCString());
+error.SetErrorStringWithFormat(
+"dlopen error: could not make function caller: %s",
+utility_error.AsCString());
 return nullptr;
   }
   
@@ -717,8 +718,9 @@
permissions,
utility_error);
   if (path_addr == LLDB_INVALID_ADDRESS) {
-error.SetErrorStringWithFormat("dlopen error: could not allocate memory"
-"for path: %s", utility_error.AsCString());
+error.SetErrorStringWithFormat(
+"dlopen error: could not allocate memory for path: %s",
+utility_error.AsCString());
 return LLDB_INVALID_IMAGE_TOKEN;
   }
 
@@ -730,8 +732,9 @@
 
   process->WriteMemory(path_addr, path.c_str(), path_len, utility_error);
   if (utility_error.Fail()) {
-error.SetErrorStringWithFormat("dlopen error: could not write path string:"
-" %s", utility_error.AsCString());
+error.SetErrorStringWithFormat(
+"dlopen error: could not write path string: %s",
+utility_error.AsCString());
 return LLDB_INVALID_IMAGE_TOKEN;
   }
   
@@ -742,8 +745,9 @@
   permissions,
   utility_error);
   if (utility_error.Fail()) {
-error.SetErrorStringWithFormat("dlopen error: could not allocate memory"
-"for path: %s", utility_error.AsCString());
+error.SetErrorStringWithFormat(
+"dlopen error: could not allocate memory for path: %s",
+utility_error.AsCString());
 return LLDB_INVALID_IMAGE_TOKEN;
   }
   
@@ -791,9 +795,9 @@
   permissions,
   utility_error);
 if (path_array_addr == LLDB_INVALID_ADDRESS) {
-  error.SetErrorStringWithFormat("dlopen error: could not allocate memory"
-  "for path array: %s", 
-  utility_error.AsCString());
+  error.SetErrorStringWithFormat(
+  "dlopen error: could not allocate memory for path array: %s",
+  utility_error.AsCString());
   return LLDB_INVALID_IMAGE_TOKEN;
 }
 
@@ -807,8 +811,9 @@
  path_array.size(), utility_error);
 
 if (utility_error.Fail()) {
-  error.SetErrorStringWithFormat("dlopen error: could not write path array:"
- " %s", utility_error.AsCString());
+  error.SetErrorStringWithFormat(
+  "dlopen error: could not write path array: %s",
+  utility_error.AsCString());
   return LLDB_INVALID_IMAGE_TOKEN;
 }
 // Now make spaces in the target for the buffer.  We need to add one for
@@ -819,9 +824,9 @@
   permissions,
   utility_error);
 if (buffer_addr == LLDB_INVALID_ADDRESS) {
-  error.SetErrorStringWit

[Lldb-commits] [PATCH] D126078: [lldb] Improve formatting of dlopen error messages (NFC)

2022-05-20 Thread 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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126078

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

[Lldb-commits] [PATCH] D126013: Add [opt] suffix to optimized stack frame in lldb-vscode

2022-05-20 Thread jeffrey tan via Phabricator via lldb-commits

yinghuitan updated this revision to Diff 431008.
yinghuitan added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126013

Files:
  lldb/test/API/tools/lldb-vscode/optimized/Makefile
  lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py
  lldb/test/API/tools/lldb-vscode/optimized/main.cpp
  lldb/tools/lldb-vscode/JSONUtils.cpp


Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -751,7 +751,19 @@
   llvm::json::Object object;
   int64_t frame_id = MakeVSCodeFrameID(frame);
   object.try_emplace("id", frame_id);
-  EmplaceSafeString(object, "name", frame.GetFunctionName());
+
+  std::string frame_name;
+  const char *func_name = frame.GetFunctionName();
+  if (func_name)
+frame_name = func_name;
+  else
+frame_name = "";
+  bool is_optimized = frame.GetFunction().GetIsOptimized();
+  if (is_optimized)
+frame_name += " [opt]";
+  EmplaceSafeString(object, "name", frame_name);
+  object.try_emplace("optimized", is_optimized);
+
   int64_t disasm_line = 0;
   object.try_emplace("source", CreateSource(frame, disasm_line));
 
Index: lldb/test/API/tools/lldb-vscode/optimized/main.cpp
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/optimized/main.cpp
@@ -0,0 +1,16 @@
+#include 
+#include 
+
+int foo(int x, int y) {
+  printf("Got input %d, %d\n", x, y);
+  return x + y + 3; // breakpoint 1
+}
+
+int main(int argc, char const *argv[]) {
+  int optimized = argc > 1 ? std::stoi(argv[1]) : 0;
+
+  printf("argc: %d, optimized: %d\n", argc, optimized);
+  int result = foo(argc, 20);
+  printf("result: %d\n", result); // breakpoint 2
+  return 0;
+}
Index: lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py
@@ -0,0 +1,35 @@
+"""
+Test lldb-vscode variables/stackTrace request for optimized code
+"""
+
+from __future__ import print_function
+
+import unittest2
+import vscode
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+import lldbvscode_testcase
+
+
+class TestVSCode_optimized(lldbvscode_testcase.VSCodeTestCaseBase):
+mydir = TestBase.compute_mydir(__file__)
+
+@skipIfWindows
+@skipIfRemote
+def test_stack_frame_name(self):
+''' Test optimized frame has special name suffix.
+'''
+program = self.getBuildArtifact("a.out")
+self.build_and_launch(program)
+source = 'main.cpp'
+breakpoint_line = line_number(source, '// breakpoint 1')
+lines = [breakpoint_line]
+breakpoint_ids = self.set_source_breakpoints(source, lines)
+self.assertEqual(len(breakpoint_ids), len(lines),
+"expect correct number of breakpoints")
+self.continue_to_breakpoints(breakpoint_ids)
+leaf_frame = self.vscode.get_stackFrame(frameIndex=0)
+self.assertTrue(leaf_frame['name'].endswith(' [opt]'))
+parent_frame = self.vscode.get_stackFrame(frameIndex=1)
+self.assertTrue(parent_frame['name'].endswith(' [opt]'))
Index: lldb/test/API/tools/lldb-vscode/optimized/Makefile
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/optimized/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+CXXFLAGS_EXTRAS := -O3 -glldb
+include Makefile.rules


Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -751,7 +751,19 @@
   llvm::json::Object object;
   int64_t frame_id = MakeVSCodeFrameID(frame);
   object.try_emplace("id", frame_id);
-  EmplaceSafeString(object, "name", frame.GetFunctionName());
+
+  std::string frame_name;
+  const char *func_name = frame.GetFunctionName();
+  if (func_name)
+frame_name = func_name;
+  else
+frame_name = "";
+  bool is_optimized = frame.GetFunction().GetIsOptimized();
+  if (is_optimized)
+frame_name += " [opt]";
+  EmplaceSafeString(object, "name", frame_name);
+  object.try_emplace("optimized", is_optimized);
+
   int64_t disasm_line = 0;
   object.try_emplace("source", CreateSource(frame, disasm_line));
 
Index: lldb/test/API/tools/lldb-vscode/optimized/main.cpp
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/optimized/main.cpp
@@ -0,0 +1,16 @@
+#include 
+#include 
+
+int foo(int x, int y) {
+  printf("Got input %d, %d\n", x, y);
+  return x + y + 3; // breakpoint 1
+}
+
+int main(int argc, char const *argv[]) {
+

[Lldb-commits] [PATCH] D126080: Adapt C++ std::string dataformatter for D125496

2022-05-20 Thread Adrian Prantl via Phabricator via lldb-commits

aprantl created this revision.
aprantl added reviewers: ldionne, shafik, jingham.
aprantl added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
aprantl requested review of this revision.

https://reviews.llvm.org/D125496 changed the layout of std::string without 
updating the LLDB dataformatter. This patch adds code to recognize the now 
format.

How do we usually add tests for STL data structures?


https://reviews.llvm.org/D126080

Files:
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp


Index: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -546,7 +546,7 @@
nullptr, nullptr, &valobj, false, 
false);
 }
 
-// the field layout in a libc++ string (cap, side, data or data, size, cap)
+/// The field layout in a libc++ string (cap, side, data or data, size, cap).
 enum LibcxxStringLayoutMode {
   eLibcxxStringLayoutModeCSD = 0,
   eLibcxxStringLayoutModeDSC = 1,
@@ -626,6 +626,9 @@
   return {};
 ValueObjectSP location_sp = short_sp->GetChildAtIndex(
 (layout == eLibcxxStringLayoutModeDSC) ? 0 : 1, true);
+// After D125496, there is a flat layout.
+if (location_sp->GetName() == g_size_name)
+  location_sp = short_sp->GetChildAtIndex(3, true);
 if (using_bitmasks)
   size = (layout == eLibcxxStringLayoutModeDSC)
  ? size_mode_value


Index: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -546,7 +546,7 @@
nullptr, nullptr, &valobj, false, false);
 }
 
-// the field layout in a libc++ string (cap, side, data or data, size, cap)
+/// The field layout in a libc++ string (cap, side, data or data, size, cap).
 enum LibcxxStringLayoutMode {
   eLibcxxStringLayoutModeCSD = 0,
   eLibcxxStringLayoutModeDSC = 1,
@@ -626,6 +626,9 @@
   return {};
 ValueObjectSP location_sp = short_sp->GetChildAtIndex(
 (layout == eLibcxxStringLayoutModeDSC) ? 0 : 1, true);
+// After D125496, there is a flat layout.
+if (location_sp->GetName() == g_size_name)
+  location_sp = short_sp->GetChildAtIndex(3, true);
 if (using_bitmasks)
   size = (layout == eLibcxxStringLayoutModeDSC)
  ? size_mode_value
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D126080: Adapt C++ std::string dataformatter for D125496

2022-05-20 Thread Adrian Prantl via Phabricator via lldb-commits

aprantl added a comment.

I'm going to land this quickly to get the bots going again, but a thorough 
review would still be very much appreciated!


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

https://reviews.llvm.org/D126080

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

[Lldb-commits] [lldb] f4570ce - Adapt C++ std::string dataformatter for D125496

2022-05-20 Thread Adrian Prantl via lldb-commits


Author: Adrian Prantl
Date: 2022-05-20T11:25:42-07:00
New Revision: f4570ce442b492d436a0e314c6ea14bf299af0d3

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

LOG: Adapt C++ std::string dataformatter for D125496

https://reviews.llvm.org/D125496 changed the layout of std::string
without updating the LLDB dataformatter. This patch adds code to
recognize the new format.

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

Added: 


Modified: 
lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
index f8052f62babd3..cd09c3e7d1e9b 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -546,7 +546,7 @@ bool 
lldb_private::formatters::LibcxxContainerSummaryProvider(
nullptr, nullptr, &valobj, false, 
false);
 }
 
-// the field layout in a libc++ string (cap, side, data or data, size, cap)
+/// The field layout in a libc++ string (cap, side, data or data, size, cap).
 enum LibcxxStringLayoutMode {
   eLibcxxStringLayoutModeCSD = 0,
   eLibcxxStringLayoutModeDSC = 1,
@@ -626,6 +626,9 @@ ExtractLibcxxStringInfo(ValueObject &valobj) {
   return {};
 ValueObjectSP location_sp = short_sp->GetChildAtIndex(
 (layout == eLibcxxStringLayoutModeDSC) ? 0 : 1, true);
+// After D125496, there is a flat layout.
+if (location_sp->GetName() == g_size_name)
+  location_sp = short_sp->GetChildAtIndex(3, true);
 if (using_bitmasks)
   size = (layout == eLibcxxStringLayoutModeDSC)
  ? size_mode_value



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

[Lldb-commits] [PATCH] D126080: Adapt C++ std::string dataformatter for D125496

2022-05-20 Thread Adrian Prantl via Phabricator via lldb-commits

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf4570ce442b4: Adapt C++ std::string dataformatter for 
D125496 (authored by aprantl).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126080

Files:
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp


Index: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -546,7 +546,7 @@
nullptr, nullptr, &valobj, false, 
false);
 }
 
-// the field layout in a libc++ string (cap, side, data or data, size, cap)
+/// The field layout in a libc++ string (cap, side, data or data, size, cap).
 enum LibcxxStringLayoutMode {
   eLibcxxStringLayoutModeCSD = 0,
   eLibcxxStringLayoutModeDSC = 1,
@@ -626,6 +626,9 @@
   return {};
 ValueObjectSP location_sp = short_sp->GetChildAtIndex(
 (layout == eLibcxxStringLayoutModeDSC) ? 0 : 1, true);
+// After D125496, there is a flat layout.
+if (location_sp->GetName() == g_size_name)
+  location_sp = short_sp->GetChildAtIndex(3, true);
 if (using_bitmasks)
   size = (layout == eLibcxxStringLayoutModeDSC)
  ? size_mode_value


Index: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -546,7 +546,7 @@
nullptr, nullptr, &valobj, false, false);
 }
 
-// the field layout in a libc++ string (cap, side, data or data, size, cap)
+/// The field layout in a libc++ string (cap, side, data or data, size, cap).
 enum LibcxxStringLayoutMode {
   eLibcxxStringLayoutModeCSD = 0,
   eLibcxxStringLayoutModeDSC = 1,
@@ -626,6 +626,9 @@
   return {};
 ValueObjectSP location_sp = short_sp->GetChildAtIndex(
 (layout == eLibcxxStringLayoutModeDSC) ? 0 : 1, true);
+// After D125496, there is a flat layout.
+if (location_sp->GetName() == g_size_name)
+  location_sp = short_sp->GetChildAtIndex(3, true);
 if (using_bitmasks)
   size = (layout == eLibcxxStringLayoutModeDSC)
  ? size_mode_value
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 9385a6d - Add some diagnostics to diagnose bot-only failures for TestIgnoredExceptions.py

2022-05-20 Thread Jim Ingham via lldb-commits


Author: Jim Ingham
Date: 2022-05-20T11:38:37-07:00
New Revision: 9385a6d6eaa3f4caebdee2eb53b5bf4ca1f1b832

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

LOG: Add some diagnostics to diagnose bot-only failures for 
TestIgnoredExceptions.py

The test for commit bff4673b41781ec5bff6b96b52cf321d2271726c is failing on the
GreenDragon bot but none of us can repro the failure locally.  Adding some 
logging
to the test failure to help diagnose the issue.

Added: 


Modified: 
lldb/test/API/macosx/ignore_exceptions/TestIgnoredExceptions.py

Removed: 




diff  --git a/lldb/test/API/macosx/ignore_exceptions/TestIgnoredExceptions.py 
b/lldb/test/API/macosx/ignore_exceptions/TestIgnoredExceptions.py
index 378ce7a7b6b01..288602199e2e4 100644
--- a/lldb/test/API/macosx/ignore_exceptions/TestIgnoredExceptions.py
+++ b/lldb/test/API/macosx/ignore_exceptions/TestIgnoredExceptions.py
@@ -42,6 +42,11 @@ def suspended_thread_test(self):
 # Now continue, and we should stop with a stop reason of SIGBUS:
 process.Continue()
 self.assertEqual(process.state, lldb.eStateStopped, "Stopped after 
continue to SIGBUS")
+if thread.stop_reason == lldb.eStopReasonBreakpoint:
+id = thread.GetStopReasonDataAtIndex(0)
+name = thread.frame[0].name
+self.fail("Hit breakpoint {0} in '{1}' rather than getting a 
SIGBUS".format(id, name))
+
 self.assertEqual(thread.stop_reason, lldb.eStopReasonSignal)
 self.assertEqual(thread.GetStopReasonDataAtIndex(0), 10, "Got a 
SIGBUS")
 



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

[Lldb-commits] [PATCH] D126081: [lldb] Fix spurious lldb_assert in PrintCommandOutput

2022-05-20 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere created this revision.
JDevlieghere added reviewers: labath, teemperor, mib.
Herald added a project: All.
JDevlieghere requested review of this revision.

When the string passed to PrintCommandOutput  doesn't end with a newline, 
`writte`n will exceed `size` and result in an lldbassert. After 
8e776bb660dda6c51ce7ca6cea641db1f47aa9cf 
 we don't 
really need `written` anymore and we can check whether `str` is empty instead.


https://reviews.llvm.org/D126081

Files:
  lldb/source/Interpreter/CommandInterpreter.cpp


Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2989,22 +2989,18 @@
   lldb::StreamFileSP stream = is_stdout ? io_handler.GetOutputStreamFileSP()
 : io_handler.GetErrorStreamFileSP();
   // Split the output into lines and poll for interrupt requests
-  size_t size = str.size();
-  while (size > 0 && !WasInterrupted()) {
+  while (!str.empty() && !WasInterrupted()) {
 llvm::StringRef line;
-size_t written = 0;
 std::tie(line, str) = str.split('\n');
 {
   std::lock_guard guard(io_handler.GetOutputMutex());
-  written += stream->Write(line.data(), line.size());
-  written += stream->Write("\n", 1);
+  stream->Write(line.data(), line.size());
+  stream->Write("\n", 1);
 }
-lldbassert(size >= written);
-size -= written;
   }
 
   std::lock_guard guard(io_handler.GetOutputMutex());
-  if (size > 0)
+  if (!str.empty())
 stream->Printf("\n... Interrupted.\n");
   stream->Flush();
 }


Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2989,22 +2989,18 @@
   lldb::StreamFileSP stream = is_stdout ? io_handler.GetOutputStreamFileSP()
 : io_handler.GetErrorStreamFileSP();
   // Split the output into lines and poll for interrupt requests
-  size_t size = str.size();
-  while (size > 0 && !WasInterrupted()) {
+  while (!str.empty() && !WasInterrupted()) {
 llvm::StringRef line;
-size_t written = 0;
 std::tie(line, str) = str.split('\n');
 {
   std::lock_guard guard(io_handler.GetOutputMutex());
-  written += stream->Write(line.data(), line.size());
-  written += stream->Write("\n", 1);
+  stream->Write(line.data(), line.size());
+  stream->Write("\n", 1);
 }
-lldbassert(size >= written);
-size -= written;
   }
 
   std::lock_guard guard(io_handler.GetOutputMutex());
-  if (size > 0)
+  if (!str.empty())
 stream->Printf("\n... Interrupted.\n");
   stream->Flush();
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D126080: Adapt C++ std::string dataformatter for D125496

2022-05-20 Thread Adrian Prantl via Phabricator via lldb-commits

aprantl added a comment.

That seemed to have done the trick.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126080

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

[Lldb-commits] [PATCH] D126080: Adapt C++ std::string dataformatter for D125496

2022-05-20 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126080

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

[Lldb-commits] [PATCH] D125509: [LLDB][NFC] Decouple dwarf location table from DWARFExpression.

2022-05-20 Thread Zequan Wu via Phabricator via lldb-commits

zequanwu updated this revision to Diff 431022.
zequanwu marked 3 inline comments as done.
zequanwu added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125509

Files:
  lldb/include/lldb/Expression/DWARFExpression.h
  lldb/include/lldb/Expression/DWARFExpressionList.h
  lldb/include/lldb/Symbol/Function.h
  lldb/include/lldb/Symbol/Variable.h
  lldb/include/lldb/Target/StackFrame.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/Core/ValueObjectVariable.cpp
  lldb/source/Expression/CMakeLists.txt
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Expression/DWARFExpressionList.cpp
  lldb/source/Expression/Materializer.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Symbol/Function.cpp
  lldb/source/Symbol/Variable.cpp
  lldb/source/Target/RegisterContextUnwind.cpp
  lldb/source/Target/StackFrame.cpp
  lldb/test/Shell/SymbolFile/DWARF/x86/debug_loc.s
  lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
  llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp
  llvm/utils/gn/secondary/lldb/source/Expression/BUILD.gn

Index: llvm/utils/gn/secondary/lldb/source/Expression/BUILD.gn
===
--- llvm/utils/gn/secondary/lldb/source/Expression/BUILD.gn
+++ llvm/utils/gn/secondary/lldb/source/Expression/BUILD.gn
@@ -23,6 +23,7 @@
   include_dirs = [ ".." ]
   sources = [
 "DWARFExpression.cpp",
+"DWARFExpressionList.cpp",
 "DiagnosticManager.cpp",
 "Expression.cpp",
 "ExpressionVariable.cpp",
Index: llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp
===
--- llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp
+++ llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp
@@ -177,8 +177,8 @@
   return visitLocationList(&Offset, [&](const DWARFLocationEntry &E) {
 Expected> Loc = Interp.Interpret(E);
 if (!Loc)
-  return Callback(Loc.takeError());
-if (*Loc)
+  consumeError(Loc.takeError());
+else if (*Loc)
   return Callback(**Loc);
 return true;
   });
Index: lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
===
--- lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
+++ lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
@@ -19,29 +19,29 @@
 # SYMBOLS-NEXT:   Variable{{.*}}, name = "A", {{.*}}, location = DW_OP_GNU_addr_index 0x0
 # SYMBOLS-NEXT:   Function{{.*}}, demangled = F0
 # SYMBOLS-NEXT:   Block{{.*}}, ranges = [0x-0x0001)
-# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location = 
-# SYMBOLS-NEXT:   DW_LLE_startx_length   (0x0001, 0x0001): DW_OP_reg0 RAX
+# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location =
+# SYMBOLS-NEXT:   [0x, 0x0001): DW_OP_reg0 RAX
 # SYMBOLS-EMPTY:
 # SYMBOLS-NEXT: CompileUnit{0x0001}, language = "", file = '1.c'
 # SYMBOLS-NEXT:   Variable{{.*}}, name = "A", {{.*}}, location = DW_OP_GNU_addr_index 0x2
 # SYMBOLS-NEXT:   Function{{.*}}, demangled = F1
 # SYMBOLS-NEXT:   Block{{.*}}, ranges = [0x0001-0x0002)
-# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location = 
-# SYMBOLS-NEXT:   DW_LLE_startx_length   (0x0003, 0x0001): DW_OP_reg1 RDX
+# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location =
+# SYMBOLS-NEXT:   [0x0001, 0x0002): DW_OP_reg1 RDX
 # SYMBOLS-EMPTY:
 # SYMBOLS-NEXT: CompileUnit{0x0002}, language = "", file = '2.c'
 # SYMBOLS-NEXT:   Variable{{.*}}, name = "A", {{.*}}, location = DW_OP_GNU_addr_index 0x4
 # SYMBOLS-NEXT:   Function{{.*}}, demangled = F2
 # SYMBOLS-NEXT:   Block{{.*}}, ranges = [0x0002-0x0003)
-# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location = 
-# SYMBOLS-NEXT:   DW_LLE_startx_length   (0x0005, 0x0001): DW_OP_reg2 RCX
+# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location =
+# SYMBOLS-NEXT:   [0x0002, 0x0003): DW_OP_reg2 RCX
 # SYMBOLS-EMPTY:
 # SYMBOLS-NEXT: CompileUnit{0x0003}, language = "", file = '3.c'
 # SYMBOLS-NEXT:   Variable{{.*}}, name = "A", {{.*}}, location = DW_OP_GNU_addr_index 0x6

[Lldb-commits] [PATCH] D125509: [LLDB][NFC] Decouple dwarf location table from DWARFExpression.

2022-05-20 Thread Zequan Wu via Phabricator via lldb-commits

zequanwu added inline comments.



Comment at: lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s:22-23
 # SYMBOLS-NEXT:   Block{{.*}}, ranges = [0x-0x0001)
-# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location = 
-# SYMBOLS-NEXT:   DW_LLE_startx_length   (0x0001, 
0x0001): DW_OP_reg0 RAX
+# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location =
+# SYMBOLS-NEXT:   [0x, 0x0001): DW_OP_reg0 RAX
 # SYMBOLS-EMPTY:

JDevlieghere wrote:
> I guess the patch is not NFC is the output changes? Would it be possible to 
> split the functional and non-functional part of this patch into separate 
> patches?
Originally, lldb-test uses `DWARFLocationTable::dumpLocationList` to dump the 
raw range info from dwarf expression, which are not interpreted to be readable 
range info. 
In this case, the raw range info is `DW_LLE_startx_length 0x1 0x1` where the 
first value is address index and second value is range length. Now, it's 
dumping the readable range info that is interpreted from the raw range info. 
So, the variable range is within the block range.

Another test case change is `debug_loc.s`, because we don't accept invalid 
range info now. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125509

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

[Lldb-commits] [PATCH] D124731: [lldb] Consider binary as module of last resort

2022-05-20 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

Please let me know if you need someone to land this on your behalf.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124731

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

[Lldb-commits] [PATCH] D126080: Adapt C++ std::string dataformatter for D125496

2022-05-20 Thread Adrian Prantl via Phabricator via lldb-commits

aprantl added a comment.

> How do we usually add tests for STL data structures?

I guess that matrix bot sort of covers this. 
https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake-matrix/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126080

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

[Lldb-commits] [PATCH] D124731: [lldb] Consider binary as module of last resort

2022-05-20 Thread Will Hawkins via Phabricator via lldb-commits

hawkinsw added a comment.

In D124731#3528269 , @JDevlieghere 
wrote:

> Please let me know if you need someone to land this on your behalf.

Unfortunately, I do! Sorry! I have not yet completed the committer process. I 
still feel too new. Thanks for the mentorship on this patch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124731

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

[Lldb-commits] [lldb] a3c3482 - [lldb] Consider binary as module of last resort

2022-05-20 Thread Jonas Devlieghere via lldb-commits


Author: Will Hawkins
Date: 2022-05-20T14:01:05-07:00
New Revision: a3c3482ceb529206b0ae4e7782e5496da5e0879d

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

LOG: [lldb] Consider binary as module of last resort

When setting an address breakpoint using a non-section address in lldb
before having ever run the program, the binary itself is not considered
a module. As a result, the breakpoint is unresolved (and never gets
resolved subsequently).

This patch changes that behavior: as a last resort, the binary is
considered as a module when resolving a non-section address breakpoint.

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

Added: 
lldb/test/API/commands/breakpoint/set/address-nomodule/Makefile

lldb/test/API/commands/breakpoint/set/address-nomodule/TestBreakpointAddressNoModule.py
lldb/test/API/commands/breakpoint/set/address-nomodule/inferior.c

Modified: 
lldb/source/Breakpoint/BreakpointResolverAddress.cpp
lldb/source/Commands/Options.td

Removed: 




diff  --git a/lldb/source/Breakpoint/BreakpointResolverAddress.cpp 
b/lldb/source/Breakpoint/BreakpointResolverAddress.cpp
index c173fc0c2a7a6..9b6b6d29cbce8 100644
--- a/lldb/source/Breakpoint/BreakpointResolverAddress.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolverAddress.cpp
@@ -121,16 +121,27 @@ Searcher::CallbackReturn 
BreakpointResolverAddress::SearchCallback(
 
   if (filter.AddressPasses(m_addr)) {
 if (breakpoint.GetNumLocations() == 0) {
-  // If the address is just an offset, and we're given a module, see if we
-  // can find the appropriate module loaded in the binary, and fix up
-  // m_addr to use that.
-  if (!m_addr.IsSectionOffset() && m_module_filespec) {
+  // If the address is just an offset ...
+  if (!m_addr.IsSectionOffset()) {
+ModuleSP containing_module_sp = nullptr;
 Target &target = breakpoint.GetTarget();
-ModuleSpec module_spec(m_module_filespec);
-ModuleSP module_sp = target.GetImages().FindFirstModule(module_spec);
-if (module_sp) {
+if (m_module_filespec) {
+  // ... and we're given a module, see if we can find the
+  // appropriate module loaded in the binary, and fix up
+  // m_addr to use that.
+  ModuleSpec module_spec(m_module_filespec);
+  containing_module_sp =
+  target.GetImages().FindFirstModule(module_spec);
+} else {
+  // ... and we're not given a module, see if the offset is
+  // somewhere in the executable module. If it is, then we'll
+  // fix up m_addr to use that.
+  containing_module_sp = target.GetExecutableModule();
+}
+if (containing_module_sp) {
   Address tmp_address;
-  if (module_sp->ResolveFileAddress(m_addr.GetOffset(), tmp_address))
+  if (containing_module_sp->ResolveFileAddress(m_addr.GetOffset(),
+   tmp_address))
 m_addr = tmp_address;
 }
   }

diff  --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td
index c326f8a320748..284437887a0b9 100644
--- a/lldb/source/Commands/Options.td
+++ b/lldb/source/Commands/Options.td
@@ -128,13 +128,15 @@ let Command = "breakpoint set" in {
 Arg<"AddressOrExpression">, Required,
 Desc<"Set the breakpoint at the specified address.  If the address maps "
 "uniquely to a particular binary, then the address will be converted to "
-"a \"file\"address, so that the breakpoint will track that binary+offset "
+"a \"file\" address, so that the breakpoint will track that binary+offset "
 "no matter where the binary eventually loads.  Alternately, if you also "
 "specify the module - with the -s option - then the address will be "
 "treated as a file address in that module, and resolved accordingly.  "
 "Again, this will allow lldb to track that offset on subsequent reloads.  "
 "The module need not have been loaded at the time you specify this "
-"breakpoint, and will get resolved when the module is loaded.">;
+"breakpoint, and will get resolved when the module is loaded.  If no "
+"module is specified, the binary being debugged is considered as a "
+"fallback.">;
   def breakpoint_set_name : Option<"name", "n">, Group<3>, Arg<"FunctionName">,
 Completion<"Symbol">, Required,
 Desc<"Set the breakpoint by function name.  Can be repeated multiple times 
"

diff  --git a/lldb/test/API/commands/breakpoint/set/address-nomodule/Makefile 
b/lldb/test/API/commands/breakpoint/set/address-nomodule/Makefile
new file mode 100644
index 0..7c82a2dde6c2e
--- /dev/null
+++ b/lldb/test/API/commands/breakpoint/set/address-nomodule/Makefile
@@ -0,0 +1,3

[Lldb-commits] [PATCH] D124731: [lldb] Consider binary as module of last resort

2022-05-20 Thread Jonas Devlieghere via Phabricator via lldb-commits

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa3c3482ceb52: [lldb] Consider binary as module of last 
resort (authored by hawkinsw, committed by JDevlieghere).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124731

Files:
  lldb/source/Breakpoint/BreakpointResolverAddress.cpp
  lldb/source/Commands/Options.td
  lldb/test/API/commands/breakpoint/set/address-nomodule/Makefile
  
lldb/test/API/commands/breakpoint/set/address-nomodule/TestBreakpointAddressNoModule.py
  lldb/test/API/commands/breakpoint/set/address-nomodule/inferior.c

Index: lldb/test/API/commands/breakpoint/set/address-nomodule/inferior.c
===
--- /dev/null
+++ lldb/test/API/commands/breakpoint/set/address-nomodule/inferior.c
@@ -0,0 +1,6 @@
+int function(int a) { return a; }
+
+int main() {
+  int f = function(10);
+  return f;
+}
Index: lldb/test/API/commands/breakpoint/set/address-nomodule/TestBreakpointAddressNoModule.py
===
--- /dev/null
+++ lldb/test/API/commands/breakpoint/set/address-nomodule/TestBreakpointAddressNoModule.py
@@ -0,0 +1,36 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def get_address_from_symbol(self, symbol):
+target = lldbutil.run_to_breakpoint_make_target(self, "a.out", True)
+bp = target.BreakpointCreateByName(symbol, None)
+address = bp.GetLocationAtIndex(0).GetAddress().GetFileAddress()
+return address
+
+def test_set_address_no_module(self):
+self.build()
+
+main_address = self.get_address_from_symbol("main")
+
+target = lldbutil.run_to_breakpoint_make_target(self)
+debugger = target.GetDebugger()
+
+debugger.HandleCommand(f"break set -a {main_address:#x}")
+self.assertEquals(target.GetNumBreakpoints(), 1)
+
+bp = target.GetBreakpointAtIndex(0)
+self.assertIsNotNone(bp)
+
+_, _, thread, _ = lldbutil.run_to_breakpoint_do_run(self, target, bp)
+self.assertGreaterEqual(thread.GetNumFrames(), 1)
+
+thread_pc = thread.GetFrameAtIndex(0).GetPCAddress()
+self.assertNotEqual(thread_pc, None)
+
+self.assertEquals(main_address, thread_pc.GetFileAddress())
Index: lldb/test/API/commands/breakpoint/set/address-nomodule/Makefile
===
--- /dev/null
+++ lldb/test/API/commands/breakpoint/set/address-nomodule/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := inferior.c
+
+include Makefile.rules
Index: lldb/source/Commands/Options.td
===
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -128,13 +128,15 @@
 Arg<"AddressOrExpression">, Required,
 Desc<"Set the breakpoint at the specified address.  If the address maps "
 "uniquely to a particular binary, then the address will be converted to "
-"a \"file\"address, so that the breakpoint will track that binary+offset "
+"a \"file\" address, so that the breakpoint will track that binary+offset "
 "no matter where the binary eventually loads.  Alternately, if you also "
 "specify the module - with the -s option - then the address will be "
 "treated as a file address in that module, and resolved accordingly.  "
 "Again, this will allow lldb to track that offset on subsequent reloads.  "
 "The module need not have been loaded at the time you specify this "
-"breakpoint, and will get resolved when the module is loaded.">;
+"breakpoint, and will get resolved when the module is loaded.  If no "
+"module is specified, the binary being debugged is considered as a "
+"fallback.">;
   def breakpoint_set_name : Option<"name", "n">, Group<3>, Arg<"FunctionName">,
 Completion<"Symbol">, Required,
 Desc<"Set the breakpoint by function name.  Can be repeated multiple times "
Index: lldb/source/Breakpoint/BreakpointResolverAddress.cpp
===
--- lldb/source/Breakpoint/BreakpointResolverAddress.cpp
+++ lldb/source/Breakpoint/BreakpointResolverAddress.cpp
@@ -121,16 +121,27 @@
 
   if (filter.AddressPasses(m_addr)) {
 if (breakpoint.GetNumLocations() == 0) {
-  // If the address is just an offset, and we're given a module, see if we
-  // can find the appropriate module loaded in the binary, and fix up
-  // m_addr to use that.
-  if (!m_addr.IsSectionOffset() && m_module_filespec) {
+  // If the address is just an offset ...
+  if (!m_addr.IsSectionOffset()) {
+Mo

[Lldb-commits] [lldb] d252d92 - [lldb] Fix spurious assertion in PrintCommandOutput

2022-05-20 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2022-05-20T14:15:01-07:00
New Revision: d252d9231c4a5008a69c6791db498aaf95bda8e7

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

LOG: [lldb] Fix spurious assertion in PrintCommandOutput

When the string passed to PrintCommandOutput doesn't end with a newline,
`written` will exceed `size` and result in an lldbassert.

After 8e776bb660dda6c51ce7ca6cea641db1f47aa9cf we don't really need
written anymore and we can check whether `str` is empty instead. This
patch simplifies the code and removes the assert that's no longer
relevant.

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

Added: 


Modified: 
lldb/source/Interpreter/CommandInterpreter.cpp

Removed: 




diff  --git a/lldb/source/Interpreter/CommandInterpreter.cpp 
b/lldb/source/Interpreter/CommandInterpreter.cpp
index 01a2e3f950aa7..e3d2aa8ebd37f 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2989,22 +2989,18 @@ void CommandInterpreter::PrintCommandOutput(IOHandler 
&io_handler,
   lldb::StreamFileSP stream = is_stdout ? io_handler.GetOutputStreamFileSP()
 : io_handler.GetErrorStreamFileSP();
   // Split the output into lines and poll for interrupt requests
-  size_t size = str.size();
-  while (size > 0 && !WasInterrupted()) {
+  while (!str.empty() && !WasInterrupted()) {
 llvm::StringRef line;
-size_t written = 0;
 std::tie(line, str) = str.split('\n');
 {
   std::lock_guard guard(io_handler.GetOutputMutex());
-  written += stream->Write(line.data(), line.size());
-  written += stream->Write("\n", 1);
+  stream->Write(line.data(), line.size());
+  stream->Write("\n", 1);
 }
-lldbassert(size >= written);
-size -= written;
   }
 
   std::lock_guard guard(io_handler.GetOutputMutex());
-  if (size > 0)
+  if (!str.empty())
 stream->Printf("\n... Interrupted.\n");
   stream->Flush();
 }



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

[Lldb-commits] [PATCH] D126081: [lldb] Fix spurious lldb_assert in PrintCommandOutput

2022-05-20 Thread Jonas Devlieghere via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd252d9231c4a: [lldb] Fix spurious assertion in 
PrintCommandOutput (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126081

Files:
  lldb/source/Interpreter/CommandInterpreter.cpp


Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2989,22 +2989,18 @@
   lldb::StreamFileSP stream = is_stdout ? io_handler.GetOutputStreamFileSP()
 : io_handler.GetErrorStreamFileSP();
   // Split the output into lines and poll for interrupt requests
-  size_t size = str.size();
-  while (size > 0 && !WasInterrupted()) {
+  while (!str.empty() && !WasInterrupted()) {
 llvm::StringRef line;
-size_t written = 0;
 std::tie(line, str) = str.split('\n');
 {
   std::lock_guard guard(io_handler.GetOutputMutex());
-  written += stream->Write(line.data(), line.size());
-  written += stream->Write("\n", 1);
+  stream->Write(line.data(), line.size());
+  stream->Write("\n", 1);
 }
-lldbassert(size >= written);
-size -= written;
   }
 
   std::lock_guard guard(io_handler.GetOutputMutex());
-  if (size > 0)
+  if (!str.empty())
 stream->Printf("\n... Interrupted.\n");
   stream->Flush();
 }


Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2989,22 +2989,18 @@
   lldb::StreamFileSP stream = is_stdout ? io_handler.GetOutputStreamFileSP()
 : io_handler.GetErrorStreamFileSP();
   // Split the output into lines and poll for interrupt requests
-  size_t size = str.size();
-  while (size > 0 && !WasInterrupted()) {
+  while (!str.empty() && !WasInterrupted()) {
 llvm::StringRef line;
-size_t written = 0;
 std::tie(line, str) = str.split('\n');
 {
   std::lock_guard guard(io_handler.GetOutputMutex());
-  written += stream->Write(line.data(), line.size());
-  written += stream->Write("\n", 1);
+  stream->Write(line.data(), line.size());
+  stream->Write("\n", 1);
 }
-lldbassert(size >= written);
-size -= written;
   }
 
   std::lock_guard guard(io_handler.GetOutputMutex());
-  if (size > 0)
+  if (!str.empty())
 stream->Printf("\n... Interrupted.\n");
   stream->Flush();
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D124731: [lldb] Consider binary as module of last resort

2022-05-20 Thread Will Hawkins via Phabricator via lldb-commits

hawkinsw added a comment.

I am going to investigate why the buildbot failed. When I ran the tests locally 
everything ran fine. I am terribly, terribly sorry!

@JDevlieghere -- If you could help me debug the failure, or point me to the 
documentation on how to read buildbot output, that would be great!

Sorry again!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124731

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

[Lldb-commits] [PATCH] D124731: [lldb] Consider binary as module of last resort

2022-05-20 Thread Will Hawkins via Phabricator via lldb-commits

hawkinsw added a comment.

I realize the reason for the failure. It would appear that there are tests that 
are otherwise affected by the change in the behavior introduced here. I will 
take care of updating these tests as quickly as possible. I am sorry for the 
trouble, @JDevlieghere !


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124731

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

[Lldb-commits] [lldb] b369762 - Convert the test file for TestIgnoredExceptions.py to the mach_vm API.

2022-05-20 Thread Jim Ingham via lldb-commits


Author: Jim Ingham
Date: 2022-05-20T15:16:24-07:00
New Revision: b369762beb70dfef22c7e793aed79b94d7dc0757

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

LOG: Convert the test file for TestIgnoredExceptions.py to the mach_vm API.

The previous version of this test uses mprotect, and that seemed to be
flakey on older systems.  I converted the test to use the underlying
mach_vm API's.  The test only runs on Darwin anyway, so this is not a
real limitation, and I'm hoping the lower level API's work more
consistently.

Added: 


Modified: 
lldb/test/API/macosx/ignore_exceptions/main.c

Removed: 




diff  --git a/lldb/test/API/macosx/ignore_exceptions/main.c 
b/lldb/test/API/macosx/ignore_exceptions/main.c
index 7b89dbf88152b..682c5f23627e0 100644
--- a/lldb/test/API/macosx/ignore_exceptions/main.c
+++ b/lldb/test/API/macosx/ignore_exceptions/main.c
@@ -3,25 +3,34 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
-int g_ints[] = {10, 20, 30, 40, 50, 60};
+int *g_int_ptr = NULL;
+size_t g_size = 10*sizeof(int);
 
 void
 saction_handler(int signo, siginfo_t info, void *baton) {
-  printf("Got into handler.\n");
-  mprotect(g_ints, sizeof(g_ints), PROT_READ|PROT_WRITE); // stop here in the 
signal handler
-  g_ints[0] = 20;
+  printf("Got into handler.\n");   // stop here in the signal handler
+  kern_return_t success
+  = mach_vm_protect(mach_task_self(), g_int_ptr,
+g_size, 0, VM_PROT_READ|VM_PROT_WRITE);
+  g_int_ptr[1] = 20;
 }
 int
 main()
 {
-  mprotect(g_ints, 10*sizeof(int) , PROT_NONE);
+  kern_return_t vm_result = vm_allocate(mach_task_self(), &g_int_ptr, g_size, 
VM_FLAGS_ANYWHERE);
+  for (int i = 0; i < 10; i++)
+g_int_ptr[i] = i * 10;
+  
+  vm_result = mach_vm_protect(mach_task_self(), g_int_ptr, g_size, 0, 
VM_PROT_NONE);
   struct sigaction my_action;
   sigemptyset(&my_action.sa_mask);
   my_action.sa_handler = (void (*)(int)) saction_handler;
   my_action.sa_flags = SA_SIGINFO;
 
   sigaction(SIGBUS, &my_action, NULL); // Stop here to get things going.
-  int local_value = g_ints[1];
+  int local_value = g_int_ptr[1];
   return local_value; // Break here to make sure we got past the signal handler
 }



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

[Lldb-commits] [PATCH] D126103: Remove `friend` classes from TypeCategoryMap

2022-05-20 Thread Jorge Gorbe Moya via Phabricator via lldb-commits

jgorbe created this revision.
jgorbe added reviewers: labath, JDevlieghere.
Herald added a project: All.
jgorbe requested review of this revision.
Herald added a project: LLDB.

As far as I can tell, the only thing those `friend` classes access is the
`ValueSP` typedef.

Given that this is a map-ish class, with "Map" in its name, it doesn't seem like
a stretch to make `KeyType`, `ValueType` and `ValueSP` public. More so when the
public methods of the class have `KeyType` and `ValueSP` arguments and clearly
`ValueSP` needs to be accessed from the outside.

`friend` complicates local reasoning about who can access private members, which
is valuable in a class like this that has every method locking a mutex to
prevent concurrent access.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126103

Files:
  lldb/include/lldb/DataFormatters/TypeCategoryMap.h


Index: lldb/include/lldb/DataFormatters/TypeCategoryMap.h
===
--- lldb/include/lldb/DataFormatters/TypeCategoryMap.h
+++ lldb/include/lldb/DataFormatters/TypeCategoryMap.h
@@ -23,13 +23,13 @@
 namespace lldb_private {
 class TypeCategoryMap {
 private:
-  typedef ConstString KeyType;
-  typedef TypeCategoryImpl ValueType;
-  typedef ValueType::SharedPointer ValueSP;
   typedef std::list ActiveCategoriesList;
   typedef ActiveCategoriesList::iterator ActiveCategoriesIterator;
 
 public:
+  typedef ConstString KeyType;
+  typedef TypeCategoryImpl ValueType;
+  typedef ValueType::SharedPointer ValueSP;
   typedef std::map MapType;
   typedef MapType::iterator MapIterator;
   typedef std::function ForEachCallback;
@@ -103,9 +103,6 @@
   ActiveCategoriesList &active_list() { return m_active_categories; }
 
   std::recursive_mutex &mutex() { return m_map_mutex; }
-
-  friend class FormattersContainer;
-  friend class FormatManager;
 };
 } // namespace lldb_private
 


Index: lldb/include/lldb/DataFormatters/TypeCategoryMap.h
===
--- lldb/include/lldb/DataFormatters/TypeCategoryMap.h
+++ lldb/include/lldb/DataFormatters/TypeCategoryMap.h
@@ -23,13 +23,13 @@
 namespace lldb_private {
 class TypeCategoryMap {
 private:
-  typedef ConstString KeyType;
-  typedef TypeCategoryImpl ValueType;
-  typedef ValueType::SharedPointer ValueSP;
   typedef std::list ActiveCategoriesList;
   typedef ActiveCategoriesList::iterator ActiveCategoriesIterator;
 
 public:
+  typedef ConstString KeyType;
+  typedef TypeCategoryImpl ValueType;
+  typedef ValueType::SharedPointer ValueSP;
   typedef std::map MapType;
   typedef MapType::iterator MapIterator;
   typedef std::function ForEachCallback;
@@ -103,9 +103,6 @@
   ActiveCategoriesList &active_list() { return m_active_categories; }
 
   std::recursive_mutex &mutex() { return m_map_mutex; }
-
-  friend class FormattersContainer;
-  friend class FormatManager;
 };
 } // namespace lldb_private
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D126103: Remove `friend` classes from TypeCategoryMap

2022-05-20 Thread 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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126103

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

[Lldb-commits] [PATCH] D126109: [lldb] Fix broken bad-address-breakpoint test

2022-05-20 Thread Will Hawkins via Phabricator via lldb-commits

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

After changing the "fallback" behavior when a user sets a breakpoint
without specifying a module the bad-address-breakpoint test case failed
incorrectly. This patch updates that test case in order to more
thoroughly discover an illegal address and use that as the means for
testing whether a breakpoint set at an illegal address fails to resolve.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126109

Files:
  
lldb/test/API/functionalities/breakpoint/address_breakpoints/TestBadAddressBreakpoints.py


Index: 
lldb/test/API/functionalities/breakpoint/address_breakpoints/TestBadAddressBreakpoints.py
===
--- 
lldb/test/API/functionalities/breakpoint/address_breakpoints/TestBadAddressBreakpoints.py
+++ 
lldb/test/API/functionalities/breakpoint/address_breakpoints/TestBadAddressBreakpoints.py
@@ -27,14 +27,25 @@
   "Set a breakpoint here",
   lldb.SBFileSpec("main.c"))
 
-# Now see if we can read from 0.  If I can't do that, I don't
-# have a good way to know what an illegal address is...
+
+
+illegal_address = 0x0
 error = lldb.SBError()
 
-ptr = process.ReadPointerFromMemory(0x0, error)
+# Walk through all the memory regions in the process and
+# find an address that is invalid.
+ptr = process.ReadPointerFromMemory(illegal_address, error)
+regions = process.GetMemoryRegions()
+for region_idx in range(regions.GetSize()):
+region = lldb.SBMemoryRegionInfo()
+regions.GetMemoryRegionAtIndex(region_idx, region)
+if region.GetRegionEnd() > illegal_address:
+illegal_address = region.GetRegionEnd()
 
 if not error.Success():
-bkpt = target.BreakpointCreateByAddress(0x0)
+# Now, set a breakpoint at the address we know is illegal.
+bkpt = target.BreakpointCreateByAddress(illegal_address)
+# Verify that breakpoint is not resolved.
 for bp_loc in bkpt:
 self.assertEquals(bp_loc.IsResolved(), False)
 else:


Index: lldb/test/API/functionalities/breakpoint/address_breakpoints/TestBadAddressBreakpoints.py
===
--- lldb/test/API/functionalities/breakpoint/address_breakpoints/TestBadAddressBreakpoints.py
+++ lldb/test/API/functionalities/breakpoint/address_breakpoints/TestBadAddressBreakpoints.py
@@ -27,14 +27,25 @@
   "Set a breakpoint here",
   lldb.SBFileSpec("main.c"))
 
-# Now see if we can read from 0.  If I can't do that, I don't
-# have a good way to know what an illegal address is...
+
+
+illegal_address = 0x0
 error = lldb.SBError()
 
-ptr = process.ReadPointerFromMemory(0x0, error)
+# Walk through all the memory regions in the process and
+# find an address that is invalid.
+ptr = process.ReadPointerFromMemory(illegal_address, error)
+regions = process.GetMemoryRegions()
+for region_idx in range(regions.GetSize()):
+region = lldb.SBMemoryRegionInfo()
+regions.GetMemoryRegionAtIndex(region_idx, region)
+if region.GetRegionEnd() > illegal_address:
+illegal_address = region.GetRegionEnd()
 
 if not error.Success():
-bkpt = target.BreakpointCreateByAddress(0x0)
+# Now, set a breakpoint at the address we know is illegal.
+bkpt = target.BreakpointCreateByAddress(illegal_address)
+# Verify that breakpoint is not resolved.
 for bp_loc in bkpt:
 self.assertEquals(bp_loc.IsResolved(), False)
 else:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D125575: [lldb] [llgs] Implement non-stop style stop notification packets

2022-05-20 Thread Michał Górny via Phabricator via lldb-commits

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

Fix missing `OK` response for c/C/vCont/… packets.


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

https://reviews.llvm.org/D125575

Files:
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/test/API/tools/lldb-server/TestLldbGdbServer.py

Index: lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
===
--- lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
+++ lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
@@ -1410,3 +1410,82 @@
 self.assertEqual(decoded[errno_idx], 0)  # si_errno
 self.assertEqual(decoded[code_idx], SEGV_MAPERR)  # si_code
 self.assertEqual(decoded[addr_idx], 0)  # si_addr
+
+def test_QNonStop(self):
+self.build()
+self.set_inferior_startup_launch()
+thread_num = 3
+procs = self.prep_debug_monitor_and_inferior(
+inferior_args=["thread:segfault"] + thread_num * ["thread:new"])
+self.test_sequence.add_log_lines(
+["read packet: $QNonStop:1#00",
+ "send packet: $OK#00",
+ "read packet: $c#63",
+ "send packet: $OK#00",
+ ], True)
+self.expect_gdbremote_sequence()
+
+segv_signo = lldbutil.get_signal_number('SIGSEGV')
+all_threads = set()
+all_segv_threads = []
+
+# we should get segfaults from all the threads
+for segv_no in range(thread_num):
+# first wait for the notification event
+self.reset_test_sequence()
+self.test_sequence.add_log_lines(
+[{"direction": "send",
+  "regex": r"^%Stop:T([0-9a-fA-F]{2})thread:([0-9a-fA-F]+);",
+  "capture": {1: "signo", 2: "thread_id"},
+  },
+ ], True)
+threads = [self.expect_gdbremote_sequence()]
+
+# then we may get events for the remaining threads
+# (but note that not all threads may have been started yet)
+while True:
+self.reset_test_sequence()
+self.test_sequence.add_log_lines(
+["read packet: $vStopped#00",
+ {"direction": "send",
+  "regex": r"^\$(OK|T([0-9a-fA-F]{2})thread:([0-9a-fA-F]+);)",
+  "capture": {1: "packet", 2: "signo", 3: "thread_id"},
+  },
+ ], True)
+m = self.expect_gdbremote_sequence()
+if m["packet"] == "OK":
+break
+threads.append(m)
+
+segv_threads = []
+other_threads = []
+for t in threads:
+signo = int(t["signo"], 16)
+if signo == segv_signo:
+segv_threads.append(t["thread_id"])
+else:
+self.assertEqual(signo, 0)
+other_threads.append(t["thread_id"])
+
+# verify that exactly one thread segfaulted
+self.assertEqual(len(segv_threads), 1)
+# we should get only one segv from every thread
+self.assertNotIn(segv_threads[0], all_segv_threads)
+all_segv_threads.extend(segv_threads)
+# segv_threads + other_threads should always be a superset
+# of all_threads, i.e. we should get states for all threads
+# already started
+self.assertFalse(
+all_threads.difference(other_threads + segv_threads))
+all_threads.update(other_threads + segv_threads)
+
+self.reset_test_sequence()
+self.test_sequence.add_log_lines(
+["read packet: $vCont;C{:02x}:{};c#00"
+ .format(segv_signo, segv_threads[0]),
+ "send packet: $OK#00",
+ ], True)
+self.expect_gdbremote_sequence()
+
+# finally, verify that all threads have started
+self.assertEqual(len(all_threads), thread_num + 1)
Index: lldb/source/Utility/StringExtractorGDBRemote.cpp
===
--- lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ lldb/source/Utility/StringExtractorGDBRemote.cpp
@@ -150,6 +150,11 @@
 return eServerPacketType_QMemTags;
   break;
 
+case 'N':
+  if (PACKET_STARTS_WITH("QNonStop:"))
+

39 matches

Mail list logo