Re: PATCH: Add --compress-debug-sections=[none|zlib|zlib-gnu|zlib-gabi] to ld

[Alan Modra <amodra at gmail dot com>]

On Mon, Apr 13, 2015 at 05:38:24AM -0700, H.J. Lu wrote:
> On Sun, Apr 12, 2015 at 6:13 PM, Alan Modra <amodra@gmail.com> wrote:
> > On Thu, Apr 09, 2015 at 03:07:20PM -0700, H.J. Lu wrote:
> >> +  if (arg->link_info
> >> +      && (arg->link_info->compress_debug & COMPRESS_DEBUG)
> >> +      && arg->link_info->compress_debug != COMPRESS_DEBUG_GABI_ZLIB
> >> +      && (asect->flags & SEC_DEBUGGING)
> >> +      && name[1] == 'd'
> >> +      && name[6] == '_')
> >
> > Here and in a couple of other places you omit checking the full prefix
> > of the name.  Are you certain you won't see something like a ".de"
> > section here?  It you might, then name[6] is a buffer overflow and a
> > potential segfault.
> 
> elf.c has
> 
>   if ((flags & SEC_ALLOC) == 0)
>     {
>       /* The debugging sections appear to be recognized only by name,
>          not any sort of flag.  Their SEC_ALLOC bits are cleared.  */

OK, good.

> Here is the completed patch.  OK for master?

Would you mind looking at doing this a little cleaner?  The patch as
it stands looks very much like a bolt-on.  I mean, you probably
wouldn't write the code this way if elflink.c had compression in mind
in the beginning..

So, I'm wondering whether things would look better if
elf_link_input_bfd allocated the extra buffer needed rather than in
both _bfd_write_merged_section and _bfd_elf_set_section_contents.
Of course, _bfd_write_merged_section would need two extra parameters,
contents and offset, and write to "contents" if non-NULL rather than
to the output file.

-- 
Alan Modra
Australia Development Lab, IBM