Don't munge yacc's #line directives

Message ID 1420678878-14065-1-git-send-email-patrick@parcs.ath.cx
State New, archived
Headers

Commit Message

Patrick Palka Jan. 8, 2015, 1:01 a.m. UTC
  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(-)
  

Comments

Pedro Alves Jan. 8, 2015, 12:02 p.m. UTC | #1
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:
>
  
Patrick Palka Jan. 8, 2015, 12:24 p.m. UTC | #2
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:
>>
>
>
  
Patrick Palka Jan. 8, 2015, 12:48 p.m. UTC | #3
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:
>>>
>>
>>
  
Pedro Alves Jan. 8, 2015, 2:06 p.m. UTC | #4
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
  
Patrick Palka Jan. 9, 2015, 10:23 p.m. UTC | #5
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
>
  

Patch

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: