From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6062 invoked by alias); 24 Apr 2003 15:12:25 -0000 Mailing-List: contact gdb-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-owner@sources.redhat.com Received: (qmail 6043 invoked from network); 24 Apr 2003 15:12:24 -0000 Received: from unknown (HELO mailhub.lss.emc.com) (168.159.2.8) by sources.redhat.com with SMTP; 24 Apr 2003 15:12:24 -0000 Received: from emc.com (lul1179.lss.emc.com [168.159.33.179]) by mailhub.lss.emc.com (Switch-2.2.5/Switch-2.2.0) with ESMTP id h3OFCG518027; Thu, 24 Apr 2003 11:12:16 -0400 (EDT) Message-Id: <200304241512.h3OFCG518027@mailhub.lss.emc.com> To: Jim Blandy cc: gdb@sources.redhat.com Subject: Re: macros, debug information, and parse_macro_definition In-Reply-To: Your message of "23 Apr 2003 22:12:07 CDT." References: <200304221640.h3MGelj05733@mailhub.lss.emc.com> Date: Thu, 24 Apr 2003 15:12:00 -0000 From: David Taylor X-SW-Source: 2003-04/txt/msg00294.txt.bz2 > From: Jim Blandy > Date: 23 Apr 2003 22:12:07 -0500 > David Taylor 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