* [PATCH] tracepoint: fix tfile byte order issue
@ 2010-10-14 6:41 Hui Zhu
2010-10-19 8:48 ` Hui Zhu
0 siblings, 1 reply; 6+ messages in thread
From: Hui Zhu @ 2010-10-14 6:41 UTC (permalink / raw)
To: gdb-patches ml
Hi,
I found that the "target tfile" doesn't handle the issue that byte
order of target part is different with host part.
For example:
In target part:
In gdbserver/tracepoint.c:add_traceframe
tframe->tpnum = tpoint->number;
It set this number directly.
In gdbserver/tracepoint.c:agent_mem_read
memcpy (mspace, &from, sizeof (from));
The address is set to memory directly.
In host part:
In ttacepoint.c:tfile_trace_find
gotten = read (trace_fd, &tpnum, 2);
After read, it is used directly.
If the byte order of target and host is same, everything is OK.
But if the target and host is not same, it will get error.
So I make a patch to let host part use extract_unsigned_integer covert
the number. It will not affect current code. But can handle byte
order issue.
Thanks,
Hui
2010-10-14 Hui Zhu <teawater@gmail.com>
* tracepoint.c (tfile_get_traceframe_address): Call
extract_unsigned_integer.
(tfile_trace_find): Ditto.
(tfile_fetch_registers): Ditto.
(tfile_xfer_partial): Ditto.
(tfile_get_trace_state_variable_value): Ditto.
---
tracepoint.c | 31 ++++++++++++++++++++++++++++++-
1 file changed, 30 insertions(+), 1 deletion(-)
--- a/tracepoint.c
+++ b/tracepoint.c
@@ -3665,6 +3665,9 @@ tfile_get_traceframe_address (off_t tfra
perror_with_name (trace_filename);
else if (gotten < 2)
error (_("Premature end of file while reading trace file"));
+ tpnum = (short) extract_unsigned_integer ((gdb_byte *)&tpnum, 2,
+ gdbarch_byte_order
+ (get_current_arch ()));
tp = get_tracepoint_by_number_on_target (tpnum);
/* FIXME this is a poor heuristic if multiple locations */
@@ -3703,6 +3706,9 @@ tfile_trace_find (enum trace_find_type t
perror_with_name (trace_filename);
else if (gotten < 2)
error (_("Premature end of file while reading trace file"));
+ tpnum = (short) extract_unsigned_integer ((gdb_byte *)&tpnum, 2,
+ gdbarch_byte_order
+ (get_current_arch ()));
offset += 2;
if (tpnum == 0)
break;
@@ -3711,6 +3717,9 @@ tfile_trace_find (enum trace_find_type t
perror_with_name (trace_filename);
else if (gotten < 4)
error (_("Premature end of file while reading trace file"));
+ data_size = (int) extract_unsigned_integer ((gdb_byte *)&data_size, 4,
+ gdbarch_byte_order
+ (get_current_arch ()));
offset += 4;
switch (type)
{
@@ -3832,6 +3841,10 @@ tfile_fetch_registers (struct target_ops
perror_with_name (trace_filename);
else if (gotten < 2)
error (_("Premature end of file while reading trace file"));
+ mlen = (unsigned short)
+ extract_unsigned_integer ((gdb_byte *)&mlen, 2,
+ gdbarch_byte_order
+ (get_current_arch ()));
lseek (trace_fd, mlen, SEEK_CUR);
pos += (8 + 2 + mlen);
break;
@@ -3924,12 +3937,18 @@ tfile_xfer_partial (struct target_ops *o
perror_with_name (trace_filename);
else if (gotten < 8)
error (_("Premature end of file while reading trace file"));
-
+ maddr = extract_unsigned_integer ((gdb_byte *)&maddr, 8,
+ gdbarch_byte_order
+ (get_current_arch ()));
gotten = read (trace_fd, &mlen, 2);
if (gotten < 0)
perror_with_name (trace_filename);
else if (gotten < 2)
error (_("Premature end of file while reading trace file"));
+ mlen = (unsigned short)
+ extract_unsigned_integer ((gdb_byte *)&mlen, 2,
+ gdbarch_byte_order
+ (get_current_arch ()));
/* If the block includes the first part of the desired
range, return as much it has; GDB will re-request the
remainder, which might be in a different block of this
@@ -4032,6 +4051,10 @@ tfile_get_trace_state_variable_value (in
perror_with_name (trace_filename);
else if (gotten < 2)
error (_("Premature end of file while reading trace file"));
+ mlen = (unsigned short)
+ extract_unsigned_integer ((gdb_byte *)&mlen, 2,
+ gdbarch_byte_order
+ (get_current_arch ()));
lseek (trace_fd, mlen, SEEK_CUR);
pos += (8 + 2 + mlen);
break;
@@ -4041,6 +4064,9 @@ tfile_get_trace_state_variable_value (in
perror_with_name (trace_filename);
else if (gotten < 4)
error (_("Premature end of file while reading trace file"));
+ vnum = (int) extract_unsigned_integer ((gdb_byte *)&vnum, 4,
+ gdbarch_byte_order
+ (get_current_arch ()));
if (tsvnum == vnum)
{
gotten = read (trace_fd, val, 8);
@@ -4048,6 +4074,9 @@ tfile_get_trace_state_variable_value (in
perror_with_name (trace_filename);
else if (gotten < 8)
error (_("Premature end of file while reading trace file"));
+ *val = extract_unsigned_integer ((gdb_byte *)val, 8,
+ gdbarch_byte_order
+ (get_current_arch ()));
return 1;
}
lseek (trace_fd, 8, SEEK_CUR);
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] tracepoint: fix tfile byte order issue
2010-10-14 6:41 [PATCH] tracepoint: fix tfile byte order issue Hui Zhu
@ 2010-10-19 8:48 ` Hui Zhu
2010-10-19 9:20 ` Pedro Alves
0 siblings, 1 reply; 6+ messages in thread
From: Hui Zhu @ 2010-10-19 8:48 UTC (permalink / raw)
To: gdb-patches ml
Ping
---------- Forwarded message ----------
From: Hui Zhu <teawater@gmail.com>
Date: Thu, Oct 14, 2010 at 14:40
Subject: [PATCH] tracepoint: fix tfile byte order issue
To: gdb-patches ml <gdb-patches@sourceware.org>
Hi,
I found that the "target tfile" doesn't handle the issue that byte
order of target part is different with host part.
For example:
In target part:
In gdbserver/tracepoint.c:add_traceframe
tframe->tpnum = tpoint->number;
It set this number directly.
In gdbserver/tracepoint.c:agent_mem_read
memcpy (mspace, &from, sizeof (from));
The address is set to memory directly.
In host part:
In ttacepoint.c:tfile_trace_find
gotten = read (trace_fd, &tpnum, 2);
After read, it is used directly.
If the byte order of target and host is same, everything is OK.
But if the target and host is not same, it will get error.
So I make a patch to let host part use extract_unsigned_integer covert
the number. It will not affect current code. But can handle byte
order issue.
Thanks,
Hui
2010-10-14 Hui Zhu <teawater@gmail.com>
* tracepoint.c (tfile_get_traceframe_address): Call
extract_unsigned_integer.
(tfile_trace_find): Ditto.
(tfile_fetch_registers): Ditto.
(tfile_xfer_partial): Ditto.
(tfile_get_trace_state_variable_value): Ditto.
---
tracepoint.c | 31 ++++++++++++++++++++++++++++++-
1 file changed, 30 insertions(+), 1 deletion(-)
--- a/tracepoint.c
+++ b/tracepoint.c
@@ -3665,6 +3665,9 @@ tfile_get_traceframe_address (off_t tfra
perror_with_name (trace_filename);
else if (gotten < 2)
error (_("Premature end of file while reading trace file"));
+ tpnum = (short) extract_unsigned_integer ((gdb_byte *)&tpnum, 2,
+ gdbarch_byte_order
+ (get_current_arch ()));
tp = get_tracepoint_by_number_on_target (tpnum);
/* FIXME this is a poor heuristic if multiple locations */
@@ -3703,6 +3706,9 @@ tfile_trace_find (enum trace_find_type t
perror_with_name (trace_filename);
else if (gotten < 2)
error (_("Premature end of file while reading trace file"));
+ tpnum = (short) extract_unsigned_integer ((gdb_byte *)&tpnum, 2,
+ gdbarch_byte_order
+ (get_current_arch ()));
offset += 2;
if (tpnum == 0)
break;
@@ -3711,6 +3717,9 @@ tfile_trace_find (enum trace_find_type t
perror_with_name (trace_filename);
else if (gotten < 4)
error (_("Premature end of file while reading trace file"));
+ data_size = (int) extract_unsigned_integer ((gdb_byte *)&data_size, 4,
+ gdbarch_byte_order
+ (get_current_arch ()));
offset += 4;
switch (type)
{
@@ -3832,6 +3841,10 @@ tfile_fetch_registers (struct target_ops
perror_with_name (trace_filename);
else if (gotten < 2)
error (_("Premature end of file while reading trace file"));
+ mlen = (unsigned short)
+ extract_unsigned_integer ((gdb_byte *)&mlen, 2,
+ gdbarch_byte_order
+ (get_current_arch ()));
lseek (trace_fd, mlen, SEEK_CUR);
pos += (8 + 2 + mlen);
break;
@@ -3924,12 +3937,18 @@ tfile_xfer_partial (struct target_ops *o
perror_with_name (trace_filename);
else if (gotten < 8)
error (_("Premature end of file while reading trace file"));
-
+ maddr = extract_unsigned_integer ((gdb_byte *)&maddr, 8,
+ gdbarch_byte_order
+ (get_current_arch ()));
gotten = read (trace_fd, &mlen, 2);
if (gotten < 0)
perror_with_name (trace_filename);
else if (gotten < 2)
error (_("Premature end of file while reading trace file"));
+ mlen = (unsigned short)
+ extract_unsigned_integer ((gdb_byte *)&mlen, 2,
+ gdbarch_byte_order
+ (get_current_arch ()));
/* If the block includes the first part of the desired
range, return as much it has; GDB will re-request the
remainder, which might be in a different block of this
@@ -4032,6 +4051,10 @@ tfile_get_trace_state_variable_value (in
perror_with_name (trace_filename);
else if (gotten < 2)
error (_("Premature end of file while reading trace file"));
+ mlen = (unsigned short)
+ extract_unsigned_integer ((gdb_byte *)&mlen, 2,
+ gdbarch_byte_order
+ (get_current_arch ()));
lseek (trace_fd, mlen, SEEK_CUR);
pos += (8 + 2 + mlen);
break;
@@ -4041,6 +4064,9 @@ tfile_get_trace_state_variable_value (in
perror_with_name (trace_filename);
else if (gotten < 4)
error (_("Premature end of file while reading trace file"));
+ vnum = (int) extract_unsigned_integer ((gdb_byte *)&vnum, 4,
+ gdbarch_byte_order
+ (get_current_arch ()));
if (tsvnum == vnum)
{
gotten = read (trace_fd, val, 8);
@@ -4048,6 +4074,9 @@ tfile_get_trace_state_variable_value (in
perror_with_name (trace_filename);
else if (gotten < 8)
error (_("Premature end of file while reading trace file"));
+ *val = extract_unsigned_integer ((gdb_byte *)val, 8,
+ gdbarch_byte_order
+ (get_current_arch ()));
return 1;
}
lseek (trace_fd, 8, SEEK_CUR);
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] tracepoint: fix tfile byte order issue
2010-10-19 8:48 ` Hui Zhu
@ 2010-10-19 9:20 ` Pedro Alves
2010-10-20 8:50 ` Hui Zhu
0 siblings, 1 reply; 6+ messages in thread
From: Pedro Alves @ 2010-10-19 9:20 UTC (permalink / raw)
To: gdb-patches; +Cc: Hui Zhu
On Tuesday 19 October 2010 09:48:28, Hui Zhu wrote:
> @@ -3665,6 +3665,9 @@ tfile_get_traceframe_address (off_t tfra
> perror_with_name (trace_filename);
> else if (gotten < 2)
> error (_("Premature end of file while reading trace file"));
> + tpnum = (short) extract_unsigned_integer ((gdb_byte *)&tpnum, 2,
> + gdbarch_byte_order
> + (get_current_arch ()));
>
> tp = get_tracepoint_by_number_on_target (tpnum);
> /* FIXME this is a poor heuristic if multiple locations */
> @@ -3703,6 +3706,9 @@ tfile_trace_find (enum trace_find_type t
> perror_with_name (trace_filename);
> else if (gotten < 2)
> error (_("Premature end of file while reading trace file"));
> + tpnum = (short) extract_unsigned_integer ((gdb_byte *)&tpnum, 2,
> + gdbarch_byte_order
> + (get_current_arch ()));
"The data in this section is raw binary, not a
hexadecimal or other encoding; its endianness matches the target's
endianness."
Please use target_gdbarch instead.
And use extract_signed_integer thus avoiding the casts.
--
Pedro Alves
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] tracepoint: fix tfile byte order issue
2010-10-19 9:20 ` Pedro Alves
@ 2010-10-20 8:50 ` Hui Zhu
2010-10-20 10:28 ` Pedro Alves
0 siblings, 1 reply; 6+ messages in thread
From: Hui Zhu @ 2010-10-20 8:50 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On Tue, Oct 19, 2010 at 17:20, Pedro Alves <pedro@codesourcery.com> wrote:
> On Tuesday 19 October 2010 09:48:28, Hui Zhu wrote:
>
>> @@ -3665,6 +3665,9 @@ tfile_get_traceframe_address (off_t tfra
>> perror_with_name (trace_filename);
>> else if (gotten < 2)
>> error (_("Premature end of file while reading trace file"));
>> + tpnum = (short) extract_unsigned_integer ((gdb_byte *)&tpnum, 2,
>> + gdbarch_byte_order
>> + (get_current_arch ()));
>>
>> tp = get_tracepoint_by_number_on_target (tpnum);
>> /* FIXME this is a poor heuristic if multiple locations */
>> @@ -3703,6 +3706,9 @@ tfile_trace_find (enum trace_find_type t
>> perror_with_name (trace_filename);
>> else if (gotten < 2)
>> error (_("Premature end of file while reading trace file"));
>> + tpnum = (short) extract_unsigned_integer ((gdb_byte *)&tpnum, 2,
>> + gdbarch_byte_order
>> + (get_current_arch ()));
>
> "The data in this section is raw binary, not a
> hexadecimal or other encoding; its endianness matches the target's
> endianness."
>
> Please use target_gdbarch instead.
>
> And use extract_signed_integer thus avoiding the casts.
>
> --
> Pedro Alves
>
Thanks Pedro.
I make a new patch change to use target_gdbarch and use
extract_signed_integer in signed value.
And I found that tfile_trace_find define the size of a frame to a int:
int data_size;
But I found that gdbserver/tracepoint.c:struct traceframe:
unsigned int data_size : 32;
So I change this int to unsigned int.
Please help me review the new one.
Best,
Hui
2010-10-20 Hui Zhu <teawater@gmail.com>
* tracepoint.c (tfile_get_traceframe_address): Call
extract_signed_integer.
(tfile_trace_find): Call extract_signed_integer and
extract_unsigned_integer. Change data_size to unsigned int.
(tfile_fetch_registers): Call extract_unsigned_integer.
(tfile_xfer_partial): Ditto.
(tfile_get_trace_state_variable_value): Call
extract_signed_integer and extract_unsigned_integer.
---
tracepoint.c | 33 +++++++++++++++++++++++++++++++--
1 file changed, 31 insertions(+), 2 deletions(-)
--- a/tracepoint.c
+++ b/tracepoint.c
@@ -3665,6 +3665,9 @@ tfile_get_traceframe_address (off_t tfra
perror_with_name (trace_filename);
else if (gotten < 2)
error (_("Premature end of file while reading trace file"));
+ tpnum = (short) extract_signed_integer ((gdb_byte *)&tpnum, 2,
+ gdbarch_byte_order
+ (target_gdbarch));
tp = get_tracepoint_by_number_on_target (tpnum);
/* FIXME this is a poor heuristic if multiple locations */
@@ -3688,7 +3691,7 @@ tfile_trace_find (enum trace_find_type t
{
short tpnum;
int tfnum = 0, found = 0, gotten;
- int data_size;
+ unsigned int data_size;
struct breakpoint *tp;
off_t offset, tframe_offset;
ULONGEST tfaddr;
@@ -3703,6 +3706,9 @@ tfile_trace_find (enum trace_find_type t
perror_with_name (trace_filename);
else if (gotten < 2)
error (_("Premature end of file while reading trace file"));
+ tpnum = (short) extract_signed_integer ((gdb_byte *)&tpnum, 2,
+ gdbarch_byte_order
+ (target_gdbarch));
offset += 2;
if (tpnum == 0)
break;
@@ -3711,6 +3717,9 @@ tfile_trace_find (enum trace_find_type t
perror_with_name (trace_filename);
else if (gotten < 4)
error (_("Premature end of file while reading trace file"));
+ data_size = (unsigned int) extract_unsigned_integer
+ ((gdb_byte *)&data_size, 4,
+ gdbarch_byte_order (target_gdbarch));
offset += 4;
switch (type)
{
@@ -3832,6 +3841,10 @@ tfile_fetch_registers (struct target_ops
perror_with_name (trace_filename);
else if (gotten < 2)
error (_("Premature end of file while reading trace file"));
+ mlen = (unsigned short)
+ extract_unsigned_integer ((gdb_byte *)&mlen, 2,
+ gdbarch_byte_order
+ (target_gdbarch));
lseek (trace_fd, mlen, SEEK_CUR);
pos += (8 + 2 + mlen);
break;
@@ -3924,12 +3937,18 @@ tfile_xfer_partial (struct target_ops *o
perror_with_name (trace_filename);
else if (gotten < 8)
error (_("Premature end of file while reading trace file"));
-
+ maddr = extract_unsigned_integer ((gdb_byte *)&maddr, 8,
+ gdbarch_byte_order
+ (target_gdbarch));
gotten = read (trace_fd, &mlen, 2);
if (gotten < 0)
perror_with_name (trace_filename);
else if (gotten < 2)
error (_("Premature end of file while reading trace file"));
+ mlen = (unsigned short)
+ extract_unsigned_integer ((gdb_byte *)&mlen, 2,
+ gdbarch_byte_order
+ (target_gdbarch));
/* If the block includes the first part of the desired
range, return as much it has; GDB will re-request the
remainder, which might be in a different block of this
@@ -4032,6 +4051,10 @@ tfile_get_trace_state_variable_value (in
perror_with_name (trace_filename);
else if (gotten < 2)
error (_("Premature end of file while reading trace file"));
+ mlen = (unsigned short)
+ extract_unsigned_integer ((gdb_byte *)&mlen, 2,
+ gdbarch_byte_order
+ (target_gdbarch));
lseek (trace_fd, mlen, SEEK_CUR);
pos += (8 + 2 + mlen);
break;
@@ -4041,6 +4064,9 @@ tfile_get_trace_state_variable_value (in
perror_with_name (trace_filename);
else if (gotten < 4)
error (_("Premature end of file while reading trace file"));
+ vnum = (int) extract_signed_integer ((gdb_byte *)&vnum, 4,
+ gdbarch_byte_order
+ (target_gdbarch));
if (tsvnum == vnum)
{
gotten = read (trace_fd, val, 8);
@@ -4048,6 +4074,9 @@ tfile_get_trace_state_variable_value (in
perror_with_name (trace_filename);
else if (gotten < 8)
error (_("Premature end of file while reading trace file"));
+ *val = extract_signed_integer ((gdb_byte *)val, 8,
+ gdbarch_byte_order
+ (target_gdbarch));
return 1;
}
lseek (trace_fd, 8, SEEK_CUR);
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] tracepoint: fix tfile byte order issue
2010-10-20 8:50 ` Hui Zhu
@ 2010-10-20 10:28 ` Pedro Alves
2010-10-20 14:04 ` Hui Zhu
0 siblings, 1 reply; 6+ messages in thread
From: Pedro Alves @ 2010-10-20 10:28 UTC (permalink / raw)
To: gdb-patches; +Cc: Hui Zhu
On Wednesday 20 October 2010 09:49:58, Hui Zhu wrote:
> I make a new patch change to use target_gdbarch and use
> extract_signed_integer in signed value.
>
> And I found that tfile_trace_find define the size of a frame to a int:
> int data_size;
>
> But I found that gdbserver/tracepoint.c:struct traceframe:
> unsigned int data_size : 32;
>
> So I change this int to unsigned int.
Thanks. It appears the format description in gdb.texinfo doesn't
make that explicit, but indeed all sizes should be unsigned.
> @@ -3665,6 +3665,9 @@ tfile_get_traceframe_address (off_t tfra
> perror_with_name (trace_filename);
> else if (gotten < 2)
> error (_("Premature end of file while reading trace file"));
> + tpnum = (short) extract_signed_integer ((gdb_byte *)&tpnum, 2,
> + gdbarch_byte_order
> + (target_gdbarch));
I don't think you need all the casts to short, int, etc.. anymore. Please
add the missing space in the second cast: "(gdb_byte *) &tpnum" everywhere too.
Otherwise okay. Thanks.
--
Pedro Alves
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] tracepoint: fix tfile byte order issue
2010-10-20 10:28 ` Pedro Alves
@ 2010-10-20 14:04 ` Hui Zhu
0 siblings, 0 replies; 6+ messages in thread
From: Hui Zhu @ 2010-10-20 14:04 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On Wed, Oct 20, 2010 at 18:28, Pedro Alves <pedro@codesourcery.com> wrote:
> On Wednesday 20 October 2010 09:49:58, Hui Zhu wrote:
>> I make a new patch change to use target_gdbarch and use
>> extract_signed_integer in signed value.
>>
>> And I found that tfile_trace_find define the size of a frame to a int:
>> int data_size;
>>
>> But I found that gdbserver/tracepoint.c:struct traceframe:
>> unsigned int data_size : 32;
>>
>> So I change this int to unsigned int.
>
> Thanks. It appears the format description in gdb.texinfo doesn't
> make that explicit, but indeed all sizes should be unsigned.
>
>> @@ -3665,6 +3665,9 @@ tfile_get_traceframe_address (off_t tfra
>> perror_with_name (trace_filename);
>> else if (gotten < 2)
>> error (_("Premature end of file while reading trace file"));
>> + tpnum = (short) extract_signed_integer ((gdb_byte *)&tpnum, 2,
>> + gdbarch_byte_order
>> + (target_gdbarch));
>
> I don't think you need all the casts to short, int, etc.. anymore. Please
> add the missing space in the second cast: "(gdb_byte *) &tpnum" everywhere too.
>
> Otherwise okay. Thanks.
>
> --
> Pedro Alves
>
Thanks Pedro. Fixed and checked in.
Hui
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-10-20 14:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-10-14 6:41 [PATCH] tracepoint: fix tfile byte order issue Hui Zhu
2010-10-19 8:48 ` Hui Zhu
2010-10-19 9:20 ` Pedro Alves
2010-10-20 8:50 ` Hui Zhu
2010-10-20 10:28 ` Pedro Alves
2010-10-20 14:04 ` Hui Zhu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox