[RFA] Define Elf32_auxv_t and Elf64_auxv_t if not available.

[RFA] Define Elf32_auxv_t and Elf64_auxv_t if not available.

[Thiago Jung Bauermann]

Hi,

glibc's <elf.h> says this about Elf32_auxv_t and Elf64_auxv_t:

   The vector is not usually defined in a standard <elf.h> file, but it
   can't hurt.  We rename it to avoid conflicts.

Android's bionic libc doesn't define these types in <elf.h>. To be
honest I couldn't find a definition for auxv_t in ARM's EABI and I'm not
aware of an Android EABI document. I confirmed that bionic internally
uses this format though, and this definition seems unlikely to change.

Ok to commit? It doesn't affect compilation in i386-linux or
armv5tel-linux-gnueabi.
-- 
[]'s
Thiago Jung Bauermann
Linaro Toolchain Working Group


2012-03-26  Thiago Jung Bauermann  <thiago.bauermann@linaro.org>

	* configure.ac: Check whether Elf32_auxv_t and Elf64_auxv_t
	are available.
	* linux-low.c [HAVE_ELF32_AUXV_T] (Elf32_auxv_t): Add typedef.
	[HAVE_ELF64_AUXV_T] (Elf64_auxv_t): Likewise.

diff --git a/gdb/gdbserver/configure.ac b/gdb/gdbserver/configure.ac
index e6e9162..6b98dc0 100644
--- a/gdb/gdbserver/configure.ac
+++ b/gdb/gdbserver/configure.ac
@@ -163,9 +163,10 @@ fi
 
 AC_CHECK_DECLS([strerror, perror, memmem, vasprintf, vsnprintf])
 
-AC_CHECK_TYPES(socklen_t, [], [],
+AC_CHECK_TYPES([socklen_t, Elf32_auxv_t, Elf64_auxv_t], [], [],
 [#include <sys/types.h>
 #include <sys/socket.h>
+#include <elf.h>
 ])
 
 ACX_PKGVERSION([GDB])
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index d2d4c1d..9aef4db 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -82,6 +82,34 @@
 #endif
 #endif
 
+#ifndef HAVE_ELF32_AUXV_T
+typedef struct
+{
+  uint32_t a_type;		/* Entry type */
+  union
+    {
+      uint32_t a_val;		/* Integer value */
+      /* We use to have pointer elements added here.  We cannot do that,
+	 though, since it does not work when using 32-bit definitions
+	 on 64-bit platforms and vice versa.  */
+    } a_un;
+} Elf32_auxv_t;
+#endif
+
+#ifndef HAVE_ELF64_AUXV_T
+typedef struct
+{
+  uint64_t a_type;		/* Entry type */
+  union
+    {
+      uint64_t a_val;		/* Integer value */
+      /* We use to have pointer elements added here.  We cannot do that,
+	 though, since it does not work when using 32-bit definitions
+	 on 64-bit platforms and vice versa.  */
+    } a_un;
+} Elf64_auxv_t;
+#endif
+
 /* ``all_threads'' is keyed by the LWP ID, which we use as the GDB protocol
    representation of the thread ID.
 

Re: [RFA] Define Elf32_auxv_t and Elf64_auxv_t if not available.

[Joel Brobecker]

> 2012-03-26  Thiago Jung Bauermann  <thiago.bauermann@linaro.org>
> 
> 	* configure.ac: Check whether Elf32_auxv_t and Elf64_auxv_t
> 	are available.
> 	* linux-low.c [HAVE_ELF32_AUXV_T] (Elf32_auxv_t): Add typedef.
> 	[HAVE_ELF64_AUXV_T] (Elf64_auxv_t): Likewise.

(you'll need to mention the fact that configure and config.in need
to be regenerated)

I am not very familiar with this area, so I am hoping someone else
might provide feedback as well.

I only have a minor question. Other than that, I don't see a problem
with the patch...

> diff --git a/gdb/gdbserver/configure.ac b/gdb/gdbserver/configure.ac
> index e6e9162..6b98dc0 100644
> --- a/gdb/gdbserver/configure.ac
> +++ b/gdb/gdbserver/configure.ac
> @@ -163,9 +163,10 @@ fi
>  
>  AC_CHECK_DECLS([strerror, perror, memmem, vasprintf, vsnprintf])
>  
> -AC_CHECK_TYPES(socklen_t, [], [],
> +AC_CHECK_TYPES([socklen_t, Elf32_auxv_t, Elf64_auxv_t], [], [],
>  [#include <sys/types.h>
>  #include <sys/socket.h>
> +#include <elf.h>
>  ])

I wonder if we shouldn't be using two separate checks here. Seems
unlikely to be a problem in practice, but we're looking for for
a type is <sys/types.h> + <sys/socket.h>, and then looking for another
collection of types in <elf.h>. We know we're most likely not going
to get any "cross-polination" finding socklen_t in <elf.h>, but if
this sort of thing were to happen, we would be in a situation where
the configure check finds the type, but the code (which presumably
might only include types.h and sockets.h) does not, leading to a build
error.

(let's wait for more feedback before we make any change - someone
might disagree)

-- 
Joel

Re: [RFA] Define Elf32_auxv_t and Elf64_auxv_t if not available.

[Thiago Jung Bauermann]

Hello Joel,

On Tue, 2012-03-27 at 08:20 -0700, Joel Brobecker wrote:
> > 2012-03-26  Thiago Jung Bauermann  <thiago.bauermann@linaro.org>
> > 
> > 	* configure.ac: Check whether Elf32_auxv_t and Elf64_auxv_t
> > 	are available.
> > 	* linux-low.c [HAVE_ELF32_AUXV_T] (Elf32_auxv_t): Add typedef.
> > 	[HAVE_ELF64_AUXV_T] (Elf64_auxv_t): Likewise.
> 
> (you'll need to mention the fact that configure and config.in need
> to be regenerated)

Oops, I forgot. Fixed.

> I am not very familiar with this area, so I am hoping someone else
> might provide feedback as well.

Thanks for your review!

> > diff --git a/gdb/gdbserver/configure.ac b/gdb/gdbserver/configure.ac
> > index e6e9162..6b98dc0 100644
> > --- a/gdb/gdbserver/configure.ac
> > +++ b/gdb/gdbserver/configure.ac
> > @@ -163,9 +163,10 @@ fi
> >  
> >  AC_CHECK_DECLS([strerror, perror, memmem, vasprintf, vsnprintf])
> >  
> > -AC_CHECK_TYPES(socklen_t, [], [],
> > +AC_CHECK_TYPES([socklen_t, Elf32_auxv_t, Elf64_auxv_t], [], [],
> >  [#include <sys/types.h>
> >  #include <sys/socket.h>
> > +#include <elf.h>
> >  ])
> 
> I wonder if we shouldn't be using two separate checks here. Seems
> unlikely to be a problem in practice, but we're looking for for
> a type is <sys/types.h> + <sys/socket.h>, and then looking for another
> collection of types in <elf.h>. We know we're most likely not going
> to get any "cross-polination" finding socklen_t in <elf.h>, but if
> this sort of thing were to happen, we would be in a situation where
> the configure check finds the type, but the code (which presumably
> might only include types.h and sockets.h) does not, leading to a build
> error.

That's a good point, I didn't think of that. Like you say it's unlikely
to happen in practice but it's cleaner indeed.

> (let's wait for more feedback before we make any change - someone
> might disagree)

It's an easy change so I did it anyway. Here's the new version.
-- 
[]'s
Thiago Jung Bauermann
Linaro Toolchain Working Group


2012-03-27  Thiago Jung Bauermann  <thiago.bauermann@linaro.org>

        * configure.ac: Check whether Elf32_auxv_t and Elf64_auxv_t
	are available.
	linux-low.c [HAVE_ELF32_AUXV_T] (Elf32_auxv_t): Add typedef.
	[HAVE_ELF64_AUXV_T] (Elf64_auxv_t): Likewise.
	* config.in: Regenerate.
	* configure: Likewise.


diff --git a/gdb/gdbserver/configure.ac b/gdb/gdbserver/configure.ac
index e6e9162..5d1e094 100644
--- a/gdb/gdbserver/configure.ac
+++ b/gdb/gdbserver/configure.ac
@@ -168,6 +168,10 @@ AC_CHECK_TYPES(socklen_t, [], [],
 #include <sys/socket.h>
 ])
 
+AC_CHECK_TYPES([Elf32_auxv_t, Elf64_auxv_t], [], [],
+#include <elf.h>
+)
+
 ACX_PKGVERSION([GDB])
 ACX_BUGURL([http://www.gnu.org/software/gdb/bugs/])
 AC_DEFINE_UNQUOTED([PKGVERSION], ["$PKGVERSION"], [Additional package description])
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 8becf78..1caff5a 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -82,6 +82,34 @@
 #endif
 #endif
 
+#ifndef HAVE_ELF32_AUXV_T
+typedef struct
+{
+  uint32_t a_type;		/* Entry type */
+  union
+    {
+      uint32_t a_val;		/* Integer value */
+      /* We use to have pointer elements added here.  We cannot do that,
+	 though, since it does not work when using 32-bit definitions
+	 on 64-bit platforms and vice versa.  */
+    } a_un;
+} Elf32_auxv_t;
+#endif
+
+#ifndef HAVE_ELF64_AUXV_T
+typedef struct
+{
+  uint64_t a_type;		/* Entry type */
+  union
+    {
+      uint64_t a_val;		/* Integer value */
+      /* We use to have pointer elements added here.  We cannot do that,
+	 though, since it does not work when using 32-bit definitions
+	 on 64-bit platforms and vice versa.  */
+    } a_un;
+} Elf64_auxv_t;
+#endif
+
 /* ``all_threads'' is keyed by the LWP ID, which we use as the GDB protocol
    representation of the thread ID.
 

Re: [RFA] Define Elf32_auxv_t and Elf64_auxv_t if not available.

[Pedro Alves]

Hello Thiago,

On 03/28/2012 01:11 AM, Thiago Jung Bauermann wrote:

> On Tue, 2012-03-27 at 08:20 -0700, Joel Brobecker wrote:
>>> > > 2012-03-26  Thiago Jung Bauermann  <thiago.bauermann@linaro.org>


>>> > > diff --git a/gdb/gdbserver/configure.ac b/gdb/gdbserver/configure.ac
>>> > > index e6e9162..6b98dc0 100644
>>> > > --- a/gdb/gdbserver/configure.ac
>>> > > +++ b/gdb/gdbserver/configure.ac
>>> > > @@ -163,9 +163,10 @@ fi
>>> > >  
>>> > >  AC_CHECK_DECLS([strerror, perror, memmem, vasprintf, vsnprintf])
>>> > >  
>>> > > -AC_CHECK_TYPES(socklen_t, [], [],
>>> > > +AC_CHECK_TYPES([socklen_t, Elf32_auxv_t, Elf64_auxv_t], [], [],
>>> > >  [#include <sys/types.h>
>>> > >  #include <sys/socket.h>
>>> > > +#include <elf.h>
>>> > >  ])
>> > 
>> > I wonder if we shouldn't be using two separate checks here. Seems
>> > unlikely to be a problem in practice, but we're looking for for
>> > a type is <sys/types.h> + <sys/socket.h>, and then looking for another
>> > collection of types in <elf.h>. We know we're most likely not going
>> > to get any "cross-polination" finding socklen_t in <elf.h>, but if
>> > this sort of thing were to happen, we would be in a situation where
>> > the configure check finds the type, but the code (which presumably
>> > might only include types.h and sockets.h) does not, leading to a build
>> > error.
> That's a good point, I didn't think of that. Like you say it's unlikely
> to happen in practice but it's cleaner indeed.


Separate tests is definitely the way to go, but for the reason that you'd
be always breaking the socklen_t test on non-elf ports, such
as the Windows port, where elf.h doesn't even exist.

> 2012-03-27  Thiago Jung Bauermann  <thiago.bauermann@linaro.org>
>
>         * configure.ac: Check whether Elf32_auxv_t and Elf64_auxv_t
> 	are available.
> 	linux-low.c [HAVE_ELF32_AUXV_T] (Elf32_auxv_t): Add typedef.
> 	[HAVE_ELF64_AUXV_T] (Elf64_auxv_t): Likewise.
> 	* config.in: Regenerate.
> 	* configure: Likewise.

This version looks good to me.


A "Copied from glibc's elf.h." small comment here would be good,
I think.  Otherwise, the "We use to have pointer elements added here"
comment is misleading.  :-)

> +#ifndef HAVE_ELF32_AUXV_T
> +typedef struct
> +{
> +  uint32_t a_type;		/* Entry type */
> +  union
> +    {
> +      uint32_t a_val;		/* Integer value */
> +      /* We use to have pointer elements added here.  We cannot do that,
> +	 though, since it does not work when using 32-bit definitions
> +	 on 64-bit platforms and vice versa.  */
> +    } a_un;
> +} Elf32_auxv_t;
> +#endif
> +

-- 
Pedro Alves

Re: [RFA] Define Elf32_auxv_t and Elf64_auxv_t if not available.

[Thiago Jung Bauermann]

On Thu, 2012-03-29 at 17:02 +0100, Pedro Alves wrote:
> On 03/28/2012 01:11 AM, Thiago Jung Bauermann wrote:
> > That's a good point, I didn't think of that. Like you say it's unlikely
> > to happen in practice but it's cleaner indeed.
> 
> 
> Separate tests is definitely the way to go, but for the reason that you'd
> be always breaking the socklen_t test on non-elf ports, such
> as the Windows port, where elf.h doesn't even exist.

Aha.

> > 2012-03-27  Thiago Jung Bauermann  <thiago.bauermann@linaro.org>
> >
> >         * configure.ac: Check whether Elf32_auxv_t and Elf64_auxv_t
> > 	are available.
> > 	linux-low.c [HAVE_ELF32_AUXV_T] (Elf32_auxv_t): Add typedef.
> > 	[HAVE_ELF64_AUXV_T] (Elf64_auxv_t): Likewise.
> > 	* config.in: Regenerate.
> > 	* configure: Likewise.
> 
> This version looks good to me.

Great, thanks for the review!

> A "Copied from glibc's elf.h." small comment here would be good,
> I think.  Otherwise, the "We use to have pointer elements added here"
> comment is misleading.  :-)

Indeed. I added the comment and committed the patch below.

-- 
[]'s
Thiago Jung Bauermann
Linaro Toolchain Working Group


2012-03-30  Thiago Jung Bauermann  <thiago.bauermann@linaro.org>

	* configure.ac: Check whether Elf32_auxv_t and Elf64_auxv_t
	are available.
	* linux-low.c [HAVE_ELF32_AUXV_T] (Elf32_auxv_t): Add typedef.
	[HAVE_ELF64_AUXV_T] (Elf64_auxv_t): Likewise.
	* config.in: Regenerate.
	* configure: Likewise.


diff --git a/gdb/gdbserver/configure.ac b/gdb/gdbserver/configure.ac
index e6e9162..5d1e094 100644
--- a/gdb/gdbserver/configure.ac
+++ b/gdb/gdbserver/configure.ac
@@ -168,6 +168,10 @@ AC_CHECK_TYPES(socklen_t, [], [],
 #include <sys/socket.h>
 ])
 
+AC_CHECK_TYPES([Elf32_auxv_t, Elf64_auxv_t], [], [],
+#include <elf.h>
+)
+
 ACX_PKGVERSION([GDB])
 ACX_BUGURL([http://www.gnu.org/software/gdb/bugs/])
 AC_DEFINE_UNQUOTED([PKGVERSION], ["$PKGVERSION"], [Additional package description])
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index ab87570..043451d 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -82,6 +82,36 @@
 #endif
 #endif
 
+#ifndef HAVE_ELF32_AUXV_T
+/* Copied from glibc's elf.h.  */
+typedef struct
+{
+  uint32_t a_type;		/* Entry type */
+  union
+    {
+      uint32_t a_val;		/* Integer value */
+      /* We use to have pointer elements added here.  We cannot do that,
+	 though, since it does not work when using 32-bit definitions
+	 on 64-bit platforms and vice versa.  */
+    } a_un;
+} Elf32_auxv_t;
+#endif
+
+#ifndef HAVE_ELF64_AUXV_T
+/* Copied from glibc's elf.h.  */
+typedef struct
+{
+  uint64_t a_type;		/* Entry type */
+  union
+    {
+      uint64_t a_val;		/* Integer value */
+      /* We use to have pointer elements added here.  We cannot do that,
+	 though, since it does not work when using 32-bit definitions
+	 on 64-bit platforms and vice versa.  */
+    } a_un;
+} Elf64_auxv_t;
+#endif
+
 /* ``all_threads'' is keyed by the LWP ID, which we use as the GDB protocol
    representation of the thread ID.