PATCH: PR ld/5303: splay-tree doesn't support 64bit value on 32bit host
Doug Kwan (關振德)
dougkwan@google.com
Tue Dec 4 19:40:00 GMT 2007
Hi HJ,
I don't have right to check into the binutils source tree. So my
comments cannot be counted as a review :).
There are other ways to fix this without affecting gcc. This is a
speed optimization. The easiest way is to turn this off when spray
tree nodes cannot store 64-bit target addresses. Currently there is
only one use of the arange set data structure inside lib BFD and there
is logic to determine when this is turned on. We can simply change
that.
Another way to fix this is always allocate a structure to store
target VMA and have the spray tree storing pointers to structures
instead of storing VMA directly. That uses a bit more memory and
cycles. I can do the changes if we think this is a better approach.
-Doug
2007/12/4, H.J. Lu <hjl@lucon.org>:
> Hi Doug,
>
> Thanks for your comments.
>
> Although this bug doesn't affect gcc since gcc doesn't store 64bit
> values in splay-tree on 32bit hosts, splay-tree is shared by both gcc
> and binutils. Could someone from gcc review my patches to gcc and
> splay-tree? Mark, DJ, can you look into them?
>
> Thanks.
>
>
> H.J.
> ---
> On Mon, Dec 03, 2007 at 07:39:38AM -0800, Doug Kwan (關振德) wrote:
> > Hi H.J.
> >
> > Thanks for fixing this. I was on vacation in the last 3 weeks and
> > out of the country. I only returned to work to day. My apology for not
> > having tested this on the 32-bit host/64-bit target in the first
> > place.
> >
> > -Doug
> >
> > 2007/11/14, H.J. Lu <hjl@lucon.org>:
> > > On Tue, Nov 13, 2007 at 03:32:38PM -0800, H.J. Lu wrote:
> > > > On Wed, Nov 14, 2007 at 09:05:08AM +1030, Alan Modra wrote:
> > > > > On Tue, Nov 13, 2007 at 09:18:45AM -0800, H.J. Lu wrote:
> > > > > > Current linker is broken on 32bit host with 64bit BFD. The problem
> > > > > > is splay-tree doesn't support 64bit value on bit host, but arange-set.c
> > > > > > uses splay-tree on bfd_vma. Fixing it isn't easy since splay-tree is
> > > > > > also used in gcc. We can't use #ifdef in splay-tree.h in gcc due
> > > > > > to lack of #ifdef in gengtype in gcc. This patch will abort the
> > > > > > linker if it detects such situation.
> > > > >
> > > > > Aborting isn't a solution. I'll revert Doug's patch in a week or so
> > > > > if someone can't find a proper solution, eg. fallback to old scheme
> > > > > if splay tree support is inadeqaute.
> > > > >
> > > >
> > > > Here are 2 patches to update splay tree. The idea is to provide both
> > > > long and long long interfaces for splay-tree if they are different.
> > > > The default interface is the smallest one which can hold a pointer.
> > > > Gcc support is tricky since gengtype doesn't support #ifdef. I
> > > > have to use splay-tree-default.h and let gcc override it. I will
> > > > post a patch for arange-set.c if these 2 patches are approved.
> > > >
> > > > Thanks.
> > > >
> > > >
> > >
> > > Here is the patch for bfd.
> > >
> > >
> > > H.J.
> > > ----
> > > 2007-11-14 H.J. Lu <hongjiu.lu@intel.com>
> > >
> > > PR ld/5303
> > > * arange-set.c (USE_SPLAY_TREE_LONG_LONG): Define if BFD64 is
> > > defined.
> > > Don't include "splay-tree.h".
> > > (arange_set_delete_value_container): Add cast to
> > > arange_set_uhostptr_t.
> > > (arange_set_node_high): Likewise.
> > > (arange_set_node_set_high): Likewise.
> > > (arange_set_node_value): Likewise.
> > > (arange_set_node_set_value): Likewise.
> > > (arange_set_splay_tree_insert): Likewise.
> > >
> > > * arange-set.h: Include "splay-tree.h".
> > > (arange_set_uhostptr_t): Defined as splay_tree_uhostptr_t.
> > >
> > > * configure.in: Check if long long exists.
> > > * config.in: Regenerated.
> > > * configure: Likewise.
> > >
> > > --- bfd/arange-set.c.vma 2007-09-21 09:16:17.000000000 -0700
> > > +++ bfd/arange-set.c 2007-11-13 06:46:18.000000000 -0800
> > > @@ -23,8 +23,12 @@
> > > #include "bfd.h"
> > > #include "libiberty.h"
> > > #include "libbfd.h"
> > > +
> > > +#ifdef BFD64
> > > +#define USE_SPLAY_TREE_LONG_LONG
> > > +#endif
> > > +
> > > #include "arange-set.h"
> > > -#include "splay-tree.h"
> > >
> > > /* Implementation of an arange-set. The set is implemented using the
> > > splay tree support in libiberty. The advantage of using this is
> > > @@ -142,7 +146,8 @@ arange_set_delete_value_container (splay
> > > {
> > > arange_value_container_t *container;
> > >
> > > - container = (arange_value_container_t*) value;
> > > + container
> > > + = (arange_value_container_t*) (arange_set_uhostptr_t)value;
> > > arange_set_delete_value (container->set, container->value);
> > > arange_set_deallocate (container->set, container);
> > > }
> > > @@ -255,7 +260,8 @@ arange_set_node_high (arange_set set, sp
> > >
> > > if (set->value_p)
> > > {
> > > - container = (arange_value_container_t*) node->value;
> > > + container
> > > + = (arange_value_container_t*) (arange_set_uhostptr_t)node->value;
> > > return container->high;
> > > }
> > >
> > > @@ -271,7 +277,8 @@ arange_set_node_set_high (arange_set set
> > >
> > > if (set->value_p)
> > > {
> > > - container = (arange_value_container_t*) node->value;
> > > + container
> > > + = (arange_value_container_t*) (arange_set_uhostptr_t)node->value;
> > > container->high = address;
> > > }
> > > else
> > > @@ -296,7 +303,8 @@ arange_set_node_value (arange_set set, s
> > >
> > > if (set->value_p)
> > > {
> > > - container = (arange_value_container_t*) node->value;
> > > + container
> > > + = (arange_value_container_t*) (arange_set_uhostptr_t)node->value;
> > > return container->value;
> > > }
> > >
> > > @@ -315,7 +323,8 @@ arange_set_node_set_value (arange_set se
> > >
> > > if (set->value_p)
> > > {
> > > - container = (arange_value_container_t*) node->value;
> > > + container
> > > + = (arange_value_container_t*) (arange_set_uhostptr_t)node->value;
> > > container->value = value;
> > > }
> > > }
> > > @@ -445,7 +454,7 @@ arange_set_splay_tree_insert (arange_set
> > > container->set = set;
> > >
> > > container->value = value;
> > > - sp_value = (splay_tree_value) container;
> > > + sp_value = (splay_tree_value) (arange_set_uhostptr_t)container;
> > > }
> > > else
> > > sp_value = (splay_tree_value) high;
> > > --- bfd/arange-set.h.vma 2007-09-21 09:16:17.000000000 -0700
> > > +++ bfd/arange-set.h 2007-11-13 06:41:48.000000000 -0800
> > > @@ -75,16 +75,13 @@
> > >
> > > #include "sysdep.h"
> > > #include "bfd.h"
> > > +#include "splay-tree.h"
> > >
> > > /* An arange_set is a pointer to an arange_set_s struct, whose implementation
> > > is opaque to clients using the arange set. */
> > > typedef struct arange_set_s *arange_set;
> > >
> > > -#ifndef _WIN64
> > > - typedef unsigned long int arange_set_uhostptr_t;
> > > -#else
> > > - typedef unsigned long long arange_set_uhostptr_t;
> > > -#endif
> > > +typedef splay_tree_uhostptr_t arange_set_uhostptr_t;
> > >
> > > /* Type of value attached to an arange. This should be wide enough to be
> > > converted from and back to any type without loss. */
> > > --- bfd/config.in.vma 2007-10-30 11:48:31.000000000 -0700
> > > +++ bfd/config.in 2007-11-13 06:31:39.000000000 -0800
> > > @@ -105,6 +105,9 @@
> > > /* Define to 1 if you have the <inttypes.h> header file. */
> > > #undef HAVE_INTTYPES_H
> > >
> > > +/* Define to 1 if the system has the type `long long'. */
> > > +#undef HAVE_LONG_LONG
> > > +
> > > /* Define if <sys/procfs.h> has lwpstatus_t. */
> > > #undef HAVE_LWPSTATUS_T
> > >
> > > --- bfd/configure.in.vma 2007-10-30 11:48:31.000000000 -0700
> > > +++ bfd/configure.in 2007-11-13 06:31:22.000000000 -0800
> > > @@ -135,7 +135,7 @@ BFD_HOST_64_BIT=
> > > BFD_HOST_U_64_BIT=
> > > BFD_HOSTPTR_T="unsigned long"
> > >
> > > -AC_CHECK_SIZEOF(long long)
> > > +AC_CHECK_TYPES([long long], [AC_CHECK_SIZEOF(long long)])
> > > AC_CHECK_SIZEOF(void *)
> > > AC_CHECK_SIZEOF(long)
> > >
> > >
>
More information about the Binutils
mailing list