Re: macros, debug information, and parse_macro_definition

[Jim Blandy <jimb@redhat.com>]

David Taylor <dtaylor@emc.com> writes:
> One of my projects at work is to do the necessary gcc and gdb work to
> allow users of sgdb (our GUI on top of GDB) to do macro expansion.
> 
> Now, we use ELF/STABS, not ELF/DWARF...
> 
> The encoding of macros that I have chosen is very very similar to the
> DWARF-2 encoding.  In particular, the string is the same.  (I see no
> reason to invent a new encoding.)
> 
> As a result, at some point I will need to call a function which will
> either be identical to or 99% identical to parse_macro_definition.
> So, I'd like to propose that the function parse_macro_definition be
> made non static and that it and its support functions (copy_string,
> dwarf2_macro_malformed_definition_complaint, consume_improper_spaces
> -- all three of which are called *ONLY* by parse_macro_defintion) be
> moved to another file -- since they are not DWARF specific anymore.
> 
> Any objections?  File name?
> 
> My inclination is to move them to macrotab.c since the function
> parse_macro_defintion calls functions within that file and can be
> thought of as a thin veneer above the functions macro_define_object
> and macro_define_function.

This seems like obviously the right thing to do, but I'm hesitating.

If there are two independent specs for how to encode something, then
GDB should have two independent readers, one for each spec.  Even if
the specs happen to be identical.  Specs change, and code has bugs; we
want to be able to upgrade or fix each reader without worrying that
we're breaking the other one.

If you think the syntax is too trivial to worry about this kind of
thing, then maybe sharing code could be okay.  Or if you can persuade
folks to make stabs.texinfo say, "the format is defined to be the same
as that used in Dwarf .debug_macinfo DW_MACINFO_define records; here's
what it looks like, but the Dwarf spec is authoritative", then that
would be okay.

If that can't be swung, I think duplicating the code would be the
right thing to do.  It's only ~170 lines.

Maybe I'm being silly.  But I'm reminded of the tragic case of
monitor.c:monitor_supply_register, used by lots of different *-rom.c
files to parse register dumps.  It had a nice set of heuristics for
groveling through all the garbage that typical ROM monitors print out
when you ask them to dump the register set, and finding the actual
meaningful hex digits amidst all the noise.  The problem was, once the
function was being used by too many ROM monitors, you couldn't change
its behavior for fear of breaking some other *-rom.c file for some
monitor you'd never be able to get your hands on.  It just wasn't
clear exactly what behaviors people relied upon and which they didn't.
Thus the following change:

    revision 1.6
    date: 2001/09/07 21:27:36;  author: jimb;  state: Exp;  lines: +79 -1
    Correctly parse register values provided by the monitor.
    * rom68k-rom.c: #include "value.h".
    (is_hex_digit, hex_digit_value, is_whitespace,
    rom68k_supply_one_register): New static functions.
    (rom68k_supply_register): Call rom68k_supply_one_register, instead
    of monitor_supply_register; the latter was incorrectly parsing
    the values.
    * Makefile.in (rom68k-rom.o): Note that this now #includes value.h.

I had to give rom68k-rom.c a new parser function of its own, that I
could fix and adjust without having to worry about breaking other roms.