public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: "Martin Liška" <mliska@suse.cz>
To: Alan Modra <amodra@gmail.com>
Cc: binutils@sourceware.org, Fangrui Song <maskray@google.com>
Subject: Re: [PATCH] compress .gnu.debuglto_.debug_* sections if requested
Date: Tue, 4 Oct 2022 10:12:46 +0200	[thread overview]
Message-ID: <373ed7fc-5b05-0049-7df0-35958fd2857c@suse.cz> (raw)
In-Reply-To: <YzvptsEykrCENkgC@squeak.grove.modra.org>

On 10/4/22 10:07, Alan Modra wrote:
> On Fri, Sep 30, 2022 at 09:19:49AM +0200, Martin Liška wrote:
>> 	* write.c (compress_debug): Compress also ".gnu.debuglto_.debug_"
>> 	if the compression algorithm is different from zlib-gnu.
> 
> OK, choosing to skip compression for zlib-gnu is a reasonable approach
> given that zlib-gnu has the clunky .debug_* to .zdebug_* section
> renaming scheme.  I'm going to commit your patch, but then enable
> zlib-gnu with a followup patch of mine that doesn't rename the lto
> debug sections.  ld.bfd and the binutils work fine without renaming,
> but there may be some other reason why zlib-gnu must have .zdebug
> sections.  If that turns out to be the case, my patch below can be
> reverted.

Makes sense. On the other hand, I would somehow slowly (but surely) make zlib-gnu
obsolete. What do you think? For the future, people will likely move to the more
modern and faster zstd compression algorithm. We're planning doing that once
new binutils is released in openSUSE Tumbleweed.

Cheers,
Martin

> 
> bfd/
> 	* elf.c (elf_fake_sections): Replace "." with ".z" in debug
> 	section names only when name was ".d*", ie. ".debug_*".
> 	(_bfd_elf_assign_file_positions_for_non_load): Likewise.
> gas/
> 	* write.c (compress_debug): Compress .gnu.debuglto_.debug_*
> 	for zlib-gnu too.  Compress .gnu.linkonce.wi.*.
> 
> diff --git a/bfd/elf.c b/bfd/elf.c
> index 8cd257fc65c..420b524aae8 100644
> --- a/bfd/elf.c
> +++ b/bfd/elf.c
> @@ -3234,7 +3234,8 @@ elf_fake_sections (bfd *abfd, asection *asect, void *fsarg)
>  	      name = new_name;
>  	    }
>  	}
> -      else if (asect->compress_status == COMPRESS_SECTION_DONE)
> +      else if (asect->compress_status == COMPRESS_SECTION_DONE
> +	       && name[1] == 'd')
>  	{
>  	  /* PR binutils/18087: Compression does not always make a
>  	     section smaller.  So only rename the section when
> @@ -3246,7 +3247,6 @@ elf_fake_sections (bfd *abfd, asection *asect, void *fsarg)
>  	      arg->failed = true;
>  	      return;
>  	    }
> -	  BFD_ASSERT (name[1] != 'z');
>  	  name = new_name;
>  	}
>      }
> @@ -6689,7 +6689,8 @@ _bfd_elf_assign_file_positions_for_non_load (bfd *abfd)
>  		    return false;
>  
>  		  if (sec->compress_status == COMPRESS_SECTION_DONE
> -		      && (abfd->flags & BFD_COMPRESS_GABI) == 0)
> +		      && (abfd->flags & BFD_COMPRESS_GABI) == 0
> +		      && name[1] == 'd')
>  		    {
>  		      /* If section is compressed with zlib-gnu, convert
>  			 section name from .debug_* to .zdebug_*.  */
> diff --git a/gas/write.c b/gas/write.c
> index b8d5253006c..30f6c110908 100644
> --- a/gas/write.c
> +++ b/gas/write.c
> @@ -1473,7 +1473,7 @@ compress_debug (bfd *abfd, asection *sec, void *xxx ATTRIBUTE_UNUSED)
>    char *header;
>    int x;
>    flagword flags = bfd_section_flags (sec);
> -  unsigned int header_size, compression_header_size;
> +  unsigned int header_size;
>  
>    if (seginfo == NULL
>        || sec->size < 32
> @@ -1482,8 +1482,8 @@ compress_debug (bfd *abfd, asection *sec, void *xxx ATTRIBUTE_UNUSED)
>  
>    section_name = bfd_section_name (sec);
>    if (!startswith (section_name, ".debug_")
> -      && (!startswith (section_name, ".gnu.debuglto_.debug_")
> -	  || flag_compress_debug == COMPRESS_DEBUG_GNU_ZLIB))
> +      && !startswith (section_name, ".gnu.debuglto_.debug_")
> +      && !startswith (section_name, ".gnu.linkonce.wi."))
>      return;
>  
>    bool use_zstd = abfd->flags & BFD_COMPRESS_ZSTD;
> @@ -1492,16 +1492,9 @@ compress_debug (bfd *abfd, asection *sec, void *xxx ATTRIBUTE_UNUSED)
>      return;
>  
>    if (flag_compress_debug == COMPRESS_DEBUG_GNU_ZLIB)
> -    {
> -      compression_header_size = 0;
> -      header_size = 12;
> -    }
> +    header_size = 12;
>    else
> -    {
> -      compression_header_size
> -	= bfd_get_compression_header_size (stdoutput, NULL);
> -      header_size = compression_header_size;
> -    }
> +    header_size = bfd_get_compression_header_size (stdoutput, NULL);
>  
>    /* Create a new frag to contain the compression header.  */
>    first_newf = frag_alloc (ob);
> @@ -1609,7 +1602,8 @@ compress_debug (bfd *abfd, asection *sec, void *xxx ATTRIBUTE_UNUSED)
>    bfd_update_compression_header (abfd, (bfd_byte *) header, sec);
>    x = bfd_set_section_size (sec, compressed_size);
>    gas_assert (x);
> -  if (!compression_header_size)
> +  if (flag_compress_debug == COMPRESS_DEBUG_GNU_ZLIB
> +      && section_name[1] == 'd')
>      {
>        compressed_name = concat (".z", section_name + 1, (char *) NULL);
>        bfd_rename_section (sec, compressed_name);
> 
> 


  reply	other threads:[~2022-10-04  8:12 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-29 13:35 Martin Liška
2022-09-30  0:20 ` Alan Modra
2022-09-30  7:19   ` Martin Liška
2022-10-04  8:07     ` Alan Modra
2022-10-04  8:12       ` Martin Liška [this message]
2022-10-04 13:41         ` Alan Modra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=373ed7fc-5b05-0049-7df0-35958fd2857c@suse.cz \
    --to=mliska@suse.cz \
    --cc=amodra@gmail.com \
    --cc=binutils@sourceware.org \
    --cc=maskray@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).