Re: [patch] Add --compress-debug-sections option to gas

[Cary Coutant <ccoutant at google dot com>]

>> + ? ? ?/* Reserve all the space we can. ?If none is available, force
>> + ? ? ?the obstack to grow by at least as many bytes as we have
>> + ? ? ?remaining in the input stream. ?*/
>> + ? ? ?room = obstack_room (ob);
>> + ? ? ?if (room <= 0)
>> + ? ? room = strm->avail_in;
>> + ? ? ?obstack_blank (ob, room);
> obstack_blank_fast. ?You don't really need to zero the mem do you?

In the (room <= 0) case, I needed to use obstack_blank so that it
would reallocate the current obstack object. I wasn't happy with that
approach anyway, since it didn't make effective use of the obstack, so
I've rewritten the patch to fill each new compressed frag until
there's no room left in the current obstack chunk, at which point it
will create another frag (instead of creating one new frag for each
original frag). With this rewrite, the isolation of the zlib interface
is also cleaner.

>> + ?/* Create a new frag to contain the "ZLIB" header. ?*/
>> + ?obstack_blank (ob, SIZEOF_STRUCT_FRAG + 12);
>
> You should use frag_alloc here and later where you create frags

I couldn't use frag_alloc because I was relying on obstack_blank
reallocating as necessary, and I needed the frag structure to get
reallocated with the data that followed. With this new patch, I can
now use frag_alloc as you suggest.

Since this new patch changes a bit more than you asked for, I'll wait
for a new approval.

(Side question: I'm puzzled by the alignment stuff in frag_alloc() --
it seems to be setting the alignment mask to 0 before calling
obstack_alloc(). Doesn't that mean that the frag structure will be
allocated on an arbitrary byte boundary?)

(Also: in relax_frag(), I found the following code:

      newf = frag_alloc (ob);
      obstack_blank_fast (ob, fragP->fr_var);
      obstack_finish (ob);

But I couldn't find any code that would guarantee that there was
enough room for the obstack_blank_fast call to succeed. Am I missing
something, or is there a potential bug there?)

Thanks so much for the quick and helpful reviews!

-cary


bfd/ChangeLog:

        * compress.c (bfd_uncompress_section_contents): Add ATTRIBUTE_UNUSED.
        * dwarf2.c (read_and_uncompress_section): New function.
        (read_section): Call it.
        (find_line): Likewise.

binutils/ChangeLog:

        * objdump.c (load_specific_debug_section): Decompress section contents
        before applying relocations.
        * readelf.c (load_specific_debug_section): Update section size after
        decompression.

gas/ChangeLog:

        * Makefile.am: Add compress-debug.c and compress-debug.h.
        * Makefile.in: Regenerate.
        * config.in: Add HAVE_ZLIB_H.
        * configure.in: Check for zlib.h.
        * configure: Regenerate.

        * as.c (parse_args): Add --compress-debug-sections and
        --nocompress-debug-sections.
        * as.h (flag_compress_debug): New variable.
        * compress-debug.c: New file.
        * compress-debug.h: New file.
        * write.c: Include compress-debug.h.
        (compress_frag): New function.
        (compress_debug): New function.
        (write_object_file): Compress debug sections if requested.

Attachment: compress-debug-patch-3.txt
Description: Text document