[PATCH 1/8] gdb.trace: Fix write_inferior_data_ptr on 32-bit big-endian machines.

[PATCH 1/8] gdb.trace: Fix write_inferior_data_ptr on 32-bit big-endian machines.

[Marcin Kościelnicki]

Noticed and tested on s390.  This bug caused completely broken fast
tracepoints.  No regressions on i686, x86_64, s390x.

gdb/gdbserver/ChangeLog:

	* tracepoint.c (write_inferior_data_ptr): Cast to uintptr_t, so that
	it works properly on big-endian machines where sizeof (CORE_ADDR)
	!= sizeof (void *).
---
Another patch for upcoming s390 tracepoint support.  31-bit fast tracepoints
are still broken, but now less so.

It seems tracepoints are still broken in multiple ways on big-endian
targets, and I'm going to fix that.  However, I'm not sure what to do
about the bi-arch case mentioned in the comment.  While it'd be nice
to handle libinproctrace.so like libgcc.so and build it for all multilib
configurations, atm C structures involving pointers are shared between
gdbserver and the IPA.  This obviously would have to be refactored before
that's possible, but might be worth it - configuring a full 31-bit build
just to get fast tracepoints is a pain.

 gdb/gdbserver/ChangeLog    | 6 ++++++
 gdb/gdbserver/tracepoint.c | 6 +++---
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index 9c42fcf..6ddd9ce 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,3 +1,9 @@
+2016-01-22  Marcin Kościelnicki  <koriakin@0x04.net>
+
+	* tracepoint.c (write_inferior_data_ptr): Cast to uintptr_t, so that
+	it works properly on big-endian machines where sizeof (CORE_ADDR)
+	!= sizeof (void *).
+
 2016-01-21  Pedro Alves  <palves@redhat.com>
 
 	* Makefile.in (COMPILER_CFLAGS, CXXFLAGS): New.
diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index 40d0da9..536cdd5 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -5931,14 +5931,14 @@ compile_tracepoint_condition (struct tracepoint *tpoint,
   *jump_entry += 16;
 }
 
-/* We'll need to adjust these when we consider bi-arch setups, and big
-   endian machines.  */
+/* We'll need to adjust these when we consider bi-arch setups.  */
 
 static int
 write_inferior_data_ptr (CORE_ADDR where, CORE_ADDR ptr)
 {
+  uintptr_t pptr = ptr;
   return write_inferior_memory (where,
-				(unsigned char *) &ptr, sizeof (void *));
+				(unsigned char *) &pptr, sizeof pptr);
 }
 
 /* The base pointer of the IPA's heap.  This is the only memory the
-- 
2.7.0

Re: [PATCH 1/8] gdb.trace: Fix write_inferior_data_ptr on 32-bit big-endian machines.

[Pedro Alves]

OK with empty line after declaration.

Thanks,
Pedro Alves

Re: [PATCH 1/8] gdb.trace: Fix write_inferior_data_ptr on 32-bit big-endian machines.

[Marcin Kościelnicki]

On 22/01/16 15:01, Pedro Alves wrote:
> OK with empty line after declaration.
>
> Thanks,
> Pedro Alves
>

Thanks, fixed & pushed.

Any comment about the bi-arch case?  Does that seem like something worth 
doing?

BTW, don't mind the 1/8 in subject, that was a git-format-patch 
accident.  The remaining 7 patches are not yet review-ready :)

Marcin Kościelnicki

Re: [PATCH 1/8] gdb.trace: Fix write_inferior_data_ptr on 32-bit big-endian machines.

[Pedro Alves]

On 01/22/2016 02:06 PM, Marcin Kościelnicki wrote:
> On 22/01/16 15:01, Pedro Alves wrote:
>> OK with empty line after declaration.
>>
>> Thanks,
>> Pedro Alves
>>
> 
> Thanks, fixed & pushed.
> 
> Any comment about the bi-arch case?  Does that seem like something worth 
> doing?

I think it'd be nice if that worked.

> 
> BTW, don't mind the 1/8 in subject, that was a git-format-patch 
> accident.  The remaining 7 patches are not yet review-ready :)

Thanks,
Pedro Alves

Re: [PATCH 1/8] gdb.trace: Fix write_inferior_data_ptr on 32-bit big-endian machines.

[Ulrich Weigand]

Marcin KoÅcielnicki wrote:

> 	* tracepoint.c (write_inferior_data_ptr): Cast to uintptr_t, so that
> 	it works properly on big-endian machines where sizeof (CORE_ADDR)
> 	!= sizeof (void *).

This is the same problem that was detected on PowerPC here:
https://sourceware.org/ml/gdb-patches/2015-03/msg00996.html

Unfortunately that patch, while approved, never got committed.  It still
seems to be the better solution, since now there's two basically identical
routines write_inferior_data_ptr and write_inferior_data_pointer for no
particular reason ...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

Re: [PATCH 1/8] gdb.trace: Fix write_inferior_data_ptr on 32-bit big-endian machines.

[Marcin Kościelnicki]

On 26/01/16 14:32, Ulrich Weigand wrote:
> Marcin KoÃ…cielnicki wrote:
>
>> 	* tracepoint.c (write_inferior_data_ptr): Cast to uintptr_t, so that
>> 	it works properly on big-endian machines where sizeof (CORE_ADDR)
>> 	!= sizeof (void *).
>
> This is the same problem that was detected on PowerPC here:
> https://sourceware.org/ml/gdb-patches/2015-03/msg00996.html
>
> Unfortunately that patch, while approved, never got committed.  It still
> seems to be the better solution, since now there's two basically identical
> routines write_inferior_data_ptr and write_inferior_data_pointer for no
> particular reason ...
>
> Bye,
> Ulrich
>


I'll remove the duplicated function.

Is there a chance of getting this patchset commited?  I see I duplicated 
a good chunk of work done there, it's a matter of time before someone 
likewise stumbles upon other already-fixed issues...

Marcin Kościelnicki

Re: [PATCH 1/8] gdb.trace: Fix write_inferior_data_ptr on 32-bit big-endian machines.

[Ulrich Weigand]

Marcin Kościelnicki wrote:

> I'll remove the duplicated function.

Thanks.

> Is there a chance of getting this patchset commited?  I see I duplicated 
> a good chunk of work done there, it's a matter of time before someone 
> likewise stumbles upon other already-fixed issues...

Agreed.  I've now pinged Wei-cheng off-list whether he's still planning
on committing this.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com