[libbacktrace] Fix and simplify xcoff_%.c pattern rule

Message ID 8e0a3f07-3326-b4f8-f1e4-64ac1f5c7aed@suse.de
State New
Headers show
Series
  • [libbacktrace] Fix and simplify xcoff_%.c pattern rule
Related show

Commit Message

Tom de Vries Jan. 28, 2019, 9:25 a.m.
[ was: Re: [backtrace] Avoid segfault  ]
On 27-01-19 22:53, Ian Lance Taylor wrote:
> On Sun, Jan 27, 2019 at 1:16 PM Tom de Vries <tdevries@suse.de> wrote:

>>

>> On 25-01-19 18:15, Nathan Sidwell wrote:

>>> On 1/25/19 5:28 AM, Tom de Vries wrote:

>>>>

>>>> This patch fixes it by passing "" instead of NULL, in the call to

>>>> elf_add at line 3083 (for .gnu_debugaltlink), not the call to elf_add at

>>>> line 3044 (for .gnu_debuglink) mentioned above.

>>>>

>>>> Nathan, does this fix the problem for you? If not, can you provide a

>>>> reproducer, or give a hint on how one could be constructed?

>>>

>>> I still hit the problem, and am installing this as sufficiently obvious.

>>>  I'm on a fedora system debugging pr88995.  The debuglink_name is

>>> "../../.dwz/isl-0.16.1-7.fc29.x86_64"

>>>

>>

>> I've managed to reproduce this segfault instance by adding a test-case

>> that uses both build-id and dwz.

>>

>> OK for trunk?

> 

>> +elf_for_test.c: elf.c

>> + PWD=$$(pwd -P); \

>> + BUILD_ID_DIR="usr/lib/debug/.build-id/"; \

>> + SEARCH='#define SYSTEM_BUILD_ID_DIR'; \

>> + REPLACE="#define SYSTEM_BUILD_ID_DIR \"$$PWD/$$BUILD_ID_DIR\""; \

>> + $(SED) "s%^$$SEARCH.*\$$%$$REPLACE%" \

>> + $< \

>> + > $@

> 

> You need to use a temporary file, such as $@.tmp, for the final sed

> command, followed by a mv to $@.  Otherwise a failure in the sed will

> leave what appears to be an up to date file.


I noticed the same problem in the xcoff_%.c pattern rule.

OK for trunk?

Thanks,
- Tom

Comments

Ian Lance Taylor Jan. 28, 2019, 6:31 p.m. | #1
On Mon, Jan 28, 2019 at 1:25 AM Tom de Vries <tdevries@suse.de> wrote:
>

> [ was: Re: [backtrace] Avoid segfault  ]

> On 27-01-19 22:53, Ian Lance Taylor wrote:

> > On Sun, Jan 27, 2019 at 1:16 PM Tom de Vries <tdevries@suse.de> wrote:

> >>

> >> On 25-01-19 18:15, Nathan Sidwell wrote:

> >>> On 1/25/19 5:28 AM, Tom de Vries wrote:

> >>>>

> >>>> This patch fixes it by passing "" instead of NULL, in the call to

> >>>> elf_add at line 3083 (for .gnu_debugaltlink), not the call to elf_add at

> >>>> line 3044 (for .gnu_debuglink) mentioned above.

> >>>>

> >>>> Nathan, does this fix the problem for you? If not, can you provide a

> >>>> reproducer, or give a hint on how one could be constructed?

> >>>

> >>> I still hit the problem, and am installing this as sufficiently obvious.

> >>>  I'm on a fedora system debugging pr88995.  The debuglink_name is

> >>> "../../.dwz/isl-0.16.1-7.fc29.x86_64"

> >>>

> >>

> >> I've managed to reproduce this segfault instance by adding a test-case

> >> that uses both build-id and dwz.

> >>

> >> OK for trunk?

> >

> >> +elf_for_test.c: elf.c

> >> + PWD=$$(pwd -P); \

> >> + BUILD_ID_DIR="usr/lib/debug/.build-id/"; \

> >> + SEARCH='#define SYSTEM_BUILD_ID_DIR'; \

> >> + REPLACE="#define SYSTEM_BUILD_ID_DIR \"$$PWD/$$BUILD_ID_DIR\""; \

> >> + $(SED) "s%^$$SEARCH.*\$$%$$REPLACE%" \

> >> + $< \

> >> + > $@

> >

> > You need to use a temporary file, such as $@.tmp, for the final sed

> > command, followed by a mv to $@.  Otherwise a failure in the sed will

> > leave what appears to be an up to date file.

>

> I noticed the same problem in the xcoff_%.c pattern rule.

>

> OK for trunk?


This is OK.

Thanks.

Ian

Patch

[libbacktrace] Fix and simplify xcoff_%.c pattern rule

When generating xcoff_%.c, the last command is a sed command.  In case of a
sed failure, this will leave an incomplete file, which will appear as up to
date to make, so consequently it will not be regenerated.  Fix this by
sedding into a temporary file instead.

Also, use $< to access the prerequisite xcoff.c, instead of spelling out the
file name once more.

2019-01-28  Tom de Vries  <tdevries@suse.de>

	* Makefile.am (xcoff_%.c): Generate sed result into temporary file.
	Use $< to access prerequisite.
	* Makefile.in: Regenerate.

---
 libbacktrace/Makefile.am | 5 +++--
 libbacktrace/Makefile.in | 5 +++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/libbacktrace/Makefile.am b/libbacktrace/Makefile.am
index 997a535dff4..0d5b3193e25 100644
--- a/libbacktrace/Makefile.am
+++ b/libbacktrace/Makefile.am
@@ -108,8 +108,9 @@  xcoff_%.c: xcoff.c
 	REPLACE='#undef BACKTRACE_XCOFF_SIZE\
 	#define BACKTRACE_XCOFF_SIZE'; \
 	$(SED) "s/^$$SEARCH\$$/$$REPLACE $*/" \
-		$(srcdir)/xcoff.c \
-		> $@
+		$< \
+		> $@.tmp
+	mv $@.tmp $@
 
 test_elf_SOURCES = test_format.c testlib.c
 test_elf_LDADD = libbacktrace_noformat.la elf.lo
diff --git a/libbacktrace/Makefile.in b/libbacktrace/Makefile.in
index f04577066f8..b25ac92aeda 100644
--- a/libbacktrace/Makefile.in
+++ b/libbacktrace/Makefile.in
@@ -1754,8 +1754,9 @@  uninstall-am:
 @NATIVE_TRUE@	REPLACE='#undef BACKTRACE_XCOFF_SIZE\
 @NATIVE_TRUE@	#define BACKTRACE_XCOFF_SIZE'; \
 @NATIVE_TRUE@	$(SED) "s/^$$SEARCH\$$/$$REPLACE $*/" \
-@NATIVE_TRUE@		$(srcdir)/xcoff.c \
-@NATIVE_TRUE@		> $@
+@NATIVE_TRUE@		$< \
+@NATIVE_TRUE@		> $@.tmp
+@NATIVE_TRUE@	mv $@.tmp $@
 
 @NATIVE_TRUE@instrumented_alloc.lo: alloc.c