[patch] Fix jit-reader.h for multilib

[patch] Fix jit-reader.h for multilib

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 641 bytes --]

Hi,

I got reported jit-reader.h is not multi-lib safe:

--- /usr/include/gdb/jit-reader.h on x86_64	2015-01-07 11:54:27.705802129 -0500
+++ /usr/include/gdb/jit-reader.h on i686	2015-01-07 11:54:46.853774165 -0500
@@ -56,7 +56,7 @@ extern "C" {
 
 /* Represents an address on the target system.  */
 
-typedef unsigned long GDB_CORE_ADDR;
+typedef unsigned long long GDB_CORE_ADDR;
 
 /* Return status codes.  */
 

multi-lib safety means that files with the same pathname (which is
/usr/include, contrary to /usr/lib vs. /usr/lib64 for example).

The patch will make it on x86_64 also 'unsigned long long'.

OK for check-in?


Thanks,
Jan

[-- Attachment #2: multilib.patch --]
[-- Type: text/plain, Size: 969 bytes --]

gdb/ChangeLog
2015-01-07  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* configure: Regenerate.
	* configure.ac (TARGET_PTR): Try "unsigned long long" first.

diff --git a/gdb/configure.ac b/gdb/configure.ac
index ec776d7..c02ace9 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -648,10 +648,12 @@ AC_CHECK_SIZEOF(unsigned long long)
 AC_CHECK_SIZEOF(unsigned long)
 AC_CHECK_SIZEOF(unsigned __int128)
 
-if test "x${ac_cv_sizeof_unsigned_long}" = "x8"; then
-  TARGET_PTR="unsigned long"
-elif test "x${ac_cv_sizeof_unsigned_long_long}" = "x8"; then
+# Try to keep TARGET_PTR the same across archs so that jit-reader.h file
+# content is the same for multilib distributions.
+if test "x${ac_cv_sizeof_unsigned_long_long}" = "x8"; then
   TARGET_PTR="unsigned long long"
+elif test "x${ac_cv_sizeof_unsigned_long}" = "x8"; then
+  TARGET_PTR="unsigned long"
 elif test "x${ac_cv_sizeof_unsigned___int128}" = "x16"; then
   TARGET_PTR="unsigned __int128"
 else

Re: [patch] Fix jit-reader.h for multilib

[Yao Qi]

Jan Kratochvil <jan.kratochvil@redhat.com> writes:

> I got reported jit-reader.h is not multi-lib safe:
>
> --- /usr/include/gdb/jit-reader.h on x86_64	2015-01-07 11:54:27.705802129 -0500
> +++ /usr/include/gdb/jit-reader.h on i686	2015-01-07 11:54:46.853774165 -0500
> @@ -56,7 +56,7 @@ extern "C" {
>  
>  /* Represents an address on the target system.  */
>  
> -typedef unsigned long GDB_CORE_ADDR;
> +typedef unsigned long long GDB_CORE_ADDR;
>  
>  /* Return status codes.  */
>  
>
> multi-lib safety means that files with the same pathname (which is
> /usr/include, contrary to /usr/lib vs. /usr/lib64 for example).

Hi Jan,
I am sorry I can't understand what you mean here, can you elaborate?
Do you mean sizeof (GDB_CORE_ADDR) should be the same on different
multi-lib (i686 vs x86_64)?

-- 
Yao (齐尧)

Re: [patch] Fix jit-reader.h for multilib

[Jan Kratochvil]

On Sun, 11 Jan 2015 13:02:55 +0100, Yao Qi wrote:
> Jan Kratochvil <jan.kratochvil@redhat.com> writes:
> 
> > I got reported jit-reader.h is not multi-lib safe:
> >
> > --- /usr/include/gdb/jit-reader.h on x86_64	2015-01-07 11:54:27.705802129 -0500
> > +++ /usr/include/gdb/jit-reader.h on i686	2015-01-07 11:54:46.853774165 -0500
> > @@ -56,7 +56,7 @@ extern "C" {
> >  
> >  /* Represents an address on the target system.  */
> >  
> > -typedef unsigned long GDB_CORE_ADDR;
> > +typedef unsigned long long GDB_CORE_ADDR;
> >  
> >  /* Return status codes.  */
> >  
> >
> > multi-lib safety means that files with the same pathname (which is
> > /usr/include, contrary to /usr/lib vs. /usr/lib64 for example).
> 
> I am sorry I can't understand what you mean here, can you elaborate?

Sorry I see I haven't finished the sentence, it should have been:

# multi-lib safety means that files with the same pathname (which is
# /usr/include, contrary to /usr/lib vs. /usr/lib64 for example)
# contain exactly the same content.

See also:
https://fedoraproject.org/wiki/PackagingDrafts/MultilibTricks#File_conflicts


When we discuss it personally I do not think multilib should be applied to the
GDB package as there is nothing like /usr/lib{,64}/libgdb.so.  There is only
/usr/bin/gdb and that has always just one arch on one system.  So installing
gdb.i686 and gdb.x86_64 simultaneously does not have any benefits / makes
sense.  But despites this I need it for Red Hat packaging so I am fine to also
keep it just as a downstream patch.  OTOH I guess other OS packagers may also
face it so it may be easier for everyone to do it upstream.


> Do you mean sizeof (GDB_CORE_ADDR) should be the same on different
> multi-lib (i686 vs x86_64)?

For multilib it does not matter what is the runtime GDB_CORE_ADDR type but the
text defining it (in /usr/include/gdb/jit-reader.h) has to be the same for
archs sharing multilib.

IIUC GDB requires for GDB_CORE_ADDR:
 * sizeof (GDB_CORE_ADDR) >= sizeof (CORE_ADDR)
 * sizeof (GDB_CORE_ADDR) is preferrably the smallest available size


Jan

Re: [patch] Fix jit-reader.h for multilib

[Yao Qi]

Jan Kratochvil <jan.kratochvil@redhat.com> writes:

> # multi-lib safety means that files with the same pathname (which is
> # /usr/include, contrary to /usr/lib vs. /usr/lib64 for example)
> # contain exactly the same content.
>

Thanks, that is clear to me now.

> See also:
> https://fedoraproject.org/wiki/PackagingDrafts/MultilibTricks#File_conflicts
>
>
> When we discuss it personally I do not think multilib should be applied to the
> GDB package as there is nothing like /usr/lib{,64}/libgdb.so.  There is only
> /usr/bin/gdb and that has always just one arch on one system.  So installing
> gdb.i686 and gdb.x86_64 simultaneously does not have any benefits / makes
> sense.  But despites this I need it for Red Hat packaging so I am fine to also
> keep it just as a downstream patch.  OTOH I guess other OS packagers may also
> face it so it may be easier for everyone to do it upstream.

Is multi-lib safety a Fedora/Red Hat specific packaging rule? or do
other distributions need this too?  If answer is yes, I don't object to
your patch as it adjusts the order of looking for desired types.

-- 
Yao (齐尧)

Re: [patch] Fix jit-reader.h for multilib

[Jan Kratochvil]

On Mon, 12 Jan 2015 03:33:37 +0100, Yao Qi wrote:
> Jan Kratochvil <jan.kratochvil@redhat.com> writes:
> > When we discuss it personally I do not think multilib should be applied to the
> > GDB package as there is nothing like /usr/lib{,64}/libgdb.so.  There is only
> > /usr/bin/gdb and that has always just one arch on one system.  So installing
> > gdb.i686 and gdb.x86_64 simultaneously does not have any benefits / makes
> > sense.  But despites this I need it for Red Hat packaging so I am fine to also
> > keep it just as a downstream patch.  OTOH I guess other OS packagers may also
> > face it so it may be easier for everyone to do it upstream.
> 
> Is multi-lib safety a Fedora/Red Hat specific packaging rule? or do
> other distributions need this too?

I do not know how other distributions do that. Debian 'gdb' does not seem to
ship jit-reader.h at all if I read Debian package listing correctly.

For other libraries Debian ships the file like Fedora does:

https://packages.debian.org/sid/amd64/libgpm-dev/filelist
	/usr/include/gpm.h
	/usr/lib/x86_64-linux-gnu/libgpm.so
https://packages.debian.org/sid/i386/libgpm-dev/filelist
	/usr/include/gpm.h
	/usr/lib/i386-linux-gnu/libgpm.so

What would happen if /usr/include/gpm.h content differs between amd64 and i386
build?  Fedora would refuse to install the second package due to conflicting
file content (sure unless forced to do so).

As I said this patch may not affect much/any other distros because
 * at least Debian does not ship jit-reader.h at all
 * other distros may not consider GDB as a multilib package


Jan

Re: [patch] Fix jit-reader.h for multilib

[Joel Brobecker]

> As I said this patch may not affect much/any other distros because
>  * at least Debian does not ship jit-reader.h at all
>  * other distros may not consider GDB as a multilib package

I don't think the problem would be specific to Fedora - it is
possible that other distros might benefit from the fix as well.
The only thing that gave me pause was the relatively strange
fix, and the fact that I didn't really understand the issue,
and therefore didn't understand the fix. I don't know much
about jit-reader.h, and the impact of the fix, but I have
a feeling that it would beneficial to others and therefore should
be considered for inclusion.

-- 
Joel

ping: [patch] Fix jit-reader.h for multilib

[Jan Kratochvil]

Message-ID: <87iogcsp6m.fsf@codesourcery.com>
On Mon, 12 Jan 2015 03:33:37 +0100, Yao Qi wrote:
> or do
> other distributions need this too?  If answer is yes, I don't object to
> your patch as it adjusts the order of looking for desired types.

Message-ID: <20150112095711.GZ5445@adacore.com>
On Mon, 12 Jan 2015 10:57:11 +0100, Joel Brobecker wrote:
> I don't think the problem would be specific to Fedora - it is
> possible that other distros might benefit from the fix as well.
[...]
> but I have a feeling that it would beneficial to others and therefore should
> be considered for inclusion.

These mails look to me as if maybe the patch has been double-approved but I am
not sure if it was really approved or not.  Was it?


Thanks,
Jan