This is the mail archive of the gdb-patches@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] |
This problem is happening on Tru64 5.1A. GDB crashed while reading the debugging information of an application created by one of our customers. Their application is a mix of C++ and Ada. In order to investigate, this customer gave us the binaries of their application as we don't have a C++ compiler on Tru64. We managed to find the source of the problem, and fix it (hopefuly :), but not having a C++ compiler, we are not able to produce a testcase for it. Here is what happened: In their application, they had one class with a few constructors, one destructor, and then some class variables. In mdebugread.c, GDB read class types as TYPE_CODE_STRUCTs. For reasons probably related to the ECOFF format (my knowledge of this format is still a bit partial), GDB creates the struct type in several passes. In the first pass, when GDB encounters the struct type definition for the first time, it does the following: - create a type object, saves the name of the type, and a few other info. - counts the number of fields in the struct, and allocates enough memory to holds these fields... The problem in our case was that GDB was mis-counting (under-counting actually) the number of fields of the C++ class. The method used in GDB to do the counting is fairly simple. GDB knows that structs definitions are started with a stBlock, that they end with a stEnd, and that all stMember symbol records between the 2 markers are fields in the struct. Simplifying it a bit the current counting code, we have: nfields = 0; ALL_SYMBOL_RECORDS (tsym) { if (tsym.st == stEnd) break; else if (tsym.st == stMember) nfields++; } Unfortunately, in the case of C++ classes, the struct usually contains methods which are encoded in something ressembling the following sequence of symbol records: stProc stParam stParam [...] stEnd The algorithm above is therefore not resistant to procedure definitions nested in struct definitions, as GDB then ends the counting prematurely. In our case, GDB stopped counting after finding 0 fields, and therefore allocated 0 bytes for the struct fields: TYPE_FIELDS (t) = f = ((struct field *) TYPE_ALLOC (t, nfields * sizeof (struct field))); The trouble starts during the second pass, when we actually try to fill in the info for the fields... case stMember: /* member of struct or union */ f = &TYPE_FIELDS (top_stack->cur_type)[top_stack->cur_field++]; FIELD_NAME (*f) = name; FIELD_BITPOS (*f) = sh->value; bitsize = 0; FIELD_TYPE (*f) = parse_type (cur_fd, ax, sh->index, &bitsize, bigend, name); FIELD_BITSIZE (*f) = bitsize; break; Notice how we don't check for the number of fields allocated before accessing field number top_stack->cur_field... Because we did not allocate enough space to store all fields, we end up with a buffer overflow! The memory for these fields was allocated on the obstack. In the time interval between the moment when we allocated the 0 bytes for the fields, and the moment we overflow, GDB did also allocate a new type object, also on the same obstack. Understandably, the obstack placed this new type at the same location as the fields. Argh! While we happily store the information for each field, we corrupt the data for the other type. It is only a bit later when we try to dereference one of the fields in this corrupted type that GDB crashes with a SEGV... The following patch is, I admit, a minimal attempt at fixing the problem. It would probably be more complete to handle StProc symbol records in the counting loop and skip the whole stProc sequence, just as we do for stBlock et al: case stBlock: case [...]: case stStruct: { if (tsym.index != 0) { /* This is something like a struct within a struct. Skip over the fields of the inner struct. The -1 is because the for loop will increment ext_tsym. */ ext_tsym = ((char *) debug_info->external_sym + ((cur_fdr->isymBase + tsym.index - 1) * external_sym_size)); } Unfornately, I lack the time to do this. Instead, I did the following trivial change, which just ignores any stEnd symbol records if they are not the one ending the struct definition. This is done by matching the symbol name associated to the stEnd SYMR against the name of the struct. This change fixes the problem reported by our customer, and does not introduce any regression in the testsuite. Unfortunately, we don't have a C++ compiler, so the C++ part of the testsuite is inoperable for us, and we could not test the effect of this change on C++. Given my current analysis, this change seems sensible. I am therefore recommending it for inclusion. If acceptable, we may also want to include it in the 5.3 branch as well, as it fixes a crash. Any feedback from somebody having a C++ compiler would be greatly appreciated. 2002-12-31 J. Brobecker <brobecker@gnat.com> * mdebugread.c (parse_symbol): Make sure to identify the correct stEnd symbol record while counting the number of fields when parsing the debugging information for a structure. Otherwise, GDB sometimes ends up under-counting the number of felds in the struct, and this causes later a memory corruption responsible for a GDB crash when running or attaching to the application. Fixes [B927-009] Ok to commit? Thanks, -- Joel
Attachment:
mdebugread.c.diff
Description: Text document
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |