This is the mail archive of the gdb@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: macros, debug information, and parse_macro_definition


> From: Jim Blandy <jimb at redhat dot com>
> Date: 23 Apr 2003 22:12:07 -0500

> David Taylor <dtaylor at emc dot com> writes:

  [...]

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

The DWARF 2.0.0 macinfo types that correspond to the proposed STABS
types N_MAC_UNDEF and N_MAC_DEFINE are DW_MACINFO_undef and
DW_MACINFO_define, respectively.  Each type is defined to take two
operands -- an integer and a string.  The integer is the line number
on which the #undef or #define appears.

Now, .stabs has a place to put the line number (the 'n_desc' field),
so that's not a problem.  I'm proposing that the strings be encoded
the same in the proposed STABS extension as the strings are encoded in
DWARF 2.0.0.

Here's the relevant portions of the DWARF 2.0.0 spec (from section
6.3.1.1 Define and Undefine Entries):

[For N_MAC_UNDEF:]

    In the case of a DW_MACINFO_undef entry, the value of this string
    will be simply the name of the pre-processor symbol which was
    undefined at the indicated source file.

[Seems simple enough, no?  For N_MAC_DEFINE:]

    In the case of a DW_MACINFO_define entry, the value of this string
    will be the name of the pre-processor symbol that was defined at
    the indicated source file, followed immediately by the macro
    formal parameter list including the surrounding parentheses (in
    the case of a function-like macro) followed by the definition
    string for the macro.  If there is no formal parameter list, then
    the name of the defined macro is followed directly by its
    definition string.

    In the case of a function-like macro definition, no whitespace
    characters should appear between the name of the defined macro and
    the following left parenthesis.  Also, no whitespace characters
    should appear between successive formal paramters in the formal
    parameter list.  (Successive formal parameters should, however, be
    separated by commas.)  Also, exactly one space character should
    separate the right parenthesis which terminates the formal
    parameter list and the following definition string.

    In the case of a ``normal'' (i.e. non-function-like) macro
    definition, exactly one space character should separate the name
    of the macro from the following definition text.

[A bit of verbiage about whitespace, but again it seems pretty simple.]

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

These encodings are simple enough that I wouldn't expect them to
change over time.

I would think it more likely that there would be a bug, either in
gcc's encoding of them or in gdb's processing of them.  And if the
code is duplicated, then both locations need to be modified to fix the
bug -- with the attendant risk that someone will modify one and forget
to modify the other.  Or will modify it, but accidentally make it
slightly different.

I don't know how people would feel about having STABS have a
description of the encoding combined with a caveat that the DWARF
2.0.0 spec is authoritative.  I would certainly prefer that the STABS
document remain self contained.

I wouldn't mind a statement saying, roughly:

    The above encoding of the string operands to N_MAC_DEFINE and
    N_MAC_UNDEF is meant to be identical to the encoding of the string
    operands to the DWARF 2.0.0 macinfo types DW_MACINFO_define and
    DW_MACINFO_undef.  Any differences between the two are
    unintentional.

So, it stops short of saying that DWARF 2.0.0 is authoritative; 

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

GDB supports fewer debug format readers than monitors.  And I do not
forsee a lot of gdb's debug format readers using these routines.

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

David


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]