[patch] compress debug sections

Eric Christopher echristo at gmail.com
Mon Mar 24 14:51:22 PDT 2014


On Wed, Mar 19, 2014 at 3:34 PM, David Blaikie <dblaikie at gmail.com> wrote:
> So I've poked around with trying to implement
> -Wa,-compress-debug-sections and come up with the following patch.
>
> (there's some Clang changes too, to initialize the MCContext flag -
> but for now if you want to experiment with this you can just change
> the ctor initialization of that flag to force compression on always)
>

Awesome.

> I have a bunch of uncertainties about this approach
>
> * compressing on a per-fragment basis. This 'works', since we never
> create more than one fragment for each debug info section (so far as I
> know) - but I don't understand the fragments well enough to know if
> this is guaranteed. If this isn't guaranteed, then we have to move the
> compression up into somewhere like ELFObjectWriter::WriteObject (since
> we'll have to do a cross-fragment computation of size and emission)
>

This should be ok. As we (and Jim) discussed, we should only create a
new fragment for things that could be resized - and given that we run
computeSizeAndOffsets for debug info I'm reasonably confident that we
should never resize parts of the debug info section. Adding a couple
of asserts might be good though.

> * Is there a nicer way to detect the sections that should be
> compressed (than just check for a .debug_ prefix?)
>

Nothing comes to mind. I worry about using ELF attributes for this (I
know that's not what you were thinking but just going there) since
there aren't any that aren't particularly specific here that we could
depend upon without creating a new one.

> * A nicer way to detect that we're in a compressed section so we
> create a compressed fragment
>

Not sure what you mean here.

> * Caching the compressed data - should that cache be invalidated or
> regenerated if the underlying data ever changes? Or can we rely on the
> size/contents never changing after its queried for during object
> emission?

I think we're ok here, it probably shouldn't be changed. That said,
again with the asserts or just discard and regenerate.

In general I think the patch seems like a reasonable first patch that
can be improved upon, I don't have any particular concerns about it.
Some comments and description in the source could be nice. Testcases
:)

-eric



More information about the llvm-commits mailing list