[Lldb-commits] [lldb] [lldb][Windows] Fixed the test gdb_remote_client/TestGDBRemotePlatformFile (PR #92088)
https://github.com/slydiman closed https://github.com/llvm/llvm-project/pull/92088 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Windows] Fixed the test gdb_remote_client/TestGDBRemotePlatformFile (PR #92088)
https://github.com/labath approved this pull request. Sounds good. Looking forward to it. https://github.com/llvm/llvm-project/pull/92088 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Windows] Fixed the test gdb_remote_client/TestGDBRemotePlatformFile (PR #92088)
slydiman wrote: Now we are in the middle of configuring buildbot and trying to get it green. I have added [the issues 92255](https://github.com/llvm/llvm-project/issues/92255). Hope we will fix it later. Thanks. https://github.com/llvm/llvm-project/pull/92088 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Windows] Fixed the test gdb_remote_client/TestGDBRemotePlatformFile (PR #92088)
https://github.com/slydiman updated https://github.com/llvm/llvm-project/pull/92088 >From 7dcfe773b6eef27aabbcc7fc68cd6448bc3c2e88 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Tue, 14 May 2024 13:08:35 +0400 Subject: [PATCH 1/3] [lldb][Windows] Fixed the test gdb_remote_client/TestGDBRemotePlatformFile The tests `test_file_permissions` and `test_file_permissions_fallback` are disabled for Windows target. These tests use MockGDBServerResponder and do not depend on the real target. These tests failed in case of Windows host and Linux target. Disable them for Windows host too. --- .../gdb_remote_client/TestGDBRemotePlatformFile.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py index 2be5ae3132038..9ef0954af1fe3 100644 --- a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py +++ b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py @@ -148,6 +148,7 @@ def vFile(self, packet): ) @skipIfWindows +@skipIf(hostoslist=["windows"]) def test_file_permissions(self): """Test 'platform get-permissions'""" @@ -168,6 +169,7 @@ def vFile(self, packet): ) @skipIfWindows +@skipIf(hostoslist=["windows"]) def test_file_permissions_fallback(self): """Test 'platform get-permissions' fallback to fstat""" >From 478d251691d511916cae5fc344d549450222d584 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Tue, 14 May 2024 20:23:56 +0400 Subject: [PATCH 2/3] Removed @skipIfWindows --- .../gdb_remote_client/TestGDBRemotePlatformFile.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py index 9ef0954af1fe3..b1c6f0822d1a8 100644 --- a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py +++ b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py @@ -147,7 +147,6 @@ def vFile(self, packet): log=server2.responder.packetLog, ) -@skipIfWindows @skipIf(hostoslist=["windows"]) def test_file_permissions(self): """Test 'platform get-permissions'""" @@ -168,7 +167,6 @@ def vFile(self, packet): ] ) -@skipIfWindows @skipIf(hostoslist=["windows"]) def test_file_permissions_fallback(self): """Test 'platform get-permissions' fallback to fstat""" >From 4f85d2379350306d915a94e0dd67377feb049fdb Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Wed, 15 May 2024 17:10:26 +0400 Subject: [PATCH 3/3] Replaced @skipIf with @expectedFailureAll and bugnumber --- .../gdb_remote_client/TestGDBRemotePlatformFile.py| 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py index b1c6f0822d1a8..c902722a2f74b 100644 --- a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py +++ b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py @@ -147,7 +147,9 @@ def vFile(self, packet): log=server2.responder.packetLog, ) -@skipIf(hostoslist=["windows"]) +@expectedFailureAll( +hostoslist=["windows"], bugnumber="github.com/llvm/llvm-project/issues/92255" +) def test_file_permissions(self): """Test 'platform get-permissions'""" @@ -167,7 +169,9 @@ def vFile(self, packet): ] ) -@skipIf(hostoslist=["windows"]) +@expectedFailureAll( +hostoslist=["windows"], bugnumber="github.com/llvm/llvm-project/issues/92255" +) def test_file_permissions_fallback(self): """Test 'platform get-permissions' fallback to fstat""" ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Windows] Fixed the test gdb_remote_client/TestGDBRemotePlatformFile (PR #92088)
labath wrote: > The problem is here > lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp, line > 3235 inside GDBRemoteCommunicationClient::GetFilePermissions() > > ``` > file_permissions = mode & (S_IRWXU | S_IRWXG | S_IRWXO); > ``` Okay, so I think this is the bug. The protocol code should not be depending on the host definitions of the permission constants. Would you be interested in creating a patch to change this so that it uses lldb's `eFilePermissions***` enums (which are the same everywhere(*)). I don't think it is necessarily up to you to fix this issue, but you do seem to care about the windows->linux configs, and this is approximately the only configuration in which this bug manifests itself. And it's definitely not a big bug, but it could cause some surprises when debugging in these setups. In case you do not want to make that change, I'd say that the appropriate decorator is `@expectedFailureAll(hostoslist=["windows"])` with a reference to this issue, as this is something that is supposed to work, but fails due to an lldb bug. (*) Technically, there is no requirement that the [gdb-protocol values](https://github.com/llvm/llvm-project/blob/b6f050fa129b08b6bc35168f0b8010742cd1ed9d/lldb/docs/resources/lldbgdbremote.md?plain=1#L2302) match `lldb` public enums. Puristically, one ought to define separate constants for the protocol and then convert them between the two. However, the two definitions happen to match in this case, and both of them are part of stable APIs so they can't go out of sync and I think we can skip the conversion step. https://github.com/llvm/llvm-project/pull/92088 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Windows] Fixed the test gdb_remote_client/TestGDBRemotePlatformFile (PR #92088)
slydiman wrote: The problem is here lldb\source\Plugins\Process\gdb-remote\GDBRemoteCommunicationClient.cpp, line 3235 inside GDBRemoteCommunicationClient::GetFilePermissions() ``` file_permissions = mode & (S_IRWXU | S_IRWXG | S_IRWXO); ``` But S_IRWXU, S_IRWXG and S_IRWXO are 0 on Windows. https://github.com/llvm/llvm-project/pull/92088 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Windows] Fixed the test gdb_remote_client/TestGDBRemotePlatformFile (PR #92088)
labath wrote: > The problem is that fstat() is fake on Windows. File::GetPermissions() > returns 0 always. The test got 'File permissions of /some/file.txt (remote): > 0o' So any permissions manipulations are useles on Windows and these > tests cannot be adapted. But who is calling File::GetPermissions? The way I see it, this test is checking that `platform get-permissions /some/file.txt` generates the correct gdb-remote packet. It does that by having the test talk to a simulated/mock gdb server and hardcoding the responses (see line 156, 0x1a4 == 0o664). In theory that should work as there are no real files involved (and for all lldb cares, the remote platform does actually have a functional permission implementation). https://github.com/llvm/llvm-project/pull/92088 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Windows] Fixed the test gdb_remote_client/TestGDBRemotePlatformFile (PR #92088)
slydiman wrote: I have removed @skipIfWindows because these tests are host specific. https://github.com/llvm/llvm-project/pull/92088 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Windows] Fixed the test gdb_remote_client/TestGDBRemotePlatformFile (PR #92088)
https://github.com/slydiman updated https://github.com/llvm/llvm-project/pull/92088 >From 7dcfe773b6eef27aabbcc7fc68cd6448bc3c2e88 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Tue, 14 May 2024 13:08:35 +0400 Subject: [PATCH 1/2] [lldb][Windows] Fixed the test gdb_remote_client/TestGDBRemotePlatformFile The tests `test_file_permissions` and `test_file_permissions_fallback` are disabled for Windows target. These tests use MockGDBServerResponder and do not depend on the real target. These tests failed in case of Windows host and Linux target. Disable them for Windows host too. --- .../gdb_remote_client/TestGDBRemotePlatformFile.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py index 2be5ae3132038..9ef0954af1fe3 100644 --- a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py +++ b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py @@ -148,6 +148,7 @@ def vFile(self, packet): ) @skipIfWindows +@skipIf(hostoslist=["windows"]) def test_file_permissions(self): """Test 'platform get-permissions'""" @@ -168,6 +169,7 @@ def vFile(self, packet): ) @skipIfWindows +@skipIf(hostoslist=["windows"]) def test_file_permissions_fallback(self): """Test 'platform get-permissions' fallback to fstat""" >From 478d251691d511916cae5fc344d549450222d584 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Tue, 14 May 2024 20:23:56 +0400 Subject: [PATCH 2/2] Removed @skipIfWindows --- .../gdb_remote_client/TestGDBRemotePlatformFile.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py index 9ef0954af1fe3..b1c6f0822d1a8 100644 --- a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py +++ b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py @@ -147,7 +147,6 @@ def vFile(self, packet): log=server2.responder.packetLog, ) -@skipIfWindows @skipIf(hostoslist=["windows"]) def test_file_permissions(self): """Test 'platform get-permissions'""" @@ -168,7 +167,6 @@ def vFile(self, packet): ] ) -@skipIfWindows @skipIf(hostoslist=["windows"]) def test_file_permissions_fallback(self): """Test 'platform get-permissions' fallback to fstat""" ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Windows] Fixed the test gdb_remote_client/TestGDBRemotePlatformFile (PR #92088)
slydiman wrote: The problem is that fstat() is fake on Windows. File::GetPermissions() returns 0 always. The test got 'File permissions of /some/file.txt (remote): 0o' So any permissions manipulations are useles on Windows and these tests cannot be adapted. https://github.com/llvm/llvm-project/pull/92088 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Windows] Fixed the test gdb_remote_client/TestGDBRemotePlatformFile (PR #92088)
labath wrote: Given that the tests use a mock server, why is it a problem that they're running on a windows host? What's the actual failure? Could they be adapted so that they run everywhere? https://github.com/llvm/llvm-project/pull/92088 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Windows] Fixed the test gdb_remote_client/TestGDBRemotePlatformFile (PR #92088)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Dmitry Vasilyev (slydiman) Changes The tests `test_file_permissions` and `test_file_permissions_fallback` are disabled for Windows target. These tests use MockGDBServerResponder and do not depend on the real target. These tests failed in case of Windows host and Linux target. Disable them for Windows host too. --- Full diff: https://github.com/llvm/llvm-project/pull/92088.diff 1 Files Affected: - (modified) lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py (+2) ``diff diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py index 2be5ae3132038..9ef0954af1fe3 100644 --- a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py +++ b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py @@ -148,6 +148,7 @@ def vFile(self, packet): ) @skipIfWindows +@skipIf(hostoslist=["windows"]) def test_file_permissions(self): """Test 'platform get-permissions'""" @@ -168,6 +169,7 @@ def vFile(self, packet): ) @skipIfWindows +@skipIf(hostoslist=["windows"]) def test_file_permissions_fallback(self): """Test 'platform get-permissions' fallback to fstat""" `` https://github.com/llvm/llvm-project/pull/92088 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Windows] Fixed the test gdb_remote_client/TestGDBRemotePlatformFile (PR #92088)
https://github.com/slydiman created https://github.com/llvm/llvm-project/pull/92088 The tests `test_file_permissions` and `test_file_permissions_fallback` are disabled for Windows target. These tests use MockGDBServerResponder and do not depend on the real target. These tests failed in case of Windows host and Linux target. Disable them for Windows host too. >From 7dcfe773b6eef27aabbcc7fc68cd6448bc3c2e88 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Tue, 14 May 2024 13:08:35 +0400 Subject: [PATCH] [lldb][Windows] Fixed the test gdb_remote_client/TestGDBRemotePlatformFile The tests `test_file_permissions` and `test_file_permissions_fallback` are disabled for Windows target. These tests use MockGDBServerResponder and do not depend on the real target. These tests failed in case of Windows host and Linux target. Disable them for Windows host too. --- .../gdb_remote_client/TestGDBRemotePlatformFile.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py index 2be5ae3132038..9ef0954af1fe3 100644 --- a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py +++ b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py @@ -148,6 +148,7 @@ def vFile(self, packet): ) @skipIfWindows +@skipIf(hostoslist=["windows"]) def test_file_permissions(self): """Test 'platform get-permissions'""" @@ -168,6 +169,7 @@ def vFile(self, packet): ) @skipIfWindows +@skipIf(hostoslist=["windows"]) def test_file_permissions_fallback(self): """Test 'platform get-permissions' fallback to fstat""" ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
