From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29375 invoked by alias); 30 Jul 2010 15:45:15 -0000 Received: (qmail 29366 invoked by uid 22791); 30 Jul 2010 15:45:14 -0000 X-SWARE-Spam-Status: No, hits=-6.0 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 30 Jul 2010 15:45:09 +0000 Received: from int-mx03.intmail.prod.int.phx2.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.16]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o6UFijRI028279 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 30 Jul 2010 11:44:45 -0400 Received: from host1.dyn.jankratochvil.net (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx03.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id o6UFihcN031995 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 30 Jul 2010 11:44:45 -0400 Received: from host1.dyn.jankratochvil.net (localhost [127.0.0.1]) by host1.dyn.jankratochvil.net (8.14.4/8.14.4) with ESMTP id o6UFigFd026037; Fri, 30 Jul 2010 17:44:42 +0200 Received: (from jkratoch@localhost) by host1.dyn.jankratochvil.net (8.14.4/8.14.4/Submit) id o6UFig4p026036; Fri, 30 Jul 2010 17:44:42 +0200 Date: Fri, 30 Jul 2010 15:45:00 -0000 From: Jan Kratochvil To: Joel Brobecker Cc: gdb-patches@sourceware.org Subject: Re: [patch] Fix regression on prelinked executables Message-ID: <20100730154442.GA25162@host1.dyn.jankratochvil.net> References: <20100715095457.GA22922@host0.dyn.jankratochvil.net> <20100729163648.GA13267@adacore.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20100729163648.GA13267@adacore.com> User-Agent: Mutt/1.5.20 (2009-12-10) 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: 2010-07/txt/msg00579.txt.bz2 On Thu, 29 Jul 2010 18:36:48 +0200, Joel Brobecker wrote: > > Curiously GDB already uses at > > many places > > baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile)); > > instead of using offset for the appropriate section at that place and nobody > > complains. > > It's something I actually noticed almost 10 years ago, now, while > working on Tru64, I think. But I never really found a situation that > required me to work on that, so... OK, I will one day try to turn section_offsets array into a single variable. > > if (!(strcmp (sect_name, ".gnu.liblist") == 0 > > || strcmp (sect_name, ".gnu.conflict") == 0 > > - || strcmp (sect_name, ".dynbss") == 0 > > - || strcmp (sect_name, ".sdynbss") == 0)) > > + || (strcmp (sect_name, ".bss") == 0 > > + && i > 0 > > + && strcmp (addrs->other[i - 1].name, ".dynbss") == 0 > > + && addrs_to_abfd_addrs[i - 1] != NULL) > > + || (strcmp (sect_name, ".sbss") == 0 > > + && i > 0 > > + && strcmp (addrs->other[i - 1].name, ".sdynbss") == 0 > > + && addrs_to_abfd_addrs[i - 1] != NULL))) warning (_("section %s not found in %s"), sect_name, bfd_get_filename (abfd)); > > I had to think over this for a while, and I think that the two new checks > are correct. However, I'm still trying to figure out why it's correct > to also remove the simple check for .dynbss. In other words, if .dynbss > is missing from the separate debug object file, shouldn't we emit a > warning? The meaning is reversed. It is !(a || b || c). Which I find more readable than (!a && !b && !c). But I will have to change my mind as it seems to not everyone may consider my chosen format as more readable. The code above says do NOT warn if section is either .gnu.liblist or .gnu.conflict or etc. That is it was very irresponsible from me to put .dynbss and .sdynbss there before. I should have examined what they really mean, when they appear etc. I have seen them as added by prelink so I thought we can ignore them. I did not consider they contain the right address for .bss/.sbss as the real .bss/.sbss address is invalid (from the GDB point of view) that time already. That is the change above reducing the enumerated list in fact increases the probability we give a warning. Another issue is that we no longer suppress the warning for .dynbss/.sdynbss as before and it works. It is due to the new addr_section_name function, originally .dynbss/.sdynbss got to this point while now .dynbss/.sdynbss get processed but .bss/.sbss gets to this point instead. Making the code if (!(strcmp (sect_name, ".gnu.liblist") == 0 || strcmp (sect_name, ".gnu.conflict") == 0 || strcmp (sect_name, ".bss") == 0 || strcmp (sect_name, ".sbss") == 0)) is probably too obviously incorrect so we have to restrict the .bss/.sbss entries warning suppression to the ones which only come exactly after the prelink-generated .dynbss/.sdynbss sections. I already got the approval but delaying it to resolving this issue. OK to check-in? Thanks, Jan