[PATCH] Don't munge yacc's #line directives

[PATCH] Don't munge yacc's #line directives

[Patrick Palka]

The #line directives within GDB's autogenerated yacc files (e.g.
c-exp.c) are being munged by a dubious sed expression that is causing
these directives to refer to nonexistent source files.  As a result it
is currently not possible to debug these source files at source level.

The culprit sed expression was added by commit 954d8cae for non-obvious
reasons.  My guess is that the expression was added to work around a bug
in ylwrap which has since been fixed upstream: if I revert the November
2014 update to ylwrap, commit be3046511, then the culprit sed line no
longer causes the above mentioned issue.

So this patch removes the culprit sed script since it does not seem
needed anymore; the emitted #line directives look and work fine without
it.

gdb/ChangeLog:

2015-01-07  Patrick Palka  <patrick@parcs.ath.cx>

	* Makefile.in (.y.c): Don't munge yacc's #line
	directives.
---
 gdb/Makefile.in | 1 -
 1 file changed, 1 deletion(-)

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 31c8a4c..97d0045 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -1856,7 +1856,6 @@ po/$(PACKAGE).pot: force
 	     -e 's/\([ \t;,(]\)free\([ \t]*[&(),]\)/\1xfree\2/g' \
 	     -e 's/\([ \t;,(]\)free$$/\1xfree/g' \
 	     -e '/^#line.*y.tab.c/d' \
-	     -e "s/^\(#line.*\)`basename $<`/\1`echo $<|sed 's/\//\\\\\//g'`/" \
 	  < $@.tmp > $@
 	rm -f $@.tmp
 .l.c:
-- 
2.2.1.212.gc5b9256

Re: [PATCH] Don't munge yacc's #line directives

[Pedro Alves]

On 01/08/2015 01:01 AM, Patrick Palka wrote:
> The #line directives within GDB's autogenerated yacc files (e.g.
> c-exp.c) are being munged by a dubious sed expression that is causing
> these directives to refer to nonexistent source files.  As a result it
> is currently not possible to debug these source files at source level.
> 
> The culprit sed expression was added by commit 954d8cae for non-obvious
> reasons.  

That predates when we started putting more complete descriptions
in the commit log.  Did you look for the mailing list patch submission?
That should have included a description.  If not, then maybe Jan
recalls.  The expression refers to basename and slashes, it makes
me wonder whether this was build-in-srcdir vs build-out-of-srcdir
related.


> My guess is that the expression was added to work around a bug
> in ylwrap which has since been fixed upstream: if I revert the November
> 2014 update to ylwrap, commit be3046511, then the culprit sed line no

OOC, got url for that git repo?

Thanks,
Pedro Alves

> longer causes the above mentioned issue.
> 
> So this patch removes the culprit sed script since it does not seem
> needed anymore; the emitted #line directives look and work fine without
> it.
> 
> gdb/ChangeLog:
> 
> 2015-01-07  Patrick Palka  <patrick@parcs.ath.cx>
> 
> 	* Makefile.in (.y.c): Don't munge yacc's #line
> 	directives.
> ---
>  gdb/Makefile.in | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index 31c8a4c..97d0045 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -1856,7 +1856,6 @@ po/$(PACKAGE).pot: force
>  	     -e 's/\([ \t;,(]\)free\([ \t]*[&(),]\)/\1xfree\2/g' \
>  	     -e 's/\([ \t;,(]\)free$$/\1xfree/g' \
>  	     -e '/^#line.*y.tab.c/d' \
> -	     -e "s/^\(#line.*\)`basename $<`/\1`echo $<|sed 's/\//\\\\\//g'`/" \
>  	  < $@.tmp > $@
>  	rm -f $@.tmp
>  .l.c:
> 

Re: [PATCH] Don't munge yacc's #line directives

[Patrick Palka]

On Thu, Jan 8, 2015 at 7:02 AM, Pedro Alves <palves@redhat.com> wrote:
> On 01/08/2015 01:01 AM, Patrick Palka wrote:
>> The #line directives within GDB's autogenerated yacc files (e.g.
>> c-exp.c) are being munged by a dubious sed expression that is causing
>> these directives to refer to nonexistent source files.  As a result it
>> is currently not possible to debug these source files at source level.
>>
>> The culprit sed expression was added by commit 954d8cae for non-obvious
>> reasons.
>
> That predates when we started putting more complete descriptions
> in the commit log.  Did you look for the mailing list patch submission?
> That should have included a description.  If not, then maybe Jan
> recalls.  The expression refers to basename and slashes, it makes
> me wonder whether this was build-in-srcdir vs build-out-of-srcdir
> related.

Here is the corresponding ML listing:
https://sourceware.org/ml/gdb-patches/2010-11/msg00265.html
So it seems that ylwrap once used relative paths when referring to the
source yacc file instead of absolute paths, and the sed expression was
there to fix that.  But our copy of ylwrap doesn't seem to have this
problem anymore.

>
>
>> My guess is that the expression was added to work around a bug
>> in ylwrap which has since been fixed upstream: if I revert the November
>> 2014 update to ylwrap, commit be3046511, then the culprit sed line no
>
> OOC, got url for that git repo?

Oops, I truncated the commit hash too much.  The commit in question is
e30465112 from within the binutils-gdb repo.  The commit synced some
files with upstream automake whose git repo is
http://git.savannah.gnu.org/cgit/automake.git.

>
> Thanks,
> Pedro Alves
>
>> longer causes the above mentioned issue.
>>
>> So this patch removes the culprit sed script since it does not seem
>> needed anymore; the emitted #line directives look and work fine without
>> it.
>>
>> gdb/ChangeLog:
>>
>> 2015-01-07  Patrick Palka  <patrick@parcs.ath.cx>
>>
>>       * Makefile.in (.y.c): Don't munge yacc's #line
>>       directives.
>> ---
>>  gdb/Makefile.in | 1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
>> index 31c8a4c..97d0045 100644
>> --- a/gdb/Makefile.in
>> +++ b/gdb/Makefile.in
>> @@ -1856,7 +1856,6 @@ po/$(PACKAGE).pot: force
>>            -e 's/\([ \t;,(]\)free\([ \t]*[&(),]\)/\1xfree\2/g' \
>>            -e 's/\([ \t;,(]\)free$$/\1xfree/g' \
>>            -e '/^#line.*y.tab.c/d' \
>> -          -e "s/^\(#line.*\)`basename $<`/\1`echo $<|sed 's/\//\\\\\//g'`/" \
>>         < $@.tmp > $@
>>       rm -f $@.tmp
>>  .l.c:
>>
>
>

Re: [PATCH] Don't munge yacc's #line directives

[Patrick Palka]

On Thu, Jan 8, 2015 at 7:24 AM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> On Thu, Jan 8, 2015 at 7:02 AM, Pedro Alves <palves@redhat.com> wrote:
>> On 01/08/2015 01:01 AM, Patrick Palka wrote:
>>> The #line directives within GDB's autogenerated yacc files (e.g.
>>> c-exp.c) are being munged by a dubious sed expression that is causing
>>> these directives to refer to nonexistent source files.  As a result it
>>> is currently not possible to debug these source files at source level.
>>>
>>> The culprit sed expression was added by commit 954d8cae for non-obvious
>>> reasons.
>>
>> That predates when we started putting more complete descriptions
>> in the commit log.  Did you look for the mailing list patch submission?
>> That should have included a description.  If not, then maybe Jan
>> recalls.  The expression refers to basename and slashes, it makes
>> me wonder whether this was build-in-srcdir vs build-out-of-srcdir
>> related.
>
> Here is the corresponding ML listing:
> https://sourceware.org/ml/gdb-patches/2010-11/msg00265.html
> So it seems that ylwrap once used relative paths when referring to the
> source yacc file instead of absolute paths, and the sed expression was
> there to fix that.  But our copy of ylwrap doesn't seem to have this
> problem anymore.
>
>>
>>
>>> My guess is that the expression was added to work around a bug
>>> in ylwrap which has since been fixed upstream: if I revert the November
>>> 2014 update to ylwrap, commit be3046511, then the culprit sed line no
>>
>> OOC, got url for that git repo?
>
> Oops, I truncated the commit hash too much.  The commit in question is
> e30465112 from within the binutils-gdb repo.  The commit synced some
> files with upstream automake whose git repo is
> http://git.savannah.gnu.org/cgit/automake.git.

Here is the upstream automake patch that is probably responsible for
making the sed expression in question obsolete:
http://git.savannah.gnu.org/cgit/automake.git/commit/lib/ylwrap?id=b6359a5f310160c8a4a2e8e8c0105408412ce400

>
>>
>> Thanks,
>> Pedro Alves
>>
>>> longer causes the above mentioned issue.
>>>
>>> So this patch removes the culprit sed script since it does not seem
>>> needed anymore; the emitted #line directives look and work fine without
>>> it.
>>>
>>> gdb/ChangeLog:
>>>
>>> 2015-01-07  Patrick Palka  <patrick@parcs.ath.cx>
>>>
>>>       * Makefile.in (.y.c): Don't munge yacc's #line
>>>       directives.
>>> ---
>>>  gdb/Makefile.in | 1 -
>>>  1 file changed, 1 deletion(-)
>>>
>>> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
>>> index 31c8a4c..97d0045 100644
>>> --- a/gdb/Makefile.in
>>> +++ b/gdb/Makefile.in
>>> @@ -1856,7 +1856,6 @@ po/$(PACKAGE).pot: force
>>>            -e 's/\([ \t;,(]\)free\([ \t]*[&(),]\)/\1xfree\2/g' \
>>>            -e 's/\([ \t;,(]\)free$$/\1xfree/g' \
>>>            -e '/^#line.*y.tab.c/d' \
>>> -          -e "s/^\(#line.*\)`basename $<`/\1`echo $<|sed 's/\//\\\\\//g'`/" \
>>>         < $@.tmp > $@
>>>       rm -f $@.tmp
>>>  .l.c:
>>>
>>
>>

Re: [PATCH] Don't munge yacc's #line directives

[Pedro Alves]

On 01/08/2015 12:48 PM, Patrick Palka wrote:

> Here is the upstream automake patch that is probably responsible for
> making the sed expression in question obsolete:
> http://git.savannah.gnu.org/cgit/automake.git/commit/lib/ylwrap?id=b6359a5f310160c8a4a2e8e8c0105408412ce400

I find that a bit confusing, since the comment says:

 # We don't want the resulting debug information to point at
 # an absolute srcdir.

(the upstream master version still has the comment, though
reformatted).

In any case, I think we've done due diligence already.

Could you update the commit log with the new findings and
references, and please also include an before/after example
of a #line line?  With that, the patch is OK.

E.g., here's what I see, before/after your patch:

 -#line 36 "/home/pedro/gdb/mygit/src/gdb//home/pedro/gdb/mygit/src/gdb/ada-exp.y"
 +#line 36 "/home/pedro/gdb/mygit/src/gdb/ada-exp.y"

Thanks,
Pedro Alves

Re: [PATCH] Don't munge yacc's #line directives

[Patrick Palka]

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

On Thu, Jan 8, 2015 at 9:06 AM, Pedro Alves <palves@redhat.com> wrote:
> On 01/08/2015 12:48 PM, Patrick Palka wrote:
>
>> Here is the upstream automake patch that is probably responsible for
>> making the sed expression in question obsolete:
>> http://git.savannah.gnu.org/cgit/automake.git/commit/lib/ylwrap?id=b6359a5f310160c8a4a2e8e8c0105408412ce400
>
> I find that a bit confusing, since the comment says:
>
>  # We don't want the resulting debug information to point at
>  # an absolute srcdir.
>
> (the upstream master version still has the comment, though
> reformatted).
>
> In any case, I think we've done due diligence already.
>
> Could you update the commit log with the new findings and
> references, and please also include an before/after example
> of a #line line?  With that, the patch is OK.
>
> E.g., here's what I see, before/after your patch:
>
>  -#line 36 "/home/pedro/gdb/mygit/src/gdb//home/pedro/gdb/mygit/src/gdb/ada-exp.y"
>  +#line 36 "/home/pedro/gdb/mygit/src/gdb/ada-exp.y"

Attached is what I committed.

>
> Thanks,
> Pedro Alves
>

[-- Attachment #2: 0001-Don-t-munge-yacc-s-line-directives.patch --]
[-- Type: application/octet-stream, Size: 2414 bytes --]

From 6bf045cd32d07ae55d7eec8ff94bd937c6bb2bce Mon Sep 17 00:00:00 2001
From: Patrick Palka <patrick@parcs.ath.cx>
Date: Sun, 30 Nov 2014 11:47:16 -0500
Subject: [PATCH] Don't munge yacc's #line directives

The #line directives within GDB's autogenerated yacc files (e.g.
c-exp.c) are being incorrectly munged, causing these directives to refer
to nonexistent source files, e.g.

 #line 36 "/home/patrick/binutils-gdb/gdb//home/patrick/binutils-gdb/gdb/c-exp.y"

as opposed to

  #line 36 "/home/patrick/binutils-gdb/gdb/c-exp.y"

The munging happens due to a sed expression added by commit 954d8cae
whose intended purpose[1] was to work around the fact that ylwrap emitted #line
directives without any directory information, e.g.

  #line 36 "c-exp.y"

So the sed expression was meant to munge such directives to refer to
absolute paths instead.  But the behavior of ylwrap was changed some
years ago[2] to emit absolute paths within #line directives.  And when
our local copy of ylwrap was synced by commit e30465112, the sed
expression in question became unnecessary, and indeed harmful.

This patch removes the now-obsolete sed expression.  The emitted #line
directives are now correct without it.

gdb/ChangeLog:

	* Makefile.in (.y.c): Don't munge yacc's #line
	directives.

[1]: https://sourceware.org/ml/gdb-patches/2010-11/msg00265.html
[2]: http://git.savannah.gnu.org/cgit/automake.git/commit/lib/ylwrap?id=b6359a5f3
---
 gdb/ChangeLog   | 5 +++++
 gdb/Makefile.in | 1 -
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 2e9816a..c830053 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2015-01-09  Patrick Palka  <patrick@parcs.ath.cx>
 
+	* Makefile.in (.y.c): Don't munge yacc's #line
+	directives.
+
+2015-01-09  Patrick Palka  <patrick@parcs.ath.cx>
+
 	* utils.c (defaulted_query): Rewrite to use gdb_readline_wrapper
 	to prompt for input.
 	* tui/tui-hooks.c (tui_query_hook): Remove.
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 5dae3e6..86ab1be 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -1858,7 +1858,6 @@ po/$(PACKAGE).pot: force
 	     -e 's/\([ \t;,(]\)free\([ \t]*[&(),]\)/\1xfree\2/g' \
 	     -e 's/\([ \t;,(]\)free$$/\1xfree/g' \
 	     -e '/^#line.*y.tab.c/d' \
-	     -e "s/^\(#line.*\)`basename $<`/\1`echo $<|sed 's/\//\\\\\//g'`/" \
 	  < $@.tmp > $@
 	rm -f $@.tmp
 .l.c:
-- 
2.2.1.212.gc5b9256