[patch] Fix regression on prelinked executables

[patch] Fix regression on prelinked executables

[Jan Kratochvil]

Hi,

there is a regression since gdb-7.0 for a combination of:
 * prelinked
 * main executable
 * using separate debug info
 * using copy relocations

It is since a patch for both PIE and (AFAIK) OSX support:
	[commit] syms_from_objfile: Relativize also MAINLINE
	http://sourceware.org/ml/gdb-patches/2010-01/msg00080.html

which started to use problematic addr_info_make_relative even for main
executables.  prelink<->gdb discussion at:
	https://bugzilla.redhat.com/show_bug.cgi?id=614659

Currently in the unfortunately executables GDB has invalid displcement for
symbols in .bss:
	int bssvar, *bssvarp = &bssvar;
	(gdb) p &bssvar
	$1 = (int *) 0x600b54
	(gdb) p bssvarp
	$2 = (int *) 0x600b50

<abstract-higher-point-of-view>
addr_info_make_relative could just simply subtract entry point address and
provide single CORE_ADDR objfile->offset (instead of the current
section_offsets array with offsets specific for each section).  Linux systems
use always single offset for the whole objfile.  AFAIK these per-section
offsets are there for some embedded targets.  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.
</abstract-higher-point-of-view>

No regressions on {x86_64,x86_64-m32,i686}-fedora13-linux-gnu.

Proposing for the gdb-7.2 branch.  I had problems fixing up my crashing X.


Thanks,
Jan


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.

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.

--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -547,6 +547,23 @@ relative_addr_info_to_section_offsets (struct section_offsets *section_offsets,
     }
 }
 
+/* Transform section name S for a name comparison.  prelink can split section
+   `.bss' into two sections `.dynbss' and `.bss' (in this order).  Similarly
+   prelink can split `.sbss' into `.sdynbss' and `.sbss'.  Use virtual address
+   of the new `.dynbss' (`.sdynbss') section as the adjacent new `.bss'
+   (`.sbss') section has invalid (increased) virtual address.  */
+
+static const char *
+addr_section_name (const char *s)
+{
+  if (strcmp (s, ".dynbss") == 0)
+    return ".bss";
+  if (strcmp (s, ".sdynbss") == 0)
+    return ".sbss";
+
+  return s;
+}
+
 /* qsort comparator for addrs_section_sort.  Sort entries in ascending order by
    their (name, sectindex) pair.  sectindex makes the sort by name stable.  */
 
@@ -557,7 +574,7 @@ addrs_section_compar (const void *ap, const void *bp)
   const struct other_sections *b = *((struct other_sections **) bp);
   int retval, a_idx, b_idx;
 
-  retval = strcmp (a->name, b->name);
+  retval = strcmp (addr_section_name (a->name), addr_section_name (b->name));
   if (retval)
     return retval;
 
@@ -641,14 +658,16 @@ addr_info_make_relative (struct section_addr_info *addrs, bfd *abfd)
 
   while (*addrs_sorted)
     {
-      const char *sect_name = (*addrs_sorted)->name;
+      const char *sect_name = addr_section_name ((*addrs_sorted)->name);
 
       while (*abfd_addrs_sorted
-	     && strcmp ((*abfd_addrs_sorted)->name, sect_name) < 0)
+	     && strcmp (addr_section_name ((*abfd_addrs_sorted)->name),
+			sect_name) < 0)
 	abfd_addrs_sorted++;
 
       if (*abfd_addrs_sorted
-	  && strcmp ((*abfd_addrs_sorted)->name, sect_name) == 0)
+	  && strcmp (addr_section_name ((*abfd_addrs_sorted)->name),
+		     sect_name) == 0)
 	{
 	  int index_in_addrs;
 
@@ -676,7 +695,6 @@ addr_info_make_relative (struct section_addr_info *addrs, bfd *abfd)
 
   for (i = 0; i < addrs->num_sections && addrs->other[i].name; i++)
     {
-      const char *sect_name = addrs->other[i].name;
       struct other_sections *sect = addrs_to_abfd_addrs[i];
 
       if (sect)
@@ -694,6 +712,9 @@ addr_info_make_relative (struct section_addr_info *addrs, bfd *abfd)
 	}
       else
 	{
+	  /* addr_section_name transformation is not used for SECT_NAME.  */
+	  const char *sect_name = addrs->other[i].name;
+
 	  /* This section does not exist in ABFD, which is normally
 	     unexpected and we want to issue a warning.
 
@@ -704,12 +725,20 @@ addr_info_make_relative (struct section_addr_info *addrs, bfd *abfd)
 	     a warning.  Shared libraries contain just the section
 	     ".gnu.liblist" but it is not marked as loadable there.  There is
 	     no other way to identify them than by their name as the sections
-	     created by prelink have no special flags.  */
+	     created by prelink have no special flags.
+
+	     For the sections `.bss' and `.sbss' see addr_section_name.  */
 
 	  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));
 
--- a/gdb/testsuite/gdb.base/prelink-lib.c
+++ b/gdb/testsuite/gdb.base/prelink-lib.c
@@ -16,6 +16,8 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.
 */
 
+int copyreloc = 1;
+
 int
 g (void (*p)(void))
 {
--- a/gdb/testsuite/gdb.base/prelink.c
+++ b/gdb/testsuite/gdb.base/prelink.c
@@ -18,6 +18,11 @@
 
 #include <stdio.h>
 
+extern int copyreloc;
+
+/* Test GDB itself finds `&bssvar' right.   */
+static int bssvar, *bssvarp = &bssvar;
+
 extern void (*h (void)) (void (*)(void));
 
 int
@@ -25,5 +30,6 @@ main (void)
 {
   void (*f) (void (*)(void)) = h ();
   printf ("%p\n", f);
+  printf ("%d\n", copyreloc);
   f (0);
 }
--- a/gdb/testsuite/gdb.base/prelink.exp
+++ b/gdb/testsuite/gdb.base/prelink.exp
@@ -57,6 +57,13 @@ if {$prelink_args == ""} {
     return -1
 }
 
+set test "split debug of executable"
+if [gdb_gnu_strip_debug $binfile] {
+    fail $test
+} else {
+    pass $test
+}
+
 if ![prelink_yes $prelink_args] {
     # Maybe we don't have prelink.
     return -1
@@ -105,3 +112,5 @@ clean_restart $executable
 gdb_test_no_output "set verbose on"
 
 gdb_test "core-file $objdir/$subdir/prelink.core" "Using PIC \\(Position Independent Code\\) prelink displacement 0x\[^0\]\[0-9a-f\]* for \[^\r\n\]*[file tail ${libfile}].*" "seen displacement message"
+
+gdb_test "p &bssvar == bssvarp" " = 1" ".dynbss vs. .bss address shift"

ping: [patch] Fix regression on prelinked executables

[Jan Kratochvil]

Hi,

it is after a week and I would not like to miss the 7.2 release.
While the patch is not nice it is probably a minimal change to fix it for 7.2.


Thu, 15 Jul 2010 11:54:57 +0200, Jan Kratochvil wrote:
Hi,

there is a regression since gdb-7.0 for a combination of:
 * prelinked
 * main executable
 * using separate debug info
 * using copy relocations

It is since a patch for both PIE and (AFAIK) OSX support:
	[commit] syms_from_objfile: Relativize also MAINLINE
	http://sourceware.org/ml/gdb-patches/2010-01/msg00080.html

which started to use problematic addr_info_make_relative even for main
executables.  prelink<->gdb discussion at:
	https://bugzilla.redhat.com/show_bug.cgi?id=614659

Currently in the unfortunately executables GDB has invalid displcement for
symbols in .bss:
	int bssvar, *bssvarp = &bssvar;
	(gdb) p &bssvar
	$1 = (int *) 0x600b54
	(gdb) p bssvarp
	$2 = (int *) 0x600b50

<abstract-higher-point-of-view>
addr_info_make_relative could just simply subtract entry point address and
provide single CORE_ADDR objfile->offset (instead of the current
section_offsets array with offsets specific for each section).  Linux systems
use always single offset for the whole objfile.  AFAIK these per-section
offsets are there for some embedded targets.  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.
</abstract-higher-point-of-view>

No regressions on {x86_64,x86_64-m32,i686}-fedora13-linux-gnu.

Proposing for the gdb-7.2 branch.  I had problems fixing up my crashing X.


Thanks,
Jan


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.

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.

--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -547,6 +547,23 @@ relative_addr_info_to_section_offsets (struct section_offsets *section_offsets,
     }
 }
 
+/* Transform section name S for a name comparison.  prelink can split section
+   `.bss' into two sections `.dynbss' and `.bss' (in this order).  Similarly
+   prelink can split `.sbss' into `.sdynbss' and `.sbss'.  Use virtual address
+   of the new `.dynbss' (`.sdynbss') section as the adjacent new `.bss'
+   (`.sbss') section has invalid (increased) virtual address.  */
+
+static const char *
+addr_section_name (const char *s)
+{
+  if (strcmp (s, ".dynbss") == 0)
+    return ".bss";
+  if (strcmp (s, ".sdynbss") == 0)
+    return ".sbss";
+
+  return s;
+}
+
 /* qsort comparator for addrs_section_sort.  Sort entries in ascending order by
    their (name, sectindex) pair.  sectindex makes the sort by name stable.  */
 
@@ -557,7 +574,7 @@ addrs_section_compar (const void *ap, const void *bp)
   const struct other_sections *b = *((struct other_sections **) bp);
   int retval, a_idx, b_idx;
 
-  retval = strcmp (a->name, b->name);
+  retval = strcmp (addr_section_name (a->name), addr_section_name (b->name));
   if (retval)
     return retval;
 
@@ -641,14 +658,16 @@ addr_info_make_relative (struct section_addr_info *addrs, bfd *abfd)
 
   while (*addrs_sorted)
     {
-      const char *sect_name = (*addrs_sorted)->name;
+      const char *sect_name = addr_section_name ((*addrs_sorted)->name);
 
       while (*abfd_addrs_sorted
-	     && strcmp ((*abfd_addrs_sorted)->name, sect_name) < 0)
+	     && strcmp (addr_section_name ((*abfd_addrs_sorted)->name),
+			sect_name) < 0)
 	abfd_addrs_sorted++;
 
       if (*abfd_addrs_sorted
-	  && strcmp ((*abfd_addrs_sorted)->name, sect_name) == 0)
+	  && strcmp (addr_section_name ((*abfd_addrs_sorted)->name),
+		     sect_name) == 0)
 	{
 	  int index_in_addrs;
 
@@ -676,7 +695,6 @@ addr_info_make_relative (struct section_addr_info *addrs, bfd *abfd)
 
   for (i = 0; i < addrs->num_sections && addrs->other[i].name; i++)
     {
-      const char *sect_name = addrs->other[i].name;
       struct other_sections *sect = addrs_to_abfd_addrs[i];
 
       if (sect)
@@ -694,6 +712,9 @@ addr_info_make_relative (struct section_addr_info *addrs, bfd *abfd)
 	}
       else
 	{
+	  /* addr_section_name transformation is not used for SECT_NAME.  */
+	  const char *sect_name = addrs->other[i].name;
+
 	  /* This section does not exist in ABFD, which is normally
 	     unexpected and we want to issue a warning.
 
@@ -704,12 +725,20 @@ addr_info_make_relative (struct section_addr_info *addrs, bfd *abfd)
 	     a warning.  Shared libraries contain just the section
 	     ".gnu.liblist" but it is not marked as loadable there.  There is
 	     no other way to identify them than by their name as the sections
-	     created by prelink have no special flags.  */
+	     created by prelink have no special flags.
+
+	     For the sections `.bss' and `.sbss' see addr_section_name.  */
 
 	  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));
 
--- a/gdb/testsuite/gdb.base/prelink-lib.c
+++ b/gdb/testsuite/gdb.base/prelink-lib.c
@@ -16,6 +16,8 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.
 */
 
+int copyreloc = 1;
+
 int
 g (void (*p)(void))
 {
--- a/gdb/testsuite/gdb.base/prelink.c
+++ b/gdb/testsuite/gdb.base/prelink.c
@@ -18,6 +18,11 @@
 
 #include <stdio.h>
 
+extern int copyreloc;
+
+/* Test GDB itself finds `&bssvar' right.   */
+static int bssvar, *bssvarp = &bssvar;
+
 extern void (*h (void)) (void (*)(void));
 
 int
@@ -25,5 +30,6 @@ main (void)
 {
   void (*f) (void (*)(void)) = h ();
   printf ("%p\n", f);
+  printf ("%d\n", copyreloc);
   f (0);
 }
--- a/gdb/testsuite/gdb.base/prelink.exp
+++ b/gdb/testsuite/gdb.base/prelink.exp
@@ -57,6 +57,13 @@ if {$prelink_args == ""} {
     return -1
 }
 
+set test "split debug of executable"
+if [gdb_gnu_strip_debug $binfile] {
+    fail $test
+} else {
+    pass $test
+}
+
 if ![prelink_yes $prelink_args] {
     # Maybe we don't have prelink.
     return -1
@@ -105,3 +112,5 @@ clean_restart $executable
 gdb_test_no_output "set verbose on"
 
 gdb_test "core-file $objdir/$subdir/prelink.core" "Using PIC \\(Position Independent Code\\) prelink displacement 0x\[^0\]\[0-9a-f\]* for \[^\r\n\]*[file tail ${libfile}].*" "seen displacement message"
+
+gdb_test "p &bssvar == bssvarp" " = 1" ".dynbss vs. .bss address shift"

Re: [patch] Fix regression on prelinked executables

[Joel Brobecker]

> 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

Re: [patch] Fix regression on prelinked executables

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

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

This is fine by me -- but AFAIK Darwin is pretty broken already.  At
least, there is an open bug with what looks to be a simple crasher.

Tom

Re: [patch] Fix regression on prelinked executables

[Joel Brobecker]

> This is fine by me -- but AFAIK Darwin is pretty broken already.  At
> least, there is an open bug with what looks to be a simple crasher.

I wasn't aware of that. As far as we can tell, it's working fine at
AdaCore. One of the issues is that neither Tristan nor myself monitor
the open bugs in bugzilla. If something like this gets reported, I think
it's work sending Tristan a note.

Regarding the patch - if a second GM thinks it's safe for 7.2, then
that's also good enough for me.

-- 
Joel

Re: [patch] Fix regression on prelinked executables

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

>> This is fine by me -- but AFAIK Darwin is pretty broken already.  At
>> least, there is an open bug with what looks to be a simple crasher.

Joel> I wasn't aware of that. As far as we can tell, it's working fine at
Joel> AdaCore. One of the issues is that neither Tristan nor myself monitor
Joel> the open bugs in bugzilla. If something like this gets reported, I think
Joel> it's work sending Tristan a note.

Ok.  I wasn't sure how active he wanted to be with regard to Darwin
bugs.

The one I am thinking of is
http://sourceware.org/bugzilla/show_bug.cgi?id=11488

It looks like some other Darwin bugs may be closeable, just search for
"darwin" in the summary.  I see 7, I'd say 5 are probably obsolete --
but that is just guessing.

Joel> Regarding the patch - if a second GM thinks it's safe for 7.2, then
Joel> that's also good enough for me.

I didn't look that closely :)

Tom

Re: [patch] Fix regression on prelinked executables

[Jan Kratochvil]

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

Re: [patch] Fix regression on prelinked executables

[Joel Brobecker]

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

I just decided to read the condition as "suppress the warning if (a || b)".
That helped mentally absorb it.  But I think that's a detail.

> That is the change above reducing the enumerated list in fact
> increases the probability we give a warning.

OK - just wanted to make sure that this was the intent.

> OK to check-in?

Sure.

-- 
Joel

Re: [patch] Fix regression on prelinked executables

[Jan Kratochvil]

On Fri, 30 Jul 2010 17:54:56 +0200, Joel Brobecker wrote:
> OK - just wanted to make sure that this was the intent.

Thanks for the recheck.  This code is very fragile with the section_offsets
vector having to end up always fully zeroed. :-)


Checked-in:
	http://sourceware.org/ml/gdb-cvs/2010-07/msg00213.html


Thanks,
Jan

[7.2.x-checkin] Re: [patch] Fix regression on prelinked executables

[Jan Kratochvil]

On Thu, 29 Jul 2010 18:36:48 +0200, Joel Brobecker wrote:
> 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.

On Fri, 30 Jul 2010 00:14:23 +0200, Tom Tromey wrote:
> >>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:
> Joel> Regarding the patch - if a second GM thinks it's safe for 7.2, then
> Joel> that's also good enough for me.
> 
> I didn't look that closely :)

http://sourceware.org/ml/gdb-cvs/2010-10/msg00028.html

Branch: 	gdb_7_2-branch

gdb/
	* 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.
	
gdb/testsuite/
	* 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.

Checked-in for 7.2.1, if any.


Thanks,
Jan