From mboxrd@z Thu Jan 1 00:00:00 1970 From: Fernando Nasser To: Jim Blandy Cc: Fernando Nasser , Christopher Faylor , gdb-patches@sources.redhat.com Subject: Re: Simple but crucial bug fix to gdb Date: Thu, 31 May 2001 05:30:00 -0000 Message-id: <3B1638A2.79AE4BCF@redhat.com> References: <3.0.5.32.20010530142745.01470ec0@pophost.pdxuxbre.lmc.com> <20010530173650.A21397@redhat.com> <3B15711D.BEA4B77E@cygnus.com> X-SW-Source: 2001-05/msg00503.html Jim Blandy wrote: > > Fernando Nasser writes: > > I cannot approve the patch because I am not the maintainer of the > > stabs reader, but (for the reasons you've explained above) this > > seems to fall clearly into the obvious fix rule. > > This is *not* an obvious fix. > > The stabs reader has seen a symbol definition, before it has any idea > what compilation unit that symbol belongs to. Are you *sure* it's > okay to just ignore that symbol definition? Won't the psymtab's start > and end addresses get set wrong? Irrelevant to the fact that his compiler may not be doing the right thing, GDB should not be dumping core. Furthermore, as Chris said, the previous test already test for 'pst' being set, so he would not be making it any worse than it was. Furthermore, it would not affect any one with a more sane compiler. Furthermore, the condition of _this_ if already test for 'pst' after 'textlow_not_set' is reset (i.e., on every subsequent pass of the loop after this test succeeds the first time). So, original: if (textlow_not_set || (pst && CUR_SYMBOL_VALUE < pst->textlow && CUR_SYMBOL_VALUE != ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile)))) and changed: if (pst && textlow_not_set || (pst && CUR_SYMBOL_VALUE < pst->textlow && CUR_SYMBOL_VALUE != ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile)))) are either as right or as wrong as each other, except that the second one is more defensive and IMHO more right than the first one, where we guard from one condition (pst not set) on the second part of the if and not on the first). This is an obvious fix! If the original code there is wrong, it has been so for a while. You, as the maintainer, should go there and see what is wrong (or understand why it does that ;-) ). But letting GDB dump core on someone is just not the right thing to do. I sincerely expect that you provide us with the "more correct" fix asap. Regards, Fernando ------------------------code from the file----------------------- case 'f': CUR_SYMBOL_VALUE += ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile)); #ifdef DBXREAD_ONLY /* Kludges for ELF/STABS with Sun ACC */ last_function_name = namestring; #ifdef SOFUN_ADDRESS_MAYBE_MISSING /* Do not fix textlow==0 for .o or NLM files, as 0 is a legit value for the bottom of the text seg in those cases. */ if (pst && textlow_not_set) { pst->textlow = find_stab_function_addr (namestring, pst->filename, objfile); textlow_not_set = 0; } #endif /* End kludge. */ /* Keep track of the start of the last function so we can handle end of function symbols. */ last_function_start = CUR_SYMBOL_VALUE; /* In reordered executables this function may lie outside the bounds created by N_SO symbols. If that's the case use the address of this function as the low bound for the partial symbol table. */ if (textlow_not_set || (pst && CUR_SYMBOL_VALUE < pst->textlow && CUR_SYMBOL_VALUE != ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile)))) { pst->textlow = CUR_SYMBOL_VALUE; textlow_not_set = 0; } #endif /* DBXREAD_ONLY */ add_psymbol_to_list (namestring, p - namestring, VAR_NAMESPACE, LOC_BLOCK, &objfile->static_psymbols, 0, CUR_SYMBOL_VALUE, psymtab_language, objfile); continue; -- Fernando Nasser Red Hat Canada Ltd. E-Mail: fnasser@redhat.com 2323 Yonge Street, Suite #300 Toronto, Ontario M4P 2C9