Re: [PATCH] Honour errors when processing more than one file

[Tom de Vries <tdevries@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 3198 bytes --]

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  <tdevries@suse.de>
>>>
>>> 	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



[-- Attachment #2: 0001-Honour-errors-when-processing-more-than-one-file.patch --]
[-- Type: text/x-patch, Size: 2025 bytes --]

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  <tdevries@suse.de>

	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