From patchwork Mon Feb 27 11:43:09 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom de Vries X-Patchwork-Id: 61924 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:5915:0:0:0:0:0 with SMTP id v21csp2365103wrd; Mon, 27 Feb 2023 03:43:14 -0800 (PST) X-Google-Smtp-Source: AK7set+/oti0eQpsj9FHEhl9TnASl/ggZNsCItnIHd96hV/5M522iKVgYCPlBMr+g1uaJAoMWBz7 X-Received: by 2002:a05:6402:1a59:b0:4b1:2051:43b1 with SMTP id bf25-20020a0564021a5900b004b1205143b1mr7280416edb.37.1677498194486; Mon, 27 Feb 2023 03:43:14 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1677498194; cv=none; d=google.com; s=arc-20160816; b=jx0DMpE+ZjnoVcOdsFKtMlPeCH92OGcjS2CcRxGwOdEOuSXIG7q03EO2zkWK2rwKAX qd7vEcsFQ7xAGntkqsa9KmUUQLOlYIKEzzjPvGmSC5wnY89Ie65nGM1E7ffcaVpsxu+N Rd8gMxwCAC7Hc51FCZx55b8VSvoOZoHt0ueG1oLsrWobsV8bufGzPHrm7y6D77LD0Qim xP76CthLAU3lT/h5nE+Q2PHYAKyQTt6t36ZPKT+xaXirz+Wm0fUXYMQpZ+uIjlgDnLqf Tj1sh9bjXk6KjJ/bpq+uSP3GXOhuLpV57LaI+ksu/5E7PxCbMDKig1f/A+/pUERoGyss 9l0Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:reply-to:from:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence:in-reply-to :references:cc:to:content-language:subject:user-agent:mime-version :date:message-id:dmarc-filter:delivered-to:dkim-signature :dkim-filter; bh=UTPDZ3S/WBDp25nXOsUFOTpRW0O7F+H50WIE3BeJOJU=; b=utAvo1OLPR+hBST+2UXyVhOkw7rEcDGUKVP7jZL9cGVAnCRLptZJcLHxl+emgEf7vP N/mxo0W/x3DScqteiR1bbjW7fwQnQt6PFLJ4rnEH+LQKMz0BBDujjA+H31UuWvkpD1Sq WLbQUDrNb1fRqpTFZ3ge1jqyMDEnX6B0+77mnKBd5TFZSPVCDQXa0G9drVjlvJ9xjCan lB507+CzXlazTVtS4wSJexbYVLsKVu9VQobGzIZ8Rvspj/Qw2+04T5hSsiW1YLqvF2Nt EdLz9muaB2XzQQ9iWpAyKgtkPMhm1/Hk8IUE2DAwxaw6E9EnYqQXemwqXkynzzneYnLh dbVA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@sourceware.org header.s=default header.b=oprJFMOv; spf=pass (google.com: domain of binutils-bounces+ouuuleilei=gmail.com@sourceware.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="binutils-bounces+ouuuleilei=gmail.com@sourceware.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=sourceware.org Received: from sourceware.org (server2.sourceware.org. [2620:52:3:1:0:246e:9693:128c]) by mx.google.com with ESMTPS id bl5-20020a170906c24500b008b8a4ed82fcsi8451269ejb.947.2023.02.27.03.43.14 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 27 Feb 2023 03:43:14 -0800 (PST) Received-SPF: pass (google.com: domain of binutils-bounces+ouuuleilei=gmail.com@sourceware.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) client-ip=2620:52:3:1:0:246e:9693:128c; Authentication-Results: mx.google.com; dkim=pass header.i=@sourceware.org header.s=default header.b=oprJFMOv; spf=pass (google.com: domain of binutils-bounces+ouuuleilei=gmail.com@sourceware.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="binutils-bounces+ouuuleilei=gmail.com@sourceware.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 30066385828E for ; Mon, 27 Feb 2023 11:43:13 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 30066385828E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1677498193; bh=UTPDZ3S/WBDp25nXOsUFOTpRW0O7F+H50WIE3BeJOJU=; h=Date:Subject:To:Cc:References:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=oprJFMOvU0m5lqeCpw56lByEyrDqvPnjeVEeub87Bd2C8iPywJNS9N9aJxLO/gIw7 cF31K07GmKKwgzRUlj+6zTLrxiDfvl3X7TRJUaNRsjHyle19vyf6pz2WjTKs5yuQke pLbFHIo+R9FLRQo7A2BD45+tFP7Fmdf4dUMv2IeM= X-Original-To: binutils@sourceware.org Delivered-To: binutils@sourceware.org Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id 324213858D20 for ; Mon, 27 Feb 2023 11:43:04 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 324213858D20 Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 5B0C4219E1; Mon, 27 Feb 2023 11:43:03 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 33FDE13912; Mon, 27 Feb 2023 11:43:03 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id e9UXC0eX/GOkIwAAMHmgww (envelope-from ); Mon, 27 Feb 2023 11:43:03 +0000 Message-ID: <9eb9eaf6-2cff-360b-3de6-072d3f8185dc@suse.de> Date: Mon, 27 Feb 2023 12:43:09 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.7.1 Subject: [PATCH] gas: Add --compress-debug-sections=force Content-Language: en-US To: Jan Beulich Cc: binutils@sourceware.org, Michael Matz , Nick Clifton , Alan Modra References: <20230223124519.4228-1-tdevries@suse.de> <7dcb7bfb-f65d-aed8-78d4-944211ef5127@suse.de> <819f729a-da9c-3b8a-3769-7995c009704b@suse.com> <14a2defc-5371-84bc-2d59-9980755b112a@suse.de> <02dcf47c-4256-c5e5-de9e-814b60da8ce8@suse.com> <7cb226d0-1a91-9bad-181c-46f79c4d6eaf@suse.de> <0c60eef7-c612-ec37-8c3f-36b746ff8d95@suse.de> <711d9da5-8f31-3f1e-530b-5ae01780fcad@suse.de> In-Reply-To: X-Spam-Status: No, score=-12.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Tom de Vries via Binutils From: Tom de Vries Reply-To: Tom de Vries Errors-To: binutils-bounces+ouuuleilei=gmail.com@sourceware.org Sender: "Binutils" X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1758984346810090966?= X-GMAIL-MSGID: =?utf-8?q?1758984346810090966?= [ was: Re: [PATCH] gas: Add --force-compress-debug-sections ] On 2/27/23 10:03, Jan Beulich wrote: > On 24.02.2023 15:57, Tom de Vries wrote: >> On 2/24/23 15:26, Jan Beulich wrote: >>> On 24.02.2023 15:11, Tom de Vries wrote: >>>> On 2/24/23 14:23, Jan Beulich wrote: >>>>> On 24.02.2023 13:21, Tom de Vries wrote: >>>>>> On 2/24/23 12:28, Jan Beulich wrote: >>>>>>> I also wouldn't see anything wrong with something >>>>>>> like "...=force,zstd,none" - the last one(s) win. That's no different >>>>>>> from specifying a second instance of the option. And without that it >>>>>>> looks as if the parsing would end up simpler. >>>>>> >>>>>> OK, gave that a try. >>>>> >>>>> That's still accumulating none and force across the entire sequence >>>>> (and then giving none priority over force, no matter that force may >>>>> have been specified last), >>>> >>>> Um, so you're saying that none+zstd+force is currently interpreted as none? >>>> >>>> Lets try: >>>> ... >>>> $ gcc ~/hello.c -c -Wa,-gdwarf-5 -Xassembler >>>> --compress-debug-sections=none+zstd+force >>>> $ readelf -S -W hello.o | grep " .debug" >>>> [ 9] .debug_line PROGBITS 0000a8 000064 00 C 0 0 8 >>>> [11] .debug_line_str PROGBITS 000110 000046 01 MSC 0 0 8 >>>> [12] .debug_info PROGBITS 000158 000046 00 C 0 0 8 >>>> [14] .debug_abbrev PROGBITS 0001a0 000049 00 C 0 0 8 >>>> [15] .debug_aranges PROGBITS 0001f0 000034 00 C 0 0 8 >>>> [17] .debug_str PROGBITS 000228 00005a 01 MSC 0 0 8 >>>> >>>> ... >>>> >>>> So, that doesn't seem to be the case, compression is done, as expected. >>> >>> Oh, I've overlooked that you explicitly clear *none when you set *force >>> (my attention was mainly on the bottom of parse_compress_debug_optarg()). >>> I think that's more involved than necessary (possibly merely a result of >>> you having worked incrementally from your earlier version), and less >>> obviously doing the same as would happen when multiple separate options >>> were parsed. >> >> I've tried to simplify further. >> >> Is this more how you want it? > > I have to admit that I'm still puzzled by the presence of > finalize_parse_compress_debug_optarg() as well as you needing both a new > static variable and a new global one. The latter I've fixed, by exporting the static variable. That eliminates one half of finalize_parse_compress_debug_optarg(). The remaining half is necessary because we accumulate "none" into compress_debug_action instead of into flag_compress_debug, so after parsing is over we need to assign the accumulation result, if it is indeed cda_none, to flag_compress_debug. And the reason we have that complicated setup is related to preventing none from zapping compression type. > But I guess whether that's really > needed first of all depends on the semantics we want e.g. > > --nocompress-debug-sections --compress-debug-sections=force > > to have (which, with how you have it presently, could also be expressed > as > > --compress-debug-sections=none+force > > or > > --compress-debug-sections=none --compress-debug-sections=force > > afaict). I view the present meaning as one sensible one, but I could > also see "none" (or equivalent) simply zapping the compression type > (and hence rendering "force" meaningless) as another sensible one. Sure, that's another possible semantics. Should I implement that instead? > A > change in meaning may then also result in the three option combinations > above possibly not all doing the same. > > As an aside: As you update the patch, please try to keep the title in > line with what the patch actually does. > I'm assuming you mean the email title (the patch title looks ok to me), and I've updated it. > Also, ftaod, I don't mean to stand in the way of another maintainer > approving any of the forms proposed so far. This specifically also > includes the use of '+' as a separator, which I personally don't > (currently) intend to approve. Well, in case that's blocking you from giving approval, I've updated the patch to use the ',' separator. Thanks, - Tom From 0fb4ed6d648a69799cdbf6b19fd36c6125c5fc41 Mon Sep 17 00:00:00 2001 From: Tom de Vries Date: Thu, 23 Feb 2023 12:53:40 +0100 Subject: [PATCH] gas: Add --compress-debug-sections=force Gas has an option --compress-debug-sections that allows it to generate compressed debug sections. That does not guarantee that the debug sections are in fact compressed: ... $ gcc hello.c -Wa,-gdwarf-5 -c -Wa,--compress-debug-sections=zstd $ readelf -S -W hello.o | grep " .debug" [ 9] .debug_line PROGBITS 0000a8 000053 00 0 0 1 [11] .debug_line_str PROGBITS 0000fb 000025 01 MS 0 0 1 [12] .debug_info PROGBITS 000120 000039 00 0 0 1 [14] .debug_abbrev PROGBITS 000159 000028 00 0 0 1 [15] .debug_aranges PROGBITS 000190 000030 00 0 0 16 [17] .debug_str PROGBITS 0001c0 000039 01 MS 0 0 1 ... Sensibly so, they're only compressed if that provides a size benefit. However, for the purpose of testing components consuming dwarf we may want the sections to be compressed regardless. Add a new suboption --compress-debug-sections=force that ignores the size heuristic, such that we have instead: ... $ gcc hello.c -Wa,-gdwarf-5 -c -Xassembler \ --compress-debug-sections=zstd,force $ readelf -S -W hello.o | grep " .debug" [ 9] .debug_line PROGBITS 0000a8 000064 00 C 0 0 8 [11] .debug_line_str PROGBITS 000110 000046 01 MSC 0 0 8 [12] .debug_info PROGBITS 000158 000046 00 C 0 0 8 [14] .debug_abbrev PROGBITS 0001a0 000049 00 C 0 0 8 [15] .debug_aranges PROGBITS 0001f0 000034 00 C 0 0 8 [17] .debug_str PROGBITS 000228 00005a 01 MSC 0 0 8 ... Advertised as: ... $ as --help ... --compress-debug-sections[={none||force|force,}] where is {zlib|zlib-gnu|zlib-gabi|zstd} compress DWARF debug sections Default: zstd ... Tested on x86_64-linux. --- gas/as.c | 106 +++++++++++++++++++++++++++++++++++++++--------- gas/as.h | 10 +++++ gas/doc/as.texi | 10 ++++- gas/write.c | 5 ++- 4 files changed, 108 insertions(+), 23 deletions(-) diff --git a/gas/as.c b/gas/as.c index 598bfd56cf5..acbccfb5dce 100644 --- a/gas/as.c +++ b/gas/as.c @@ -230,6 +230,8 @@ enum compressed_debug_section_type flag_compress_debug = DEFAULT_COMPRESSED_DEBUG_ALGORITHM; #endif +enum compress_debug_action compress_debug_action = cda_default; + static void show_usage (FILE * stream) { @@ -252,7 +254,8 @@ Options:\n\ fprintf (stream, _("\ --alternate initially turn on alternate macro syntax\n")); fprintf (stream, _("\ - --compress-debug-sections[={none|zlib|zlib-gnu|zlib-gabi|zstd}]\n\ + --compress-debug-sections[={none||force|force,}]\n\ + where is {zlib|zlib-gnu|zlib-gabi|zstd}\n\ compress DWARF debug sections\n")), fprintf (stream, _("\ Default: %s\n"), @@ -418,6 +421,84 @@ Options:\n\ fprintf (stream, _("Report bugs to %s\n"), REPORT_BUGS_TO); } +static void +parse_compress_debug_optarg_1 (const char *optarg) +{ + gas_assert (optarg != NULL); + + if (strcmp (optarg, "force") == 0) + { + compress_debug_action = cda_force; + return; + } + + enum compressed_debug_section_type tmp + = bfd_get_compression_algorithm (optarg); + +#ifndef HAVE_ZSTD + if (tmp == COMPRESS_DEBUG_ZSTD) + as_fatal (_ ("--compress-debug-sections=zstd: gas is not " + "built with zstd support")); +#endif + + if (tmp == COMPRESS_UNKNOWN) + as_fatal (_("Invalid --compress-debug-sections option: `%s'"), + optarg); + + if (tmp == COMPRESS_DEBUG_NONE) + { + compress_debug_action = cda_none; + return; + } + + compress_debug_action = cda_yes; + flag_compress_debug = tmp; +} + +static void +parse_compress_debug_optarg (const char *optarg) +{ +#if !defined OBJ_ELF && !defined OBJ_MAYBE_ELF + as_fatal (_("--compress-debug-sections=%s is unsupported"), + optarg); +#endif + + /* Tokenize subopts pass to parse_compress_debug_optarg_1. */ + char sep = ','; + while (true) + { + const char *idx = optarg; + while (*idx != '\0' && *idx != sep) + idx++; + + size_t len = idx - optarg; + if (len == 0) + { + /* Generate error. */ + parse_compress_debug_optarg_1 (""); + break; + } + + char *tmp = xstrndup (optarg, len); + parse_compress_debug_optarg_1 (tmp); + free (tmp); + + if (*idx == '\0') + break; + + /* Step over separator and continue tokenizing. */ + gas_assert (*idx == sep); + optarg = idx + 1; + } +} + +static void +finalize_parse_compress_debug_optarg (void) +{ + if (compress_debug_action == cda_none) + flag_compress_debug = COMPRESS_DEBUG_NONE; +} + /* Since it is easy to do here we interpret the special arg "-" to mean "use stdin" and we set that argv[] pointing to "". After we have munged argv[], the only things left are source file @@ -747,28 +828,13 @@ This program has absolutely no warranty.\n")); case OPTION_COMPRESS_DEBUG: if (optarg) - { -#if defined OBJ_ELF || defined OBJ_MAYBE_ELF - flag_compress_debug = bfd_get_compression_algorithm (optarg); -#ifndef HAVE_ZSTD - if (flag_compress_debug == COMPRESS_DEBUG_ZSTD) - as_fatal (_ ("--compress-debug-sections=zstd: gas is not " - "built with zstd support")); -#endif - if (flag_compress_debug == COMPRESS_UNKNOWN) - as_fatal (_("Invalid --compress-debug-sections option: `%s'"), - optarg); -#else - as_fatal (_("--compress-debug-sections=%s is unsupported"), - optarg); -#endif - } + parse_compress_debug_optarg (optarg); else - flag_compress_debug = COMPRESS_DEBUG_GABI_ZLIB; + parse_compress_debug_optarg ("zlib-gabi"); break; case OPTION_NOCOMPRESS_DEBUG: - flag_compress_debug = COMPRESS_DEBUG_NONE; + parse_compress_debug_optarg ("none"); break; case OPTION_DEBUG_PREFIX_MAP: @@ -1136,6 +1202,8 @@ This program has absolutely no warranty.\n")); *pargc = new_argc; *pargv = new_argv; + finalize_parse_compress_debug_optarg (); + #ifdef md_after_parse_args md_after_parse_args (); #endif diff --git a/gas/as.h b/gas/as.h index 4c5fa9ecf7d..bc0fe7f5cd9 100644 --- a/gas/as.h +++ b/gas/as.h @@ -331,6 +331,16 @@ COMMON int flag_traditional_format; /* Type of compressed debug sections we should generate. */ COMMON enum compressed_debug_section_type flag_compress_debug; +/* Whether we should compress debug sections. */ +enum compress_debug_action +{ + cda_default, + cda_none, + cda_force, + cda_yes, +}; +COMMON enum compress_debug_action compress_debug_action; + /* TRUE if .note.GNU-stack section with SEC_CODE should be created */ COMMON int flag_execstack; diff --git a/gas/doc/as.texi b/gas/doc/as.texi index bbdfa4bfdca..f0140c09779 100644 --- a/gas/doc/as.texi +++ b/gas/doc/as.texi @@ -718,7 +718,8 @@ Begin in alternate macro mode. Compress DWARF debug sections using zlib with SHF_COMPRESSED from the ELF ABI. The resulting object file may not be compatible with older linkers and object file utilities. Note if compression would make a -given section @emph{larger} then it is not compressed. +given section @emph{larger} then it is not compressed, unless +@option{--compress-debug-section=force} is used. @ifset ELF @cindex @samp{--compress-debug-sections=} option @@ -727,6 +728,7 @@ given section @emph{larger} then it is not compressed. @itemx --compress-debug-sections=zlib-gnu @itemx --compress-debug-sections=zlib-gabi @itemx --compress-debug-sections=zstd +@itemx --compress-debug-sections=force These options control how DWARF debug sections are compressed. @option{--compress-debug-sections=none} is equivalent to @option{--nocompress-debug-sections}. @@ -738,7 +740,11 @@ using the obsoleted zlib-gnu format. The debug sections are renamed to begin with @samp{.zdebug}. @option{--compress-debug-sections=zstd} compresses DWARF debug sections using zstd. Note - if compression would actually make a section -@emph{larger}, then it is not compressed nor renamed. +@emph{larger}, then it is not compressed nor renamed, unless +@option{--compress-debug-section=force} is used. +@option{--compress-debug-sections=force} compresses DWARF debug sections, +even if this does not reduce size. It can be used in conjunction with a format +selection, for instance @option{--compress-debug-section=zstd,force}. @end ifset diff --git a/gas/write.c b/gas/write.c index 8273b7a42f1..1fa3b54a03b 100644 --- a/gas/write.c +++ b/gas/write.c @@ -1463,9 +1463,10 @@ compress_debug (bfd *abfd, asection *sec, void *xxx ATTRIBUTE_UNUSED) segment_info_type *seginfo = seg_info (sec); bfd_size_type uncompressed_size = sec->size; flagword flags = bfd_section_flags (sec); + bool force_compress_debug = compress_debug_action == cda_force; if (seginfo == NULL - || uncompressed_size < 32 + || (!force_compress_debug && uncompressed_size < 32) || (flags & SEC_HAS_CONTENTS) == 0) return; @@ -1582,7 +1583,7 @@ compress_debug (bfd *abfd, asection *sec, void *xxx ATTRIBUTE_UNUSED) /* PR binutils/18087: If compression didn't make the section smaller, just keep it uncompressed. */ - if (compressed_size >= uncompressed_size) + if (!force_compress_debug && compressed_size >= uncompressed_size) return; /* Replace the uncompressed frag list with the compressed frag list. */ -- 2.35.3