[v3,1/2] gdbreplay: Calculate the checksum if missing
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gdb_build--master-arm |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-arm |
success
|
Test passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
success
|
Test passed
|
Commit Message
Recalculate the checksum and replace whatever is at the end
of the packet with the newly calculated checksum. Then
replay the packet with the checksum added.
The motivation for this change is that I'd like to add a TCL test
which starts a communication with gdbsever setting the remotelog file.
Then, it modifies the remotelog, injects an error message instead of
the expected replay to some packet in order to test GDB reacts to
the error response properly.
---
v3: - use 'const std::string &buf' to avoid a copy
- use line.resize
gdbserver/gdbreplay.cc | 52 ++++++++++++++++++++++++++++++++++++++----
1 file changed, 48 insertions(+), 4 deletions(-)
Comments
>>>>> "Alexandra" == Alexandra Hájková <ahajkova@redhat.com> writes:
Alexandra> Recalculate the checksum and replace whatever is at the end
Alexandra> of the packet with the newly calculated checksum. Then
Alexandra> replay the packet with the checksum added.
Alexandra> The motivation for this change is that I'd like to add a TCL test
Alexandra> which starts a communication with gdbsever setting the remotelog file.
Alexandra> Then, it modifies the remotelog, injects an error message instead of
Alexandra> the expected replay to some packet in order to test GDB reacts to
Alexandra> the error response properly.
This still isn't really what I was hoping for, but I think it's probably
not really worth the effort we're putting into it.
There is one nit to fix before pushing, see below.
Approved-By: Tom Tromey <tom@tromey.com>
Alexandra> + line.resize(where_csum + 3);
Space before "("
Tom
On Fri, Nov 22, 2024 at 4:36 PM Tom Tromey <tom@tromey.com> wrote:
> >>>>> "Alexandra" == Alexandra Hájková <ahajkova@redhat.com> writes:
>
> Alexandra> Recalculate the checksum and replace whatever is at the end
> Alexandra> of the packet with the newly calculated checksum. Then
> Alexandra> replay the packet with the checksum added.
>
> Alexandra> The motivation for this change is that I'd like to add a TCL
> test
> Alexandra> which starts a communication with gdbsever setting the
> remotelog file.
> Alexandra> Then, it modifies the remotelog, injects an error message
> instead of
> Alexandra> the expected replay to some packet in order to test GDB reacts
> to
> Alexandra> the error response properly.
>
> This still isn't really what I was hoping for, but I think it's probably
> not really worth the effort we're putting into it.
>
What was the implementation you were hoping for? I'm open for ideas.
>
> There is one nit to fix before pushing, see below.
>
> Approved-By: Tom Tromey <tom@tromey.com>
>
> Thank you!
@@ -402,6 +402,20 @@ expect (FILE *fp)
}
}
+/* Calculate checksum for the packet stored in buffer buf. Store
+ the checksum in a hexadecimal format in a checksum_hex variable. */
+static void
+recalculate_csum (const std::string &buf, int cnt, unsigned char *checksum_hex)
+{
+ unsigned char csum = 0;
+
+ for (int i = 0; i < cnt; i++)
+ csum += buf[i];
+
+ checksum_hex[0] = tohex ((csum >> 4) & 0xf);
+ checksum_hex[1] = tohex (csum & 0xf);
+}
+
/* Play data back to gdb from fp (after skipping leading blank) up until a
\n is read from fp (which is discarded and not sent to gdb). */
@@ -409,7 +423,10 @@ static void
play (FILE *fp)
{
int fromlog;
- char ch;
+ int where_csum = 0, offset = 1;
+ unsigned char checksum[2] = {0, 0};
+ std::string line;
+
if ((fromlog = logchar (fp, false)) != ' ')
{
@@ -418,10 +435,37 @@ play (FILE *fp)
}
while ((fromlog = logchar (fp, false)) != EOL)
{
- ch = fromlog;
- if (write (remote_desc_out, &ch, 1) != 1)
- remote_error ("Error during write to gdb");
+ line.push_back (fromlog);
+ if (line[line.length ()] == '#')
+ where_csum = line.length ();
}
+
+ /* Packet starts with '+$' or '$', we don't want to calculate those
+ to the checksum, substract the offset to adjust the line length.
+ If the line starts with '$', the offset remains set to 1. */
+ if (line[0] == '+')
+ offset = 2;
+
+ /* If '#' is missing at the end of the line, add it and adjust the line
+ length. */
+ if (where_csum == 0)
+ {
+ where_csum = line.length ();
+ line.push_back ('#');
+ }
+ recalculate_csum (line.substr (offset), where_csum - offset, checksum);
+
+ /* Check if the checksum is missing and adjust the line length to be able
+ to fit the checksum. */
+ if (where_csum + 1 >= line.length ())
+ line.resize(where_csum + 3);
+
+ /* Replace what is at the end of the packet with the checksum. */
+ line[where_csum + 1] = checksum[0];
+ line[where_csum + 2] = checksum[1];
+
+ if (write (remote_desc_out, line.data (), line.size ()) != line.size ())
+ remote_error ("Error during write to gdb");
}
static void