public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Joel Brobecker <brobecker@adacore.com>
To: Jan Kratochvil <jan.kratochvil@redhat.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [patch] Fix regression on prelinked executables
Date: Thu, 29 Jul 2010 16:37:00 -0000	[thread overview]
Message-ID: <20100729163648.GA13267@adacore.com> (raw)
In-Reply-To: <20100715095457.GA22922@host0.dyn.jankratochvil.net>

> 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...

> gdb/
> 2010-07-15  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* symfile.c (addr_section_name): New function.
> 	(addrs_section_compar): Use it.
> 	(addr_info_make_relative): Use it.  Move variable sect_name into a more
> 	inner block.  Make ".dynbss" and ".sdynbss" checks more strict.

I think that this is OK.

I'm not sure about putting it on the 7.2 branch, however.  Given that
this can directly affect Darwin, I'd rather give this patch an observation
period and potentially put it in 7.2.1.

> gdb/testsuite/
> 2010-07-15  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* gdb.base/prelink-lib.c (copyreloc): New initialized variable.
> 	* gdb.base/prelink.c (copyreloc, bssvar, bssvarp): New variables.
> 	(main): Use copyreloc.
> 	* gdb.base/prelink.exp (split debug of executable)
> 	(.dynbss vs. .bss address shift): New tests.

This part is OK.

>  	  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)))

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?

-- 
Joel

  parent reply	other threads:[~2010-07-29 16:37 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-07-15  9:55 Jan Kratochvil
2010-07-27 19:22 ` ping: " Jan Kratochvil
2010-07-29 16:37 ` Joel Brobecker [this message]
2010-07-29 20:14   ` Tom Tromey
2010-07-29 21:38     ` Joel Brobecker
2010-07-29 22:14       ` Tom Tromey
2010-07-30 15:45   ` Jan Kratochvil
2010-07-30 15:55     ` Joel Brobecker
2010-07-30 16:06       ` Jan Kratochvil
2010-10-05 20:56   ` [7.2.x-checkin] " Jan Kratochvil

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100729163648.GA13267@adacore.com \
    --to=brobecker@adacore.com \
    --cc=gdb-patches@sourceware.org \
    --cc=jan.kratochvil@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).