Re: PATCH: Support SHF_GNU_COMPRESSED

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Wed, Jul 21, 2010 at 7:21 AM, Alan Modra <amodra@gmail.com> wrote:
> On Tue, Jul 20, 2010 at 04:38:56PM -0700, H.J. Lu wrote:
>> SHF_GNU_COMPRESSED isn't strictly required. I can peek into
>> .debug_* sections like gdb does. On the other hand, SHF_GNU_COMPRESSED
>> can avoid the peek. I can prepare a patch without SHF_GNU_COMPRESSED.
>> Any comments on that?
>
> A linker that doesn't know about compressed sections can't possibly
> apply relocations for them correctly, so I think we should be setting
> SHF_OS_NONCONFORMING on these sections. ?I didn't have a preference
> for SHF_GNU_COMPRESSED or SHT_GNU_COMPRESSED until I noticed that GNU
> ld doesn't emit a diagnostic for SHF_OS_NONCONFORMING with unknown
> flags (which is a bug), but does do so for unknown types. ?So I favour
> SHT_GNU_COMPRESSED. ?That way an older GNU ld will sensibly give up if
> presented a new compressed debug object file.

I will keep .zdebug sections and won't use SHT_GNU_COMPRESSED.

> Now, some comments on the patch.
>
>> ? ? ? * bfd.c (BFD_COMPRESS): New.
>> ? ? ? (BFD_DECOMPRESS): Likewise.
>
> Is there a reason why we shouldn't always decompress? ?I think that
> would make handling these sections so much easier. ?Oh, I see you are
> making objdump -s dump out the compressed contents. ?I wonder if it
> wouldn't be more useful to dump out the contents after decompression
> anyway?

For size, ar, nm, string and some objdump/objcopy, we shouldn't
decompress.

>
>> + ?unsigned int compress_status : 2;
>
> If you always decompress, then you probably don't need this.

That is true. There are 3 advantages with the current scheme:

1. We can avoid decompress if the uncompressed contents
are never used.
2. We can decompress directly into user provided buffer
directly without decompress and copy.
3. I use compress_status in _bfd_generic_get_section_contents
to catch usages of bfd_get_section_contents which should
be replaced by bfd_get_full_section_contents. I have found it
is very useful.

>> + ?/* The compressed size of the section in octets. ?*/
>> + ?bfd_size_type compressed_size;
>
> Seems to me this is unnecessary. ?For an input section, "rawsize"
> should be the on-disk size, with "size" that after decompression.
>

I will give it a try.

-- 
H.J.