On 2019/03/01 16:10, Jeremie Courreges-Anglas wrote:
> On Mon, Feb 25 2019, Jeremie Courreges-Anglas <j...@wxcvbn.org> wrote:
> > +cc sthen since ports/textproc/mupdf is affected by the bug:
> >
> >
> > http://build-failures.rhaalovely.net//sparc64/2019-02-03/textproc/mupdf.log
> >
> > On Sun, Feb 24 2019, Jeremie Courreges-Anglas <j...@wxcvbn.org> wrote:
> >> On Sat, Feb 23 2019, Aaron Miller <aaron.mille...@gmail.com> wrote:
> >>> On February 23, 2019 2:50:46 AM PST, Jeremie Courreges-Anglas
> >>> <j...@wxcvbn.org> wrote:
> >>>>On Sat, May 07 2016, Stefan Kempf <sisnk...@gmail.com> wrote:
> >>
> >> [...]
> >>
> >>> Hi Jeremie,
> >>>
> >>> That is concerning. I'm on my phone and haven't had a chance to
> >>> investigate, but from the code in the gdb output above, it looks like the
> >>> author of the diff forgot to set the pointer to NULL after freeing. For
> >>> example:
> >>> if (elf_tdata (sub)->symbuf) {
> >>> free (elf_tdata (sub)->symbuf);
> >>> elf_tdata (sub)->symbuf = NULL;
> >>> }
> >>>
> >>> This is not tested at all. I hope this works!
> >>
> >> It doesn't, which is consistent with the error seen with
> >> MALLOC_OPTIONS=S: "free (ptr=0xdbdbdbdbdbdbdbdb)" points out that the
> >> code uses uninitialized memory (0xdb). The 0xdf pattern in the sparc64
> >> build failure is likely newly allocated, uninitialized memory which
> >> had previously been junked by free(3).
> >
> > The following diff fixes the issue here. The "bfd_get_flavour (sub) ==
> > bfd_target_elf_flavour" check is the important part, it mirrors the
> > checks done in bfd_elf_match_symbols_in_sections().
> >
> > The "symbuf = NULL" part is not needed to avoid the crash, but if it can
> > avoid someone another dive in this codebase, I think it's worth it. ;)
> >
> > Quick way to test the diff:
> >
> > MALLOC_OPTIONS=S ld.bfd -r -b binary ~/.profile -o /tmp/garbage
> >
> > ok?
>
> Still looking for feedback. textproc/mupdf being broken knocks out cups
> and its consumers in bulk builds, this is not nice. I'll commit this on
> sunday unless I hear objections.
I'm not exactly the best person to comment on binutils diffs(!) but
as far as I'm concerned it's OK with me.
> > Index: bfd/elflink.c
> > ===================================================================
> > RCS file: /cvs/src/gnu/usr.bin/binutils-2.17/bfd/elflink.c,v
> > retrieving revision 1.22
> > diff -u -p -r1.22 elflink.c
> > --- bfd/elflink.c 3 Dec 2018 02:59:51 -0000 1.22
> > +++ bfd/elflink.c 25 Feb 2019 09:49:05 -0000
> > @@ -8619,8 +8619,13 @@ bfd_elf_final_link (bfd *abfd, struct bf
> > if (!info->reduce_memory_overheads)
> > {
> > for (sub = info->input_bfds; sub != NULL; sub = sub->link_next)
> > - if (elf_tdata (sub)->symbuf)
> > - free (elf_tdata (sub)->symbuf);
> > + {
> > + if (bfd_get_flavour (sub) == bfd_target_elf_flavour)
> > + {
> > + free (elf_tdata (sub)->symbuf);
> > + elf_tdata (sub)->symbuf = NULL;
> > + }
> > + }
> > }
> >
> > /* Output any global symbols that got converted to local in a
>
> --
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
