Oh, I messed up the diff. Spoiled by github, never done it this way. The testing makes sense now. Might be the weekend before I can make much progress on that. Thanks for the fast reply! On Mon, Oct 23, 2023 at 4:40 AM Andrew Burgess wrote: > Zeck S writes: > > > First off, I apologize if I'm doing this process wrong. I have sent an > > email to assign@gnu.org trying to get the paperwork required for > copyright > > assignment. I think that's the correct thing to do? > > > > While I wait on that, I'm not sure exactly what is required for these > > changes. > > > > Here's what I fixed in mdebug support. > > > > info sym funcName would segfault > > The first problem was that no compunit_symtab was set for the > global_block > > on blockvectors in new_symtab. This caused a crash in block.c. > > initialize_block_iterator called get_block_compunit_symtab and the > > assertion gdb_assert (gb->compunit_symtab != NULL); would fail. > > > > info types would segfault > > The second problem was memory corruption. struct global_block is a larger > > and different type from plain block and blockvector is expected to have > > index 0 be a global_block struct. This can be seen done correctly in > jit.c > > near /* Now add the special blocks */ under if (i == GLOBAL_BLOCK). > Failing > > to allocate this correctly leads to crashes for me (usually) in > > set_compunit_symtab where the assertion gdb_assert (gb->compunit_symtab > == > > NULL); would randomly fail. This fix is also in new_symtab. > > > > info line file:line did not work > > The third problem was finding lines never worked because add_line never > set > > .is_stmt to true, so in symtab.c find_line_common never saw item->is_stmt > > as true, do it always went down the /* Ignore non-statements. */ path in > > its main loop. > > I was confused by this description as the only change I see is you > removing this line 'lt->item[lt->nitems].is_stmt = 1;' , but I suspect > you generated your diff the wrong way round. > > You should consider creating your diff as a git commit, then use 'git > send-email' to send out patches, I found this site > https://git-send-email.io/ a pretty useful guide for setting up git & > email sending. > > > > > I looked in the gdb/testsuite directory, and I don't see a directory for > > mips or mdebug? Unsure how to set up a test for this. To make files with > > mdebug symbols, I used the old IRIX IDO compiler running under a kind of > > qemu setup used by N64 game reverse engineering projects. (N64 dev is why > > I'm interested in this symbol format. I can connect vscode to gdb and gdb > > to an n64 emulator with a gdb stub to debug with symbols) > > You might not need to add any new tests at all, IF you can identify some > existing tests that are fixed by your changes. > > Most tests are not separated based on which compiler or environment is > used, though clearly there are exceptions, e.g. gdb.arch/*.exp does > contain some architecture specific tests. Instead most tests are > written based on the GDB feature being tested. For example, > gdb.base/infoline.exp tests the 'info line' command. > > The expectation is that if someone has a more niche compiler or > environment then they will perform their own regression testing using > their setup. > > So, hopefully, if you can get the GDB tests running using your > toolchain, then without your patch you'll see some failures in (maybe) > gdb.base/infoline.exp, and after your patch some of the failures would > be resolved, you'd then mention some (or all) of these improvements in > your commit message. > > Of course, if your particular situation isn't covered by an existing > test then you might need to extend an existing test -- or create a new > test -- whatever seems most appropriate. > > > > > diff --git a/gdb/mdebugread.c b/gdb/mdebugread.c > > index 4b0a1eb255f..9cb30ce0acd 100644 > > --- a/gdb/mdebugread.c > > +++ b/gdb/mdebugread.c > > @@ -239,9 +239,6 @@ enum block_type { FUNCTION_BLOCK, NON_FUNCTION_BLOCK > }; > > static struct block *new_block (struct objfile *objfile, > > enum block_type, enum language); > > > > -static struct block *new_global_block (struct objfile *objfile, > > - enum block_type, enum language); > > - > > static struct compunit_symtab *new_symtab (const char *, int, struct > > objfile *); > > > > static struct linetable *new_linetable (int); > > @@ -4545,7 +4542,6 @@ add_line (struct linetable *lt, int lineno, > CORE_ADDR > > adr, int last) > > return lineno; > > > > lt->item[lt->nitems].line = lineno; > > - lt->item[lt->nitems].is_stmt = 1; > > lt->item[lt->nitems++].set_unrelocated_pc (unrelocated_addr (adr << > 2)); > > return lineno; > > } > > @@ -4638,10 +4634,9 @@ new_symtab (const char *name, int maxlines, struct > > objfile *objfile) > > > > /* All symtabs must have at least two blocks. */ > > bv = new_bvect (2); > > - bv->set_block (GLOBAL_BLOCK, new_global_block (objfile, > > NON_FUNCTION_BLOCK, lang)); > > + bv->set_block (GLOBAL_BLOCK, new_block (objfile, NON_FUNCTION_BLOCK, > > lang)); > > bv->set_block (STATIC_BLOCK, new_block (objfile, NON_FUNCTION_BLOCK, > > lang)); > > bv->static_block ()->set_superblock (bv->global_block ()); > > - bv->global_block ()->set_compunit_symtab(cust); > > cust->set_blockvector (bv); > > > > cust->set_debugformat ("ECOFF"); > > @@ -4740,21 +4735,6 @@ new_block (struct objfile *objfile, enum > block_type > > type, > > return retval; > > } > > > > -static struct block * > > -new_global_block (struct objfile *objfile, enum block_type type, > > - enum language language) > > Static functions should have a comment before them. In this case > something as simple as: > > /* Like new_block, but create a global_block. */ > > Though I wonder if we could/should just give new_block an extra > parameter so its declaration becomes: > > static struct block *new_block (struct objfile *objfile, > enum block_type, enum language, > bool global_block = false); > > Hopefully it's obvious how the new parameter would be used :) > > Thanks, > Andrew > > > > -{ > > - struct block *retval = new (&objfile->objfile_obstack) global_block; > > - > > - if (type == FUNCTION_BLOCK) > > - retval->set_multidict (mdict_create_linear_expandable (language)); > > - else > > - retval->set_multidict (mdict_create_hashed_expandable (language)); > > - > > - return retval; > > -} > > - > > - > > /* Create a new symbol with printname NAME. */ > > > > static struct symbol * > >