Awfa wrote:
Thanks for taking a look at the build. I will do a new PR addressing the issues.
https://github.com/llvm/llvm-project/pull/88812
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
bulbazord wrote:
> I got an email that the build failed and I should revert because of this:
> https://lab.llvm.org/buildbot/#/builders/68/builds/72623
>
> @bulbazord can you or someone with write permissions revert this PR so I have
> time to triage the issue?
As David said, no need since
DavidSpickett wrote:
Please address @JDevlieghere's comments in a new PR.
https://github.com/llvm/llvm-project/pull/88812
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
DavidSpickett wrote:
> @bulbazord can you or someone with write permissions revert this PR so I have
> time to triage the issue?
The next build is green, those DAP tests do fail once in a while so it's
unrelated to your change.
https://github.com/llvm/llvm-project/pull/88812
DavidSpickett wrote:
> So a good follow up PR would be to list it in that document. It's self
> explanatory but still, weird that it's not there.
https://github.com/llvm/llvm-project/pull/89357
https://github.com/llvm/llvm-project/pull/88812
___
Awfa wrote:
I got an email that the build failed and I should revert because of this:
https://lab.llvm.org/buildbot/#/builders/68/builds/72623
@bulbazord can you or someone with write permissions revert this PR so I have
time to triage the issue?
@@ -3419,7 +3419,7 @@ bool GDBRemoteCommunicationClient::GetFileExists(
}
bool GDBRemoteCommunicationClient::CalculateMD5(
-const lldb_private::FileSpec _spec, uint64_t , uint64_t ) {
+const lldb_private::FileSpec _spec, uint64_t , uint64_t ) {
@@ -1197,6 +1197,32 @@ Status Platform::PutFile(const FileSpec , const
FileSpec ,
if (!source_file)
return Status(source_file.takeError());
Status error;
+
+ bool requires_upload = true;
+ uint64_t dest_md5_low, dest_md5_high;
+ bool success =
github-actions[bot] wrote:
@Awfa Congratulations on having your first Pull Request (PR) merged into the
LLVM Project!
Your changes will be combined with recent changes from other authors, then
tested
by our [build bots](https://lab.llvm.org/buildbot/). If there is a problem with
a build,
https://github.com/bulbazord closed
https://github.com/llvm/llvm-project/pull/88812
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
bulbazord wrote:
> Someone will have to merge for me because it says "Only those with write
> access to this repository can merge pull requests."
>
> Am I supposed to request write access somewhere before making these PRs? I
> haven't contributed before.
First time contributors aren't
Awfa wrote:
Someone will have to merge for me because it says "Only those with write access
to this repository can merge pull requests."
Am I supposed to request write access somewhere before making these PRs? I
haven't contributed before.
https://github.com/llvm/llvm-project/pull/88812
emaste wrote:
Ok, submitted https://github.com/llvm/llvm-project/issues/89271 for the MD5
migration. I agree that issue does not block this change.
https://github.com/llvm/llvm-project/pull/88812
___
lldb-commits mailing list
https://github.com/bulbazord approved this pull request.
LGTM
https://github.com/llvm/llvm-project/pull/88812
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
DavidSpickett wrote:
@bulbazord please give the final approval if it looks good to you.
We can sort out moving from MD5 on an issue.
https://github.com/llvm/llvm-project/pull/88812
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
DavidSpickett wrote:
If it were a GDB packet it would be in
https://sourceware.org/gdb/current/onlinedocs/gdb.html/Host-I_002fO-Packets.html#Host-I_002fO-Packets
but I see no sign of it, or in GDB's sources.
The first appearance of `vFile:MD5` is e0f8f574c7f41f5b61ec01aa290d52fc55a3f3c9
Awfa wrote:
> MD5 is insufficient for claiming that files are identical; how do we migrate
> this to a secure hash?
Is there an attack vector you're concerned about?
Or are you wary of workflow friction when a file won't upload to the remote
platform because the hashes accidently collide?
emaste wrote:
MD5 is insufficient for claiming that files are identical; how do we migrate
this to a secure hash?
https://github.com/llvm/llvm-project/pull/88812
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
@@ -1197,6 +1197,34 @@ Status Platform::PutFile(const FileSpec , const
FileSpec ,
if (!source_file)
return Status(source_file.takeError());
Status error;
+
+ bool requires_upload = true;
+ {
Awfa wrote:
Just removed the scoping - so the md5 vars
@@ -1184,7 +1184,7 @@ bool Platform::IsCompatibleArchitecture(const ArchSpec
,
Status Platform::PutFile(const FileSpec , const FileSpec ,
uint32_t uid, uint32_t gid) {
Log *log = GetLog(LLDBLog::Platform);
- LLDB_LOGF(log, "[PutFile] Using block by
https://github.com/Awfa updated https://github.com/llvm/llvm-project/pull/88812
>From 095696411172034f80233f1722e293c1f458d67f Mon Sep 17 00:00:00 2001
From: Anthony Ha
Date: Mon, 15 Apr 2024 19:34:19 +
Subject: [PATCH 1/3] [lldb] Skip remote PutFile when MD5 hashes equal
---
https://github.com/bulbazord commented:
Looking better! Let's get the comments and style sorted out, but I think this
is looking good :)
https://github.com/llvm/llvm-project/pull/88812
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
@@ -684,6 +684,14 @@ Status PlatformRemoteGDBServer::RunShellCommand(
signo_ptr, command_output, timeout);
}
+bool PlatformRemoteGDBServer::CalculateMD5(const FileSpec _spec,
+ uint64_t ,
https://github.com/bulbazord edited
https://github.com/llvm/llvm-project/pull/88812
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
@@ -1197,6 +1197,34 @@ Status Platform::PutFile(const FileSpec , const
FileSpec ,
if (!source_file)
return Status(source_file.takeError());
Status error;
+
+ bool requires_upload = true;
+ {
bulbazord wrote:
I see. I wouldn't say it's a bad thing
DavidSpickett wrote:
@weliveindetail I think this might fix the problems you were having remote
debugging clang-repl (I can't seem to find the actual Discourse thread).
https://github.com/llvm/llvm-project/pull/88812
___
lldb-commits mailing list
@@ -1197,6 +1197,34 @@ Status Platform::PutFile(const FileSpec , const
FileSpec ,
if (!source_file)
return Status(source_file.takeError());
Status error;
+
+ bool requires_upload = true;
+ {
DavidSpickett wrote:
If you really need a new scope, a
@@ -1184,7 +1184,7 @@ bool Platform::IsCompatibleArchitecture(const ArchSpec
,
Status Platform::PutFile(const FileSpec , const FileSpec ,
uint32_t uid, uint32_t gid) {
Log *log = GetLog(LLDBLog::Platform);
- LLDB_LOGF(log, "[PutFile] Using block by
@@ -3433,8 +3433,40 @@ bool GDBRemoteCommunicationClient::CalculateMD5(
return false;
if (response.Peek() && *response.Peek() == 'x')
return false;
-low = response.GetHexMaxU64(false, UINT64_MAX);
-high = response.GetHexMaxU64(false, UINT64_MAX);
+
+
@@ -684,6 +684,15 @@ Status PlatformRemoteGDBServer::RunShellCommand(
signo_ptr, command_output, timeout);
}
+bool PlatformRemoteGDBServer::CalculateMD5(const FileSpec _spec,
+ uint64_t ,
https://github.com/Awfa updated https://github.com/llvm/llvm-project/pull/88812
>From 095696411172034f80233f1722e293c1f458d67f Mon Sep 17 00:00:00 2001
From: Anthony Ha
Date: Mon, 15 Apr 2024 19:34:19 +
Subject: [PATCH 1/2] [lldb] Skip remote PutFile when MD5 hashes equal
---
@@ -1197,6 +1197,34 @@ Status Platform::PutFile(const FileSpec , const
FileSpec ,
if (!source_file)
return Status(source_file.takeError());
Status error;
+
+ bool requires_upload = true;
+ {
+uint64_t dest_md5_low, dest_md5_high;
+bool success =
@@ -3433,8 +3433,40 @@ bool GDBRemoteCommunicationClient::CalculateMD5(
return false;
if (response.Peek() && *response.Peek() == 'x')
return false;
-low = response.GetHexMaxU64(false, UINT64_MAX);
-high = response.GetHexMaxU64(false, UINT64_MAX);
+
+
@@ -1197,6 +1197,34 @@ Status Platform::PutFile(const FileSpec , const
FileSpec ,
if (!source_file)
return Status(source_file.takeError());
Status error;
+
+ bool requires_upload = true;
+ {
Awfa wrote:
This is how I write code to scope the
@@ -1184,7 +1184,7 @@ bool Platform::IsCompatibleArchitecture(const ArchSpec
,
Status Platform::PutFile(const FileSpec , const FileSpec ,
uint32_t uid, uint32_t gid) {
Log *log = GetLog(LLDBLog::Platform);
- LLDB_LOGF(log, "[PutFile] Using block by
@@ -684,6 +684,15 @@ Status PlatformRemoteGDBServer::RunShellCommand(
signo_ptr, command_output, timeout);
}
+bool PlatformRemoteGDBServer::CalculateMD5(const FileSpec _spec,
+ uint64_t ,
@@ -684,6 +684,15 @@ Status PlatformRemoteGDBServer::RunShellCommand(
signo_ptr, command_output, timeout);
}
+bool PlatformRemoteGDBServer::CalculateMD5(const FileSpec _spec,
+ uint64_t ,
@@ -3433,8 +3433,40 @@ bool GDBRemoteCommunicationClient::CalculateMD5(
return false;
if (response.Peek() && *response.Peek() == 'x')
return false;
-low = response.GetHexMaxU64(false, UINT64_MAX);
-high = response.GetHexMaxU64(false, UINT64_MAX);
+
+
@@ -3433,8 +3433,40 @@ bool GDBRemoteCommunicationClient::CalculateMD5(
return false;
if (response.Peek() && *response.Peek() == 'x')
return false;
-low = response.GetHexMaxU64(false, UINT64_MAX);
-high = response.GetHexMaxU64(false, UINT64_MAX);
+
+
@@ -3433,8 +3433,40 @@ bool GDBRemoteCommunicationClient::CalculateMD5(
return false;
if (response.Peek() && *response.Peek() == 'x')
return false;
-low = response.GetHexMaxU64(false, UINT64_MAX);
-high = response.GetHexMaxU64(false, UINT64_MAX);
+
+
@@ -684,6 +684,15 @@ Status PlatformRemoteGDBServer::RunShellCommand(
signo_ptr, command_output, timeout);
}
+bool PlatformRemoteGDBServer::CalculateMD5(const FileSpec _spec,
+ uint64_t ,
@@ -1197,6 +1197,34 @@ Status Platform::PutFile(const FileSpec , const
FileSpec ,
if (!source_file)
return Status(source_file.takeError());
Status error;
+
+ bool requires_upload = true;
+ {
+uint64_t dest_md5_low, dest_md5_high;
+bool success =
@@ -1184,7 +1184,7 @@ bool Platform::IsCompatibleArchitecture(const ArchSpec
,
Status Platform::PutFile(const FileSpec , const FileSpec ,
uint32_t uid, uint32_t gid) {
Log *log = GetLog(LLDBLog::Platform);
- LLDB_LOGF(log, "[PutFile] Using block by
@@ -1197,6 +1197,34 @@ Status Platform::PutFile(const FileSpec , const
FileSpec ,
if (!source_file)
return Status(source_file.takeError());
Status error;
+
+ bool requires_upload = true;
+ {
bulbazord wrote:
Why is this all in its own block?
@@ -3433,8 +3433,40 @@ bool GDBRemoteCommunicationClient::CalculateMD5(
return false;
if (response.Peek() && *response.Peek() == 'x')
return false;
-low = response.GetHexMaxU64(false, UINT64_MAX);
-high = response.GetHexMaxU64(false, UINT64_MAX);
+
+
@@ -684,6 +684,15 @@ Status PlatformRemoteGDBServer::RunShellCommand(
signo_ptr, command_output, timeout);
}
+bool PlatformRemoteGDBServer::CalculateMD5(const FileSpec _spec,
+ uint64_t ,
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Anthony Ha (Awfa)
Changes
This PR adds a check within `PutFile` to exit early when both local and
destination files have matching MD5 hashes. If they differ, or there is trouble
getting the hashes, the regular code path to put the file is
github-actions[bot] wrote:
Thank you for submitting a Pull Request (PR) to the LLVM Project!
This PR will be automatically labeled and the relevant teams will be
notified.
If you wish to, you can add reviewers by using the "Reviewers" section on this
page.
If this is not working for you,
https://github.com/Awfa created https://github.com/llvm/llvm-project/pull/88812
This PR adds a check within `PutFile` to exit early when both local and
destination files have matching MD5 hashes. If they differ, or there is trouble
getting the hashes, the regular code path to put the file is
49 matches
Mail list logo
