labath added inline comments.
Herald added a subscriber: dexonsmith.
================
Comment at:
packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteKill.py:17-23
+ reg_expr = r"^\$X[0-9a-fA-F]+([^#]*)#[0-9A-Fa-f]{2}"
+ triple = self.dbg.GetSelectedPlatform().GetTriple()
+
+ # No signal support on Windwos. Only W* response is sent.
+ if re.match(".*-.*-windows", triple):
+ reg_expr = r"^\$W[0-9a-fA-F]+([^#]*)#[0-9A-Fa-f]{2}"
+
----------------
If I understand correctly, the regexes differ only in the first letter. If
that's the case, then you could just factor out the rest of the regex and just
compute the letter differently:
```
stop_type = 'W' if windows else 'X'
reg_expr = r"^\${}...".format(stop_type)
```
================
Comment at:
packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteModuleInfo.py:24-27
+ # Replace path separators in the json string either with "\\\\" or "/"
on Windows.
+ triple = self.dbg.GetSelectedPlatform().GetTriple()
+ if re.match(".*-.*-windows", triple):
+ module_path = module_path.replace(os.path.sep, '/')
----------------
It sounds like you could just unconditionally replace all backslashes with
double-backslashes here. That would result in us also correctly handling posix
paths that happen to contain a backslash.
================
Comment at:
packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteProcessInfo.py:178-191
@skipUnlessPlatform(["linux"])
@llgs_test
def test_qProcessInfo_contains_triple_llgs_linux(self):
self.init_llgs_test()
self.build()
self.qProcessInfo_contains_keys(set(['triple']))
----------------
I'm not sure why the original test was made linux-specific, but I think you can
just drop the os name from the test and have a single test which checks for
both the presence of `parent-pid` and `triple`. No `@skipUnless` needed as all
platforms that currently have a working lldb-server should support these fields.
================
Comment at:
packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py:208-212
+ # In current implementation of llgs on Windows, as a response to '\x03'
packet, the debugger
+ # of the native process will trigger a call to DebugBreakProcess that will
create a new thread
+ # to handle the exception debug event. So one more stop thread will be
notified to the
+ # delegate, e.g. llgs. So tests below to assert the stop threads number
will all fail.
+ @expectedFailureAll(oslist=["windows"])
----------------
Is this something that we consider to be a bug, or is it just how debugging is
supposed to work on windows? If it's a bug then fine, but if not then we might
consider adjusting the expectations in the test (or just skipping it).
================
Comment at:
packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemote_qThreadStopInfo.py:146-153
# All but one thread should report no stop reason.
- self.assertEqual(no_stop_reason_count, thread_count - 1)
+ triple = self.dbg.GetSelectedPlatform().GetTriple()
+
+ # Consider one more thread created by calling DebugBreakProcess.
+ if re.match(".*-.*-windows", triple):
+ self.assertGreaterEqual(no_stop_reason_count, thread_count - 1)
+ else:
----------------
I think this assertion should just be deleted. If the assertion below (that one
thread has a stop reason) is true, then it trivially true that the rest of the
threads don't have one. Whether or not a platforms spawns an extra thread does
not seem to be relevant for this test.
================
Comment at:
packages/Python/lldbsuite/test/tools/lldb-server/inferior-crash/TestGdbRemoteAbort.py:40
+ @skipIfWindows # For now the signo in T* packet is always 0.
@llgs_test
----------------
Do you have any plans for changing this? Given that windows does not support
signals, maybe it should stay 0...
Repository:
rLLDB LLDB
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61687/new/
https://reviews.llvm.org/D61687
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
