From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7777 invoked by alias); 10 May 2019 07:35:11 -0000 Mailing-List: contact dwz-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Post: List-Help: List-Subscribe: Sender: dwz-owner@sourceware.org Received: (qmail 7705 invoked by uid 89); 10 May 2019 07:35:11 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Checked: by ClamAV 0.100.3 on sourceware.org X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.4 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_PASS autolearn=ham version=3.3.1 spammy=9,7, processed, UD:info X-Spam-Status: No, score=-25.4 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_PASS autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on sourceware.org X-Spam-Level: X-HELO: mx1.suse.de X-Virus-Scanned: by amavisd-new at test-mx.suse.de Subject: Re: [PATCH] Honour errors when processing more than one file From: Tom de Vries To: Jakub Jelinek Cc: dwz@sourceware.org References: <20190304164925.GA27794@delia> <20190304170957.GO7611@tucnak> Message-ID: <43f9bb8f-609d-caa5-6dae-1fe9c1b9b321@suse.de> Date: Tue, 01 Jan 2019 00:00:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.5.1 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/mixed; boundary="------------9415A6C912AAF7BD17D15F7E" Content-Language: en-US X-SW-Source: 2019-q2/txt/msg00052.txt.bz2 This is a multi-part message in MIME format. --------------9415A6C912AAF7BD17D15F7E Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Content-length: 3198 On 05-03-19 21:32, Tom de Vries wrote: > On 04-03-19 18:09, Jakub Jelinek wrote: >> On Mon, Mar 04, 2019 at 05:49:26PM +0100, Tom de Vries wrote: >>> 2019-03-04 Tom de Vries >>> >>> PR dwz/24301 >>> * dwz.c (main): Handle dwz returning 1 if processing more than one file. >>> * testsuite/dwz.tests/two-files-too-many-dies.sh: New test. >> >> That looks wrong to me, that means once an error is reported for any single >> one, the rest will not be processed. That was not the intent and is >> undesirable for the distro compression, we want to compress as much as >> possible. >> > > I see, understood. Updated the test-case accordingly. > >> Guess that >> else if (resa[i - optind].res == 0) >> successcount++; >> should be >> if (resa[i - optind].res == 0) >> successcount++; >> else >> ret = 1; >> or something similar, I must say I don't really remember the difference >> between res->res value and return value from dwz etc., > > The return value of the dwz function indicates whether a problem > occurred during dwz execution. > > The res->res value, as far as I can infer from the source, is related to > file status, and can be (as added in the patch for PR24275): > ... > /* -2: Already processed under different name. > -1: Ignore. > 0: Processed, changed. > 1: Processed, unchanged. > ... > and unchanged can be either due to an error, or due to not being able to > compress. > > [ The PR24275 is about that fact that dwz never in fact returns with > res->res set to 1. ] > >> but it seems weird >> that if if some file isn't processed in normal mode and when switched into lowmem >> mode succeeds, we don't count it as successcount, > > The successcount variable tracks the amount of unique files (that is, in > hardlink mode we count two hardlinks to the same file as one) that > succeeded in regular mode, which, if >=2 enables multifile mode. > > AFAIU, the fact that low-mem files are not optimized in multifile mode > is listed explicitly in the file header documentation: > ... > If some executable or shared library has too large debug information > > (number of DIEs in .debug_info section) that there would be > > risk of too high memory consumption, that file isn't multifile > > optimized, instead it is processed by dwz () in a low-memory mode > > with low_mem flag set. This can decrease memory consumption to > > half in some very large cases. */ > ... > > So, I'd say we want to keep the else in: > ... > else if (resa[i - optind].res == 0) > ... > >> while if we already are in >> lowmem mode from the previous file, we count it. > > The lowmem mode is a per-file mode. If a previous file was processed in > low-mem mode, we still try the current file again in regular mode, > before possibly switching back to low-mem mode. > > --- > > Also, we cannot use and else here: > ... > else > ret = 1; > ... > because that also triggers for res->res == -2 (which would make > hardlink.sh fail). So I use: > ... > else if (thisret == 1) > ... > instead. > > OK like this? Committed as attached. Thanks, - Tom --------------9415A6C912AAF7BD17D15F7E Content-Type: text/x-patch; name="0001-Honour-errors-when-processing-more-than-one-file.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename*0="0001-Honour-errors-when-processing-more-than-one-file.patch" Content-length: 2025 Honour errors when processing more than one file When using dwz -L0 on a hello world a.out, it fails: ... $ dwz -L0 a.out $ echo $? 1 ... But when we do the same for a.out and a copy b.out, it passes: ... $ cp a.out b.out $ dwz -L0 a.out b.out $ echo $? 0 ... Fix this by honouring dwz return codes when processing more than one file. 2019-05-10 Tom de Vries PR dwz/24301 * dwz.c (main): Handle dwz returning 1 if processing more than one file. * testsuite/dwz.tests/pr24171.sh: Update expected return status. * testsuite/dwz.tests/two-files-too-many-dies.sh: Same. --- dwz.c | 2 ++ testsuite/dwz.tests/pr24171.sh | 2 +- testsuite/dwz.tests/two-files-too-many-dies.sh | 6 +----- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/dwz.c b/dwz.c index 51300d3..6f34a0c 100644 --- a/dwz.c +++ b/dwz.c @@ -12291,6 +12291,8 @@ main (int argc, char *argv[]) } else if (resa[i - optind].res == 0) successcount++; + else if (thisret == 1) + ret = 1; if (hardlink && resa[i - optind].res >= 0 && resa[i - optind].nlink > 1) diff --git a/testsuite/dwz.tests/pr24171.sh b/testsuite/dwz.tests/pr24171.sh index 0705489..31b2439 100644 --- a/testsuite/dwz.tests/pr24171.sh +++ b/testsuite/dwz.tests/pr24171.sh @@ -10,6 +10,6 @@ if ! grep -q "DW_AT_stmt_list not DW_FORM_sec_offset or DW_FORM_data4" dwz.err; exit 1 fi -[ $status -eq 0 ] +[ $status -eq 1 ] rm -f 1 2 dwz.err diff --git a/testsuite/dwz.tests/two-files-too-many-dies.sh b/testsuite/dwz.tests/two-files-too-many-dies.sh index fb7d496..13950ba 100644 --- a/testsuite/dwz.tests/two-files-too-many-dies.sh +++ b/testsuite/dwz.tests/two-files-too-many-dies.sh @@ -9,11 +9,7 @@ if ! grep -q "Too many DIEs, not optimizing" dwz.err; then exit 1 fi -if [ $status -eq 0 ]; then - echo "PR24301 workaround used" > dwz.info -else - [ $status -eq 1 ] -fi +[ $status -eq 1 ] cmp 1 $execs/hello cmp 2 $execs/hello --------------9415A6C912AAF7BD17D15F7E--