[1/3] bfd/PE: fix debug section compression for pei-*

  bfd supports compressing debug sections for PE/Coff since a29a8af823af
("ChangeLog bfd"). It works for pe, but never worked for pei in a full
round trip. For example:

 $ x86_64-w64-mingw32-gcc -g -o testprog testprog.c
 $ objcopy --compress-debug-sections testprog.exe testprog_z.exe
 $ gdb --batch -ex "break fn" -ex "info line fn" testprog_z.exe
 warning: Can't read data for section '.zdebug_info' in file '/.../testprog_z.exe'
 [...]
 No line number information available for address 0x140001490 <fn>

PE objects use SizeOfRawData to store the compressed size. PE images
can't, because SizeOfRawData must be file aligned per specification. PE
images could instead fall back to VirtualSize, but this was not
implemented correctly.

When BFD opened a compressed file for decompression,
peicode.h:coff_swap_scnhdr_in set the lesser of VirtualSize and
SizeOfRawData as s_size, which was later used for decompression
verification. This was wrong in any case. VirtualSize held the
uncompressed size and SizeOfRawData the padded size. Instead we need the
*compressed* size to let zlib verification conclude it successfully
consumed all bytes.

Fix it by storing the compressed size rather than the uncompressed size
in VirtualSize. As an important detail, defer osec->virt_size assignment
to coff_compute_section_file_positions when a size change due to
(de)compression is expected. Otherwise, a successful decompression would
not update from compressed size to decompressed size.

Signed-off-by: Tobias Deiminger <tobias.deiminger@linutronix.de>
---
 bfd/peXXigen.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
  

On 28.09.2026 11:01, Tobias Deiminger wrote:
> bfd supports compressing debug sections for PE/Coff since a29a8af823af
> ("ChangeLog bfd"). It works for pe, but never worked for pei in a full
> round trip. For example:
> 
>  $ x86_64-w64-mingw32-gcc -g -o testprog testprog.c
>  $ objcopy --compress-debug-sections testprog.exe testprog_z.exe
>  $ gdb --batch -ex "break fn" -ex "info line fn" testprog_z.exe
>  warning: Can't read data for section '.zdebug_info' in file '/.../testprog_z.exe'
>  [...]
>  No line number information available for address 0x140001490 <fn>
> 
> PE objects use SizeOfRawData to store the compressed size. PE images
> can't, because SizeOfRawData must be file aligned per specification. PE
> images could instead fall back to VirtualSize, but this was not
> implemented correctly.

As you say "could" - there must have been agreement on what needs
recording in the image, before anything was committed? Is "could"
perhaps meant to be something like "are supposed to"?

> When BFD opened a compressed file for decompression,
> peicode.h:coff_swap_scnhdr_in set the lesser of VirtualSize and
> SizeOfRawData as s_size, which was later used for decompression
> verification. This was wrong in any case.

I'm struggling with the description. That's only in part because it's
written in past tense (as if it would describe behavior in the past,
not current behavior). In the last sentence above, what is "This"? It
could be the use for decompression verification, or it could be the
using of the lesser of the two sizes (or it could be both).

> VirtualSize held the
> uncompressed size and SizeOfRawData the padded size. Instead we need the
> *compressed* size to let zlib verification conclude it successfully
> consumed all bytes.

Yet then VirtualSize is what is necessary to determine how much VA
space to allocate for a section. While debug info commonly isn't
loaded into memory when an image is loaded, there's nothing excluding
such a scenario. Similarly, which non-debug-info sections commonly
aren't compressed, it's not quite clear to me what would prevent
someone from doing so.

Overall, ...

> Fix it by storing the compressed size rather than the uncompressed size
> in VirtualSize. As an important detail, defer osec->virt_size assignment
> to coff_compute_section_file_positions when a size change due to
> (de)compression is expected. Otherwise, a successful decompression would
> not update from compressed size to decompressed size.

... with SizeOfRawData's need to be file aligned, I'm having difficulty
seeing how section compression can be implemented correctly for PE.
(Already for COFF it's a stretch, I think.) Which isn't to say that I
wouldn't (eventually, i.e. once I understand the description) approve
this change: Better a working hack than a broken one.

Jan
  

Hi Jan,

thanks for reviewing. And sorry, I missed important things.

Am Freitag, 2. Oktober 2026, 14:20:06 Mitteleuropäische Sommerzeit schrieb Jan Beulich:
> On 28.09.2026 11:01, Tobias Deiminger wrote:
> > bfd supports compressing debug sections for PE/Coff since a29a8af823af
> > ("ChangeLog bfd"). It works for pe, but never worked for pei in a full
> > round trip.

Wrong claim. It actually worked for binutils <= 2.44 and broke only recently
with 0921b99dafd5 ("PR 33033, Support compressed debug sections larger th..").

> > 
> >  $ x86_64-w64-mingw32-gcc -g -o testprog testprog.c
> >  $ objcopy --compress-debug-sections testprog.exe testprog_z.exe
> >  $ gdb --batch -ex "break fn" -ex "info line fn" testprog_z.exe
> >  warning: Can't read data for section '.zdebug_info' in file '/.../testprog_z.exe'
> >  [...]
> >  No line number information available for address 0x140001490 <fn>
> > 
> > PE objects use SizeOfRawData to store the compressed size. PE images
> > can't, because SizeOfRawData must be file aligned per specification. PE
> > images could instead fall back to VirtualSize, but this was not
> > implemented correctly.
> 
> As you say "could" - there must have been agreement on what needs
> recording in the image, before anything was committed? Is "could"
> perhaps meant to be something like "are supposed to"?

Meanwhile I found PR 18087, where basically the same change was proposed before,
then revoked with good reasoning in one of the patches [1]:

 Updating the virtual size when the section is compressed doesn't seem
 possible, as that breaks the COFF requirement that sections much be
 contiguous

[1] https://sourceware.org/bugzilla/attachment.cgi?id=8178&

I wasn't aware of that requirement, but now read it in the PE spec an
confirm it. I.e., patch 1/3 here is invalid, alternative proposal follows.

> > When BFD opened a compressed file for decompression,
> > peicode.h:coff_swap_scnhdr_in set the lesser of VirtualSize and
> > SizeOfRawData as s_size, which was later used for decompression
> > verification. This was wrong in any case.
> 
> I'm struggling with the description. That's only in part because it's
> written in past tense (as if it would describe behavior in the past,
> not current behavior).

Yes, past tense = behavior before applying the patch, present tense = after
applying the patch. How would you prefer before / after comparison?

> In the last sentence above, what is "This"? It could be the use for
> decompression verification, or it could be the using of the lesser of the
> two sizes (or it could be both).

"This" is the lesser of the two sizes, sorry for being ambiguous.

I thought we need to communicate the compressed size. But turns we don't have
to. zlib already has end-of-stream markers, and the ZLIB header communicates
how much uncompressed bytes we expect. In binutils <= 2.44 this was considered
enough information for determining successful end of decompression. If some
trailing bytes (padding that may result from how PE sets VirtualSize) were not
processed, it was just accepted to be ok.

I think we can safely restore that behavior by adjusting the new decompression
loop from 0921b99dafd5 as follows:

diff --git a/bfd/compress.c b/bfd/compress.c
index c9710178f39..095aacc591f 100644
--- a/bfd/compress.c
+++ b/bfd/compress.c
@@ -554,3 +554,3 @@ decompress_contents (bool is_zstd, bfd_byte *compressed_buffer,
   while (compressed_size > 0 && uncompressed_size > 0);
-  return compressed_size == 0 && uncompressed_size == 0;
+  return uncompressed_size == 0;
 }

Will post this in a v2 unless there are objections.

CC-ed Rui. Rui, if there's a reason to not relax your result calculation let me
know.

Tobias

> > VirtualSize held the
> > uncompressed size and SizeOfRawData the padded size. Instead we need the
> > *compressed* size to let zlib verification conclude it successfully
> > consumed all bytes.
> 
> Yet then VirtualSize is what is necessary to determine how much VA
> space to allocate for a section. While debug info commonly isn't
> loaded into memory when an image is loaded, there's nothing excluding
> such a scenario. Similarly, which non-debug-info sections commonly
> aren't compressed, it's not quite clear to me what would prevent
> someone from doing so.
> 
> Overall, ...
> 
> > Fix it by storing the compressed size rather than the uncompressed size
> > in VirtualSize. As an important detail, defer osec->virt_size assignment
> > to coff_compute_section_file_positions when a size change due to
> > (de)compression is expected. Otherwise, a successful decompression would
> > not update from compressed size to decompressed size.
> 
> ... with SizeOfRawData's need to be file aligned, I'm having difficulty
> seeing how section compression can be implemented correctly for PE.
> (Already for COFF it's a stretch, I think.) Which isn't to say that I
> wouldn't (eventually, i.e. once I understand the description) approve
> this change: Better a working hack than a broken one.
> 
> Jan
>
  

On 06.10.2026 09:20, Tobias Deiminger wrote:
> Am Freitag, 2. Oktober 2026, 14:20:06 Mitteleuropäische Sommerzeit schrieb Jan Beulich:
>> On 28.09.2026 11:01, Tobias Deiminger wrote:
>>> When BFD opened a compressed file for decompression,
>>> peicode.h:coff_swap_scnhdr_in set the lesser of VirtualSize and
>>> SizeOfRawData as s_size, which was later used for decompression
>>> verification. This was wrong in any case.
>>
>> I'm struggling with the description. That's only in part because it's
>> written in past tense (as if it would describe behavior in the past,
>> not current behavior).
> 
> Yes, past tense = behavior before applying the patch, present tense = after
> applying the patch. How would you prefer before / after comparison?

First, specifically here: "When BFD opened ..." describes not just behavior
before your change, I suppose. More generally, a good pattern I've been
seeing people use to describe how to best write descriptions is "This is
the current behavior. This is the problem. Fix it by." I'm not going to
insist on this model (as I often don't strictly follow it myself), but I
think it gets across how tenses may want using to avoid ambiguity.

>> In the last sentence above, what is "This"? It could be the use for
>> decompression verification, or it could be the using of the lesser of the
>> two sizes (or it could be both).
> 
> "This" is the lesser of the two sizes, sorry for being ambiguous.
> 
> I thought we need to communicate the compressed size. But turns we don't have
> to. zlib already has end-of-stream markers, and the ZLIB header communicates
> how much uncompressed bytes we expect. In binutils <= 2.44 this was considered
> enough information for determining successful end of decompression. If some
> trailing bytes (padding that may result from how PE sets VirtualSize) were not
> processed, it was just accepted to be ok.
> 
> I think we can safely restore that behavior by adjusting the new decompression
> loop from 0921b99dafd5 as follows:
> 
> diff --git a/bfd/compress.c b/bfd/compress.c
> index c9710178f39..095aacc591f 100644
> --- a/bfd/compress.c
> +++ b/bfd/compress.c
> @@ -554,3 +554,3 @@ decompress_contents (bool is_zstd, bfd_byte *compressed_buffer,
>    while (compressed_size > 0 && uncompressed_size > 0);
> -  return compressed_size == 0 && uncompressed_size == 0;
> +  return uncompressed_size == 0;
>  }

Hmm. For well-formed inputs this ought to be okay. But I'm concerned of its
use (also) as a sanity check: What if the uncompressed size communicated
doesn't match the actual data? If compressed_size remains larger than zero,
might we not want to check that the remaining space is indeed padding (e.g.
all zeroes, or maybe all minus-ones)?

Relaxing the check also looks like isn't necessary for ELF, so we may want
to consider retaining the stronger check there.

> CC-ed Rui. Rui, if there's a reason to not relax your result calculation let me
> know.

Let's also Cc Alan right away.

Jan