From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x102c.google.com (mail-pj1-x102c.google.com [IPv6:2607:f8b0:4864:20::102c]) by sourceware.org (Postfix) with ESMTPS id 053133858C42 for ; Mon, 4 Dec 2023 03:36:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 053133858C42 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 053133858C42 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::102c ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701661020; cv=none; b=aqQweaVusj+u1stLKioGGv+uymiOuMYoKVtMm9i43FvKyHIUgBt8tUKAezTKKzyEgeRv+VmgaHbT25RpV5ht3l59+7Pfjkr7a1iwcmStHtbd4pJ5GssDpcqc/FTcrJ0ohJqiEd39/wTaYZKRr8PbP9AY+P0UOFRNu2jj8CMoPx4= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701661020; c=relaxed/simple; bh=Tgwb6l2LqV9L+4pQdEf7htgUoJe2DQlOD9JMGnew6eI=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=mL+z8Y5XdohygcMlmwEpugvWfkpVaOxwRIdkaQC2M9uqPfYzB2hi+s7R71d102H9uI0wMfxyjSIsGEGP67UY68e41eNVtg/iGrYVfskD23nlbWFUuJ6HTLlzfVvTP9xTIIEl46osXEvTu90GacGbkO7c+gVjMrZdIquJAqSGAgk= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pj1-x102c.google.com with SMTP id 98e67ed59e1d1-28654179ec0so2807098a91.2 for ; Sun, 03 Dec 2023 19:36:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1701661016; x=1702265816; darn=sourceware.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=UtNLlzjUEbSDPkm19PmkDfTqKpexhaflY0ttqkq4kRg=; b=RMAk3pML1JahRQ6ii+z1mKjkALplC2r1VWhQj9YHka8QTpTvCbhNHS9HJaHE97iFIz bbqS+JQdPCQJ5THOZkDW+V1KvrSEBvKTYytGfoInU3XgeZ0luEzgP0/Pmw5D05Fwu+Iw k/9k3WprhC29uJY6Uarhp2vm5Axw5dinVjDRIie6OL7f6lwnTmJ9ViHfEc+B4KtzlFFz eSaKzC6Ae6HXGBQi+pqbQ9fwavfk7Xm+l8BBUb8G17U/WRpuGbvGbnWafMa2NKCpZG2Y IVt6O3U52T4UGcQGtiOHzLhrw9nx/wNvm465eJjD/8plFOoXUK0HIgFF14+mC8gcXdlz CX+A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701661016; x=1702265816; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=UtNLlzjUEbSDPkm19PmkDfTqKpexhaflY0ttqkq4kRg=; b=L+MgrEkublf0lW/ynReBi0BV30OByQ2edPafxDX+zkrbSUS4EEd3zgRgRarrzzcie6 aC1ReaeQpIbqVfJpTbUQS02eOdoPGLPvvTZrNUOPtEK9LBMvnp+t9AAXsr9efoqFwf8N UfIpFm8hIbzvopLSl3xbi7BSl/2BjthkqL1wXYNKEtMkiyEq9wb9g0yhOAnjmcIY47IT /7sLfC+m8SWUjW72/WkNWQFDYJ4j8lR9AbkHoi1b6PV6gFEHOudL7MUBMA+y11Jjc/SU uBJttvst1JqWL2Rrrsu4Pyau17m1OUUmhcDPBgO0ht4A4yJRqSZw2fbLtREIqxZrfl/j 165g== X-Gm-Message-State: AOJu0Yx2P5mvY/dQjuffMB32+RW2LSqbcU7HeoHm65wJZPy0ore1WU8f nV+REPnj0qDTC9n8yB1r8XrfSqi8olYsAHmmMszHAPmoD8w= X-Google-Smtp-Source: AGHT+IHN5zWl+CpE8K1DQPAi1icju1bklpCZSoPZJmjvGHi2x02ZveeCPwPAF1LDGG9YaASQPJZ7HceHM9QNto2wM1w= X-Received: by 2002:a17:90a:986:b0:286:a089:c3dd with SMTP id 6-20020a17090a098600b00286a089c3ddmr867674pjo.62.1701661015873; Sun, 03 Dec 2023 19:36:55 -0800 (PST) MIME-Version: 1.0 References: <877cnd4qy8.fsf@redhat.com> In-Reply-To: From: Zeck S Date: Sun, 3 Dec 2023 21:36:44 -0600 Message-ID: Subject: Re: [RFC][PATCH?] fixed some segfaults and bugs in mdebug support To: Andrew Burgess Cc: gdb-patches@sourceware.org Content-Type: multipart/alternative; boundary="000000000000611cec060ba6d46b" X-Spam-Status: No, score=-9.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM,GIT_PATCH_0,HTML_MESSAGE,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: --000000000000611cec060ba6d46b Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Not sure what etiquette is correct, but just so this thread isn't left hanging without an update, after nearly a month: I have created a QEMU user mode emulation board configuration, so that I can test this patch. https://sourceware.org/pipermail/gdb-patches/2023-December/204760.html After that is resolved (or I am told of a better way), I should be able to get some tests running by wrapping the compiler in a script as suggested at the bottom of this page https://sourceware.org/gdb/wiki/Running%20the%20test%20suite%20with%20a%20n= on-GCC%20compiler On Fri, Nov 10, 2023 at 9:07=E2=80=AFPM Zeck S wrote: > I'm still working on the testing. Been setting up QEMU stuff so I can run > linux mips code. Seems like the best option. > I think I should be able to get the test suite to call the IDO compiler > and use the linux mips linker. > > Also, sent the signed paperwork to the FSF for copyright assignment, and > am waiting to hear back. > > Been a few weeks, so, figured I'd post an update. Fixed one more small bug > I found as well. > > On Mon, Oct 23, 2023 at 7:25=E2=80=AFPM Zeck S wro= te: > >> 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=E2=80=AFAM 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 !=3D 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 ha= ve >>> > 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 =3D=3D 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 =3D=3D >>> > 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 =3D 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 =3D lineno; >>> > - lt->item[lt->nitems].is_stmt =3D 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 =3D 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_BLOC= K, >>> > lang)); >>> > bv->set_block (STATIC_BLOCK, new_block (objfile, NON_FUNCTION_BLOC= K, >>> > 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 =3D false); >>> >>> Hopefully it's obvious how the new parameter would be used :) >>> >>> Thanks, >>> Andrew >>> >>> >>> > -{ >>> > - struct block *retval =3D new (&objfile->objfile_obstack) global_bl= ock; >>> > - >>> > - if (type =3D=3D 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 * >>> >>> --000000000000611cec060ba6d46b--