* [PATCH] Honour errors when processing more than one file
@ 2019-01-01 0:00 Tom de Vries
2019-01-01 0:00 ` Jakub Jelinek
0 siblings, 1 reply; 4+ messages in thread
From: Tom de Vries @ 2019-01-01 0:00 UTC (permalink / raw)
To: dwz, jakub
Hi,
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.
OK for trunk?
Thanks,
- Tom
Honour errors when processing more than one file
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.
---
dwz.c | 2 ++
testsuite/dwz.tests/two-files-too-many-dies.sh | 19 +++++++++++++++++++
2 files changed, 21 insertions(+)
diff --git a/dwz.c b/dwz.c
index d348418..0c53c4b 100644
--- a/dwz.c
+++ b/dwz.c
@@ -11940,6 +11940,8 @@ main (int argc, char *argv[])
}
else if (resa[i - optind].res == 0)
successcount++;
+ if (thisret == 1)
+ return 1;
if (hardlink
&& resa[i - optind].res >= 0
&& resa[i - optind].nlink > 1)
diff --git a/testsuite/dwz.tests/two-files-too-many-dies.sh b/testsuite/dwz.tests/two-files-too-many-dies.sh
new file mode 100755
index 0000000..7214ff6
--- /dev/null
+++ b/testsuite/dwz.tests/two-files-too-many-dies.sh
@@ -0,0 +1,19 @@
+#!/bin/sh
+
+set -e
+
+cp ../hello 1
+cp ../hello 2
+
+if dwz -L0 1 2 2>/dev/null; then
+ exit 1
+fi
+
+cmp 1 ../hello
+cmp 2 ../hello
+
+ls=$(ls)
+ls=$(echo $ls)
+[ "$ls" = "1 2" ]
+
+rm -f 1 2
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Honour errors when processing more than one file
2019-01-01 0:00 ` Jakub Jelinek
@ 2019-01-01 0:00 ` Tom de Vries
2019-01-01 0:00 ` Tom de Vries
0 siblings, 1 reply; 4+ messages in thread
From: Tom de Vries @ 2019-01-01 0:00 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: dwz
[-- Attachment #1: Type: text/plain, Size: 2972 bytes --]
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?
Thanks,
- Tom
[-- Attachment #2: 0001-Honour-errors-when-processing-more-than-one-file.patch --]
[-- Type: text/x-patch, Size: 2436 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-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.
---
Makefile | 5 ++++-
dwz.c | 2 ++
hello.c | 14 ++++++++++++++
testsuite/dwz.tests/two-files-too-many-dies.sh | 22 ++++++++++++++++++++++
4 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index b318fd7..4b37732 100644
--- a/Makefile
+++ b/Makefile
@@ -17,11 +17,14 @@ clean:
PWD:=$(shell pwd -P)
-TEST_EXECS = hello
+TEST_EXECS = hello hello2
hello:
$(CC) hello.c -o $@ -g
+hello2:
+ $(CC) hello.c -o $@ -DWITH_STRUCT -g
+
check: dwz $(TEST_EXECS)
mkdir -p testsuite-bin
cd testsuite-bin; ln -sf $(PWD)/dwz .
diff --git a/dwz.c b/dwz.c
index d348418..e82d6a7 100644
--- a/dwz.c
+++ b/dwz.c
@@ -11940,6 +11940,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/hello.c b/hello.c
index 82d070e..ee65515 100644
--- a/hello.c
+++ b/hello.c
@@ -1,8 +1,22 @@
#include <stdio.h>
+#ifdef WITH_STRUCT
+struct a {
+ int a;
+ char *p;
+};
+
+struct a a;
+#endif
+
int
main (void)
{
+#ifdef WITH_STRUCT
+ a.p = "hello";
+ printf ("%s\n", a.p);
+#else
printf ("hello\n");
+#endif
return 0;
}
diff --git a/testsuite/dwz.tests/two-files-too-many-dies.sh b/testsuite/dwz.tests/two-files-too-many-dies.sh
new file mode 100755
index 0000000..da86d23
--- /dev/null
+++ b/testsuite/dwz.tests/two-files-too-many-dies.sh
@@ -0,0 +1,22 @@
+#!/bin/sh
+
+set -e
+
+cp ../hello 1
+cp ../hello2 2
+
+limit=$(readelf -w 2 | grep '(DW_TAG' | wc -l)
+limit=$((limit - 1))
+
+if dwz -L$limit 2 1 2>/dev/null; then
+ exit 1
+fi
+
+smaller-than.sh 1 ../hello
+cmp 2 ../hello2
+
+ls=$(ls)
+ls=$(echo $ls)
+[ "$ls" = "1 2" ]
+
+rm -f 1 2
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Honour errors when processing more than one file
2019-01-01 0:00 [PATCH] Honour errors when processing more than one file Tom de Vries
@ 2019-01-01 0:00 ` Jakub Jelinek
2019-01-01 0:00 ` Tom de Vries
0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2019-01-01 0:00 UTC (permalink / raw)
To: Tom de Vries; +Cc: dwz
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.
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., 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, while if we already are in
lowmem mode from the previous file, we count it.
Jakub
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Honour errors when processing more than one file
2019-01-01 0:00 ` Tom de Vries
@ 2019-01-01 0:00 ` Tom de Vries
0 siblings, 0 replies; 4+ messages in thread
From: Tom de Vries @ 2019-01-01 0:00 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: dwz
[-- 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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-05-10 7:35 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-01 0:00 [PATCH] Honour errors when processing more than one file Tom de Vries
2019-01-01 0:00 ` Jakub Jelinek
2019-01-01 0:00 ` Tom de Vries
2019-01-01 0:00 ` Tom de Vries
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).