Re: ping: [patch 3/6] PIE: Fix occasional error attaching i686 binary

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

On Mon, 05 Jul 2010 19:18:10 +0200, Joel Brobecker wrote:
> You forgot to remove the old version of the comment. :-)

Embarassing, I am sorry.

Checked-in.


Thanks,
Jan


http://sourceware.org/ml/gdb-cvs/2010-07/msg00026.html

--- src/gdb/ChangeLog	2010/07/05 17:58:55	1.11965
+++ src/gdb/ChangeLog	2010/07/05 18:00:39	1.11966
@@ -1,6 +1,15 @@
 2010-07-05  Jan Kratochvil  <jan.kratochvil@redhat.com>
 	    Joel Brobecker  <brobecker@adacore.com>
 
+	* auxv.c (memory_xfer_auxv): Update attach comment.
+	* solib-svr4.c (svr4_special_symbol_handling): Remove the call to
+	svr4_relocate_main_executable.
+	(svr4_solib_create_inferior_hook): Make the call to
+	svr4_relocate_main_executable unconditional.
+
+2010-07-05  Jan Kratochvil  <jan.kratochvil@redhat.com>
+	    Joel Brobecker  <brobecker@adacore.com>
+
 	* auxv.c (ld_so_xfer_auxv): Do not error on failed read of data_address.
 
 2010-07-05  Jan Kratochvil  <jan.kratochvil@redhat.com>
--- src/gdb/auxv.c	2010/07/05 17:58:55	1.28
+++ src/gdb/auxv.c	2010/07/05 18:00:39	1.29
@@ -208,8 +208,10 @@
   gdb_assert (readbuf || writebuf);
 
    /* ld_so_xfer_auxv is the only function safe for virtual executables being
-      executed by valgrind's memcheck.  As using ld_so_xfer_auxv is problematic
-      during inferior startup GDB does call it only for attached processes.  */
+      executed by valgrind's memcheck.  Using ld_so_xfer_auxv during inferior
+      startup is problematic, because ld.so symbol tables have not yet been
+      relocated.  So GDB uses this function only when attaching to a process.
+      */
 
   if (current_inferior ()->attach_flag != 0)
     {
--- src/gdb/solib-svr4.c	2010/07/05 17:57:50	1.135
+++ src/gdb/solib-svr4.c	2010/07/05 18:00:39	1.136
@@ -1653,7 +1653,6 @@
 static void
 svr4_special_symbol_handling (void)
 {
-  svr4_relocate_main_executable ();
 }
 
 /* Read the ELF program headers from ABFD.  Return the contents and
@@ -2096,8 +2095,7 @@
   info = get_svr4_info ();
 
   /* Relocate the main executable if necessary.  */
-  if (current_inferior ()->attach_flag == 0)
-    svr4_relocate_main_executable ();
+  svr4_relocate_main_executable ();
 
   if (!svr4_have_link_map_offsets ())
     return;
--- src/gdb/testsuite/ChangeLog	2010/07/05 17:58:56	1.2372
+++ src/gdb/testsuite/ChangeLog	2010/07/05 18:00:39	1.2373
@@ -1,4 +1,10 @@
 2010-07-05  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	* gdb.base/attach-pie-misread.exp, gdb.base/attach-pie-misread.c: New.
+	* gdb.base/break-interp.exp (reach, test_core, test_ld): Require each
+	displacement message exactly once.
+
+2010-07-05  Jan Kratochvil  <jan.kratochvil@redhat.com>
 	    Joel Brobecker  <brobecker@adacore.com>
 
 	* gdb.base/break-interp.exp (test_attach): Keep $interp changed.  Move
--- src/gdb/testsuite/gdb.base/attach-pie-misread.c
+++ src/gdb/testsuite/gdb.base/attach-pie-misread.c	2010-07-05 18:10:20.709982000 +0000
@@ -0,0 +1,47 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2010 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <stdio.h>
+#include <unistd.h>
+
+const char stub[] = {
+#ifdef GEN
+# include GEN
+#endif
+};
+
+int
+main (int argc, char **argv)
+{
+  /* Generator of GEN written in Python takes about 15s for x86_64's 4MB.  */
+  if (argc == 2)
+    {
+      long count = strtol (argv[1], NULL, 0);
+
+      while (count-- > 0)
+	puts ("0x55,");
+
+      return 0;
+    }
+  if (argc != 1)
+    return 1;
+
+  puts ("sleeping");
+  fflush (stdout);
+
+  return sleep (60);
+}
--- src/gdb/testsuite/gdb.base/attach-pie-misread.exp
+++ src/gdb/testsuite/gdb.base/attach-pie-misread.exp	2010-07-05 18:10:21.021056000 +0000
@@ -0,0 +1,209 @@
+# Copyright 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This test only works on GNU/Linux.
+if { ![isnative] || [is_remote host] || ![istarget *-linux*] || [skip_shlib_tests]} {
+    continue
+}
+
+set test "attach-pie-misread"
+set srcfile ${test}.c
+set genfile ${objdir}/${subdir}/${test}-gen.h
+set executable ${test}
+set binfile ${objdir}/${subdir}/${executable}
+
+if {[build_executable ${test}.exp $executable $srcfile [list "additional_flags=-fPIE -pie"]] == -1} {
+    return -1
+}
+
+# Program Headers:
+#   Type           Offset   VirtAddr           PhysAddr           FileSiz  MemSiz   Flg Align
+#   LOAD           0x000000 0x0000000000400000 0x0000000000400000 0x134f5ec 0x134f5ec R E 0x200000
+#   LOAD           0x134f5f0 0x000000000194f5f0 0x000000000194f5f0 0x1dbc60 0x214088 RW  0x200000
+#   DYNAMIC        0x134f618 0x000000000194f618 0x000000000194f618 0x000200 0x000200 RW  0x8
+#
+proc read_phdr {binfile test} {
+    set readelf_program [transform readelf]
+    set command "exec $readelf_program -Wl $binfile"
+    verbose -log "command is $command"
+    set result [catch $command output]
+    verbose -log "result is $result"
+    verbose -log "output is $output"
+    if {$result != 0} {
+	fail $test
+	return
+    }
+    if ![regexp {\nProgram Headers:\n *Type [^\n]* Align\n(.*?)\n\n} $output trash phdr] {
+	fail "$test (no Program Headers)"
+	return
+    }
+    if ![regexp -line {^ *DYNAMIC +0x[0-9a-f]+ +(0x[0-9a-f]+) } $phdr trash dynamic_vaddr] {
+	fail "$test (no DYNAMIC found)"
+	return
+    }
+    verbose -log "dynamic_vaddr is $dynamic_vaddr"
+    set align_max -1
+    foreach {trash align} [regexp -line -all -inline {^ *LOAD .* (0x[0-9]+)$} $phdr] {
+	if {$align_max < $align} {
+	    set align_max $align
+	}
+    }
+    verbose -log "align_max is $align_max"
+    if {$align_max == -1} {
+	fail "$test (no LOAD found)"
+	return
+    }
+    pass $test
+    return [list $dynamic_vaddr $align_max]
+}
+
+set phdr [read_phdr $binfile "readelf initial scan"]
+set dynamic_vaddr [lindex $phdr 0]
+set align_max [lindex $phdr 1]
+
+set stub_size [format 0x%x [expr "2 * $align_max - ($dynamic_vaddr & ($align_max - 1))"]]
+verbose -log "stub_size is $stub_size"
+
+# On x86_64 it is commonly about 4MB.
+if {$stub_size > 25000000} {
+    xfail "stub size $stub_size is too large"
+    return
+}
+
+set test "generate stub"
+set command "exec $binfile $stub_size >$genfile"
+verbose -log "command is $command"
+set result [catch $command output]
+verbose -log "result is $result"
+verbose -log "output is $output"
+if {$result == 0} {
+    pass $test
+} else {
+    fail $test
+}
+
+if {[build_executable ${test}.exp $executable $srcfile [list "additional_flags=-fPIE -pie -DGEN=\"$genfile\""]] == -1} {
+    return -1
+}
+
+# x86_64 file has 25MB, no need to keep it.
+file delete -- $genfile
+
+set phdr [read_phdr $binfile "readelf rebuilt with stub_size"]
+set dynamic_vaddr_prelinkno [lindex $phdr 0]
+
+set command "exec /usr/sbin/prelink -q -N --no-exec-shield -R $binfile"
+verbose -log "command is $command"
+set result [catch $command output]
+verbose -log "result is $result"
+verbose -log "output is $output"
+
+set test "prelink -R"
+if {$result == 0 && $output == ""} {
+    pass $test
+} elseif {$result == 1 && [regexp {^(couldn't execute "/usr/sbin/prelink[^\r\n]*": no such file or directory\n?)*$} $output]} {
+    untested attach-pie-misread.exp
+    return -1
+} else {
+    fail $test
+}
+
+set phdr [read_phdr $binfile "readelf with prelink -R"]
+set dynamic_vaddr_prelinkyes [lindex $phdr 0]
+
+set first_offset [format 0x%x [expr $dynamic_vaddr_prelinkyes - $dynamic_vaddr_prelinkno]]
+verbose -log "first_offset is $first_offset"
+
+set test "first offset is non-zero"
+if {$first_offset == 0} {
+    fail "$test (-fPIE -pie in effect?)"
+} else {
+    pass $test
+}
+
+set test "start inferior"
+gdb_exit
+
+set res [remote_spawn host $binfile];
+if { $res < 0 || $res == "" } {
+    perror "Spawning $binfile failed."
+    fail $test
+    return
+}
+set pid [exp_pid -i $res]
+gdb_expect {
+    -re "sleeping\r\n" {
+	pass $test
+    }
+    eof {
+	fail "$test (eof)"
+	remote_exec host "kill -9 $pid"
+	return
+    }
+    timeout {
+	fail "$test (timeout)"
+	remote_exec host "kill -9 $pid"
+	return
+    }
+}
+
+# Due to alignments it was reproducible with 1 on x86_64 but 2 on i686.
+foreach align_mult {1 2} {
+    set old_ldprefix $pf_prefix
+    lappend pf_prefix "shift-by-$align_mult:"
+
+    # FIXME: We believe there is enough room under FIRST_OFFSET.
+    set shifted_offset [format 0x%x [expr "$first_offset - $align_mult * $align_max"]]
+    verbose -log "shifted_offset is $shifted_offset"
+
+    set command "exec /usr/sbin/prelink -q -N --no-exec-shield -r $shifted_offset $binfile"
+    verbose -log "command is $command"
+    set result [catch $command output]
+    verbose -log "result is $result"
+    verbose -log "output is $output"
+
+    set test "prelink -r"
+    if {$result == 0 && $output == ""} {
+	pass $test
+    } else {
+	fail $test
+    }
+
+    clean_restart $executable
+
+    set test "attach"
+    gdb_test_multiple "attach $pid" $test {
+	-re "Attaching to program: .*, process $pid\r\n" {
+	    # Missing "$gdb_prompt $" is intentional.
+	    pass $test
+	}
+    }
+
+    set test "error on Cannot access memory at address"
+    gdb_test_multiple "" $test {
+	-re "\r\nCannot access memory at address .*$gdb_prompt $" {
+	    fail $test
+	}
+	-re "$gdb_prompt $" {
+	    pass $test
+	}
+    }
+
+    gdb_test "detach" "Detaching from program: .*"
+
+    set pf_prefix $old_ldprefix
+}
+
+remote_exec host "kill -9 $pid"
--- src/gdb/testsuite/gdb.base/break-interp.exp	2010/07/05 17:58:56	1.15
+++ src/gdb/testsuite/gdb.base/break-interp.exp	2010/07/05 18:00:40	1.16
@@ -253,9 +253,8 @@
 		}
 		if {$displacement == $case} {
 		    pass $test_displacement
-		    # Permit multiple such messages.
 		    set displacement "FOUND-$displacement"
-		} elseif {$displacement != "FOUND-$case"} {
+		} else {
 		    fail $test_displacement
 		}
 		exp_continue
@@ -310,9 +309,8 @@
 	    }
 	    if {$displacement == $case} {
 		pass $test_displacement
-		# Permit multiple such messages.
 		set displacement "FOUND-$displacement"
-	    } elseif {$displacement != "FOUND-$case"} {
+	    } else {
 		fail $test_displacement
 	    }
 	    exp_continue
@@ -368,9 +366,8 @@
 	    }
 	    if {$displacement == $case} {
 		pass $test_displacement
-		# Permit multiple such messages.
 		set displacement "FOUND-$displacement"
-	    } elseif {$displacement != "FOUND-$case"} {
+	    } else {
 		fail $test_displacement
 	    }
 	    exp_continue
@@ -468,15 +465,7 @@
     gdb_test "bt" "#0 +\[^\r\n\]*\\mdl_main\\M.*" "dl bt"
 
     if $ifmain {
-	# Displacement message will be printed the second time on initializing
-	# the linker from svr4_special_symbol_handling.  If any ANOFFSET has
-	# been already set as non-zero the detection will no longer be run.
-	if {$displacement == "NONZERO"} {
-	    set displacement_main "NONE"
-	} else {
-	    set displacement_main $displacement
-	}
-	reach "main" continue $displacement_main
+	reach "main" continue "NONE"
 
 	reach "libfunc" continue "NONE"
 
@@ -542,9 +531,8 @@
 		}
 		if {$displacement == $case} {
 		    pass $test_displacement
-		    # Permit multiple such messages.
 		    set displacement "FOUND-$displacement"
-		} elseif {$displacement != "FOUND-$case"} {
+		} else {
 		    fail $test_displacement
 		}
 		exp_continue