diff mbox series

[libbacktrace] Add gen-xcoff-n.sh

Message ID 1874cf9d-656c-ddee-070b-ef60570f69ce@suse.de
State New
Headers show
Series [libbacktrace] Add gen-xcoff-n.sh | expand

Commit Message

Tom de Vries Jan. 28, 2019, 10:24 a.m. UTC
[ was: Re: [libbacktrace] Fix and simplify xcoff_%.c pattern rule ]

On 28-01-19 10:25, Tom de Vries 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.
> 

And looking over the rule again, I wondered if it would be more readable
if split off into a separate script file.

Is this follow-up patch OK for trunk?

Thanks,
- Tom

Comments

Ian Lance Taylor Jan. 28, 2019, 10:23 p.m. UTC | #1
On Mon, Jan 28, 2019 at 2:24 AM Tom de Vries <tdevries@suse.de> wrote:
>
> [ was: Re: [libbacktrace] Fix and simplify xcoff_%.c pattern rule ]
>
> On 28-01-19 10:25, Tom de Vries 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.
> >
>
> And looking over the rule again, I wondered if it would be more readable
> if split off into a separate script file.
>
> Is this follow-up patch OK for trunk?

I guess for this case I think it's OK to keep the lines in the Makefile.

Ian
diff mbox series

Patch

[Libbacktrace] Add gen-xcoff-n.sh

Factor out xcoff_%.c generation into gen-xcoff-n.c, getting rid of the
escaping of shell variables.

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

	* Makefile.am: Factor out xcoff_%.c generation ...
	* gen-xcoff-n.sh: ... here.  New file.
	* Makefile.in: Regenerate.

---
 libbacktrace/Makefile.am    |  8 +-------
 libbacktrace/Makefile.in    |  8 +-------
 libbacktrace/gen-xcoff-n.sh | 20 ++++++++++++++++++++
 3 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/libbacktrace/Makefile.am b/libbacktrace/Makefile.am
index 0d5b3193e25..43d0c9ffd73 100644
--- a/libbacktrace/Makefile.am
+++ b/libbacktrace/Makefile.am
@@ -104,13 +104,7 @@  libbacktrace_noformat_la_LIBADD = $(BACKTRACE_FILE) $(VIEW_FILE) $(ALLOC_FILE)
 libbacktrace_noformat_la_DEPENDENCIES = $(libbacktrace_noformat_la_LIBADD)
 
 xcoff_%.c: xcoff.c
-	SEARCH='#error "Unknown BACKTRACE_XCOFF_SIZE"'; \
-	REPLACE='#undef BACKTRACE_XCOFF_SIZE\
-	#define BACKTRACE_XCOFF_SIZE'; \
-	$(SED) "s/^$$SEARCH\$$/$$REPLACE $*/" \
-		$< \
-		> $@.tmp
-	mv $@.tmp $@
+	$(srcdir)/gen-xcoff-n.sh $(SED) $< $* $@
 
 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 b25ac92aeda..650392c2988 100644
--- a/libbacktrace/Makefile.in
+++ b/libbacktrace/Makefile.in
@@ -1750,13 +1750,7 @@  uninstall-am:
 
 
 @NATIVE_TRUE@xcoff_%.c: xcoff.c
-@NATIVE_TRUE@	SEARCH='#error "Unknown BACKTRACE_XCOFF_SIZE"'; \
-@NATIVE_TRUE@	REPLACE='#undef BACKTRACE_XCOFF_SIZE\
-@NATIVE_TRUE@	#define BACKTRACE_XCOFF_SIZE'; \
-@NATIVE_TRUE@	$(SED) "s/^$$SEARCH\$$/$$REPLACE $*/" \
-@NATIVE_TRUE@		$< \
-@NATIVE_TRUE@		> $@.tmp
-@NATIVE_TRUE@	mv $@.tmp $@
+@NATIVE_TRUE@	$(srcdir)/gen-xcoff-n.sh $(SED) $< $* $@
 
 @NATIVE_TRUE@instrumented_alloc.lo: alloc.c
 
diff --git a/libbacktrace/gen-xcoff-n.sh b/libbacktrace/gen-xcoff-n.sh
new file mode 100755
index 00000000000..c8825a9deba
--- /dev/null
+++ b/libbacktrace/gen-xcoff-n.sh
@@ -0,0 +1,20 @@ 
+#!/bin/sh
+
+sed="$1"
+src="$2"
+n="$3"
+dst="$4"
+
+tmp=$dst.tmp
+
+search='^#error "Unknown BACKTRACE_XCOFF_SIZE"$'
+
+replace='#undef BACKTRACE_XCOFF_SIZE\
+	#define BACKTRACE_XCOFF_SIZE '"$n"
+
+$sed \
+    "s/$search/$replace/" \
+    $src \
+    > $tmp
+
+mv $tmp $dst