[Lldb-commits] [PATCH] D93895: Implement vAttachWait in lldb-server

2021-01-06 Thread Augusto Noronha via Phabricator via lldb-commits 
augusto2112 added a comment.

Ok, that makes sense!

Let me summarize the work that needs to be done, correct me if I get something 
wrong:

- Change back `vAttachWait` and `vAttachOrWait` to the original format of 
sending only the process name.

- Add default values for  "waitfor-interval-usec" and "waitfor-duration-sec" on 
ProcessGDBRemoteProperties.td.

- Add a new `jAttachWait` packet type, which sends a json which will read the 
values saved on the settings.  Question: would we need a `jAttachOrWait` 
variant? Or could we send something like "ignore-existing" in `jAttachWait`?

- Add a `qJAttachWaitSupported` packet. If  `jAttachWait` isn't supported, 
fallback to vAttachWait (we should also probably warn the user that their 
custom settings won't be used in this case).

- Add a test for `jAttachWait`.

Question: would we also keep the `waitfor-interval` and `waitfor-duration` 
command line options as well, and when they're missing, default to the 
settings? Or do we forego them completely?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93895

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

[Lldb-commits] [PATCH] D93481: [lldb/Lua] add support for multiline scripted breakpoints

2021-01-06 Thread Pavel Labath via Phabricator via lldb-commits 
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

This looks better. I haven't checked the rest of the patch in detail, but it 
seems ok at a quick glance and Jonas appeared to be happy with it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93481

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

[Lldb-commits] [PATCH] D94063: lldb: Add support for DW_AT_ranges on DW_TAG_subprograms

2021-01-06 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

The fix is good, but the test could be improved. Combining assembly input with 
running the inferior effectively limits the test to a single platform (assembly 
is not portable, and running requires asm to match the host). AFAICT, we don't 
actually need to run the binary to test this fix -- checking just the 
"breakpoint set" command should suffice (if you want to be more explicit in 
what is being checked, you can also run "breakpoint list -v" and test that). 
Then you'd just need to replace `%clang_host` with `%clang -target 
x86_64-pc-linux` (or something) and `UNSUPPORTED: system-windows` with 
`REQUIRES: x86`.

I'll write about variable printing in the other review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94063

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

[Lldb-commits] [PATCH] D94064: lldb: Add support for printing variables with DW_AT_ranges on DW_TAG_subprograms

2021-01-06 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

I don't think that simply setting func_lo_pc to zero will be sufficient to make 
this work. I would expect this to break in more complicated scenarios (like, 
even if we just have two of these functions). I think the only reason it works 
in this case is because for this particular function, it's base address 
(relative to the CU base) *is* zero.

The purpose of func_lo_pc is pretty weird, but it's essentially used for 
runtime relocation of location lists within the function -- we take the static 
"base" of the function, and the runtime "base", and the difference between the 
two produces the load bias. Applying the same bias to all variable location 
lists "relocates" the variables. (The reason this is so convoluted is reading 
location lists straight from (unrelocated) .o files on mac. I seriously 
considered changing this when working on debug_rnglists, but it turned out it 
wasn't really necessary, so I let it go.)

The "runtime" function address is being taken in 
https://github.com/llvm/llvm-project/blob/main/lldb/source/Core/ValueObjectVariable.cpp#L159
 (and maybe a couple of other places), and these two things need to be in sync. 
I think this could just be defined as the first address in the first address 
range of the function. That should be fine for ELF purposes (I have a feeling 
this thing would completely break down if macho started rearranging parts of 
the function), but it does bring up another problem.

LLDB's representation of a function (lldb_private::Function) assumes that all 
functions will be contiguous (Function::GetAddressRange returns a single 
range). If we make it so that this range matches the first block of the 
function (maybe that's what happens now), then maybe we could make things work 
for the case where DW_AT_ranges are used as a size optimization. However, 
getting things to work for truly discontinuous functions will require more 
work...

(Regarding the test, I think we could avoid running a binary by testing this 
via the "image lookup --verbose" command, and checking that the output contains 
the variables it should.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94064

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

[Lldb-commits] [PATCH] D93926: [lldb] Don't remove the lldb.debugger var after exiting the interpreter

2021-01-06 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

In D93926#2479749 , @JDevlieghere 
wrote:

> Your argument is correct because the interactive script interpreter always 
> belongs to a single debugger.

I'm not sure that even this is true (for python, anyway). IIUC all SBDebuggers 
share the same python interpreter (because python does not allow the creation 
of completely independent python contexts). We do some trickery to make it it 
appear as if these were independent, but I am not sure the trickery extends to 
the `lldb` module...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93926

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

[Lldb-commits] [PATCH] D94077: Support unscoped enumeration members in the expression evaluator.

2021-01-06 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

This suffers from the same problem as the other patch, where the other index 
classes (apple_names and debug_names) will essentially never be able to 
implement this feature. (Unless they re-index the dwarf themselves, that is, 
but this would defeat the purpose of having the index in the first place.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94077

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

[Lldb-commits] [PATCH] D93895: Implement vAttachWait in lldb-server

2021-01-06 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

I don't think it's a good idea to stuff all of this into a single patch. Can we 
go back to the version which just implements the basic vAttachWait packet (we 
can bikeshed on what the default interval should be)? I believe new 
commands/options/packets should be done in separate patches...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93895

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

[Lldb-commits] [PATCH] D94008: [LLDB] DynamicRegisterInfo calculate offsets in separate function

2021-01-06 Thread Pavel Labath via Phabricator via lldb-commits 
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

LG, modulo inline comment.




Comment at: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h:45-46
 
+  void ConfigureOffsets();
+
   size_t GetNumRegisters() const;

Users should not be calling this, right? Could we make this protected?


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

https://reviews.llvm.org/D94008

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

[Lldb-commits] [PATCH] D91847: [lldb] [debugserver] Add stN aliases for stmmN for compatibility

2021-01-06 Thread Pavel Labath via Phabricator via lldb-commits 
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

I think this is fine -- sorry this dropped off my radar.

@jasonmolenda, do you have any thoughts about this?


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

https://reviews.llvm.org/D91847

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

[Lldb-commits] [PATCH] D91847: [lldb] [debugserver] Add stN aliases for stmmN for compatibility

2021-01-06 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

In D91847#2410723 , @krytarowski wrote:

> I would mark stmmX as an alias to stX and keep stX as the default for all 
> platforms. stmmX could be an alias for everybody for legacy reasons.

Possibly. Though we could also do that later, as the next step towards total 
deprecation...


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

https://reviews.llvm.org/D91847

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

[Lldb-commits] [PATCH] D93895: Implement vAttachWait in lldb-server

2021-01-06 Thread Augusto Noronha via Phabricator via lldb-commits 
augusto2112 added a comment.

> I don't think it's a good idea to stuff all of this into a single patch. Can 
> we go back to the version which just implements the basic vAttachWait packet 
> (we can bikeshed on what the default interval should be)? I believe new 
> commands/options/packets should be done in separate patches...

Ok. I can open other patches after this one is merged.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93895

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

[Lldb-commits] [PATCH] D93895: Implement vAttachWait in lldb-server

2021-01-06 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

In D93895#2481990 , @augusto2112 wrote:

>> I don't think it's a good idea to stuff all of this into a single patch. Can 
>> we go back to the version which just implements the basic vAttachWait packet 
>> (we can bikeshed on what the default interval should be)? I believe new 
>> commands/options/packets should be done in separate patches...
>
> Ok. I can open other patches after this one is merged.

Thanks. Feel free to create the other patches even before that (the downside is 
lost productivity due to more rebasing).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93895

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

[Lldb-commits] [PATCH] D93895: Implement vAttachWait in lldb-server

2021-01-06 Thread Augusto Noronha via Phabricator via lldb-commits 
augusto2112 updated this revision to Diff 314874.
augusto2112 added a comment.

Back to implementation of vAttachWait


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93895

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py

Index: lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py
@@ -0,0 +1,68 @@
+
+import os
+from time import sleep
+
+import gdbremote_testcase
+import lldbgdbserverutils
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestGdbRemoteAttachWait(gdbremote_testcase.GdbRemoteTestCaseBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def test_attach_with_vAttachWait(self):
+exe = '%s_%d' % (self.testMethodName, os.getpid())
+self.build(dictionary={'EXE': exe})
+self.set_inferior_startup_attach_manually()
+
+server = self.connect_to_debug_monitor()
+self.assertIsNotNone(server)
+
+
+self.add_no_ack_remote_stream()
+self.test_sequence.add_log_lines([
+# Do the attach.
+"read packet: $vAttachWait;{}#00".format(lldbgdbserverutils.gdbremote_hex_encode_string(exe)),
+], True)
+# Run the stream until attachWait.
+context = self.expect_gdbremote_sequence()
+self.assertIsNotNone(context)
+
+# Sleep so we're sure that the inferior is launched after we ask for the attach.
+sleep(1)
+
+# Launch the inferior.
+inferior = self.launch_process_for_attach(
+inferior_args=["sleep:60"],
+exe_path=self.getBuildArtifact(exe))
+self.assertIsNotNone(inferior)
+self.assertTrue(inferior.pid > 0)
+self.assertTrue(
+lldbgdbserverutils.process_is_running(
+inferior.pid, True))
+
+# Make sure the attach succeeded.
+self.test_sequence.add_log_lines([
+{"direction": "send",
+ "regex": r"^\$T([0-9a-fA-F]{2})[^#]*#[0-9a-fA-F]{2}$",
+ "capture": {1: "stop_signal_hex"}},
+], True)
+self.add_process_info_collection_packets()
+
+
+# Run the stream sending the response..
+context = self.expect_gdbremote_sequence()
+self.assertIsNotNone(context)
+
+# Gather process info response.
+process_info = self.parse_process_info_response(context)
+self.assertIsNotNone(process_info)
+
+# Ensure the process id matches what we expected.
+pid_text = process_info.get('pid', None)
+self.assertIsNotNone(pid_text)
+reported_pid = int(pid_text, base=16)
+self.assertEqual(reported_pid, inferior.pid)
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -59,6 +59,17 @@
   /// attach operation.
   Status AttachToProcess(lldb::pid_t pid);
 
+  /// Wait to attach to a process with a given name.
+  ///
+  /// This method supports waiting for the next instance of a process
+  /// with a given name and attaching llgs to that via the configured
+  /// Platform.
+  ///
+  /// \return
+  /// An Status object indicating the success or failure of the
+  /// attach operation.
+  Status AttachWaitProcess(llvm::StringRef process_name);
+
   // NativeProcessProtocol::NativeDelegate overrides
   void InitializeDelegate(NativeProcessProtocol *process) override;
 
@@ -170,6 +181,8 @@
 
   PacketResult Handle_vAttach(StringExtractorGDBRemote &packet);
 
+  PacketResult Handle_vAttachWait(StringExtractorGDBRemote &packet);
+
   PacketResult Handle_D(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_qThreadStopInfo(StringExtractorGDBRemote &packet);
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -159,6 +159,9 @@
   RegisterMemberFunctionHandler(
   StringExtractorGDBRemote::eServerPacketType_vAttach,
   &GDBRemoteCommunicationServerLLGS::Handle_vAttach);
+  RegisterMemberFunctionHandler(
+  StringExtractorGDBRemote::eServerPacketType_vAttachWait,
+  &GDBRemoteCommunicationServerLLGS::Handle_vAttachWait);
   RegisterMemberFunctionHandler(
   StringExtractorGDBRemo

[Lldb-commits] [PATCH] D93895: Implement vAttachWait in lldb-server

2021-01-06 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

Thanks. This talk of all these packets has made me realize I did not completely 
understand the purpose of this packet. To really test the exclusion 
functionality of this packet, I guess we should be launching two instances of 
the inferior (one before sending the packet, one after), and then checking we 
attach to the right one. It looks like it should be easy to add to the existing 
test. Other than that (and some inline simplifications) this looks fine to me.




Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:360
+
+  auto is_in_exclusion_list = [&exclusion_list](ProcessInstanceInfo &info) {
+for (auto &excluded : exclusion_list) {





Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:362-364
+  if (excluded.GetProcessID() == info.GetProcessID()) {
+return true;
+  }

https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:374-377
+  loop_process_list.erase(std::remove_if(loop_process_list.begin(),
+ loop_process_list.end(),
+ is_in_exclusion_list),
+  loop_process_list.end());

You may need to `#include "llvm/ADT/STLExtras.h"` to get this...



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:389-391
+error_stream.Printf(
+"Multiple executables with name: '%s' found. Pids: ",
+process_name.str().c_str());





Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:393
+for (size_t i = 0; i < loop_process_list.size() - 1; ++i) {
+  error_stream.Printf("%lu, ", loop_process_list[i].GetProcessID());
+}





Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:395
+}
+error_stream.Printf("%lu.", loop_process_list.back().GetProcessID());
+




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93895

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

[Lldb-commits] [lldb] 4e0e79d - [lldb] Simplify some lldb-server tests

2021-01-06 Thread Pavel Labath via lldb-commits 

Author: Pavel Labath
Date: 2021-01-06T15:39:51+01:00
New Revision: 4e0e79dd349a208384449fd8dcdc9bf1644ee0f3

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

LOG: [lldb] Simplify some lldb-server tests

Remove manual test duplication.

Added: 


Modified: 
lldb/test/API/tools/lldb-server/TestGdbRemoteAuxvSupport.py

lldb/test/API/tools/lldb-server/libraries-svr4/TestGdbRemoteLibrariesSvr4Support.py

lldb/test/API/tools/lldb-server/memory-allocation/TestGdbRemoteMemoryAllocation.py
lldb/test/API/tools/lldb-server/register-reading/TestGdbRemoteGPacket.py
lldb/test/API/tools/lldb-server/thread-name/TestGdbRemoteThreadName.py

Removed: 




diff  --git a/lldb/test/API/tools/lldb-server/TestGdbRemoteAuxvSupport.py 
b/lldb/test/API/tools/lldb-server/TestGdbRemoteAuxvSupport.py
index 9d9c4d89e0b5..a16ba6dc3443 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemoteAuxvSupport.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemoteAuxvSupport.py
@@ -3,7 +3,6 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
-
 class TestGdbRemoteAuxvSupport(gdbremote_testcase.GdbRemoteTestCaseBase):
 
 mydir = TestBase.compute_mydir(__file__)
@@ -11,25 +10,9 @@ class 
TestGdbRemoteAuxvSupport(gdbremote_testcase.GdbRemoteTestCaseBase):
 AUXV_SUPPORT_FEATURE_NAME = "qXfer:auxv:read"
 
 def has_auxv_support(self):
-inferior_args = ["message:main entered", "sleep:5"]
-procs = self.prep_debug_monitor_and_inferior(
-inferior_args=inferior_args)
-
-# Don't do anything until we match the launched inferior main entry 
output.
-# Then immediately interrupt the process.
-# This prevents auxv data being asked for before it's ready and leaves
-# us in a stopped state.
-self.test_sequence.add_log_lines([
-# Start the inferior...
-"read packet: $c#63",
-# ... match output
-{"type": "output_match", "regex": self.maybe_strict_output_regex(
-r"message:main entered\r\n")},
-], True)
-# ... then interrupt.
-self.add_interrupt_packets()
-self.add_qSupported_packets()
+procs = self.prep_debug_monitor_and_inferior()
 
+self.add_qSupported_packets()
 context = self.expect_gdbremote_sequence()
 self.assertIsNotNone(context)
 
@@ -87,27 +70,19 @@ def get_raw_auxv_data(self):
 self.assertIsNotNone(content_raw)
 return (word_size, self.decode_gdbremote_binary(content_raw))
 
-def supports_auxv(self):
-# When non-auxv platforms support llgs, skip the test on platforms
-# that don't support auxv.
+@skipIfWindows # no auxv support.
+@skipIfDarwin
+def test_supports_auxv(self):
+self.build()
+self.set_inferior_startup_launch()
 self.assertTrue(self.has_auxv_support())
 
-#
-# We skip the "supports_auxv" test on debugserver.  The rest of the tests
-# appropriately skip the auxv tests if the support flag is not present
-# in the qSupported response, so the debugserver test bits are still there
-# in case debugserver code one day does have auxv support and thus those
-# tests don't get skipped.
-#
-
-@skipIfWindows # no auxv support.
-@llgs_test
-def test_supports_auxv_llgs(self):
+@skipIfWindows
+@expectedFailureNetBSD
+def test_auxv_data_is_correct_size(self):
 self.build()
 self.set_inferior_startup_launch()
-self.supports_auxv()
 
-def auxv_data_is_correct_size(self):
 (word_size, auxv_data) = self.get_raw_auxv_data()
 self.assertIsNotNone(auxv_data)
 
@@ -116,21 +91,12 @@ def auxv_data_is_correct_size(self):
 self.assertEqual(len(auxv_data) % (2 * word_size), 0)
 self.trace("auxv contains {} entries".format(len(auxv_data) / 
(2*word_size)))
 
-@debugserver_test
-def test_auxv_data_is_correct_size_debugserver(self):
-self.build()
-self.set_inferior_startup_launch()
-self.auxv_data_is_correct_size()
-
 @skipIfWindows
 @expectedFailureNetBSD
-@llgs_test
-def test_auxv_data_is_correct_size_llgs(self):
+def test_auxv_keys_look_valid(self):
 self.build()
 self.set_inferior_startup_launch()
-self.auxv_data_is_correct_size()
 
-def auxv_keys_look_valid(self):
 (word_size, auxv_data) = self.get_raw_auxv_data()
 self.assertIsNotNone(auxv_data)
 
@@ -154,21 +120,12 @@ def auxv_keys_look_valid(self):
 self.assertTrue(auxv_key <= 1000)
 self.trace("auxv dict: {}".format(auxv_dict))
 
-@debugserver_test
-def test_auxv_keys_look_valid_debugserver(self):
-self.build()
-self

[Lldb-commits] [PATCH] D93895: Implement vAttachWait in lldb-server

2021-01-06 Thread Augusto Noronha via Phabricator via lldb-commits 
augusto2112 updated this revision to Diff 314896.
augusto2112 marked 5 inline comments as done.
augusto2112 added a comment.

Changes test to launch a process before and after we ask for the attach. Minor 
code fixes as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93895

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py

Index: lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py
@@ -0,0 +1,75 @@
+
+import os
+from time import sleep
+
+import gdbremote_testcase
+import lldbgdbserverutils
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestGdbRemoteAttachWait(gdbremote_testcase.GdbRemoteTestCaseBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def test_attach_with_vAttachWait(self):
+exe = '%s_%d' % (self.testMethodName, os.getpid())
+
+def launch_inferior():
+inferior = self.launch_process_for_attach(
+inferior_args=["sleep:60"],
+exe_path=self.getBuildArtifact(exe))
+self.assertIsNotNone(inferior)
+self.assertTrue(inferior.pid > 0)
+self.assertTrue(
+lldbgdbserverutils.process_is_running(
+inferior.pid, True))
+return inferior
+
+self.build(dictionary={'EXE': exe})
+self.set_inferior_startup_attach_manually()
+
+server = self.connect_to_debug_monitor()
+self.assertIsNotNone(server)
+
+# Launch the first inferior (we shouldn't attach to this one).
+launch_inferior()
+
+self.add_no_ack_remote_stream()
+self.test_sequence.add_log_lines([
+# Do the attach.
+"read packet: $vAttachWait;{}#00".format(lldbgdbserverutils.gdbremote_hex_encode_string(exe)),
+], True)
+# Run the stream until attachWait.
+context = self.expect_gdbremote_sequence()
+self.assertIsNotNone(context)
+
+# Sleep so we're sure that the inferior is launched after we ask for the attach.
+sleep(1)
+
+# Launch the second inferior (we SHOULD attach to this one).
+inferior_to_attach = launch_inferior()
+
+# Make sure the attach succeeded.
+self.test_sequence.add_log_lines([
+{"direction": "send",
+ "regex": r"^\$T([0-9a-fA-F]{2})[^#]*#[0-9a-fA-F]{2}$",
+ "capture": {1: "stop_signal_hex"}},
+], True)
+self.add_process_info_collection_packets()
+
+
+# Run the stream sending the response..
+context = self.expect_gdbremote_sequence()
+self.assertIsNotNone(context)
+
+# Gather process info response.
+process_info = self.parse_process_info_response(context)
+self.assertIsNotNone(process_info)
+
+# Ensure the process id matches what we expected.
+pid_text = process_info.get('pid', None)
+self.assertIsNotNone(pid_text)
+reported_pid = int(pid_text, base=16)
+self.assertEqual(reported_pid, inferior_to_attach.pid)
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -59,6 +59,17 @@
   /// attach operation.
   Status AttachToProcess(lldb::pid_t pid);
 
+  /// Wait to attach to a process with a given name.
+  ///
+  /// This method supports waiting for the next instance of a process
+  /// with a given name and attaching llgs to that via the configured
+  /// Platform.
+  ///
+  /// \return
+  /// An Status object indicating the success or failure of the
+  /// attach operation.
+  Status AttachWaitProcess(llvm::StringRef process_name);
+
   // NativeProcessProtocol::NativeDelegate overrides
   void InitializeDelegate(NativeProcessProtocol *process) override;
 
@@ -170,6 +181,8 @@
 
   PacketResult Handle_vAttach(StringExtractorGDBRemote &packet);
 
+  PacketResult Handle_vAttachWait(StringExtractorGDBRemote &packet);
+
   PacketResult Handle_D(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_qThreadStopInfo(StringExtractorGDBRemote &packet);
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServ

[Lldb-commits] [PATCH] D91734: [FastISel] Flush local value map on every instruction

2021-01-06 Thread Paul Robinson via Phabricator via lldb-commits 
probinson added a comment.

This version of the patch avoids the weirdness I was seeing with prolog 
instructions in certain cases.

The gdb test suite is largely happy with this; it avoids the new failures I 
mentioned previously, and one test needed to have some "next" commands updated. 
 Here's the gdb test suite patch (against 8.3; I can provide more context if 
it's not clear how to apply to later versions).

  diff --git a/gdb-8.3/gdb/testsuite/gdb.base/foll-exec.exp 
b/gdb-8.3/gdb/testsuite/gdb.base/foll-exec.exp
  index 3e0653a1..61ee752f 100644
  --- a/gdb-8.3/gdb/testsuite/gdb.base/foll-exec.exp
  +++ b/gdb-8.3/gdb/testsuite/gdb.base/foll-exec.exp
  @@ -115,7 +115,10 @@ proc do_exec_tests {} {
  # We should stop in execd-program, at its first statement.
  #
  set execd_line [gdb_get_line_number "after-exec" $srcfile2]
  -   send_gdb "next\n"
  +# Clang will emit source locations for the parameter evaluation.
  +# Ideally we'd "next" until we saw 'execlp' again in the source display,
  +# then do one more.
  +   send_gdb "next 3\n"
  gdb_expect {
-re ".*xecuting new program: 
.*${testfile2}.*${srcfile2}:${execd_line}.*int  local_j = argc;.*$gdb_prompt $"\
{pass "step through execlp call"}
  @@ -263,7 +266,7 @@ proc do_exec_tests {} {
  # the newly-exec'd program, not after the remaining step-count
  # reaches zero.
  #
  -   send_gdb "next 2\n"
  +   send_gdb "next 3\n"
  gdb_expect {
-re ".*xecuting new program: 
.*${testfile2}.*${srcfile2}:${execd_line}.*int  local_j = argc;.*$gdb_prompt $"\
{pass "step through execl call"}

I think I might want to add one more lit test, because the LLVM suite didn't 
catch a thinko that was the root cause of that last prolog weirdness.  But this 
version is totally ready for review.


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

https://reviews.llvm.org/D91734

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

[Lldb-commits] [PATCH] D91734: [FastISel] Flush local value map on every instruction

2021-01-06 Thread David Blaikie via Phabricator via lldb-commits 
dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

In D91734#2480474 , @probinson wrote:

> This version of the patch avoids the weirdness I was seeing with prolog 
> instructions in certain cases.
>
> The gdb test suite is largely happy with this; it avoids the new failures I 
> mentioned previously, and one test needed to have some "next" commands 
> updated.  Here's the gdb test suite patch (against 8.3; I can provide more 
> context if it's not clear how to apply to later versions).
>
>   diff --git a/gdb-8.3/gdb/testsuite/gdb.base/foll-exec.exp 
> b/gdb-8.3/gdb/testsuite/gdb.base/foll-exec.exp
>   index 3e0653a1..61ee752f 100644
>   --- a/gdb-8.3/gdb/testsuite/gdb.base/foll-exec.exp
>   +++ b/gdb-8.3/gdb/testsuite/gdb.base/foll-exec.exp
>   @@ -115,7 +115,10 @@ proc do_exec_tests {} {
>   # We should stop in execd-program, at its first statement.
>   #
>   set execd_line [gdb_get_line_number "after-exec" $srcfile2]
>   -   send_gdb "next\n"
>   +# Clang will emit source locations for the parameter evaluation.
>   +# Ideally we'd "next" until we saw 'execlp' again in the source 
> display,
>   +# then do one more.
>   +   send_gdb "next 3\n"
>   gdb_expect {
> -re ".*xecuting new program: 
> .*${testfile2}.*${srcfile2}:${execd_line}.*int  local_j = argc;.*$gdb_prompt 
> $"\
> {pass "step through execlp call"}
>   @@ -263,7 +266,7 @@ proc do_exec_tests {} {
>   # the newly-exec'd program, not after the remaining step-count
>   # reaches zero.
>   #
>   -   send_gdb "next 2\n"
>   +   send_gdb "next 3\n"
>   gdb_expect {
> -re ".*xecuting new program: 
> .*${testfile2}.*${srcfile2}:${execd_line}.*int  local_j = argc;.*$gdb_prompt 
> $"\
> {pass "step through execl call"}
>
> I think I might want to add one more lit test, because the LLVM suite didn't 
> catch a thinko that was the root cause of that last prolog weirdness.  But 
> this version is totally ready for review.

Thanks - that seems to cover Google's internal gdb test execution as well. 
Appreciate the test patch!

I haven't looked closely, but I take it this one test modification seems 
reasonable to you? I guess we're stepping into some subexpressions on a 
function call that we previously didn't? (they didn't have any instructions 
attributed to them until now, but it's not unreasonable that they have them 
attributed - for materializing constants to pass to a function call when the 
function call/constants are split over multiple lines, etc)


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

https://reviews.llvm.org/D91734

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

[Lldb-commits] [PATCH] D91734: [FastISel] Flush local value map on every instruction

2021-01-06 Thread Paul Robinson via Phabricator via lldb-commits 
probinson updated this revision to Diff 314708.
probinson added a comment.

Change how PHIs are handled; if the operand has a debug location, use it, 
otherwise don't set a debug location.  Then, flushLocalValueMap() will look at 
the first local-value instruction; if it doesn't already have a debug location, 
we copy the location from the first instruction after the local-value 
instructions (which should have the debug location of the original IR 
instruction).


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

https://reviews.llvm.org/D91734

Files:
  lld/test/wasm/debug-removed-fn.ll
  lldb/test/Shell/SymbolFile/NativePDB/disassembly.cpp
  lldb/test/Shell/SymbolFile/NativePDB/load-pdb.cpp
  llvm/include/llvm/CodeGen/FastISel.h
  llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
  llvm/test/CodeGen/AArch64/arm64-abi_align.ll
  llvm/test/CodeGen/AArch64/arm64-fast-isel-call.ll
  llvm/test/CodeGen/AArch64/arm64-fast-isel-gv.ll
  llvm/test/CodeGen/AArch64/arm64-fast-isel-intrinsic.ll
  llvm/test/CodeGen/AArch64/arm64-fast-isel.ll
  llvm/test/CodeGen/AArch64/arm64-patchpoint-webkit_jscc.ll
  llvm/test/CodeGen/AArch64/cfguard-checks.ll
  llvm/test/CodeGen/AArch64/elf-globals-static.ll
  llvm/test/CodeGen/AArch64/large-stack.ll
  llvm/test/CodeGen/ARM/fast-isel-call.ll
  llvm/test/CodeGen/ARM/fast-isel-intrinsic.ll
  llvm/test/CodeGen/ARM/fast-isel-ldr-str-thumb-neg-index.ll
  llvm/test/CodeGen/ARM/fast-isel-ldrh-strh-arm.ll
  llvm/test/CodeGen/ARM/fast-isel-select.ll
  llvm/test/CodeGen/ARM/fast-isel.ll
  llvm/test/CodeGen/Mips/Fast-ISel/callabi.ll
  llvm/test/CodeGen/Mips/Fast-ISel/fastalloca.ll
  llvm/test/CodeGen/Mips/Fast-ISel/fpcmpa.ll
  llvm/test/CodeGen/Mips/Fast-ISel/icmpa.ll
  llvm/test/CodeGen/Mips/Fast-ISel/logopm.ll
  llvm/test/CodeGen/Mips/Fast-ISel/overflt.ll
  llvm/test/CodeGen/Mips/Fast-ISel/shftopm.ll
  llvm/test/CodeGen/Mips/Fast-ISel/simplestore.ll
  llvm/test/CodeGen/Mips/Fast-ISel/simplestorei.ll
  llvm/test/CodeGen/Mips/emergency-spill-slot-near-fp.ll
  llvm/test/CodeGen/PowerPC/elf-common.ll
  llvm/test/CodeGen/PowerPC/fast-isel-load-store.ll
  llvm/test/CodeGen/PowerPC/mcm-1.ll
  llvm/test/CodeGen/PowerPC/mcm-13.ll
  llvm/test/CodeGen/PowerPC/mcm-2.ll
  llvm/test/CodeGen/PowerPC/mcm-3.ll
  llvm/test/CodeGen/PowerPC/mcm-6.ll
  llvm/test/CodeGen/PowerPC/mcm-9.ll
  llvm/test/CodeGen/PowerPC/mcm-default.ll
  llvm/test/CodeGen/X86/atomic-unordered.ll
  llvm/test/CodeGen/X86/atomic64.ll
  llvm/test/CodeGen/X86/crash-O0.ll
  llvm/test/CodeGen/X86/fast-isel-constant.ll
  llvm/test/CodeGen/X86/fast-isel-mem.ll
  llvm/test/CodeGen/X86/fast-isel-select.ll
  llvm/test/CodeGen/X86/lvi-hardening-loads.ll
  llvm/test/CodeGen/X86/membarrier.ll
  llvm/test/CodeGen/X86/pr32241.ll
  llvm/test/CodeGen/X86/pr32256.ll
  llvm/test/CodeGen/X86/pr32284.ll
  llvm/test/CodeGen/X86/pr32340.ll
  llvm/test/CodeGen/X86/pr44749.ll
  llvm/test/CodeGen/X86/volatile.ll
  llvm/test/DebugInfo/COFF/lines-bb-start.ll
  llvm/test/DebugInfo/Mips/delay-slot.ll
  llvm/test/DebugInfo/X86/fission-ranges.ll

Index: llvm/test/DebugInfo/X86/fission-ranges.ll
===
--- llvm/test/DebugInfo/X86/fission-ranges.ll
+++ llvm/test/DebugInfo/X86/fission-ranges.ll
@@ -45,31 +45,31 @@
 ; if they've changed due to a bugfix, change in register allocation, etc.
 
 ; CHECK:  [[A]]:
-; CHECK-NEXT:   DW_LLE_startx_length (0x0002, 0x000f): DW_OP_consts +0, DW_OP_stack_value
-; CHECK-NEXT:   DW_LLE_startx_length (0x0003, 0x000b): DW_OP_reg0 RAX
-; CHECK-NEXT:   DW_LLE_startx_length (0x0004, 0x0012): DW_OP_breg7 RSP-4
+; CHECK-NEXT:   DW_LLE_startx_length (0x0001, 0x0011): DW_OP_consts +0, DW_OP_stack_value
+; CHECK-NEXT:   DW_LLE_startx_length (0x0002, 0x000b): DW_OP_reg0 RAX
+; CHECK-NEXT:   DW_LLE_startx_length (0x0003, 0x0012): DW_OP_breg7 RSP-4
 ; CHECK-NEXT:   DW_LLE_end_of_list   ()
 ; CHECK:  [[E]]:
-; CHECK-NEXT:   DW_LLE_startx_length (0x0005, 0x000b): DW_OP_reg0 RAX
-; CHECK-NEXT:   DW_LLE_startx_length (0x0006, 0x005a): DW_OP_breg7 RSP-48
+; CHECK-NEXT:   DW_LLE_startx_length (0x0004, 0x000b): DW_OP_reg0 RAX
+; CHECK-NEXT:   DW_LLE_startx_length (0x0005, 0x005a): DW_OP_breg7 RSP-48
 ; CHECK-NEXT:   DW_LLE_end_of_list   ()
 ; CHECK:  [[B]]:
-; CHECK-NEXT:   DW_LLE_startx_length (0x0007, 0x000b): DW_OP_reg0 RAX
-; CHECK-NEXT:   DW_LLE_startx_length (0x0008, 0x0042): DW_OP_breg7 RSP-24
+; CHECK-NEXT:   DW_LLE_startx_length (0x0006, 0x000b): DW_OP_reg0 RAX
+; CHECK-NEXT:   DW_LLE_startx_length (0x0007, 0x0042): DW_OP_breg7 RSP-24
 ; CHECK-NEXT:   DW_LLE_end_of_list   ()
 ; CHECK:  [[D]]:
-; CHECK-NEXT:   DW_LLE_startx_length (0x0009, 0x000b): DW_OP_reg0 RAX
-; CHECK-NEXT:   DW_LLE_startx_length (0x000a, 0x002a): DW_OP_breg7 RSP-12
+; CHECK-NEXT:   DW_LLE_startx_length (0x0008, 0x000b): DW_OP_reg0 RAX
+; CHECK-NEXT:   DW_LLE

[Lldb-commits] [PATCH] D91734: [FastISel] Flush local value map on every instruction

2021-01-06 Thread Paul Robinson via Phabricator via lldb-commits 
probinson added a comment.

In D91734#2480930 , @dblaikie wrote:

> I haven't looked closely, but I take it this one test modification seems 
> reasonable to you? I guess we're stepping into some subexpressions on a 
> function call that we previously didn't? (they didn't have any instructions 
> attributed to them until now, but it's not unreasonable that they have them 
> attributed - for materializing constants to pass to a function call when the 
> function call/constants are split over multiple lines, etc)

Exactly.  In each case, the test is trying to "next" over a function call; gcc 
attributes all parameter evaluation to the function name, while clang will 
attribute each parameter to its own location.  And when the parameters span 
multiple source lines, the is_stmt heuristic kicks in, so we stop on each line 
with actual parameters.

This is not ideal IMO; I'd rather be more principled about (a) stop at every 
parameter, or (b) stop at no parameters.  But without reworking how we do 
is_stmt, I think fiddling the test to do enough single-steps to actually get 
past the function call is okay.


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

https://reviews.llvm.org/D91734

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

[Lldb-commits] [PATCH] D91734: [FastISel] Flush local value map on every instruction

2021-01-06 Thread David Blaikie via Phabricator via lldb-commits 
dblaikie added a comment.

In D91734#2482280 , @probinson wrote:

> In D91734#2480930 , @dblaikie wrote:
>
>> I haven't looked closely, but I take it this one test modification seems 
>> reasonable to you? I guess we're stepping into some subexpressions on a 
>> function call that we previously didn't? (they didn't have any instructions 
>> attributed to them until now, but it's not unreasonable that they have them 
>> attributed - for materializing constants to pass to a function call when the 
>> function call/constants are split over multiple lines, etc)
>
> Exactly.  In each case, the test is trying to "next" over a function call; 
> gcc attributes all parameter evaluation to the function name, while clang 
> will attribute each parameter to its own location.  And when the parameters 
> span multiple source lines, the is_stmt heuristic kicks in, so we stop on 
> each line with actual parameters.
>
> This is not ideal IMO; I'd rather be more principled about (a) stop at every 
> parameter, or (b) stop at no parameters.  But without reworking how we do 
> is_stmt, I think fiddling the test to do enough single-steps to actually get 
> past the function call is okay.

I get that it's a bit unreliable for the user - though GCC's approach isn't so 
different. If any of the parameter evaluations are themselves function calls, I 
think it does still multistep as well.

Actually, it seems it does not. Somewhere back around GCC 6 it did what we're 
doing, but since about 8 at least, no matter what the expressions (at least I 
tested for + and function call) it attributes all the instructions to the 
function call line - even the backtrace is misleading - I don't think it's 
better, just different.

But, yes, we could possibly do better with more strategic is_stmt, but that's a 
big/complex piece of work to tackle.


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

https://reviews.llvm.org/D91734

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

[Lldb-commits] [PATCH] D91734: [FastISel] Flush local value map on every instruction

2021-01-06 Thread Paul Robinson via Phabricator via lldb-commits 
probinson added a comment.

In D91734#2482318 , @dblaikie wrote:

> But, yes, we could possibly do better with more strategic is_stmt, but that's 
> a big/complex piece of work to tackle.

Oh, absolutely.  Didn't mean to imply otherwise.  And the behavior now is at 
least easily explainable in terms of source line.

Thanks for the review, this has been an irritant for our licensees since 
forever.  And especially thanks @rnk for the flush-on-every-instruction 
suggestion.


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

https://reviews.llvm.org/D91734

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

[Lldb-commits] [lldb] 6d94eea - [lldb] Ad os_signpost support to lldb_private::Timer

2021-01-06 Thread Jonas Devlieghere via lldb-commits 

Author: Jonas Devlieghere
Date: 2021-01-06T15:16:09-08:00
New Revision: 6d94eeadd28af4d488b5875778a3ebfa0d749b52

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

LOG: [lldb] Ad os_signpost support to lldb_private::Timer

Emit os_signposts when supported from LLDB's timer class. A vast amount
of performance sensitive places in LLDB are already instrumented with
the Timer class.

By emitting signposts we can examine this information in Instruments. I
recommend looking at Daniel's differential for why this is so powerful:
https://reviews.llvm.org/D52954.

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

Added: 


Modified: 
lldb/include/lldb/Utility/Timer.h
lldb/source/Utility/Timer.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/Timer.h 
b/lldb/include/lldb/Utility/Timer.h
index 91f9c57c03c1..edc064b23b57 100644
--- a/lldb/include/lldb/Utility/Timer.h
+++ b/lldb/include/lldb/Utility/Timer.h
@@ -25,6 +25,7 @@ class Timer {
   class Category {
   public:
 explicit Category(const char *category_name);
+llvm::StringRef GetName() { return m_name; }
 
   private:
 friend class Timer;

diff  --git a/lldb/source/Utility/Timer.cpp b/lldb/source/Utility/Timer.cpp
index d55c9863117b..7ead51069529 100644
--- a/lldb/source/Utility/Timer.cpp
+++ b/lldb/source/Utility/Timer.cpp
@@ -7,6 +7,8 @@
 
//===--===//
 #include "lldb/Utility/Timer.h"
 #include "lldb/Utility/Stream.h"
+#include "llvm/Support/ManagedStatic.h"
+#include "llvm/Support/Signposts.h"
 
 #include 
 #include 
@@ -28,6 +30,9 @@ typedef std::vector TimerStack;
 static std::atomic g_categories;
 } // end of anonymous namespace
 
+/// Allows llvm::Timer to emit signposts when supported.
+static llvm::ManagedStatic Signposts;
+
 std::atomic Timer::g_quiet(true);
 std::atomic Timer::g_display_depth(0);
 static std::mutex &GetFileMutex() {
@@ -54,6 +59,7 @@ void Timer::SetQuiet(bool value) { g_quiet = value; }
 
 Timer::Timer(Timer::Category &category, const char *format, ...)
 : m_category(category), m_total_start(std::chrono::steady_clock::now()) {
+  Signposts->startInterval(this, m_category.GetName());
   TimerStack &stack = GetTimerStackForCurrentThread();
 
   stack.push_back(this);
@@ -80,6 +86,8 @@ Timer::~Timer() {
   auto total_dur = stop_time - m_total_start;
   auto timer_dur = total_dur - m_child_duration;
 
+  Signposts->endInterval(this, m_category.GetName());
+
   TimerStack &stack = GetTimerStackForCurrentThread();
   if (g_quiet && stack.size() <= g_display_depth) {
 std::lock_guard lock(GetFileMutex());



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

[Lldb-commits] [PATCH] D93657: [lldb] Ad os_signpost support to `lldb_private::Timer`

2021-01-06 Thread Jonas Devlieghere via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
JDevlieghere marked an inline comment as done.
Closed by commit rG6d94eeadd28a: [lldb] Ad os_signpost support to 
lldb_private::Timer (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93657

Files:
  lldb/include/lldb/Utility/Timer.h
  lldb/source/Utility/Timer.cpp


Index: lldb/source/Utility/Timer.cpp
===
--- lldb/source/Utility/Timer.cpp
+++ lldb/source/Utility/Timer.cpp
@@ -7,6 +7,8 @@
 
//===--===//
 #include "lldb/Utility/Timer.h"
 #include "lldb/Utility/Stream.h"
+#include "llvm/Support/ManagedStatic.h"
+#include "llvm/Support/Signposts.h"
 
 #include 
 #include 
@@ -28,6 +30,9 @@
 static std::atomic g_categories;
 } // end of anonymous namespace
 
+/// Allows llvm::Timer to emit signposts when supported.
+static llvm::ManagedStatic Signposts;
+
 std::atomic Timer::g_quiet(true);
 std::atomic Timer::g_display_depth(0);
 static std::mutex &GetFileMutex() {
@@ -54,6 +59,7 @@
 
 Timer::Timer(Timer::Category &category, const char *format, ...)
 : m_category(category), m_total_start(std::chrono::steady_clock::now()) {
+  Signposts->startInterval(this, m_category.GetName());
   TimerStack &stack = GetTimerStackForCurrentThread();
 
   stack.push_back(this);
@@ -80,6 +86,8 @@
   auto total_dur = stop_time - m_total_start;
   auto timer_dur = total_dur - m_child_duration;
 
+  Signposts->endInterval(this, m_category.GetName());
+
   TimerStack &stack = GetTimerStackForCurrentThread();
   if (g_quiet && stack.size() <= g_display_depth) {
 std::lock_guard lock(GetFileMutex());
Index: lldb/include/lldb/Utility/Timer.h
===
--- lldb/include/lldb/Utility/Timer.h
+++ lldb/include/lldb/Utility/Timer.h
@@ -25,6 +25,7 @@
   class Category {
   public:
 explicit Category(const char *category_name);
+llvm::StringRef GetName() { return m_name; }
 
   private:
 friend class Timer;


Index: lldb/source/Utility/Timer.cpp
===
--- lldb/source/Utility/Timer.cpp
+++ lldb/source/Utility/Timer.cpp
@@ -7,6 +7,8 @@
 //===--===//
 #include "lldb/Utility/Timer.h"
 #include "lldb/Utility/Stream.h"
+#include "llvm/Support/ManagedStatic.h"
+#include "llvm/Support/Signposts.h"
 
 #include 
 #include 
@@ -28,6 +30,9 @@
 static std::atomic g_categories;
 } // end of anonymous namespace
 
+/// Allows llvm::Timer to emit signposts when supported.
+static llvm::ManagedStatic Signposts;
+
 std::atomic Timer::g_quiet(true);
 std::atomic Timer::g_display_depth(0);
 static std::mutex &GetFileMutex() {
@@ -54,6 +59,7 @@
 
 Timer::Timer(Timer::Category &category, const char *format, ...)
 : m_category(category), m_total_start(std::chrono::steady_clock::now()) {
+  Signposts->startInterval(this, m_category.GetName());
   TimerStack &stack = GetTimerStackForCurrentThread();
 
   stack.push_back(this);
@@ -80,6 +86,8 @@
   auto total_dur = stop_time - m_total_start;
   auto timer_dur = total_dur - m_child_duration;
 
+  Signposts->endInterval(this, m_category.GetName());
+
   TimerStack &stack = GetTimerStackForCurrentThread();
   if (g_quiet && stack.size() <= g_display_depth) {
 std::lock_guard lock(GetFileMutex());
Index: lldb/include/lldb/Utility/Timer.h
===
--- lldb/include/lldb/Utility/Timer.h
+++ lldb/include/lldb/Utility/Timer.h
@@ -25,6 +25,7 @@
   class Category {
   public:
 explicit Category(const char *category_name);
+llvm::StringRef GetName() { return m_name; }
 
   private:
 friend class Timer;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D91847: [lldb] [debugserver] Add stN aliases for stmmN for compatibility

2021-01-06 Thread Michał Górny via Phabricator via lldb-commits 
mgorny added a comment.

I suppose this has waited long enough to deserve another test run.


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

https://reviews.llvm.org/D91847

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

[Lldb-commits] [PATCH] D93951: [vscode] Improve runInTerminal and support linux

2021-01-06 Thread walter erquinigo via Phabricator via lldb-commits 
wallace updated this revision to Diff 315015.
wallace marked 10 inline comments as done.
wallace added a comment.
Herald added a subscriber: mgorny.

Address all comments.

- Moved all the communication logic between the debug adaptor and the launcher 
to classes, simplifying the code in lldb-vscode.
- Added more comments and tests
- Created a file where lldb-vscode can write errors to.
- Now returning any error messages to the IDE, so that the user can see what's 
going on. No exit() calls are using in the debug adaptor side.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93951

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/test/API/tools/lldb-vscode/runInTerminal/TestVSCode_runInTerminal.py
  lldb/tools/lldb-vscode/CMakeLists.txt
  lldb/tools/lldb-vscode/JSONUtils.cpp
  lldb/tools/lldb-vscode/JSONUtils.h
  lldb/tools/lldb-vscode/Options.td
  lldb/tools/lldb-vscode/RunInTerminal.cpp
  lldb/tools/lldb-vscode/RunInTerminal.h
  lldb/tools/lldb-vscode/VSCode.h
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -384,12 +384,7 @@
 break;
   case lldb::eStateSuspended:
 break;
-  case lldb::eStateStopped: {
-if (g_vsc.waiting_for_run_in_terminal) {
-  g_vsc.waiting_for_run_in_terminal = false;
-  g_vsc.request_in_terminal_cv.notify_one();
-}
-  }
+  case lldb::eStateStopped:
 // Only report a stopped event if the process was not restarted.
 if (!lldb::SBProcess::GetRestartedFromEvent(event)) {
   SendStdOutStdErr(process);
@@ -461,7 +456,7 @@
 }
 body.try_emplace("module", module_value);
 module_event.try_emplace("body", std::move(body));
-g_vsc.SendJSON(llvm::json::Value(std::move(module_event)));
+// g_vsc.SendJSON(llvm::json::Value(std::move(module_event)));
   }
 }
   } else if (event.BroadcasterMatchesRef(g_vsc.broadcaster)) {
@@ -1379,9 +1374,11 @@
 filters.emplace_back(CreateExceptionBreakpointFilter(exc_bp));
   }
   body.try_emplace("exceptionBreakpointFilters", std::move(filters));
-  // The debug adapter supports launching a debugee in intergrated VSCode
-  // terminal.
+#if !defined(_WIN32)
   body.try_emplace("supportsRunInTerminalRequest", true);
+#else
+  body.try_emplace("supportsRunInTerminalRequest", false);
+#endif
   // The debug adapter supports stepping back via the stepBack and
   // reverseContinue requests.
   body.try_emplace("supportsStepBack", false);
@@ -1441,47 +1438,61 @@
   g_vsc.SendJSON(llvm::json::Value(std::move(response)));
 }
 
-void request_runInTerminal(const llvm::json::Object &launch_request,
-   llvm::json::Object &launch_response) {
-  // We have already created a target that has a valid "program" path to the
-  // executable. We will attach to the next process whose name matches that
-  // of the target's.
+llvm::Error request_runInTerminal(const llvm::json::Object &launch_request) {
+#if defined(_WIN32)
+  return llvm::createStringError(
+  llvm::inconvertibleErrorCode(),
+  "\"runInTerminal\" is not supported on Windows");
+#else
   g_vsc.is_attach = true;
   lldb::SBAttachInfo attach_info;
-  lldb::SBError error;
-  attach_info.SetWaitForLaunch(true, /*async*/ true);
-  g_vsc.target.Attach(attach_info, error);
 
-  llvm::json::Object reverse_request =
-  CreateRunInTerminalReverseRequest(launch_request);
+  llvm::Expected comm_channel_or_err =
+  RunInTerminalDebugAdaptorCommChannel::Create();
+  if (!comm_channel_or_err)
+return comm_channel_or_err.takeError();
+  RunInTerminalDebugAdaptorCommChannel &comm_channel =
+  *comm_channel_or_err.get();
+
+  llvm::json::Object reverse_request = CreateRunInTerminalReverseRequest(
+  launch_request, g_vsc.debug_adaptor_path,
+  comm_channel.GetCommFilesPrefix());
   llvm::json::Object reverse_response;
   lldb_vscode::PacketStatus status =
   g_vsc.SendReverseRequest(reverse_request, reverse_response);
   if (status != lldb_vscode::PacketStatus::Success)
-error.SetErrorString("Process cannot be launched by IDE.");
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "Process cannot be launched by the IDE. %s",
+   comm_channel.GetLauncherError().c_str());
 
-  if (error.Success()) {
-// Wait for the attach stop event to happen or for a timeout.
-g_vsc.waiting_for_run_in_terminal = true;
-static std::mutex mutex;
-std::unique_lock locker(mutex);
-g_vsc.re

[Lldb-commits] [PATCH] D93895: Implement vAttachWait in lldb-server

2021-01-06 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.

LGTM. Nice and simple. We can do another patch for vAttachOrWait as it will be 
very simple modifications and very similar to this patch. I am find avoiding 
all of the polling interval and duration settings for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93895

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

[Lldb-commits] [PATCH] D78978: [LLDB] Add support for WebAssembly debugging

2021-01-06 Thread Paolo Severini via Phabricator via lldb-commits 
paolosev added a comment.

In D78978#2481358 , @vwzm228 wrote:

> Is there any progress about such patch and D78801 
> ？
>
> I have implemented the debugging feature in our Wasm VM based on  
> https://reviews.llvm.org/D78801, and it already work to attach, set 
> breakpoint, step, show variable value, backtrace...
>
> I am not sure if I need to change LLDB part to this one, or keep using D78801 
> .
>
> But if both patches wont be merged, I have to maintain a private LLDB 
> version

Unfortunately I have not received any more feedback for this patch, or the 
alternative version D78801 , so I assumed it 
won't move forward :(
Meanwhile, I have created a personal fork 
(https://github.com/paolosevMSFT/llvm-project/tree/WasmDbg).

I still think that it would be very useful to add support for Wasm debugging to 
LLDB. Especially in scenarios where Wasm is not used as part of a web app, but 
server side (node.js) or running on micro runtime for IoT devices.
I know that there is the problem that LLDB does not support segmented address 
spaces, and so this patch requires a couple of tiny but smelly changes to core 
code.
From the mailing list I seem to remember that somebody was working to add 
support for segmented addresses, but I don't know what is the current state of 
the initiative.

I'd be happy to keep working on this, please let me know what I could do to 
progress toward an acceptable solution.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78978

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

[Lldb-commits] [PATCH] D78978: [LLDB] Add support for WebAssembly debugging

2021-01-06 Thread David Blaikie via Phabricator via lldb-commits 
dblaikie added a comment.

In D78978#2483327 , @paolosev wrote:

> In D78978#2481358 , @vwzm228 wrote:
>
>> Is there any progress about such patch and D78801 
>> ？
>>
>> I have implemented the debugging feature in our Wasm VM based on  
>> https://reviews.llvm.org/D78801, and it already work to attach, set 
>> breakpoint, step, show variable value, backtrace...
>>
>> I am not sure if I need to change LLDB part to this one, or keep using 
>> D78801 .
>>
>> But if both patches wont be merged, I have to maintain a private LLDB 
>> version
>
> Unfortunately I have not received any more feedback for this patch, or the 
> alternative version D78801 , so I assumed it 
> won't move forward :(
> Meanwhile, I have created a personal fork 
> (https://github.com/paolosevMSFT/llvm-project/tree/WasmDbg).
>
> I still think that it would be very useful to add support for Wasm debugging 
> to LLDB. Especially in scenarios where Wasm is not used as part of a web app, 
> but server side (node.js) or running on micro runtime for IoT devices.
> I know that there is the problem that LLDB does not support segmented address 
> spaces, and so this patch requires a couple of tiny but smelly changes to 
> core code.
> From the mailing list I seem to remember that somebody was working to add 
> support for segmented addresses, but I don't know what is the current state 
> of the initiative.
>
> I'd be happy to keep working on this, please let me know what I could do to 
> progress toward an acceptable solution.

Usually the thing is to ping the review thread at most weekly & maybe search 
around for specific reviewers to ask if you're met with a lot of silence.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78978

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

[Lldb-commits] [lldb] d853bd7 - [lldb/Lua] add support for multiline scripted breakpoints

2021-01-06 Thread Pedro Tammela via lldb-commits 

Author: Pedro Tammela
Date: 2021-01-07T00:31:36Z
New Revision: d853bd7a4e86a50f7d7e6a5f397fcbd1e7d844b4

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

LOG: [lldb/Lua] add support for multiline scripted breakpoints

1 - Partial Statements

The interpreter loop runs every line it receives, so partial
Lua statements are not being handled properly. This is a problem for
multiline breakpoint scripts since the interpreter loop, for this
particular case, is just an abstraction to a partially parsed function
body declaration.

This patch addresses this issue and as a side effect improves the
general Lua interpreter loop as well. It's now possible to write partial
statements in the 'script' command.

Example:
   (lldb) script
   >>>   do
   ..>   local a = 123
   ..>   print(a)
   ..>   end
   123

The technique implemented is the same as the one employed by Lua's own REPL 
implementation.
Partial statements always errors out with the '' tag in the error
message.

2 - CheckSyntax in Lua.h

In order to support (1), we need an API for just checking the syntax of string 
buffers.

3 - Multiline scripted breakpoints

Finally, with all the base features implemented this feature is
straightforward. The interpreter loop behaves exactly the same, the
difference is that it will aggregate all Lua statements into the body of
the breakpoint function. An explicit 'quit' statement is needed to exit the
interpreter loop.

Example:
   (lldb) breakpoint command add -s lua
   Enter your Lua command(s). Type 'quit' to end.
   The commands are compiled as the body of the following Lua function
   function (frame, bp_loc, ...) end
   ..> print(456)
   ..> a = 123
   ..> quit

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

Added: 
lldb/test/Shell/ScriptInterpreter/Lua/partial_statements.test

Modified: 
lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_callback.test

Removed: 




diff  --git a/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp 
b/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
index fb3628a3107c..ec946d1b97e4 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
@@ -105,6 +105,23 @@ Lua::CallBreakpointCallback(void *baton, 
lldb::StackFrameSP stop_frame_sp,
bp_loc_sp);
 }
 
+llvm::Error Lua::CheckSyntax(llvm::StringRef buffer) {
+  int error =
+  luaL_loadbuffer(m_lua_state, buffer.data(), buffer.size(), "buffer");
+  if (error == LUA_OK) {
+// Pop buffer
+lua_pop(m_lua_state, 1);
+return llvm::Error::success();
+  }
+
+  llvm::Error e = llvm::make_error(
+  llvm::formatv("{0}\n", lua_tostring(m_lua_state, -1)),
+  llvm::inconvertibleErrorCode());
+  // Pop error message from the stack.
+  lua_pop(m_lua_state, 1);
+  return e;
+}
+
 llvm::Error Lua::LoadModule(llvm::StringRef filename) {
   FileSpec file(filename);
   if (!FileSystem::Instance().Exists(file)) {

diff  --git a/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h 
b/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
index 39c39e1c43b8..84cc0148a7eb 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
+++ b/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
@@ -36,6 +36,7 @@ class Lua {
   CallBreakpointCallback(void *baton, lldb::StackFrameSP stop_frame_sp,
  lldb::BreakpointLocationSP bp_loc_sp);
   llvm::Error LoadModule(llvm::StringRef filename);
+  llvm::Error CheckSyntax(llvm::StringRef buffer);
   llvm::Error ChangeIO(FILE *out, FILE *err);
 
 private:

diff  --git 
a/lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp 
b/lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
index 239b409ac695..1dbadb90813c 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
@@ -17,23 +17,33 @@
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/StringList.h"
 #include "lldb/Utility/Timer.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/FormatAdapters.h"
 #include 
+#include 
 
 using namespace lldb;
 using namespace lldb_private;
 
 LLDB_PLUGIN_DEFINE(ScriptInterpreterLua)
 
+enum ActiveIOHandler {
+  eIOHandlerNone,
+  eIOHandlerBreakpoint,
+  eIOHandlerWatchpoint
+};
+
 class IOHandlerLuaInterpreter : public IOHandlerDelegate,
 public IOHandlerEditline {
 public:
   IOHandlerLuaInterpreter(Debugger &debugger,
-  ScriptInterpret

[Lldb-commits] [PATCH] D93481: [lldb/Lua] add support for multiline scripted breakpoints

2021-01-06 Thread Pedro Tammela 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 rGd853bd7a4e86: [lldb/Lua] add support for multiline scripted 
breakpoints (authored by tammela).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93481

Files:
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
  lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_callback.test
  lldb/test/Shell/ScriptInterpreter/Lua/partial_statements.test

Index: lldb/test/Shell/ScriptInterpreter/Lua/partial_statements.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/partial_statements.test
@@ -0,0 +1,15 @@
+# REQUIRES: lua
+# RUN: %lldb -s %s --script-language lua 2>&1 | FileCheck %s
+script
+do
+local a = 123
+print(a)
+end
+# CHECK: 123
+str = 'hello there!'
+function callme()
+print(str)
+end
+callme()
+# CHECK: hello there!
+# CHECK-NOT: error
Index: lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_callback.test
===
--- lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_callback.test
+++ lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_callback.test
@@ -1,5 +1,13 @@
 # REQUIRES: lua
-# RUN: %lldb -s %s --script-language lua 2>&1 | FileCheck %s
+# RUN: echo "int main() { return 0; }" | %clang_host -x c - -o %t
+# RUN: %lldb -s %s --script-language lua %t 2>&1 | FileCheck %s
 b main
 breakpoint command add -s lua
-# CHECK: error: This script interpreter does not support breakpoint callbacks
+local a = 123
+print(a)
+quit
+run
+# CHECK: 123
+breakpoint command add -s lua
+789_nil
+# CHECK: unexpected symbol near '789'
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
@@ -65,6 +65,10 @@
   llvm::Error EnterSession(lldb::user_id_t debugger_id);
   llvm::Error LeaveSession();
 
+  void CollectDataForBreakpointCommandCallback(
+  std::vector &bp_options_vec,
+  CommandReturnObject &result) override;
+
   Status SetBreakpointCommandCallback(BreakpointOptions *bp_options,
   const char *command_body_text) override;
 
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
@@ -17,23 +17,33 @@
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/StringList.h"
 #include "lldb/Utility/Timer.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/FormatAdapters.h"
 #include 
+#include 
 
 using namespace lldb;
 using namespace lldb_private;
 
 LLDB_PLUGIN_DEFINE(ScriptInterpreterLua)
 
+enum ActiveIOHandler {
+  eIOHandlerNone,
+  eIOHandlerBreakpoint,
+  eIOHandlerWatchpoint
+};
+
 class IOHandlerLuaInterpreter : public IOHandlerDelegate,
 public IOHandlerEditline {
 public:
   IOHandlerLuaInterpreter(Debugger &debugger,
-  ScriptInterpreterLua &script_interpreter)
+  ScriptInterpreterLua &script_interpreter,
+  ActiveIOHandler active_io_handler = eIOHandlerNone)
   : IOHandlerEditline(debugger, IOHandler::Type::LuaInterpreter, "lua",
   ">>> ", "..> ", true, debugger.GetUseColor(), 0,
   *this, nullptr),
-m_script_interpreter(script_interpreter) {
+m_script_interpreter(script_interpreter),
+m_active_io_handler(active_io_handler) {
 llvm::cantFail(m_script_interpreter.GetLua().ChangeIO(
 debugger.GetOutputFile().GetStream(),
 debugger.GetErrorFile().GetStream()));
@@ -44,20 +54,79 @@
 llvm::cantFail(m_script_interpreter.LeaveSession());
   }
 
-  void IOHandlerInputComplete(IOHandler &io_handler,
-  std::string &data) override {
-if (llvm::StringRef(data).rtrim() == "quit") {
-  io_handler.SetIsDone(true);
+  void IOHandlerActivated(IOHandler &io_handler, bool interactive) override {
+const char *instructions = nullptr;
+switch (m_active_io_handler) {
+case eIOHandlerNone:
+case eIOHandlerWatchpoint:
+  break;
+case eIOHandlerBreakpoint:
+  instructions = "Enter your Lua command(s). Type 'quit' to end.\n"
+ "The commands are compiled as the body of the following "
+ "Lua function\n

[Lldb-commits] [PATCH] D78978: [LLDB] Add support for WebAssembly debugging

2021-01-06 Thread Paolo Severini via Phabricator via lldb-commits 
paolosev added a comment.

In D78978#2483354 , @dblaikie wrote:

> Usually the thing is to ping the review thread at most weekly & maybe search 
> around for specific reviewers to ask if you're met with a lot of silence.

There was not a lot of feedback on this thread, but if you look at 
https://reviews.llvm.org/D78801, the discussion there was huge :)
But it's a good point! I'll post the same message there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78978

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

[Lldb-commits] [PATCH] D78801: [LLDB] Add class WasmProcess for WebAssembly debugging

2021-01-06 Thread Paolo Severini via Phabricator via lldb-commits 
paolosev added a subscriber: vwzm228.
paolosev added a comment.

In D78978#2481358 , @vwzm228 wrote:

> Is there any progress about such patch and D78801 
> ？
>
> I have implemented the debugging feature in our Wasm VM based on  
> https://reviews.llvm.org/D78801, and it already work to attach, set 
> breakpoint, step, show variable value, backtrace...
>
> I am not sure if I need to change LLDB part to this one, or keep using D78801 
> .
>
> But if both patches wont be merged, I have to maintain a private LLDB 
> version

Unfortunately I have not received any more feedback for this patch, or the 
alternative version D78978 , so I assumed it 
won't move forward :(
Meanwhile, I have created a personal fork 
(https://github.com/paolosevMSFT/llvm-project/tree/WasmDbg).

I still think that it would be very useful to add support for Wasm debugging to 
LLDB. Especially in scenarios where Wasm is not used as part of a web app, but 
server side (node.js) or running on micro runtime for IoT devices.
I know that there is the problem that LLDB does not support segmented address 
spaces, and so this patch requires a couple of tiny but smelly changes to core 
code.
From the mailing list I seem to remember that somebody was working to add 
support for segmented addresses, but I don't know what is the current state of 
the initiative.

I'd be happy to keep working on this, please let me know what I could do to 
progress toward an acceptable solution.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78801

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

[Lldb-commits] [lldb] b9bfe8a - [lldb] [debugserver] Add stN aliases for stmmN for compatibility

2021-01-06 Thread Michał Górny via lldb-commits 

Author: Michał Górny
Date: 2021-01-07T02:10:38+01:00
New Revision: b9bfe8a75306b211dc53291d28a31c0f37be2a2c

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

LOG: [lldb] [debugserver] Add stN aliases for stmmN for compatibility

Add stN aliases for the FPU (stmmN) registers on MacOSX.  This should
improve compatibility between MacOSX and other platforms, and partially
fix x86*-fp-write tests without having to duplicate them.  Note that
the tests are currently still broken due to ftag incompatibility.

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

Added: 


Modified: 
lldb/test/API/commands/register/register/register_command/TestRegisters.py
lldb/test/CMakeLists.txt
lldb/test/Shell/Register/x86-multithread-write.test
lldb/test/Shell/lit.cfg.py
lldb/test/Shell/lit.site.cfg.py.in
lldb/tools/debugserver/source/MacOSX/i386/DNBArchImplI386.cpp
lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp

Removed: 




diff  --git 
a/lldb/test/API/commands/register/register/register_command/TestRegisters.py 
b/lldb/test/API/commands/register/register/register_command/TestRegisters.py
index 0b53fa684c288..fa8e99aefa6ab 100644
--- a/lldb/test/API/commands/register/register/register_command/TestRegisters.py
+++ b/lldb/test/API/commands/register/register/register_command/TestRegisters.py
@@ -320,10 +320,12 @@ def fp_register_write(self):
 ]
 
 st0regname = None
-if currentFrame.FindRegister("st0").IsValid():
-st0regname = "st0"
-elif currentFrame.FindRegister("stmm0").IsValid():
+# Darwin is using stmmN by default but support stN as an alias.
+# Therefore, we need to check for stmmN first.
+if currentFrame.FindRegister("stmm0").IsValid():
 st0regname = "stmm0"
+elif currentFrame.FindRegister("st0").IsValid():
+st0regname = "st0"
 if st0regname is not None:
 # reg  value
 # must-have

diff  --git a/lldb/test/CMakeLists.txt b/lldb/test/CMakeLists.txt
index 9d17009df2cad..79fa05f2df29f 100644
--- a/lldb/test/CMakeLists.txt
+++ b/lldb/test/CMakeLists.txt
@@ -166,6 +166,7 @@ llvm_canonicalize_cmake_booleans(
   LLDB_ENABLE_LZMA
   LLVM_ENABLE_ZLIB
   LLVM_ENABLE_SHARED_LIBS
+  LLDB_USE_SYSTEM_DEBUGSERVER
   LLDB_IS_64_BITS)
 
 # Configure the individual test suites.

diff  --git a/lldb/test/Shell/Register/x86-multithread-write.test 
b/lldb/test/Shell/Register/x86-multithread-write.test
index bf0cd33c44e00..cc02b323c7263 100644
--- a/lldb/test/Shell/Register/x86-multithread-write.test
+++ b/lldb/test/Shell/Register/x86-multithread-write.test
@@ -1,6 +1,6 @@
 # XFAIL: system-windows
-# XFAIL: system-darwin
 # REQUIRES: native && (target-x86 || target-x86_64)
+# UNSUPPORTED: system-debugserver
 # RUN: %clangxx_host %p/Inputs/x86-multithread-write.cpp -o %t -pthread
 # RUN: %lldb -b -s %s %t | FileCheck %s
 

diff  --git a/lldb/test/Shell/lit.cfg.py b/lldb/test/Shell/lit.cfg.py
index 7b179c587f712..83e3ef6782adc 100644
--- a/lldb/test/Shell/lit.cfg.py
+++ b/lldb/test/Shell/lit.cfg.py
@@ -120,6 +120,9 @@ def calculate_arch_features(arch_string):
 if find_executable('xz') != None:
 config.available_features.add('xz')
 
+if config.lldb_system_debugserver:
+config.available_features.add('system-debugserver')
+
 # NetBSD permits setting dbregs either if one is root
 # or if user_set_dbregs is enabled
 can_set_dbregs = True

diff  --git a/lldb/test/Shell/lit.site.cfg.py.in 
b/lldb/test/Shell/lit.site.cfg.py.in
index 6cddd3937628d..868f0b6e7b268 100644
--- a/lldb/test/Shell/lit.site.cfg.py.in
+++ b/lldb/test/Shell/lit.site.cfg.py.in
@@ -22,6 +22,7 @@ config.lldb_bitness = 64 if @LLDB_IS_64_BITS@ else 32
 config.lldb_enable_python = @LLDB_ENABLE_PYTHON@
 config.lldb_enable_lua = @LLDB_ENABLE_LUA@
 config.lldb_build_directory = "@LLDB_TEST_BUILD_DIRECTORY@"
+config.lldb_system_debugserver = @LLDB_USE_SYSTEM_DEBUGSERVER@
 # The shell tests use their own module caches.
 config.lldb_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_LLDB@", 
"lldb-shell")
 config.clang_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_CLANG@", 
"lldb-shell")

diff  --git a/lldb/tools/debugserver/source/MacOSX/i386/DNBArchImplI386.cpp 
b/lldb/tools/debugserver/source/MacOSX/i386/DNBArchImplI386.cpp
index 27bc75110620a..2b1d360dcae5f 100644
--- a/lldb/tools/debugserver/source/MacOSX/i386/DNBArchImplI386.cpp
+++ b/lldb/tools/debugserver/source/MacOSX/i386/DNBArchImplI386.cpp
@@ -1356,28 +1356,28 @@ const DNBRegisterInfo 
DNBArchImplI386::g_fpu_registers_no_avx[] = {
  FPU_SIZE_UINT(mxcsrmask), FPU_OFFSET(mxcsrmask), INVALID_NUB_REGNUM,
  INVALID_NUB_REGNUM, INVALID_NUB_REGNUM,

[Lldb-commits] [PATCH] D91847: [lldb] [debugserver] Add stN aliases for stmmN for compatibility

2021-01-06 Thread Michał Górny via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb9bfe8a75306: [lldb] [debugserver] Add stN aliases for stmmN 
for compatibility (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91847

Files:
  lldb/test/API/commands/register/register/register_command/TestRegisters.py
  lldb/test/CMakeLists.txt
  lldb/test/Shell/Register/x86-multithread-write.test
  lldb/test/Shell/lit.cfg.py
  lldb/test/Shell/lit.site.cfg.py.in
  lldb/tools/debugserver/source/MacOSX/i386/DNBArchImplI386.cpp
  lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp

Index: lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
===
--- lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
+++ lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
@@ -1767,28 +1767,28 @@
  FPU_SIZE_UINT(mxcsrmask), FPU_OFFSET(mxcsrmask), -1U, -1U, -1U, -1U, NULL,
  NULL},
 
-{e_regSetFPU, fpu_stmm0, "stmm0", NULL, Vector, VectorOfUInt8,
+{e_regSetFPU, fpu_stmm0, "stmm0", "st0", Vector, VectorOfUInt8,
  FPU_SIZE_MMST(stmm0), FPU_OFFSET(stmm0), ehframe_dwarf_stmm0,
  ehframe_dwarf_stmm0, -1U, debugserver_stmm0, NULL, NULL},
-{e_regSetFPU, fpu_stmm1, "stmm1", NULL, Vector, VectorOfUInt8,
+{e_regSetFPU, fpu_stmm1, "stmm1", "st1", Vector, VectorOfUInt8,
  FPU_SIZE_MMST(stmm1), FPU_OFFSET(stmm1), ehframe_dwarf_stmm1,
  ehframe_dwarf_stmm1, -1U, debugserver_stmm1, NULL, NULL},
-{e_regSetFPU, fpu_stmm2, "stmm2", NULL, Vector, VectorOfUInt8,
+{e_regSetFPU, fpu_stmm2, "stmm2", "st2", Vector, VectorOfUInt8,
  FPU_SIZE_MMST(stmm2), FPU_OFFSET(stmm2), ehframe_dwarf_stmm2,
  ehframe_dwarf_stmm2, -1U, debugserver_stmm2, NULL, NULL},
-{e_regSetFPU, fpu_stmm3, "stmm3", NULL, Vector, VectorOfUInt8,
+{e_regSetFPU, fpu_stmm3, "stmm3", "st3", Vector, VectorOfUInt8,
  FPU_SIZE_MMST(stmm3), FPU_OFFSET(stmm3), ehframe_dwarf_stmm3,
  ehframe_dwarf_stmm3, -1U, debugserver_stmm3, NULL, NULL},
-{e_regSetFPU, fpu_stmm4, "stmm4", NULL, Vector, VectorOfUInt8,
+{e_regSetFPU, fpu_stmm4, "stmm4", "st4", Vector, VectorOfUInt8,
  FPU_SIZE_MMST(stmm4), FPU_OFFSET(stmm4), ehframe_dwarf_stmm4,
  ehframe_dwarf_stmm4, -1U, debugserver_stmm4, NULL, NULL},
-{e_regSetFPU, fpu_stmm5, "stmm5", NULL, Vector, VectorOfUInt8,
+{e_regSetFPU, fpu_stmm5, "stmm5", "st5", Vector, VectorOfUInt8,
  FPU_SIZE_MMST(stmm5), FPU_OFFSET(stmm5), ehframe_dwarf_stmm5,
  ehframe_dwarf_stmm5, -1U, debugserver_stmm5, NULL, NULL},
-{e_regSetFPU, fpu_stmm6, "stmm6", NULL, Vector, VectorOfUInt8,
+{e_regSetFPU, fpu_stmm6, "stmm6", "st6", Vector, VectorOfUInt8,
  FPU_SIZE_MMST(stmm6), FPU_OFFSET(stmm6), ehframe_dwarf_stmm6,
  ehframe_dwarf_stmm6, -1U, debugserver_stmm6, NULL, NULL},
-{e_regSetFPU, fpu_stmm7, "stmm7", NULL, Vector, VectorOfUInt8,
+{e_regSetFPU, fpu_stmm7, "stmm7", "st7", Vector, VectorOfUInt8,
  FPU_SIZE_MMST(stmm7), FPU_OFFSET(stmm7), ehframe_dwarf_stmm7,
  ehframe_dwarf_stmm7, -1U, debugserver_stmm7, NULL, NULL},
 
@@ -1882,28 +1882,28 @@
  FPU_SIZE_UINT(mxcsrmask), AVX_OFFSET(mxcsrmask), -1U, -1U, -1U, -1U, NULL,
  NULL},
 
-{e_regSetFPU, fpu_stmm0, "stmm0", NULL, Vector, VectorOfUInt8,
+{e_regSetFPU, fpu_stmm0, "stmm0", "st0", Vector, VectorOfUInt8,
  FPU_SIZE_MMST(stmm0), AVX_OFFSET(stmm0), ehframe_dwarf_stmm0,
  ehframe_dwarf_stmm0, -1U, debugserver_stmm0, NULL, NULL},
-{e_regSetFPU, fpu_stmm1, "stmm1", NULL, Vector, VectorOfUInt8,
+{e_regSetFPU, fpu_stmm1, "stmm1", "st1", Vector, VectorOfUInt8,
  FPU_SIZE_MMST(stmm1), AVX_OFFSET(stmm1), ehframe_dwarf_stmm1,
  ehframe_dwarf_stmm1, -1U, debugserver_stmm1, NULL, NULL},
-{e_regSetFPU, fpu_stmm2, "stmm2", NULL, Vector, VectorOfUInt8,
+{e_regSetFPU, fpu_stmm2, "stmm2", "st2", Vector, VectorOfUInt8,
  FPU_SIZE_MMST(stmm2), AVX_OFFSET(stmm2), ehframe_dwarf_stmm2,
  ehframe_dwarf_stmm2, -1U, debugserver_stmm2, NULL, NULL},
-{e_regSetFPU, fpu_stmm3, "stmm3", NULL, Vector, VectorOfUInt8,
+{e_regSetFPU, fpu_stmm3, "stmm3", "st3", Vector, VectorOfUInt8,
  FPU_SIZE_MMST(stmm3), AVX_OFFSET(stmm3), ehframe_dwarf_stmm3,
  ehframe_dwarf_stmm3, -1U, debugserver_stmm3, NULL, NULL},
-{e_regSetFPU, fpu_stmm4, "stmm4", NULL, Vector, VectorOfUInt8,
+{e_regSetFPU, fpu_stmm4, "stmm4", "st4", Vector, VectorOfUInt8,
  FPU_SIZE_MMST(stmm4), AVX_OFFSET(stmm4), ehframe_dwarf_stmm4,
  ehframe_dwarf_stmm4, -1U, debugserver_stmm4, NULL, NULL},
-{e_regSetFPU, fpu_stmm5, "stmm5", NULL, Vector, VectorOfUInt8,
+{e_regSetFPU, fpu_stmm5, "stmm5", "st5", Vector, VectorOfUInt8,
  FPU_SIZE_MMST(stmm5), AVX_OFFSET(stmm5), ehframe_dwarf_stmm5,
  ehframe_dwarf_stmm5, -1U, debugserver_st

[Lldb-commits] [lldb] fbc13e9 - [lldb] Skip scoped enum checks with Dwarf <4

2021-01-06 Thread Jonas Devlieghere via lldb-commits 

Author: Jonas Devlieghere
Date: 2021-01-06T17:13:33-08:00
New Revision: fbc13e9345c7c9607f0c28e0ccfa9a7baf254f29

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

LOG: [lldb] Skip scoped enum checks with Dwarf <4

The scoped enum tests depend on DW_AT_enum_class which was added in
Dwarf 4.

I made part of the test conditional on the Dwarf version instead of
splitting it into a separate test and using the decorator to avoid the
overhead of setting up the test.

Added: 


Modified: 
lldb/test/API/python_api/type/TestTypeList.py

Removed: 




diff  --git a/lldb/test/API/python_api/type/TestTypeList.py 
b/lldb/test/API/python_api/type/TestTypeList.py
index ff560de2f96f..359dfe8cf5a9 100644
--- a/lldb/test/API/python_api/type/TestTypeList.py
+++ b/lldb/test/API/python_api/type/TestTypeList.py
@@ -145,25 +145,26 @@ def test(self):
 self.DebugSBType(myint_type)
 self.assertTrue(myint_arr_element_type == myint_type)
 
-# Test enum methods.
-enum_type = target.FindFirstType('EnumType')
-self.assertTrue(enum_type)
-self.DebugSBType(enum_type)
-self.assertFalse(enum_type.IsScopedEnumerationType())
-
-scoped_enum_type = target.FindFirstType('ScopedEnumType')
-self.assertTrue(scoped_enum_type)
-self.DebugSBType(scoped_enum_type)
-self.assertTrue(scoped_enum_type.IsScopedEnumerationType())
-int_scoped_enum_type = scoped_enum_type.GetEnumerationIntegerType()
-self.assertTrue(int_scoped_enum_type)
-self.DebugSBType(int_scoped_enum_type)
-self.assertEquals(int_scoped_enum_type.GetName(), 'int')
-
-enum_uchar = target.FindFirstType('EnumUChar')
-self.assertTrue(enum_uchar)
-self.DebugSBType(enum_uchar)
-int_enum_uchar = enum_uchar.GetEnumerationIntegerType()
-self.assertTrue(int_enum_uchar)
-self.DebugSBType(int_enum_uchar)
-self.assertEquals(int_enum_uchar.GetName(), 'unsigned char')
+# Test enum methods. Requires DW_AT_enum_class which was added in 
Dwarf 4.
+if configuration.dwarf_version >= 4:
+enum_type = target.FindFirstType('EnumType')
+self.assertTrue(enum_type)
+self.DebugSBType(enum_type)
+self.assertFalse(enum_type.IsScopedEnumerationType())
+
+scoped_enum_type = target.FindFirstType('ScopedEnumType')
+self.assertTrue(scoped_enum_type)
+self.DebugSBType(scoped_enum_type)
+self.assertTrue(scoped_enum_type.IsScopedEnumerationType())
+int_scoped_enum_type = scoped_enum_type.GetEnumerationIntegerType()
+self.assertTrue(int_scoped_enum_type)
+self.DebugSBType(int_scoped_enum_type)
+self.assertEquals(int_scoped_enum_type.GetName(), 'int')
+
+enum_uchar = target.FindFirstType('EnumUChar')
+self.assertTrue(enum_uchar)
+self.DebugSBType(enum_uchar)
+int_enum_uchar = enum_uchar.GetEnumerationIntegerType()
+self.assertTrue(int_enum_uchar)
+self.DebugSBType(int_enum_uchar)
+self.assertEquals(int_enum_uchar.GetName(), 'unsigned char')



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

[Lldb-commits] [PATCH] D93951: [vscode] Improve runInTerminal and support linux

2021-01-06 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

So I like that we are using files in the file system for things, but we are 
using 3 of them and that seems excessive. See inlined comments and see if we 
can get down to using just one? Also, what is stopping us from being able to 
support windows here? Just mkfifo?




Comment at: lldb/tools/lldb-vscode/RunInTerminal.cpp:29
+static llvm::Error CreateFifoFile(const std::string &path) {
+  if (int err = mkfifo(path.c_str(), 0666))
+return llvm::createStringError(

These permissions seem bad, probably should be 0600 to only allow current user 
to access for security purposes.

Also, any reason we are using mkfifo over just a normal temp file? Is this the 
only thing that keeps us from using this on windows? Could we have windows 
support if just create a normal file like std::ofstream and std::ifstream?



Comment at: lldb/tools/lldb-vscode/RunInTerminal.h:32-38
+  /// File the runInTerminal launcher can write its pid to
+  std::string pid_file;
+  /// File the runInTerminal launcher can write error messages to
+  std::string err_file;
+  /// File whose existance indicates that the debug adaptor has attached to the
+  /// runInTerminal launcher.
+  std::string did_attach_file;

Can we avoid using more than one file here? Might be nice to require just one 
file that gets a JSON dictionary as the contents?
```
{ "pid": 123 }
```
or
```
{ "error" : "" }
```
Then we need to read data until we get correctly formed JSON or timeout.



Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:1377-1381
+#if !defined(_WIN32)
   body.try_emplace("supportsRunInTerminalRequest", true);
+#else
+  body.try_emplace("supportsRunInTerminalRequest", false);
+#endif

Reverse this to avoid the negate?



Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:1442-1446
+#if defined(_WIN32)
+  return llvm::createStringError(
+  llvm::inconvertibleErrorCode(),
+  "\"runInTerminal\" is not supported on Windows");
+#else

We shouldn't need any #if define(_WIN32) since we didn't enable this feature in 
the response to "initialize" right?



Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:2982
+// emitted to the debug adaptor.
+[[noreturn]] void LaunchRunInTerminalTarget(llvm::opt::Arg &target_arg,
+llvm::StringRef comm_files_prefix,

Is "[[noreturn]]" supported by all compilers that compile LLDB? There might be 
some LLVM define that we should use?



Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:2994
+  // signal to prevent being paused forever.
+  const char *timeout_env_var = getenv("LLDB_VSCODE_RIT_TIMEOUT_IN_MS");
+  int timeout_in_ms =

Why are we using env vars for some things and options values for others? We 
should probably pass this in as an argument. We are also polluting the env vars 
of the new process we will exec if we leave this in the environment



Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:3001
+  const char *target = target_arg.getValue();
+  execv(target, argv + target_arg.getIndex() + 2);
+

I assume env vars are being inherited from this process?



Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:3018
+  llvm::SmallString<256> program_path(argv[0]);
+  llvm::sys::fs::make_absolute(program_path);
+  g_vsc.debug_adaptor_path = program_path.str().str();

Might be worth checking if there is a llvm file system call to determine the 
program path already?



Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:3026
+
+  if (auto *target_arg = input_args.getLastArg(OPT_launch_target)) {
+if (auto *comm_file_prefix = input_args.getLastArg(OPT_comm_files_prefix))

Don't use auto



Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:3027
+  if (auto *target_arg = input_args.getLastArg(OPT_launch_target)) {
+if (auto *comm_file_prefix = input_args.getLastArg(OPT_comm_files_prefix))
+  LaunchRunInTerminalTarget(*target_arg, comm_file_prefix->getValue(),

don't use auto 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93951

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

[Lldb-commits] [PATCH] D93951: [vscode] Improve runInTerminal and support linux

2021-01-06 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg added a comment.

We should also add a test that makes sure that env vars set in the launch 
config are received in the program that runs in the terminal


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93951

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

[Lldb-commits] [PATCH] D93951: [vscode] Improve runInTerminal and support linux

2021-01-06 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg added a comment.

And a test for current working directory


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93951

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

[Lldb-commits] [PATCH] D94063: lldb: Add support for DW_AT_ranges on DW_TAG_subprograms

2021-01-06 Thread David Blaikie via Phabricator via lldb-commits 
dblaikie updated this revision to Diff 315035.
dblaikie added a comment.

Update test to avoid running the program


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94063

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  lldb/test/Shell/SymbolFile/DWARF/Inputs/subprogram_ranges.s
  lldb/test/Shell/SymbolFile/DWARF/subprogram_ranges.test

Index: lldb/test/Shell/SymbolFile/DWARF/subprogram_ranges.test
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/subprogram_ranges.test
@@ -0,0 +1,17 @@
+# REQUIRES: x86
+# RUN: %clang -target x86_64-pc-linux -g -O0 %S/Inputs/subprogram_ranges.s -o %t.out
+# RUN: %lldb -b -s %s %t.out 2>&1 | FileCheck %s
+
+# Test breaking on symbols and printing variables when a DW_TAG_subprogram uses
+# DW_AT_ranges instead of DW_AT_low_pc/DW_AT_high_pc.  While the assembly here
+# is a bit unrealistic - it's a single-entry range using DWARFv4 which isn't
+# useful for anything (a single-entry range with DWARFv5 can reduce address
+# relocations, and multi-entry ranges can be used for function sections), but
+# it's the simplest thing to test. If anyone's updating this test at some
+# point, feel free to replace it with another equivalent test if it's
+# especially useful, but don't dismiss it as pointless just because it's a bit
+# weird.
+
+b main
+# CHECK: (lldb) b main
+# CHECK-NEXT: Breakpoint 1: where = subprogram_ranges.test.tmp.out`main + 6 at main.c:2:7
Index: lldb/test/Shell/SymbolFile/DWARF/Inputs/subprogram_ranges.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/Inputs/subprogram_ranges.s
@@ -0,0 +1,159 @@
+	.text
+	.file	"main.c"
+	.globl	main# -- Begin function main
+	.p2align	4, 0x90
+	.type	main,@function
+main:   # @main
+.Lfunc_begin0:
+	.file	1 "/usr/local/google/home/blaikie/dev/scratch" "main.c"
+	.loc	1 1 0   # main.c:1:0
+	.cfi_startproc
+# %bb.0:# %entry
+	pushq	%rbp
+	.cfi_def_cfa_offset 16
+	.cfi_offset %rbp, -16
+	movq	%rsp, %rbp
+	.cfi_def_cfa_register %rbp
+	xorl	%eax, %eax
+.Ltmp0:
+	.loc	1 2 7 prologue_end  # main.c:2:7
+	movl	$3, -4(%rbp)
+	.loc	1 3 1   # main.c:3:1
+	popq	%rbp
+	.cfi_def_cfa %rsp, 8
+	retq
+.Ltmp1:
+.Lfunc_end0:
+	.size	main, .Lfunc_end0-main
+	.cfi_endproc
+# -- End function
+	.section	.debug_abbrev,"",@progbits
+	.byte	1   # Abbreviation Code
+	.byte	17  # DW_TAG_compile_unit
+	.byte	1   # DW_CHILDREN_yes
+	.byte	37  # DW_AT_producer
+	.byte	14  # DW_FORM_strp
+	.byte	19  # DW_AT_language
+	.byte	5   # DW_FORM_data2
+	.byte	3   # DW_AT_name
+	.byte	14  # DW_FORM_strp
+	.byte	16  # DW_AT_stmt_list
+	.byte	23  # DW_FORM_sec_offset
+	.byte	27  # DW_AT_comp_dir
+	.byte	14  # DW_FORM_strp
+	.byte	17  # DW_AT_low_pc
+	.byte	1   # DW_FORM_addr
+	.byte	18  # DW_AT_high_pc
+	.byte	6   # DW_FORM_data4
+	.byte	0   # EOM(1)
+	.byte	0   # EOM(2)
+	.byte	2   # Abbreviation Code
+	.byte	46  # DW_TAG_subprogram
+	.byte	1   # DW_CHILDREN_yes
+	.byte	85  # DW_AT_ranges
+	.byte	23  # DW_FORM_sec_offset
+	.byte	64  # DW_AT_frame_base
+	.byte	24  # DW_FORM_exprloc
+	.byte	3   # DW_AT_name
+	.byte	14  # DW_FORM_strp
+	.byte	58  # DW_AT_decl_file
+	.byte	11  # DW_FORM_data1
+	.byte	59  # DW_AT_decl_line
+	.byte	11  # DW_FORM_data1
+	.byte	73  # DW_AT_type
+	.byte	19  # DW_FORM_ref4
+	.byte	63  # DW_AT_external
+	.byte	25  # DW_FORM_flag_present
+	.byte	0   # EOM(1)
+	.byte	0   # EOM(2)
+	.byte	3   # Abbreviation Code
+	.byte	52  # DW_TAG_variable
+	.byte	0

[Lldb-commits] [PATCH] D94063: lldb: Add support for DW_AT_ranges on DW_TAG_subprograms

2021-01-06 Thread David Blaikie via Phabricator via lldb-commits 
dblaikie added a comment.

In D94063#2481867 , @labath wrote:

> The fix is good, but the test could be improved.

Yeah - haven't written lldb patches before so totally open to suggestions on 
the testing front for sure. Thanks!

> Combining assembly input with running the inferior effectively limits the 
> test to a single platform (assembly is not portable, and running requires asm 
> to match the host).

If it's better to write it using C++ source and custom clang flags I can do 
that instead (it'll be an -mllvm flag - looks like there's one other test that 
does that: `lldb/test/API/lang/objc/forward-decl/TestForwardDecl.py:
dict(CFLAGS_EXTRAS="-dwarf-version=5 -mllvm -accel-tables=Dwarf"))`) - means 
the test will be a bit more convoluted to tickle the subprogram ranges, but not 
much - just takes two functions+function-sections.

> AFAICT, we don't actually need to run the binary to test this fix -- checking 
> just the "breakpoint set" command should suffice (if you want to be more 
> explicit in what is being checked, you can also run "breakpoint list -v" and 
> test that). Then you'd just need to replace `%clang_host` with `%clang 
> -target x86_64-pc-linux` (or something) and `UNSUPPORTED: system-windows` 
> with `REQUIRES: x86`.

Yeah, I was hoping to setup the test for testing both these things in a uniform 
way, but if best practice is to test them with different mechanisms I'm OK with 
that.

> I'll write about variable printing in the other review.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94063

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

[Lldb-commits] [PATCH] D94064: lldb: Add support for printing variables with DW_AT_ranges on DW_TAG_subprograms

2021-01-06 Thread David Blaikie via Phabricator via lldb-commits 
dblaikie updated this revision to Diff 315036.
dblaikie added a comment.

Use image lookup to make test runnable without executing the test code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94064

Files:
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/test/Shell/SymbolFile/DWARF/subprogram_ranges.test


Index: lldb/test/Shell/SymbolFile/DWARF/subprogram_ranges.test
===
--- lldb/test/Shell/SymbolFile/DWARF/subprogram_ranges.test
+++ lldb/test/Shell/SymbolFile/DWARF/subprogram_ranges.test
@@ -15,3 +15,7 @@
 b main
 # CHECK: (lldb) b main
 # CHECK-NEXT: Breakpoint 1: where = subprogram_ranges.test.tmp.out`main + 6 at 
main.c:2:7
+
+image lookup -v -s main
+# CHECK: 1 symbols match 'main'
+# CHECK:  Variable: {{.*}}, name = "var", type = "int", {{.*}}, decl = main.c:2
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3040,8 +3040,13 @@
 if (sc.function) {
   DWARFDIE function_die = GetDIE(sc.function->GetID());
 
-  const dw_addr_t func_lo_pc = function_die.GetAttributeValueAsAddress(
+  dw_addr_t func_lo_pc = function_die.GetAttributeValueAsAddress(
   DW_AT_low_pc, LLDB_INVALID_ADDRESS);
+  DWARFFormValue form_value;
+  if (func_lo_pc == LLDB_INVALID_ADDRESS &&
+  function_die.GetDIE()->GetAttributeValue(function_die.GetCU(),
+   DW_AT_ranges, form_value))
+func_lo_pc = 0;
   if (func_lo_pc != LLDB_INVALID_ADDRESS) {
 const size_t num_variables = ParseVariables(
 sc, function_die.GetFirstChild(), func_lo_pc, true, true);


Index: lldb/test/Shell/SymbolFile/DWARF/subprogram_ranges.test
===
--- lldb/test/Shell/SymbolFile/DWARF/subprogram_ranges.test
+++ lldb/test/Shell/SymbolFile/DWARF/subprogram_ranges.test
@@ -15,3 +15,7 @@
 b main
 # CHECK: (lldb) b main
 # CHECK-NEXT: Breakpoint 1: where = subprogram_ranges.test.tmp.out`main + 6 at main.c:2:7
+
+image lookup -v -s main
+# CHECK: 1 symbols match 'main'
+# CHECK:  Variable: {{.*}}, name = "var", type = "int", {{.*}}, decl = main.c:2
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3040,8 +3040,13 @@
 if (sc.function) {
   DWARFDIE function_die = GetDIE(sc.function->GetID());
 
-  const dw_addr_t func_lo_pc = function_die.GetAttributeValueAsAddress(
+  dw_addr_t func_lo_pc = function_die.GetAttributeValueAsAddress(
   DW_AT_low_pc, LLDB_INVALID_ADDRESS);
+  DWARFFormValue form_value;
+  if (func_lo_pc == LLDB_INVALID_ADDRESS &&
+  function_die.GetDIE()->GetAttributeValue(function_die.GetCU(),
+   DW_AT_ranges, form_value))
+func_lo_pc = 0;
   if (func_lo_pc != LLDB_INVALID_ADDRESS) {
 const size_t num_variables = ParseVariables(
 sc, function_die.GetFirstChild(), func_lo_pc, true, true);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D94064: lldb: Add support for printing variables with DW_AT_ranges on DW_TAG_subprograms

2021-01-06 Thread David Blaikie via Phabricator via lldb-commits 
dblaikie added a comment.

In D94064#2481925 , @labath wrote:

> I don't think that simply setting func_lo_pc to zero will be sufficient to 
> make this work. I would expect this to break in more complicated scenarios 
> (like, even if we just have two of these functions). I think the only reason 
> it works in this case is because for this particular function, it's base 
> address (relative to the CU base) *is* zero.

I certainly don't have high confidence in the change, to be sure - but I think 
it's a bit more robust than that.

Here's at least one experiment where a function is at a non-zero offset 
relative to the CU:

  $ cat test.cpp
  void stop() {
  }
  void f1() {
int i = 7;
stop();
  }
  int main() {
int j = 12;
f1();
stop();
  }
  $ $ clang++-tot test.cpp -gdwarf-5 -mllvm -always-use-ranges-in-v5=Enable && 
~/dev/llvm/build/default/bin/lldb ./a.out
  (lldb) target create "./a.out"
  Current executable set to '/usr/local/google/home/blaikie/dev/scratch/a.out' 
(x86_64).
  (lldb) r
  Process 1039251 launched: '/usr/local/google/home/blaikie/dev/scratch/a.out' 
(x86_64)
  Process 1039251 exited with status = 0 (0x) 
  (lldb) b stop
  Breakpoint 1: where = a.out`stop() + 4 at test.cpp:2:1, address = 
0x00401114
  (lldb) r
  Process 1039605 launched: '/usr/local/google/home/blaikie/dev/scratch/a.out' 
(x86_64)
  Process 1039605 stopped
  * thread #1, name = 'a.out', stop reason = breakpoint 1.1
  frame #0: 0x00401114 a.out`stop() at test.cpp:2:1
 1void stop() {
  -> 2}
 3void f1() {
 4  int i = 7;
 5  stop();
 6}
 7int main() {
  (lldb) up
  frame #1: 0x00401134 a.out`f1() at test.cpp:5:3
 2}
 3void f1() {
 4  int i = 7;
  -> 5  stop();
 6}
 7int main() {
 8  int j = 12;
  (lldb) p i
  (int) $0 = 7
  (lldb) c
  Process 1039605 resuming
  Process 1039605 stopped
  * thread #1, name = 'a.out', stop reason = breakpoint 1.1
  frame #0: 0x00401114 a.out`stop() at test.cpp:2:1
 1void stop() {
  -> 2}
 3void f1() {
 4  int i = 7;
 5  stop();
 6}
 7int main() {
  (lldb) up
  frame #1: 0x00401159 a.out`main at test.cpp:10:3
 7int main() {
 8  int j = 12;
 9  f1();
  -> 10 stop();
 11   }
  (lldb) p j
  (int) $1 = 12
  $ llvm-dwarfdump a.out
  .debug_info contents:
  0x: Compile Unit: length = 0x005f, format = DWARF32, version = 
0x0005, unit_type = DW_UT_compile, abbr_offset = 0x, addr_size = 0x08 (next 
unit at 0x0063)
  
  0x000c: DW_TAG_compile_unit
DW_AT_producer("clang version 12.0.0 
(g...@github.com:llvm/llvm-project.git 
13740c7d80e6c7b47506fd34cd2ea2a7b658cdfa)")
DW_AT_language(DW_LANG_C_plus_plus_14)
DW_AT_name("test.cpp")
DW_AT_str_offsets_base(0x0008)
DW_AT_stmt_list   (0x)
DW_AT_comp_dir("/usr/local/google/home/blaikie/dev/scratch")
DW_AT_low_pc  (0x00401110)
DW_AT_high_pc (0x00401161)
DW_AT_addr_base   (0x0008)
DW_AT_rnglists_base   (0x000c)
  
  0x0027:   DW_TAG_subprogram
  DW_AT_low_pc(0x00401110)
  DW_AT_high_pc   (0x00401116)
  DW_AT_frame_base(DW_OP_reg6 RBP)
  DW_AT_linkage_name  ("_Z4stopv")
  DW_AT_name  ("stop")
  DW_AT_decl_file 
("/usr/local/google/home/blaikie/dev/scratch/test.cpp")
  DW_AT_decl_line (1)
  DW_AT_external  (true)
  
  0x0033:   DW_TAG_subprogram
  DW_AT_ranges(indexed (0x0) rangelist = 0x0014
 [0x00401120, 0x0040113a))
  DW_AT_frame_base(DW_OP_reg6 RBP)
  DW_AT_linkage_name  ("_Z2f1v")
  DW_AT_name  ("f1")
  DW_AT_decl_file 
("/usr/local/google/home/blaikie/dev/scratch/test.cpp")
  DW_AT_decl_line (3)
  DW_AT_external  (true)
  
  0x003b: DW_TAG_variable
DW_AT_location(DW_OP_fbreg -4)
DW_AT_name("i")
DW_AT_decl_file   
("/usr/local/google/home/blaikie/dev/scratch/test.cpp")
DW_AT_decl_line   (4)
DW_AT_type(0x005e "int")
  
  0x0046: NULL
  
  0x0047:   DW_TAG_subprogram
  DW_AT_ranges(indexed (0x1) rangelist = 0x0018
 [0x00401140, 0x00401161))
  DW_AT_frame_base(DW_OP_reg6 RBP)
  DW_AT_name  ("main")
  DW_AT_decl_fi