Cleanup and update to gdb 7.8

[jeffmahoney]

Hi Dave -

Our Hack Week at SUSE is next week, and I'm planning on continuing a project I've been working on to extend the Python functionality exported by GDB to be useful through crash. I'm aware of several other projects with similar goals. I've already gotten the needed bits integrated into our gdb, which is at version 7.8. When I updated the patches from an earlier version to 7.8, there were substantial differences. I didn't really want to maintain two sets of patches: one against gdb and the other against the gdb that crash uses. Since there's no real reason not to update crash's gdb version to 7.8, I went ahead and integrated it with crash.

This pull request covers a fair amount of ground.

• I've removed support for gdb versions prior to 7.6
• Cleaned up configuration of different gdb versions
• Renamed read_string within crash to men_read_string to avoid renaming gdb functions
• Use pointers to minimal_symbols for patching rather than pointers to longs (this one is needed for 7.8)
• The command hook has been removed in 7.8, so I've implemented an interpreter that just copies the cli interpreter during its own initialization and overrides the command fund.
• Constify parameters for a few functions
• Eliminate a few global variables that can be replaced with cleaner options
• Break out the gdb interface code from gdb-7.6.patch into a separate file
• Add support for gdb 7.8.

I've done some light testing with the command loop, but if you have a test system, I'd be happy to drive it a bit more.

[jeffmahoney]

There seems to be an integration issue. 'dmesg' works but printing of the variables it uses for structured logging does not.

[crash-utility]

----- Original Message -----

Hi Dave -

Our Hack Week at SUSE is next week, and I'm planning on continuing a
project I've been working on to extend the Python functionality exported
by GDB to be useful through crash. I'm aware of several other projects
with similar goals. I've already gotten the needed bits integrated into
our gdb, which is at version 7.8.

Let me get this straight -- you've had to make additional changes to gdb-7.8 for
this Python functionality? Or does gdb-7.8 work as-is for Python's purposes
(i.e., outside of crash)?

How exactly does this Python functionality get used in crash? I don't speak
Python, and the only users of it with respect to crash are those who use
the built-in Python interpreter offered by the pykdump extension module:

http://people.redhat.com/anderson/extensions.html#PYTHON

When I updated the patches from an earlier version to 7.8, there were substantial
differences. I didn't really want to maintain two sets of patches: one against
gdb and the other against the gdb that crash uses. Since there's no real reason
not to update crash's gdb version to 7.8, I went ahead and integrated it with crash.

That's debatable on this end. Historically the only reason to update the embedded
gdb was absolute necessity. Stability carries much more weight within Red Hat, so
I'm bound to that (at least as long as I'm employed here). And if you're a follower
of the crash-utility mailing list, you're probably aware of how conservative I am
w/respect to major changes. I've always maintained that whatever version of gdb
is currently in place should remain there forever -- until it has to be updated...

This pull request covers a fair amount of ground.

• I've removed support for gdb versions prior to 7.6

Why? Removing support for earlier versions is something that I don't feel comfortable
changing just because you can. There is historical precedent, where the embedded
gdb was bumped to gdb-6.1, but due to debuginfo incompatibility with certain
versions of gcc, it had to be reverted back to gdb-6.0. Because of the way
things are set up, it was trivial making the reversion -- it's basically just
changing the "default_gdb" in configure.c.

• Cleaned up configuration of different gdb versions

I grant you that it is somewhat confusing, but the #ifdef lines w/respect to
gdb versions was changed recently such that for future updates, it should be a
relatively simple matter of searching for usages of "GDB_x_y" where x_y is the
most recent version -- and then applying any newly-required changes there, or
just appending the new version to the "#if" line.

• Renamed read_string within crash to men_read_string to avoid renaming gdb
  functions

Similar to our Red Hat "KABI" for changing kernel interfaces, Red Hat is also
very sensitive to changing user-space ABI's as well. The functions that are
exported in the top-level "defs.h" file shouldn't be changed because extension
modules may depend upon them -- as you saw with dminfo.c and trace.c. For example,
although trace.c does get captured as a sample module within the crash source
package, it is carried as a separate crash-trace-command package. So that
would break that package, and require an update to that package.

Given that there are only 2 (count 'em) callers of the read_string() function
in gdb's valprint.c, it's certainly not an onerous modification to the gdb
sources.

• Use pointers to minimal_symbols for patching rather than pointers to longs
  (this one is needed for 7.8)
• The command hook has been removed in 7.8, so I've implemented an
  interpreter that just copies the cli interpreter during its own
  initialization and overrides the command fund.

Damn -- I see they've removed the deprecated_command_hook, which I was hoping
they'd keep around. I don't quite understand what you mean above, but hopefully
it works as easily as you make it sound. As I recall, it wasn't so much the
replacement of the command hook that was tricky, but rather the error handling
for errors generated within the gdb code itself.

• Constify parameters for a few functions
• Eliminate a few global variables that can be replaced with cleaner options
• Break out the gdb interface code from gdb-7.6.patch into a separate file
• Add support for gdb 7.8.

It's not clear to me how this works -- it looks like you've kept gdb-7.6
in place? But I don't seen any change in the Makefile to apply the gdb-7.8.patch?

Again, I would like to keep things working in a traditional manner. When a
new gdb-x.y version gets incorporated, there should be a new gdb-x.y.patch
file created, and the prior one removed from the Makefile. I may be misunderstanding
how this is being done.

I've done some light testing with the command loop, but if you have a
test system, I'd be happy to drive it a bit more.
You can merge this Pull Request by running:

git pull https://github.com/jeffmahoney/crash cleanup-and-update-to-gdb-7.8

Or you can view, comment on it, or merge it online at:

#2

I'm currently not using the pull functionality at github. Changes of this
magnitude (in fact any changes) need to be posted on the crash-utility mailing
list.

But as noted above, I'd rather see a "traditional" bumping of the embedded
gdb version the way it's always been done before.

Thanks,
Dave

-- Commit Summary --

• cleanup: remove support for gdb 5.x
• cleanup: remove support for gdb 6.0/6.1
• cleanup: remove support for gdb 7.0/7.3.1
• cleanup: remove static indexes for gdb version configuration
• Add missing includes
• cleanup: rename read_string to mem_read_string and leave gdb alone
• cleanup: use minimal_symbol directly instead of pointers to longs
• cleanup: use the gdb interpreter infrastructure
• cleanup: constify arguments to same_file
• cleanup: constify check_specified_module_tree
• gdb: request prettyformat versions of options
• cleanup: properly export prototypes in defs.h
• cleanup: constify untrusted_file
• cleanup: split crash-specific code out from gdb-7.6 and into crash.c
• gdb: eliminate crash_from_tty
• gdb: eliminate gdb_kernel_objfile
• gdb: add support for gdb 7.8

-- File Changes --

M Makefile (73)
M alpha.c (7)
M configure.c (124)
A crash.c (722)
A crash.h (13)
M defs.h (119)
M dev.c (13)
M extensions/dminfo.c (4)
M extensions/trace.c (12)
M filesys.c (14)
M gdb-7.6.patch (369)
A gdb-7.8.patch (804)
M gdb_interface.c (93)
M help.c (8)
M kernel.c (32)
M main.c (8)
M memory.c (40)
M net.c (2)
M ppc.c (10)
M ppc64.c (8)
M symbols.c (48)
M task.c (12)
M unwind_x86_32_64.c (2)

-- Patch Links --

https://github.com/crash-utility/crash/pull/2.patch
https://github.com/crash-utility/crash/pull/2.diff

---

Reply to this email directly or view it on GitHub:
#2