From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id ks1CC6c/NmUp+zQAWB0awg (envelope-from ) for ; Mon, 23 Oct 2023 05:40:55 -0400 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=i4PdGkXE; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 212231E0C1; Mon, 23 Oct 2023 05:40:55 -0400 (EDT) Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 087F21E00F for ; Mon, 23 Oct 2023 05:40:53 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 4E8DC3858004 for ; Mon, 23 Oct 2023 09:40:52 +0000 (GMT) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 18C013858D37 for ; Mon, 23 Oct 2023 09:40:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 18C013858D37 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 18C013858D37 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1698054039; cv=none; b=ugNYvjRT5YPSSxCOXsmchkSopO5YDmVXyApIyRNksBaY/vkSqPi66X3x4f0FhV/W1YVp3RjV2+lvqxzLunp4eVbmIoSlhRpwOvSrZKxspnYs5USmcZN7oO0ysjMpn+SdHrsRMC4tPz/A22lwbL/w5lote2dcmBzKlCYs2LfjJ3E= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1698054039; c=relaxed/simple; bh=cLEhIz1qb2nQ0MGXzgO9mAtfGXdhiGhHE3oLtz7S7s0=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=XRomcsn0o18M+/JekbYUnLs7yvTA9P1R+M3afG1CJqVy/w625PRL8pSMFscuJW9vTiOgKra74G14QsRBA4uBDM6HwdvjW+MYdm//qIfUn3Tf6I1btyhuFWX8ukGlP2PfqH5vzAMmPTkO4htf3onxzDZ6AKZLbW5VoSQgv9EdCjo= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1698054036; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=i+7uCiES9rSFMp/eP5iALySofnJV5fiEZSGjac9Cq44=; b=i4PdGkXE9sgHrdcyM4fWxRtMbhTBcXkI9QAsKuEbcBfDlbj11dPQTAzSWwN0P3Rysjldm2 9U2ShJw1xnRYckPttQNR03foZm4ezkViRNIbBaiOMPMryl4FPPPfV8G8LVyui/dZZIBzNN vwiQunavxGylyT0U6izvuyKdPhnXHEU= Received: from mail-ed1-f71.google.com (mail-ed1-f71.google.com [209.85.208.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-681-RbcAiMmQNySIVdO1SZ9YfQ-1; Mon, 23 Oct 2023 05:40:34 -0400 X-MC-Unique: RbcAiMmQNySIVdO1SZ9YfQ-1 Received: by mail-ed1-f71.google.com with SMTP id 4fb4d7f45d1cf-5344aaf2703so1789694a12.0 for ; Mon, 23 Oct 2023 02:40:34 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1698054033; x=1698658833; h=mime-version:message-id:date:references:in-reply-to:subject:to:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=i+7uCiES9rSFMp/eP5iALySofnJV5fiEZSGjac9Cq44=; b=a5rTdQLNMpEmftN9ol9C1g/f6f7kH0x/+uFrWER645X+zElqZZvmZw0Aioyq57wTg8 7JnVtF9eUFO981cUxm0HEzw3aQ6oSIYmCAy9AZTMqk+83dO3+rjGU9VfDY00fFIV5ITs WHt3jofiWHcWUQ0gKWOuvt6C2d/0l0xd63/QUaRKgK3iZTpUN/2I/wy+s6oNgANdx/X5 KuqjWIYVUu4Bw5HlpU0W2Pr8KROJETo/nImJp1GxcjegF3ymFERjNW+aly5uahe9QAm1 6+jfdXNtG0E0oK8Djvlv1hcGuezC5QdrdNybz7hJYzYAb7NUzHvHx4ZmVK5T08Ykt2q1 JHWw== X-Gm-Message-State: AOJu0YxW/yEHEBul1boZZyYkZ+USHeRmaO+6LusqxTZqju1Nv6hi21mC XG5t4UNAKBxpgI1tzaLuqBZLKwiRpFORzYCPWS5N4uMzOpG0v0ECGqbRSqkMVAhnNNc9tGytRRH /K9UtXTRf/hvFUIwFdW3sYTw+HxzxUg== X-Received: by 2002:a50:8712:0:b0:53e:232b:121c with SMTP id i18-20020a508712000000b0053e232b121cmr5418110edb.3.1698054033302; Mon, 23 Oct 2023 02:40:33 -0700 (PDT) X-Google-Smtp-Source: AGHT+IES/oF5C5oXclw4o7mBdS6mIWsQbuXVuo3TmZlusmvyQ15to2zZYmTbldKbTsONDsxBKM9o4g== X-Received: by 2002:a50:8712:0:b0:53e:232b:121c with SMTP id i18-20020a508712000000b0053e232b121cmr5418101edb.3.1698054032752; Mon, 23 Oct 2023 02:40:32 -0700 (PDT) Received: from localhost ([31.111.84.209]) by smtp.gmail.com with ESMTPSA id u17-20020a509511000000b0053e163a1ca0sm6184877eda.1.2023.10.23.02.40.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 23 Oct 2023 02:40:32 -0700 (PDT) From: Andrew Burgess To: Zeck S , gdb-patches@sourceware.org Subject: Re: [RFC][PATCH?] fixed some segfaults and bugs in mdebug support In-Reply-To: References: Date: Mon, 23 Oct 2023 10:40:31 +0100 Message-ID: <877cnd4qy8.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org 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 *