public inbox for archer@sourceware.org
 help / color / mirror / Atom feed
* find-debuginfo.sh change for gdb index
@ 2010-06-29 22:32 Tom Tromey
  2010-06-29 23:21 ` Roland McGrath
  0 siblings, 1 reply; 27+ messages in thread
From: Tom Tromey @ 2010-06-29 22:32 UTC (permalink / raw)
  To: Project Archer; +Cc: pmatilai

Hi Panu.  Roland suggested I contact you as the Fedora RPM maintainer.

This patch adds gdb index creation to find-debuginfo.sh, in support of a
feature I'm proposing for Fedora 14:

    https://fedoraproject.org/wiki/Features/GdbIndex

I've tested this just by building some RPMs locally.

If you'd prefer, I can open a bug in bugzilla for this.

I don't know what other changes may be needed to ensure that the proper
gdb is in the buildroots when this script is run.  Also, the proper gdb
is not actually available yet; cleaning up that patch series is my next
task.

I think this should probably be local to Fedora, but if you think it
should go into upstream RPM, I am happy to try that.

Tom

--- find-debuginfo.sh.orig	2010-06-29 16:19:42.000000000 -0600
+++ find-debuginfo.sh	2010-06-29 15:52:08.000000000 -0600
@@ -96,6 +96,15 @@
   chmod 444 "$1" || exit
 }
 
+# Create a gdb .index file for $1.
+make_gdb_index()
+{
+  local f="$1"
+  local d="${f%/*}"
+  # We don't care if gdb gives an error.
+  gdb --batch-silent -ex "file $f" -ex "maintenance save-gnu-index $d" > /dev/null 2>&1
+}
+
 # Make a relative symlink to $1 called $3$2
 shopt -s extglob
 link_relative()
@@ -224,9 +233,15 @@
     chmod u-w "$f"
   fi
 
+  make_gdb_index "$debugfn"
+
   if [ -n "$id" ]; then
     make_id_link "$id" "$dn/$(basename $f)"
     make_id_link "$id" "/usr/lib/debug$dn/$bn" .debug
+
+    if [ -f "${debugfn}.index" ]; then
+      make_id_link "$id" "/usr/lib/debug$dn/$bn" .debug
+    fi
   fi
 done || exit
 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: find-debuginfo.sh change for gdb index
  2010-06-29 22:32 find-debuginfo.sh change for gdb index Tom Tromey
@ 2010-06-29 23:21 ` Roland McGrath
  2010-06-30 17:35   ` Tom Tromey
  0 siblings, 1 reply; 27+ messages in thread
From: Roland McGrath @ 2010-06-29 23:21 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Project Archer, pmatilai

> I don't know what other changes may be needed to ensure that the proper
> gdb is in the buildroots when this script is run.  Also, the proper gdb
> is not actually available yet; cleaning up that patch series is my next
> task.

The rpm-build subpackage will need "Requires: gdb >= V-R".  Obviously, the
patch can't go in anywhere until that gdb is built in dist-rawhide.

> I think this should probably be local to Fedora, but if you think it
> should go into upstream RPM, I am happy to try that.

It's certainly Fedora-specific and only for Fedora 14.

> +  gdb --batch-silent -ex "file $f" -ex "maintenance save-gnu-index $d" > /dev/null 2>&1

I don't quite understand what file this writes to.
Is it implicitly "<symfile name>.index" in the argument directory?

IMHO, the file name should have "gdb" in the name.  
This is really not any very generic sort of index for the information.

>    if [ -n "$id" ]; then
>      make_id_link "$id" "$dn/$(basename $f)"
>      make_id_link "$id" "/usr/lib/debug$dn/$bn" .debug
> +
> +    if [ -f "${debugfn}.index" ]; then
> +      make_id_link "$id" "/usr/lib/debug$dn/$bn" .debug
> +    fi

What's this for?  It just repeats the work of making and recording the
build-id symlink to the .debug file.  Unless you're being quite subtle
somehow I've missed, this doesn't do anything with the index file.
Do you mean something like:

	make_id_link "$id" "/usr/lib/debug$dn/$bn" .index

?  That gets you a /usr/lib/debug/.build-id/xx/yyy.index symlink
to ../../usr/bin/foobar.index for example.


Thanks,
Roland

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: find-debuginfo.sh change for gdb index
  2010-06-29 23:21 ` Roland McGrath
@ 2010-06-30 17:35   ` Tom Tromey
  2010-06-30 18:14     ` Roland McGrath
  0 siblings, 1 reply; 27+ messages in thread
From: Tom Tromey @ 2010-06-30 17:35 UTC (permalink / raw)
  To: Roland McGrath; +Cc: Project Archer, pmatilai

>>>>> "Roland" == Roland McGrath <roland@redhat.com> writes:

Roland> I don't quite understand what file this writes to.
Roland> Is it implicitly "<symfile name>.index" in the argument directory?

Yes.

Roland> IMHO, the file name should have "gdb" in the name.  
Roland> This is really not any very generic sort of index for the information.

Ok.  What do you think of just ".gdb as the suffix?

>> +    if [ -f "${debugfn}.index" ]; then
>> +      make_id_link "$id" "/usr/lib/debug$dn/$bn" .debug
>> +    fi

Roland> What's this for?  It just repeats the work of making and recording the
Roland> build-id symlink to the .debug file.  Unless you're being quite subtle
Roland> somehow I've missed, this doesn't do anything with the index file.
Roland> Do you mean something like:

Roland> 	make_id_link "$id" "/usr/lib/debug$dn/$bn" .index

Roland> ?  That gets you a /usr/lib/debug/.build-id/xx/yyy.index symlink
Roland> to ../../usr/bin/foobar.index for example.

Yeah, oops.  I will fix this.

Tom

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: find-debuginfo.sh change for gdb index
  2010-06-30 17:35   ` Tom Tromey
@ 2010-06-30 18:14     ` Roland McGrath
  2010-06-30 18:23       ` Tom Tromey
  0 siblings, 1 reply; 27+ messages in thread
From: Roland McGrath @ 2010-06-30 18:14 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Project Archer, pmatilai

> Roland> I don't quite understand what file this writes to.
> Roland> Is it implicitly "<symfile name>.index" in the argument directory?
> 
> Yes.

Ok.  IMHO it would be better if the command just took the output file name
explicitly.  But whatever.

> Roland> IMHO, the file name should have "gdb" in the name.  
> Roland> This is really not any very generic sort of index for the information.
> 
> Ok.  What do you think of just ".gdb as the suffix?

I don't object, though I'd go with something like '.gdb-index' instead.


Thanks,
Roland

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: find-debuginfo.sh change for gdb index
  2010-06-30 18:14     ` Roland McGrath
@ 2010-06-30 18:23       ` Tom Tromey
  2010-06-30 20:42         ` Tom Tromey
  0 siblings, 1 reply; 27+ messages in thread
From: Tom Tromey @ 2010-06-30 18:23 UTC (permalink / raw)
  To: Roland McGrath; +Cc: Project Archer, pmatilai

>>>>> "Roland" == Roland McGrath <roland@redhat.com> writes:

Roland> I don't quite understand what file this writes to.
Roland> Is it implicitly "<symfile name>.index" in the argument directory?

>> Yes.

Roland> Ok.  IMHO it would be better if the command just took the output
Roland> file name explicitly.  But whatever.

It saves indices for all the loaded objfiles.
I suppose I could add another argument so you can specify which one.

Roland> I don't object, though I'd go with something like '.gdb-index' instead.

Ok, I'll do that.

Tom

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: find-debuginfo.sh change for gdb index
  2010-06-30 18:23       ` Tom Tromey
@ 2010-06-30 20:42         ` Tom Tromey
  2010-06-30 20:44           ` Roland McGrath
  0 siblings, 1 reply; 27+ messages in thread
From: Tom Tromey @ 2010-06-30 20:42 UTC (permalink / raw)
  To: Roland McGrath; +Cc: Project Archer, pmatilai

Tom> It saves indices for all the loaded objfiles.
Tom> I suppose I could add another argument so you can specify which one.

I left this as-is.

I renamed the command, though, since a different name made more sense to
me once I wrote the gdb documentation.

Here's a new find-debuginfo.sh patch.

Tom

--- find-debuginfo.sh.orig	2010-06-29 16:19:42.000000000 -0600
+++ find-debuginfo.sh	2010-06-30 14:41:19.000000000 -0600
@@ -96,6 +96,15 @@
   chmod 444 "$1" || exit
 }
 
+# Create a .gdb-index file for $1.
+make_gdb_index()
+{
+  local f="$1"
+  local d="${f%/*}"
+  # We don't care if gdb gives an error.
+  gdb --batch-silent -ex "file $f" -ex "maintenance save-gdb-index $d" > /dev/null 2>&1
+}
+
 # Make a relative symlink to $1 called $3$2
 shopt -s extglob
 link_relative()
@@ -224,6 +233,8 @@
     chmod u-w "$f"
   fi
 
+  make_gdb_index "$debugfn"
+
   if [ -n "$id" ]; then
     make_id_link "$id" "$dn/$(basename $f)"
     make_id_link "$id" "/usr/lib/debug$dn/$bn" .debug

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: find-debuginfo.sh change for gdb index
  2010-06-30 20:42         ` Tom Tromey
@ 2010-06-30 20:44           ` Roland McGrath
  2010-06-30 21:25             ` Tom Tromey
  0 siblings, 1 reply; 27+ messages in thread
From: Roland McGrath @ 2010-06-30 20:44 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Project Archer, pmatilai

> Here's a new find-debuginfo.sh patch.

So you don't need a build-id symlink for the index after all?
Where does gdb look for an index file?

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: find-debuginfo.sh change for gdb index
  2010-06-30 20:44           ` Roland McGrath
@ 2010-06-30 21:25             ` Tom Tromey
  2010-06-30 21:58               ` Tom Tromey
  2010-06-30 22:14               ` Roland McGrath
  0 siblings, 2 replies; 27+ messages in thread
From: Tom Tromey @ 2010-06-30 21:25 UTC (permalink / raw)
  To: Roland McGrath; +Cc: Project Archer, pmatilai

>>>>> "Roland" == Roland McGrath <roland@redhat.com> writes:

Tom> Here's a new find-debuginfo.sh patch.

Roland> So you don't need a build-id symlink for the index after all?

I thought not, but I am doing a much fuller check now.

Roland> Where does gdb look for an index file?

For a symbol file X, it looks for X.gdb-index.

Tom

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: find-debuginfo.sh change for gdb index
  2010-06-30 21:25             ` Tom Tromey
@ 2010-06-30 21:58               ` Tom Tromey
  2010-06-30 22:00                 ` Tom Tromey
  2010-06-30 22:14               ` Roland McGrath
  1 sibling, 1 reply; 27+ messages in thread
From: Tom Tromey @ 2010-06-30 21:58 UTC (permalink / raw)
  To: Roland McGrath; +Cc: Project Archer, pmatilai

Roland> So you don't need a build-id symlink for the index after all?

Tom> I thought not, but I am doing a much fuller check now.

I cleaned out all my old index files from /usr/lib/debug.
Then I re-built the index files -- but not for .debug files in
/usr/lib/debug/.build-id.

I think something in gdb must be realpath-ing the .build-id links.
A casual search didn't turn it up, though.

It may still be prudent to make the links, I don't know.
Maybe Jan has more insight here.

Tom

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: find-debuginfo.sh change for gdb index
  2010-06-30 21:58               ` Tom Tromey
@ 2010-06-30 22:00                 ` Tom Tromey
  0 siblings, 0 replies; 27+ messages in thread
From: Tom Tromey @ 2010-06-30 22:00 UTC (permalink / raw)
  To: Roland McGrath; +Cc: Project Archer, pmatilai

Tom> I cleaned out all my old index files from /usr/lib/debug.
Tom> Then I re-built the index files -- but not for .debug files in
Tom> /usr/lib/debug/.build-id.

I forgot to say: then I attached to a running OO.o, and used "maint
print objfiles" to see whether the index was in use for each objfile.
It was.

Tom

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: find-debuginfo.sh change for gdb index
  2010-06-30 21:25             ` Tom Tromey
  2010-06-30 21:58               ` Tom Tromey
@ 2010-06-30 22:14               ` Roland McGrath
  2010-07-02 19:55                 ` Tom Tromey
  1 sibling, 1 reply; 27+ messages in thread
From: Roland McGrath @ 2010-06-30 22:14 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Project Archer, pmatilai

> For a symbol file X, it looks for X.gdb-index.

It's not clear to me what that means in the separate .debug case.
If "symbol file" means the file with the DWARF, then that file is
foo.debug so you will be looking for foo.debug.gdb-index.

A good way to deal with the build-id symlink is to canonicalize_file_name
(aka realpath) them and call the symlink target the real file name (for
user display of a useful file name too).  If the gdb build-id support is
doing that, then foo.debug.gdb-index should be found.

I think it is preferable for the packaging not to add the new symlinks.
It doesn't seem like we really need them, since you can look at the
.build-id/xx/yyy.debug symlink target name instead.
It just adds a lot more crapola to each rpm.


Thanks,
Roland

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: find-debuginfo.sh change for gdb index
  2010-06-30 22:14               ` Roland McGrath
@ 2010-07-02 19:55                 ` Tom Tromey
  2010-07-05  9:36                   ` Panu Matilainen
  0 siblings, 1 reply; 27+ messages in thread
From: Tom Tromey @ 2010-07-02 19:55 UTC (permalink / raw)
  To: Roland McGrath; +Cc: Project Archer, pmatilai

Tom> For a symbol file X, it looks for X.gdb-index.

Roland> It's not clear to me what that means in the separate .debug case.
Roland> If "symbol file" means the file with the DWARF, then that file is
Roland> foo.debug so you will be looking for foo.debug.gdb-index.

Right, that is what we do.

Roland> I think it is preferable for the packaging not to add the new symlinks.
Roland> It doesn't seem like we really need them, since you can look at the
Roland> .build-id/xx/yyy.debug symlink target name instead.

Sounds good.

In this case I think the most recent patch I sent is the one to use.

Tom

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: find-debuginfo.sh change for gdb index
  2010-07-02 19:55                 ` Tom Tromey
@ 2010-07-05  9:36                   ` Panu Matilainen
  2010-07-05  9:48                     ` Jan Kratochvil
  2010-07-06 18:35                     ` Tom Tromey
  0 siblings, 2 replies; 27+ messages in thread
From: Panu Matilainen @ 2010-07-05  9:36 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Roland McGrath, Project Archer

On Fri, 2 Jul 2010, Tom Tromey wrote:

> Tom> For a symbol file X, it looks for X.gdb-index.
>
> Roland> It's not clear to me what that means in the separate .debug case.
> Roland> If "symbol file" means the file with the DWARF, then that file is
> Roland> foo.debug so you will be looking for foo.debug.gdb-index.
>
> Right, that is what we do.
>
> Roland> I think it is preferable for the packaging not to add the new symlinks.
> Roland> It doesn't seem like we really need them, since you can look at the
> Roland> .build-id/xx/yyy.debug symlink target name instead.
>
> Sounds good.
>
> In this case I think the most recent patch I sent is the one to use.

So it would be this patch, right?

--- find-debuginfo.sh.orig	2010-06-29 16:19:42.000000000 -0600
+++ find-debuginfo.sh	2010-06-30 14:41:19.000000000 -0600
@@ -96,6 +96,15 @@
    chmod 444 "$1" || exit
  }

+# Create a .gdb-index file for $1.
+make_gdb_index()
+{
+  local f="$1"
+  local d="${f%/*}"
+  # We don't care if gdb gives an error.
+  gdb --batch-silent -ex "file $f" -ex "maintenance save-gdb-index $d" > /dev/null 2>&1
+}
+
  # Make a relative symlink to $1 called $3$2
  shopt -s extglob
  link_relative()
@@ -224,6 +233,8 @@
      chmod u-w "$f"
    fi

+  make_gdb_index "$debugfn"
+
    if [ -n "$id" ]; then
      make_id_link "$id" "$dn/$(basename $f)"
      make_id_link "$id" "/usr/lib/debug$dn/$bn" .debug

Is the necessary patch(es) already in rawhide gdb, I dont see anything 
obviously related in gdb changelogs?

One thing this does is that it forces rpm-build to depend on gdb, which 
hardly is the end of the world, just something to note.

 	- Panu -

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: find-debuginfo.sh change for gdb index
  2010-07-05  9:36                   ` Panu Matilainen
@ 2010-07-05  9:48                     ` Jan Kratochvil
  2010-07-05 10:39                       ` Panu Matilainen
  2010-07-06 18:35                     ` Tom Tromey
  1 sibling, 1 reply; 27+ messages in thread
From: Jan Kratochvil @ 2010-07-05  9:48 UTC (permalink / raw)
  To: Panu Matilainen; +Cc: Tom Tromey, Roland McGrath, Project Archer

On Mon, 05 Jul 2010 11:36:32 +0200, Panu Matilainen wrote:
> Is the necessary patch(es) already in rawhide gdb, I dont see
> anything obviously related in gdb changelogs?

It will be before the FeatureFreeze.  I will ping you to coordinate with rpm.
So far it is only posted upstream:
	http://sourceware.org/ml/gdb-patches/2010-06/msg00696.html


Thanks,
Jan

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: find-debuginfo.sh change for gdb index
  2010-07-05  9:48                     ` Jan Kratochvil
@ 2010-07-05 10:39                       ` Panu Matilainen
  0 siblings, 0 replies; 27+ messages in thread
From: Panu Matilainen @ 2010-07-05 10:39 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: Tom Tromey, Roland McGrath, Project Archer

On Mon, 5 Jul 2010, Jan Kratochvil wrote:

> On Mon, 05 Jul 2010 11:36:32 +0200, Panu Matilainen wrote:
>> Is the necessary patch(es) already in rawhide gdb, I dont see
>> anything obviously related in gdb changelogs?
>
> It will be before the FeatureFreeze.  I will ping you to coordinate with rpm.
> So far it is only posted upstream:
> 	http://sourceware.org/ml/gdb-patches/2010-06/msg00696.html

Well, I'm starting a long vacation at the end of this week so you might 
need to ping Jindrich Novy and/or Florian Festi instead.

OTOH if we can reasonably expect the details wrt find-debuginfo to stay 
unchanged we can just apply the patch already (AFAICT it ignores errors 
anyway and wont cause build-failures)

 	- Panu -

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: find-debuginfo.sh change for gdb index
  2010-07-05  9:36                   ` Panu Matilainen
  2010-07-05  9:48                     ` Jan Kratochvil
@ 2010-07-06 18:35                     ` Tom Tromey
  2010-07-06 19:14                       ` Roland McGrath
  1 sibling, 1 reply; 27+ messages in thread
From: Tom Tromey @ 2010-07-06 18:35 UTC (permalink / raw)
  To: Panu Matilainen; +Cc: Roland McGrath, Project Archer

>>>>> "Panu" == Panu Matilainen <pmatilai@redhat.com> writes:

Panu> So it would be this patch, right?

Yes.

Panu> Is the necessary patch(es) already in rawhide gdb, I dont see anything
Panu> obviously related in gdb changelogs?

Yeah, it is still under discussion upstream.

One thing that came up is that the current approach of using file
size+mtime to determine whether the index is valid is not super.
Two competing ideas:

1. Put the build-id into the index file, then verify it.
2. Require the index to be a section in the ELF object.
   This has the nice property that no validation need be done.
   However, it would require a further tweak to find-debuginfo.sh.

Any comments?
I would like to resolve this ASAP. 

Tom

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: find-debuginfo.sh change for gdb index
  2010-07-06 18:35                     ` Tom Tromey
@ 2010-07-06 19:14                       ` Roland McGrath
  2010-07-06 19:41                         ` Tom Tromey
  2010-07-08 15:56                         ` Tom Tromey
  0 siblings, 2 replies; 27+ messages in thread
From: Roland McGrath @ 2010-07-06 19:14 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Panu Matilainen, Project Archer

> One thing that came up is that the current approach of using file
> size+mtime to determine whether the index is valid is not super.

Agreed.

> Two competing ideas:
> 
> 1. Put the build-id into the index file, then verify it.

That is the method already used for .debug files today.  (The old
.gnu_debuglink CRC32 check should be ignored when there are build IDs.
I don't know about GDB, but libdw ignores it when there is a build ID.)

> 2. Require the index to be a section in the ELF object.

This is the most obvious way to handle this information.  The only real
down-sides are a) possible bugs breaking the original file and b) somewhat
more hair later if the index format has to be redone and only data
reindexed.  I don't really think either of those is something to worry much
about.

Doing this adds another interlocking requirement.  The eu-strip (libebl)
code for -g matches the individual DWARF sections by name.  (Without -g, it
strips all non-allocated sections, but for -g it only strips the sections
with recognized names.)  So that requires a (one-line) change in elfutils
for the new section name, and we'll need the new elfutils release in
buildroots before the new index-adding procedure goes in.  (It shouldn't be
any problem to push this out quickly, but it needs to go on your checklist
so we coordinate it.)

>    This has the nice property that no validation need be done.

Put another way, it implicitly takes advantage of the existing validation
mechanisms.  If you modify the unstripped file before strip-to-file, then
even the CRC32 will be correct.  (This should not really be a concern one
way or the other for Fedora, where both --build-id is always used and all
the tools should be validating by IDs rather than CRCs.  But for generic
goodness of the scheme, it is attractive in not introducing a new wrinkle
about the CRC calculation.)

>    However, it would require a further tweak to find-debuginfo.sh.

Or perhaps less change there, after a fashion.  The place to put the new
work is either right after debugedit or right before eu-strip -f.  Off hand
I think it should be the former.  That will put an index into e.g. the
unstripped vmlinux, which gets copied into /usr/lib/debug but never passed
to eu-strip.

So one approach would be to replace the debugedit invocation with the use
of another shell script.  Then that new script can run debugedit and make
the gdb index.  It can be fiddled as needed without changing the core logic
in find-debuginfo.sh again.  We could possibly let that script be supplied
by the redhat-rpm-config rpm or the gdb rpm or something else, so that its
maintenance is not directly in the path of the rpm package maintainers.


Thanks,
Roland

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: find-debuginfo.sh change for gdb index
  2010-07-06 19:14                       ` Roland McGrath
@ 2010-07-06 19:41                         ` Tom Tromey
  2010-07-06 20:28                           ` Roland McGrath
  2010-07-08 15:56                         ` Tom Tromey
  1 sibling, 1 reply; 27+ messages in thread
From: Tom Tromey @ 2010-07-06 19:41 UTC (permalink / raw)
  To: Roland McGrath; +Cc: Panu Matilainen, Project Archer

Roland> Doing this adds another interlocking requirement.  The eu-strip
Roland> (libebl) code for -g matches the individual DWARF sections by
Roland> name.  (Without -g, it strips all non-allocated sections, but
Roland> for -g it only strips the sections with recognized names.)  So
Roland> that requires a (one-line) change in elfutils for the new
Roland> section name, and we'll need the new elfutils release in
Roland> buildroots before the new index-adding procedure goes in.  (It
Roland> shouldn't be any problem to push this out quickly, but it needs
Roland> to go on your checklist so we coordinate it.)

I was thinking we would just objcopy the data into the .debug file in
find-debuginfo.sh.

Is there an advantage to doing it before the stripping?

Tom

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: find-debuginfo.sh change for gdb index
  2010-07-06 19:41                         ` Tom Tromey
@ 2010-07-06 20:28                           ` Roland McGrath
  0 siblings, 0 replies; 27+ messages in thread
From: Roland McGrath @ 2010-07-06 20:28 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Panu Matilainen, Project Archer

> I was thinking we would just objcopy the data into the .debug file in
> find-debuginfo.sh.
> 
> Is there an advantage to doing it before the stripping?

I stated two:

1. CRC32 in .gnu_debuglink will be correct.
   
2. Do it to those that don't get stripped.  
   (AFAIK the only case is vmlinux.)

I am not especially concerned about either of those issues.  But they point
to it probably being the more right or more clean approach.

I am far more concerned about the unknown potential problems.
For example, making sure that eu-unstrip can still recombine the files.
(I think that will be fine, but it's something to worry about.)

In the past we have had various problems with objcopy/bfd deciding to
gratuitously regenerate things and breaking the data along the way.
I believe all the old issues we ever noticed have been fixed by now.
But it certainly makes me nervous.  I trust eu-strip far more.


Thanks,
Roland

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: find-debuginfo.sh change for gdb index
  2010-07-06 19:14                       ` Roland McGrath
  2010-07-06 19:41                         ` Tom Tromey
@ 2010-07-08 15:56                         ` Tom Tromey
  2010-07-08 20:36                           ` Tom Tromey
  2010-07-08 22:53                           ` Roland McGrath
  1 sibling, 2 replies; 27+ messages in thread
From: Tom Tromey @ 2010-07-08 15:56 UTC (permalink / raw)
  To: Roland McGrath; +Cc: Panu Matilainen, Project Archer

Roland> So one approach would be to replace the debugedit invocation
Roland> with the use of another shell script.

Here is a patch to just do the work directly in find-debuginfo.sh.  This
seemed simpler to me, but if you and Panu want a new script, I will do
that.

It would perhaps have been cleaner to make the gdb command simply
rewrite the objfile directly.  However, this turns out to be relatively
hairy with BFD.  So again, for simplicity I just stuck with invoking
objcopy directly.  I also verified that objcopy will preserve hard
links.

Tom

--- find-debuginfo.sh.orig	2010-06-29 16:19:42.000000000 -0600
+++ find-debuginfo.sh	2010-07-08 09:36:03.000000000 -0600
@@ -96,6 +96,15 @@
   chmod 444 "$1" || exit
 }
 
+# Create a .gdb-index file for $1.
+make_gdb_index()
+{
+  local f="$1"
+  local d="${f%/*}"
+  # We don't care if gdb gives an error.
+  gdb --batch-silent -ex "file $f" -ex "maintenance save-gdb-index $d" > /dev/null 2>&1
+}
+
 # Make a relative symlink to $1 called $3$2
 shopt -s extglob
 link_relative()
@@ -207,6 +216,12 @@
     $strict && exit 2
   fi
 
+  make_gdb_index "$f"
+  if [ -f "${f}.gdb-index" ]; then
+    objcopy --add-section .gdb_index="${f}.gdb-index" --set-section-flags .gdb_index=readonly "$f" "$f"
+    rm -f "${f}.gdb-index"
+  fi
+
   # A binary already copied into /usr/lib/debug doesn't get stripped,
   # just has its file names collected and adjusted.
   case "$dn" in

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: find-debuginfo.sh change for gdb index
  2010-07-08 15:56                         ` Tom Tromey
@ 2010-07-08 20:36                           ` Tom Tromey
  2010-07-08 22:53                           ` Roland McGrath
  1 sibling, 0 replies; 27+ messages in thread
From: Tom Tromey @ 2010-07-08 20:36 UTC (permalink / raw)
  To: Roland McGrath; +Cc: Panu Matilainen, Project Archer

Tom> Here is a patch to just do the work directly in find-debuginfo.sh.  This
Tom> seemed simpler to me, but if you and Panu want a new script, I will do
Tom> that.

An upstream maintainer wanted the command's name changed.
It is now "save gdb-index" instead of "maint save-gdb-index".

Here's the updated find-debuginfo.sh patch.

Tom

--- find-debuginfo.sh.orig	2010-06-29 16:19:42.000000000 -0600
+++ find-debuginfo.sh	2010-07-08 14:10:06.000000000 -0600
@@ -96,6 +96,15 @@
   chmod 444 "$1" || exit
 }
 
+# Create a .gdb-index file for $1.
+make_gdb_index()
+{
+  local f="$1"
+  local d="${f%/*}"
+  # We don't care if gdb gives an error.
+  gdb --batch-silent -ex "file $f" -ex "save gdb-index $d" > /dev/null 2>&1
+}
+
 # Make a relative symlink to $1 called $3$2
 shopt -s extglob
 link_relative()
@@ -207,6 +216,12 @@
     $strict && exit 2
   fi
 
+  make_gdb_index "$f"
+  if [ -f "${f}.gdb-index" ]; then
+    objcopy --add-section .gdb_index="${f}.gdb-index" --set-section-flags .gdb_index=readonly "$f" "$f"
+    rm -f "${f}.gdb-index"
+  fi
+
   # A binary already copied into /usr/lib/debug doesn't get stripped,
   # just has its file names collected and adjusted.
   case "$dn" in

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: find-debuginfo.sh change for gdb index
  2010-07-08 15:56                         ` Tom Tromey
  2010-07-08 20:36                           ` Tom Tromey
@ 2010-07-08 22:53                           ` Roland McGrath
  2010-07-09  5:07                             ` Panu Matilainen
  1 sibling, 1 reply; 27+ messages in thread
From: Roland McGrath @ 2010-07-08 22:53 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Panu Matilainen, Project Archer

IMHO that procedure is now hairy enough that it really should be
encapsulated in some script provided by gdb (and maintained upstream if
the gdb support itself is upstream).  Not to mention that all other
users of the index support would want such a script to use by hand,
integrate into a different package system, etc.

i.e., find-debuginfo.sh would just add:

    gdb-add-index "$f" > /dev/null 2>&1


Thanks,
Roland


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: find-debuginfo.sh change for gdb index
  2010-07-08 22:53                           ` Roland McGrath
@ 2010-07-09  5:07                             ` Panu Matilainen
  2010-07-09 17:48                               ` Tom Tromey
  0 siblings, 1 reply; 27+ messages in thread
From: Panu Matilainen @ 2010-07-09  5:07 UTC (permalink / raw)
  To: Roland McGrath; +Cc: Tom Tromey, Project Archer

On Thu, 8 Jul 2010, Roland McGrath wrote:

> IMHO that procedure is now hairy enough that it really should be
> encapsulated in some script provided by gdb (and maintained upstream if
> the gdb support itself is upstream).  Not to mention that all other
> users of the index support would want such a script to use by hand,
> integrate into a different package system, etc.
>
> i.e., find-debuginfo.sh would just add:
>
>    gdb-add-index "$f" > /dev/null 2>&1

Sounds good to me. Also if it were an external script that might 
not be there (depending on gdb-version), find-debuginfo.sh can do

[ -x /usr/bin/gdb-add-index ] && /usr/bin/gdb-add-index "$f" \
     > /dev/null 2>&1

making it nicer for upstreaming. Of course for F >= 14 we still want 
rpm-build to require gdb to consistently get the indexes.

 	- Panu -

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: find-debuginfo.sh change for gdb index
  2010-07-09  5:07                             ` Panu Matilainen
@ 2010-07-09 17:48                               ` Tom Tromey
  2010-07-30 21:36                                 ` Tom Tromey
  0 siblings, 1 reply; 27+ messages in thread
From: Tom Tromey @ 2010-07-09 17:48 UTC (permalink / raw)
  To: Panu Matilainen; +Cc: Roland McGrath, Project Archer

>>>>> "Panu" == Panu Matilainen <pmatilai@redhat.com> writes:

>> gdb-add-index "$f" > /dev/null 2>&1

Panu> Sounds good to me. Also if it were an external script that might not
Panu> be there (depending on gdb-version), find-debuginfo.sh can do
[...]

Indeed, the latest patch is just the line you wrote.

I sent the gdb-add-index patch to gdb-patches.  I don't know whether it
will go in (I think historically gdb has avoided installing scripts),
but in any case Jan is going to put it into the gdb RPM.

Tom

--- find-debuginfo.sh.orig	2010-06-29 16:19:42.000000000 -0600
+++ find-debuginfo.sh	2010-07-09 11:35:39.000000000 -0600
@@ -207,6 +207,8 @@
     $strict && exit 2
   fi
 
+  [ -x /usr/bin/gdb-add-index ] && /usr/bin/gdb-add-index "$f" > /dev/null 2>&1
+
   # A binary already copied into /usr/lib/debug doesn't get stripped,
   # just has its file names collected and adjusted.
   case "$dn" in

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: find-debuginfo.sh change for gdb index
  2010-07-09 17:48                               ` Tom Tromey
@ 2010-07-30 21:36                                 ` Tom Tromey
  2010-07-30 23:07                                   ` Roland McGrath
  2010-07-30 23:09                                   ` Jan Kratochvil
  0 siblings, 2 replies; 27+ messages in thread
From: Tom Tromey @ 2010-07-30 21:36 UTC (permalink / raw)
  To: Panu Matilainen; +Cc: Roland McGrath, Project Archer

Panu> Sounds good to me. Also if it were an external script that might not
Panu> be there (depending on gdb-version), find-debuginfo.sh can do
Tom> [...]

Tom> Indeed, the latest patch is just the line you wrote.

I just wanted to follow up on this, as the Fedora 14 process is moving
along rapidly.  Did the needed RPM patch go in?

Tom

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: find-debuginfo.sh change for gdb index
  2010-07-30 21:36                                 ` Tom Tromey
@ 2010-07-30 23:07                                   ` Roland McGrath
  2010-07-30 23:09                                   ` Jan Kratochvil
  1 sibling, 0 replies; 27+ messages in thread
From: Roland McGrath @ 2010-07-30 23:07 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Panu Matilainen, Project Archer

> I just wanted to follow up on this, as the Fedora 14 process is moving
> along rapidly.  Did the needed RPM patch go in?

Nope, it's not in the Fedora package nor in rpm's upstream git repository.

Note that we'll also need elfutils 0.149 to be in Fedora so that eu-strip
-g will recognize the .gdb_index section as one to move into .debug files.
(Otherwise they'll end up in libc.so and the like where -g is used, rather
than libc.so.debug, though most things don't use -g.)

I haven't released 0.149 yet, I guess I will next week.


Thanks,
Roland

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: find-debuginfo.sh change for gdb index
  2010-07-30 21:36                                 ` Tom Tromey
  2010-07-30 23:07                                   ` Roland McGrath
@ 2010-07-30 23:09                                   ` Jan Kratochvil
  1 sibling, 0 replies; 27+ messages in thread
From: Jan Kratochvil @ 2010-07-30 23:09 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Panu Matilainen, Roland McGrath, Project Archer

On Fri, 30 Jul 2010 23:36:15 +0200, Tom Tromey wrote:
> Panu> Sounds good to me. Also if it were an external script that might not
> Panu> be there (depending on gdb-version), find-debuginfo.sh can do
> Tom> [...]
> 
> Tom> Indeed, the latest patch is just the line you wrote.
> 
> I just wanted to follow up on this, as the Fedora 14 process is moving
> along rapidly.  Did the needed RPM patch go in?

It is filed for rpm and still open:
https://bugzilla.redhat.com/show_bug.cgi?id=617166


Regards,
Jan

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2010-07-30 23:09 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-06-29 22:32 find-debuginfo.sh change for gdb index Tom Tromey
2010-06-29 23:21 ` Roland McGrath
2010-06-30 17:35   ` Tom Tromey
2010-06-30 18:14     ` Roland McGrath
2010-06-30 18:23       ` Tom Tromey
2010-06-30 20:42         ` Tom Tromey
2010-06-30 20:44           ` Roland McGrath
2010-06-30 21:25             ` Tom Tromey
2010-06-30 21:58               ` Tom Tromey
2010-06-30 22:00                 ` Tom Tromey
2010-06-30 22:14               ` Roland McGrath
2010-07-02 19:55                 ` Tom Tromey
2010-07-05  9:36                   ` Panu Matilainen
2010-07-05  9:48                     ` Jan Kratochvil
2010-07-05 10:39                       ` Panu Matilainen
2010-07-06 18:35                     ` Tom Tromey
2010-07-06 19:14                       ` Roland McGrath
2010-07-06 19:41                         ` Tom Tromey
2010-07-06 20:28                           ` Roland McGrath
2010-07-08 15:56                         ` Tom Tromey
2010-07-08 20:36                           ` Tom Tromey
2010-07-08 22:53                           ` Roland McGrath
2010-07-09  5:07                             ` Panu Matilainen
2010-07-09 17:48                               ` Tom Tromey
2010-07-30 21:36                                 ` Tom Tromey
2010-07-30 23:07                                   ` Roland McGrath
2010-07-30 23:09                                   ` Jan Kratochvil

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).