On Thu, Mar 15, 2018 at 03:55:25PM -0700, William Ahern wrote:
> On Thu, Mar 15, 2018 at 05:23:24PM +0100, Patrick Wildt wrote:
> > Hi,
> >
> > LLVM 6.0.0 does now complain of code does computation on NULL pointers,
> > which apparently binutils makes use of. I think we can teach binutils
> > to stop doing that.
> >
> > Is my C foo correct? Feedback?
>
> Both (type *)0 - 1 and (type *)-1 rely on undefined behavior, but
> _different_ undefined behavior. Even presuming the former was reliable, is
> the latter?
>
> FWIW, the two expressions also evaluate to different values,
> 0xfffffffffffffffc vs 0xffffffffffffffff. That's not a problem by itself but
> suggests possible signedness (or whatever you call the analogous issue for
> pointer arithmetic) pitfalls.
You're right. It does have a different value. On the other hand, the
code is only used in three places. One place checks for NULL, the other
two places are part of the diff. And the comment above _bfd_elf_archive
_symbol_lookup actually says:
/* Return the linker hash table entry of a symbol that might be
satisfied by an archive symbol. Return -1 on error. */
> Maybe it's safer to use the address of abfd, which should be shared even if
> archive_symbol_lookup is a function pointer to an external instantiation? I
> originally thought to use a static global singleton, but the macros and
> function pointers made me doubt the caller and callee are always from the
> same object. Using abfd is simpler and I don't see where abfd might be
> wrapped or proxied.
I don't understand what you're saying, I don't know the binutils code
well enough for that.
> > Patrick
> >
> > diff --git a/gnu/usr.bin/binutils-2.17/bfd/elflink.c
> > b/gnu/usr.bin/binutils-2.17/bfd/elflink.c
> > index a6d17dcb4d9..aa010f9d0b2 100644
> > --- a/gnu/usr.bin/binutils-2.17/bfd/elflink.c
> > +++ b/gnu/usr.bin/binutils-2.17/bfd/elflink.c
> > @@ -4517,7 +4517,7 @@ _bfd_elf_archive_symbol_lookup (bfd *abfd,
> > len = strlen (name);
> > copy = bfd_alloc (abfd, len);
> > if (copy == NULL)
> > - return (struct elf_link_hash_entry *) 0 - 1;
> > + return (struct elf_link_hash_entry *)-1;
> >
> > first = p - name + 1;
> > memcpy (copy, name, first);
> > @@ -4629,7 +4629,7 @@ elf_link_add_archive_symbols (bfd *abfd, struct
> > bfd_link_info *info)
> > }
> >
> > h = archive_symbol_lookup (abfd, info, symdef->name);
> > - if (h == (struct elf_link_hash_entry *) 0 - 1)
> > + if (h == (struct elf_link_hash_entry *)-1)
> > goto error_return;
> >
> > if (h == NULL)
> > diff --git a/gnu/usr.bin/binutils-2.17/include/obstack.h
> > b/gnu/usr.bin/binutils-2.17/include/obstack.h
> > index 88c2a264adc..8839c48e95f 100644
> > --- a/gnu/usr.bin/binutils-2.17/include/obstack.h
> > +++ b/gnu/usr.bin/binutils-2.17/include/obstack.h
> > @@ -123,7 +123,7 @@ extern "C" {
> > #endif
> >
> > #ifndef __INT_TO_PTR
> > -# define __INT_TO_PTR(P) ((P) + (char *) 0)
> > +# define __INT_TO_PTR(P) ((char *)(P))
> > #endif
> >
> > /* We need the type of the resulting object. If __PTRDIFF_TYPE__ is
