From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21735 invoked by alias); 28 Jul 2009 19:51:53 -0000 Received: (qmail 21711 invoked by uid 22791); 28 Jul 2009 19:51:51 -0000 X-SWARE-Spam-Status: No, hits=-2.5 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from qnxmail.qnx.com (HELO qnxmail.qnx.com) (209.226.137.76) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 28 Jul 2009 19:51:41 +0000 Received: from Nebula.ott.qnx.com (nebula.ott.qnx.com [10.42.3.30]) by hub.ott.qnx.com (8.9.3/8.9.3) with ESMTP id PAA16183; Tue, 28 Jul 2009 15:51:29 -0400 Received: from [127.0.0.1] ([10.42.161.192]) by Nebula.ott.qnx.com with Microsoft SMTPSVC(6.0.3790.3959); Tue, 28 Jul 2009 15:51:36 -0400 Message-ID: <4A6F56C6.6090801@qnx.com> Date: Tue, 28 Jul 2009 21:48:00 -0000 From: Aleksandar Ristovski User-Agent: Thunderbird 2.0.0.22 (Windows/20090605) MIME-Version: 1.0 To: Pedro Alves CC: gdb-patches@sourceware.org Subject: Re: [patch] Set bfd field in target_section References: <200907281606.05571.pedro@codesourcery.com> <4A6F1647.70907@qnx.com> <200907281657.29337.pedro@codesourcery.com> In-Reply-To: <200907281657.29337.pedro@codesourcery.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2009-07/txt/msg00699.txt.bz2 Pedro Alves wrote: > On Tuesday 28 July 2009 16:16:23, Aleksandar Ristovski wrote: >> Pedro Alves wrote: >> I think now you broke it in a different way. Now we can end >> up trying to read from a closed bfd. > > Ooops, you're right. There are uses of the bfd after transfering > its ownership to the bfd target with target_bfd_reopen, and closing > the bfd target (target_close)... This target is modeled on fdopen, > which is why I blindly assumed it wasn't happening. I've reverted > that patch. > >> And just wondering, why not simply: >> >> Index: gdb/exec.c >> =================================================================== >> RCS file: /cvs/src/src/gdb/exec.c,v >> retrieving revision 1.90 >> diff -u -p -r1.90 exec.c >> --- gdb/exec.c 2 Jul 2009 17:21:06 -0000 1.90 >> +++ gdb/exec.c 28 Jul 2009 14:58:16 -0000 >> @@ -381,6 +381,7 @@ add_to_section_table (bfd *abfd, struct >> struct target_section **table_pp = (struct >> target_section **) table_pp_char; >> flagword aflag; >> >> + (*table_pp)->bfd = abfd; >> /* Check the section flags, but do not discard >> zero-length sections, since >> some symbols may still be attached to this section. >> For instance, we >> encountered on sparc-solaris 2.10 a shared library >> with an empty .bss >> @@ -390,7 +391,6 @@ add_to_section_table (bfd *abfd, struct >> if (!(aflag & SEC_ALLOC)) >> return; >> >> - (*table_pp)->bfd = abfd; >> (*table_pp)->the_bfd_section = asect; >> (*table_pp)->addr = bfd_section_vma (abfd, asect); >> (*table_pp)->endaddr = (*table_pp)->addr + >> bfd_section_size (abfd, asect); > > Still not right, because that's still papering over the problem, and > still hitting undefined behaviour, in the case of the > table ending up being empty. When table->sections == table->sections_end, > we should never dereference table->sections, like in 'table->sections->bfd'. > Notice how the end pointer isn't advanced if !SEC_ALLOC. > Consider the fact that we usually xmalloc enough memory that > makes that not misbehave a coincidence. E.g., see that with > a resize_section_table call, you can end up with table->sections == NULL, > and table->section_end == NULL, which is legal, because table->sections > is still equal to table->sections_end in that case (meaning empty table). > > There used to be an extra structure in bfd_target.c to hold > the sections and the bfd, which didn't look necessary when I > reimplemented bfd-target.c on top of exec.c's functions, but > it is clear now that we need it anyway. Here's a patch that > brings something like that back. > > Could you try it? Thanks. > I tried it, it works. Thanks, -- Aleksandar Ristovski QNX Software Systems