Prelink.exp troubles

Prelink.exp troubles

[Daniel Jacobowitz]

Hi Alexandre, two problems for you.

First: you're using gdb_expect directly.  I'm sorry I didn't notice
this before.  Don't do that; use gdb_test_multiple instead.  This
will get you the fallback case which includes things like "time out
quickly if you see an unexpected GDB prompt", which in this case
I do; prelink.exp is taking five minutes to fail for me.

Second:

drow@caradoc:/space/fsf/x86-64/commit-gdb/gdb/testsuite/gdb.base% prelink -v -NR prelink.so
Laying out 4 libraries in virtual address space 0000003000000000-0000004000000000
Random base 0x0000003594d00000
Assigned virtual address space slots for libraries:
/lib64/ld-linux-x86-64.so.2 0000003594d00000-0000003594e179e8
/lib/libc.so.6 0000003594f00000-000000359513dc68
prelink.so 0000003595200000-0000003595300928
/lib/libm.so.6 0000003595400000-00000035955851c8
Prelinking /lib/ld-2.3.5.so
/usr/sbin/prelink.bin: Could not set /lib/ld-2.3.5.so owner or mode: Operation not permitted
/usr/sbin/prelink.bin: Could not prelink /lib/libc.so.6 because its dependency /lib64/ld-linux-x86-64.so.2 could not be prelinked
/usr/sbin/prelink.bin: Could not prelink /lib/libm.so.6 because its dependency /lib/libc.so.6 could not be prelinked
/usr/sbin/prelink.bin: Could not prelink prelink.so because its dependency /lib/libm.so.6 could not be prelinked
drow@caradoc:/space/fsf/x86-64/commit-gdb/gdb/testsuite/gdb.base% echo $?
0

So prelink can "succeed" without prelinking the library, and will if
the system you're running on is non-prelinked.  This results in no
warning from GDB, which results in the test timing out and eventually
failing.

We need to either copy the libraries around (complicated, fragile), or
punt if the system ld.so is not prelinked.

Could you take a look at these?  TIA!

-- 
Daniel Jacobowitz
CodeSourcery

Re: Prelink.exp troubles

[Alexandre Oliva]

[-- Attachment #1: Type: text/plain, Size: 755 bytes --]

On Mar  1, 2006, Daniel Jacobowitz <drow@false.org> wrote:

> First: you're using gdb_expect directly.  I'm sorry I didn't notice
> this before.  Don't do that; use gdb_test_multiple instead.

Tricky to catch two independent messages triggered by the same command
that way while still producing a fail if either fails to match, so I
put them now in a single regexp.

> We need to either copy the libraries around (complicated, fragile), or
> punt if the system ld.so is not prelinked.

Punting it is :-)  Tricky, since prelink doesn't report an error in
this case.  Fortunately prelink -u does.

How's this?  I've tested it on amd64-linux-gnu, with and without
prelinked system libraries.  I got the warning in the latter case, and
a pass in the former.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: gdb-prelink-test-tweak.patch --]
[-- Type: text/x-patch, Size: 2971 bytes --]

for gdb/testsuite/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* gdb.base/prelink.exp: Add -q to prelink runs.  Rename
	executable to prelinkt.  Don't re-prelink if we didn't get a
	core file.  Test prelink -u exit status to tell whether
	prelinking failed.  Use gdb_test_multiple.

Index: trunk/gdb/testsuite/gdb.base/prelink.exp
===================================================================
--- trunk.orig/gdb/testsuite/gdb.base/prelink.exp	2006-02-28 01:28:27.000000000 -0300
+++ trunk/gdb/testsuite/gdb.base/prelink.exp	2006-03-03 10:57:20.000000000 -0300
@@ -40,8 +40,6 @@ if {$gcc_compiled == 0} {
 }
 
 set testfile "prelink"
-set srcfile ${testfile}.c
-set binfile ${objdir}/${subdir}/${testfile}
 
 set libsrcfile ${testfile}-lib.c
 set libfile ${objdir}/${subdir}/${testfile}.so
@@ -50,13 +48,13 @@ if { [gdb_compile "${srcdir}/${subdir}/$
     return -1
 }
 
-if {[catch "system \"prelink -NR ${libfile}\""] != 0} {
+if {[catch "system \"prelink -qNR ${libfile}\""] != 0} {
     # Maybe we don't have prelink.
     return -1
 }
 
 set srcfile ${testfile}.c
-set binfile ${objdir}/${subdir}/${testfile}
+set binfile ${objdir}/${subdir}/${testfile}t
 if { [gdb_compile "${srcdir}/${subdir}/${srcfile} ${libfile}" "${binfile}" executable [list debug "additional_flags=-Wl,-rpath,${objdir}/${subdir}"]] != ""} {
     return -1;
 }
@@ -82,9 +80,6 @@ if { $found == 0 } {
     }
 }
 
-catch "system \"prelink -u ${libfile}\""
-catch "system \"prelink -NR ${libfile}\""
-
 # Try to clean up after ourselves.
 remote_file build delete [file join $coredir coremmap.data]
 remote_exec build "rmdir $coredir"
@@ -94,6 +89,12 @@ if { $found == 0  } {
     return 0
 }
 
+if {[catch "system \"prelink -uN ${libfile}\""] != 0} {
+    warning "${libfile} was not prelinked, maybe system libraries are not prelinked?"
+    return 0
+}
+catch "system \"prelink -qNR ${libfile}\""
+
 # Start with a fresh gdb
 
 gdb_exit
@@ -101,26 +102,12 @@ gdb_start
 gdb_reinitialize_dir $srcdir/$subdir
 gdb_load ${binfile}
 
-set oldtimeout $timeout
-set timeout [expr "$timeout + 60"]
-verbose "Timeout is now $timeout seconds" 2
-send_gdb "core-file $objdir/$subdir/prelink.core\n"
-gdb_expect {
-    -re "warning: \.dynamic section.*not at the expected address" {
-	pass "changed base address"
-    }
-    -re ".*$gdb_prompt $"	{ fail "changed base address" }
-    timeout 		{ fail "(timeout) changed base address" }
-}
-gdb_expect {
-    -re "warning: difference.*caused by prelink, adjusting" {
-	pass "prelink adjustment"
+set test "prelink"
+gdb_test_multiple "core-file $objdir/$subdir/prelink.core" "$test" {
+    -re "warning: \.dynamic section.*not at the expected address.*warning: difference.*caused by prelink, adjusting expectations." {
+	pass "$test"
     }
-    -re ".*$gdb_prompt $"	{ fail "prelink adjustment" }
-    timeout 		{ fail "(timeout) prelink adjustment" }
 }
-set timeout $oldtimeout
-verbose "Timeout is now $timeout seconds" 2
 
 gdb_exit
 

[-- Attachment #3: Type: text/plain, Size: 249 bytes --]


-- 
Alexandre Oliva         http://www.lsd.ic.unicamp.br/~oliva/
Secretary for FSF Latin America        http://www.fsfla.org/
Red Hat Compiler Engineer   aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist  oliva@{lsd.ic.unicamp.br, gnu.org}

Re: Prelink.exp troubles

[Daniel Jacobowitz]

On Fri, Mar 03, 2006 at 11:04:21AM -0300, Alexandre Oliva wrote:
> On Mar  1, 2006, Daniel Jacobowitz <drow@false.org> wrote:
> 
> > First: you're using gdb_expect directly.  I'm sorry I didn't notice
> > this before.  Don't do that; use gdb_test_multiple instead.
> 
> Tricky to catch two independent messages triggered by the same command
> that way while still producing a fail if either fails to match, so I
> put them now in a single regexp.

Actually, it's very easy: set a flag when you see the each message,
exp_continue if you haven't seen the other.  This is fine too.

> > We need to either copy the libraries around (complicated, fragile), or
> > punt if the system ld.so is not prelinked.
> 
> Punting it is :-)  Tricky, since prelink doesn't report an error in
> this case.  Fortunately prelink -u does.
> 
> How's this?  I've tested it on amd64-linux-gnu, with and without
> prelinked system libraries.  I got the warning in the latter case, and
> a pass in the former.

Hmm, two things: I don't think a WARNING is appropriate (there's
nothing wrong), how about just an UNTESTED?  And, when matching
patterns in gdb_test_multiple (the same was true in gdb_expect)
it's important to anchor the end of the pattern to the GDB prompt.
Otherwise that prompt might not be consumed by expect, causing
new tests added at the end of the file to fail.

If you agree about the UNTESTED, with those changes it's fine to check
in.  My test runs thank you.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Prelink.exp troubles

[Alexandre Oliva]

[-- Attachment #1: Type: text/plain, Size: 853 bytes --]

On Mar  4, 2006, Daniel Jacobowitz <drow@false.org> wrote:

> Hmm, two things: I don't think a WARNING is appropriate (there's
> nothing wrong), how about just an UNTESTED?

Sure.

> And, when matching
> patterns in gdb_test_multiple (the same was true in gdb_expect)
> it's important to anchor the end of the pattern to the GDB prompt.
> Otherwise that prompt might not be consumed by expect, causing
> new tests added at the end of the file to fail.

Oh?  I thought gdb_test_multiple would only exit when it got to a
prompt or something, so it would be consumed automatically.  Is that
not so?

> If you agree about the UNTESTED, with those changes it's fine to check
> in.  My test runs thank you.

Here's what I'm checking in, in the hopes that the archoring you
suggested above is not needed, otherwise I'll take care of it in a
subsequent patch.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: gdb-prelink-test-tweak.patch --]
[-- Type: text/x-patch, Size: 4734 bytes --]

for gdb/testsuite/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* gdb.base/prelink.c, gdb.base/prelink-lib.c: Update FSF address.
	* gdb.base/prelink.exp: Likewise.  Add -q to prelink runs.
	Rename executable to prelinkt.  Don't re-prelink if we didn't
	get a core file.  Test prelink -u exit status to tell whether
	prelinking failed.  Use gdb_test_multiple.

Index: gdb/testsuite/gdb.base/prelink.exp
===================================================================
--- gdb/testsuite/gdb.base/prelink.exp.orig	2006-03-07 01:41:02.000000000 -0300
+++ gdb/testsuite/gdb.base/prelink.exp	2006-03-07 01:51:07.000000000 -0300
@@ -12,10 +12,10 @@
 #
 # You should have received a copy of the GNU General Public License
 # along with this program; if not, write to the Free Software
-# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA.
 
 # Please email any bugs, comments, and/or additions to this file to:
-# bug-gdb@prep.ai.mit.edu
+# bug-gdb@gnu.org
 
 # This file was written by Alexandre Oliva <aoliva@redhat.com>
 
@@ -40,8 +40,6 @@ if {$gcc_compiled == 0} {
 }
 
 set testfile "prelink"
-set srcfile ${testfile}.c
-set binfile ${objdir}/${subdir}/${testfile}
 
 set libsrcfile ${testfile}-lib.c
 set libfile ${objdir}/${subdir}/${testfile}.so
@@ -50,13 +48,13 @@ if { [gdb_compile "${srcdir}/${subdir}/$
     return -1
 }
 
-if {[catch "system \"prelink -NR ${libfile}\""] != 0} {
+if {[catch "system \"prelink -qNR ${libfile}\""] != 0} {
     # Maybe we don't have prelink.
     return -1
 }
 
 set srcfile ${testfile}.c
-set binfile ${objdir}/${subdir}/${testfile}
+set binfile ${objdir}/${subdir}/${testfile}t
 if { [gdb_compile "${srcdir}/${subdir}/${srcfile} ${libfile}" "${binfile}" executable [list debug "additional_flags=-Wl,-rpath,${objdir}/${subdir}"]] != ""} {
     return -1;
 }
@@ -82,9 +80,6 @@ if { $found == 0 } {
     }
 }
 
-catch "system \"prelink -u ${libfile}\""
-catch "system \"prelink -NR ${libfile}\""
-
 # Try to clean up after ourselves.
 remote_file build delete [file join $coredir coremmap.data]
 remote_exec build "rmdir $coredir"
@@ -94,6 +89,12 @@ if { $found == 0  } {
     return 0
 }
 
+if {[catch "system \"prelink -uN ${libfile}\""] != 0} {
+    untested "${libfile} was not prelinked, maybe system libraries are not prelinked?"
+    return 0
+}
+catch "system \"prelink -qNR ${libfile}\""
+
 # Start with a fresh gdb
 
 gdb_exit
@@ -101,26 +102,12 @@ gdb_start
 gdb_reinitialize_dir $srcdir/$subdir
 gdb_load ${binfile}
 
-set oldtimeout $timeout
-set timeout [expr "$timeout + 60"]
-verbose "Timeout is now $timeout seconds" 2
-send_gdb "core-file $objdir/$subdir/prelink.core\n"
-gdb_expect {
-    -re "warning: \.dynamic section.*not at the expected address" {
-	pass "changed base address"
-    }
-    -re ".*$gdb_prompt $"	{ fail "changed base address" }
-    timeout 		{ fail "(timeout) changed base address" }
-}
-gdb_expect {
-    -re "warning: difference.*caused by prelink, adjusting" {
-	pass "prelink adjustment"
+set test "prelink"
+gdb_test_multiple "core-file $objdir/$subdir/prelink.core" "$test" {
+    -re "warning: \.dynamic section.*not at the expected address.*warning: difference.*caused by prelink, adjusting expectations." {
+	pass "$test"
     }
-    -re ".*$gdb_prompt $"	{ fail "prelink adjustment" }
-    timeout 		{ fail "(timeout) prelink adjustment" }
 }
-set timeout $oldtimeout
-verbose "Timeout is now $timeout seconds" 2
 
 gdb_exit
 
Index: gdb/testsuite/gdb.base/prelink-lib.c
===================================================================
--- gdb/testsuite/gdb.base/prelink-lib.c.orig	2006-03-07 01:41:02.000000000 -0300
+++ gdb/testsuite/gdb.base/prelink-lib.c	2006-03-07 01:41:48.000000000 -0300
@@ -14,7 +14,7 @@
 
    You should have received a copy of the GNU General Public License
    along with this program; if not, write to the Free Software
-   Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA.
 */
 
 int
Index: gdb/testsuite/gdb.base/prelink.c
===================================================================
--- gdb/testsuite/gdb.base/prelink.c.orig	2006-03-07 01:41:02.000000000 -0300
+++ gdb/testsuite/gdb.base/prelink.c	2006-03-07 01:41:48.000000000 -0300
@@ -14,7 +14,7 @@
 
    You should have received a copy of the GNU General Public License
    along with this program; if not, write to the Free Software
-   Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA.
 */
 
 #include <stdio.h>

[-- Attachment #3: Type: text/plain, Size: 249 bytes --]


-- 
Alexandre Oliva         http://www.lsd.ic.unicamp.br/~oliva/
Secretary for FSF Latin America        http://www.fsfla.org/
Red Hat Compiler Engineer   aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist  oliva@{lsd.ic.unicamp.br, gnu.org}

Re: Prelink.exp troubles

[Daniel Jacobowitz]

On Tue, Mar 07, 2006 at 02:02:40AM -0300, Alexandre Oliva wrote:
> > And, when matching
> > patterns in gdb_test_multiple (the same was true in gdb_expect)
> > it's important to anchor the end of the pattern to the GDB prompt.
> > Otherwise that prompt might not be consumed by expect, causing
> > new tests added at the end of the file to fail.
> 
> Oh?  I thought gdb_test_multiple would only exit when it got to a
> prompt or something, so it would be consumed automatically.  Is that
> not so?

Take a look at the implementation, or other uses, please.  If a
user-procided case does not use exp_continue, the function will return.

> > If you agree about the UNTESTED, with those changes it's fine to check
> > in.  My test runs thank you.
> 
> Here's what I'm checking in, in the hopes that the archoring you
> suggested above is not needed, otherwise I'll take care of it in a
> subsequent patch.

Please fix the anchoring.

Thanks,

-- 
Daniel Jacobowitz
CodeSourcery

Re: Prelink.exp troubles

[Alexandre Oliva]

[-- Attachment #1: Type: text/plain, Size: 549 bytes --]

On Mar  7, 2006, Daniel Jacobowitz <drow@false.org> wrote:

> On Tue, Mar 07, 2006 at 02:02:40AM -0300, Alexandre Oliva wrote:

>> Here's what I'm checking in, in the hopes that the archoring you
>> suggested above is not needed, otherwise I'll take care of it in a
>> subsequent patch.

> Please fix the anchoring.

In the following patch, that I'm checking in as obvious, I fix
anchoring and also tidy up some code added in the original prelink
patch, such that it's absolutely obvious that the l_addr field is
properly initialized in all cases.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: gdb-prelink-tweak-more.patch --]
[-- Type: text/x-patch, Size: 1972 bytes --]

for gdb/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* solib-svr4.c (svr4_current_sos): Move up initialization of
	l_addr, such that it clearly covers all cases.

for gdb/testsuite/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* gdb.base/prelink.exp: Anchor tested-for regular expression
	on gdb prompt.

Index: gdb/solib-svr4.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-svr4.c,v
retrieving revision 1.56
diff -u -p -r1.56 solib-svr4.c
--- gdb/solib-svr4.c 28 Feb 2006 04:28:47 -0000 1.56
+++ gdb/solib-svr4.c 9 Mar 2006 00:07:26 -0000
@@ -704,6 +704,7 @@ svr4_current_sos (void)
       new->lm_info = xmalloc (sizeof (struct lm_info));
       make_cleanup (xfree, new->lm_info);
 
+      new->lm_info->l_addr = (CORE_ADDR)-1;
       new->lm_info->lm = xzalloc (lmo->link_map_size);
       make_cleanup (xfree, new->lm_info->lm);
 
@@ -744,8 +745,6 @@ svr4_current_sos (void)
 	    free_so (new);
 	  else
 	    {
-	      new->lm_info->l_addr = (CORE_ADDR)-1;
-
 	      new->next = 0;
 	      *link_ptr = new;
 	      link_ptr = &new->next;
Index: gdb/testsuite/gdb.base/prelink.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/prelink.exp,v
retrieving revision 1.2
diff -u -p -r1.2 prelink.exp
--- gdb/testsuite/gdb.base/prelink.exp 7 Mar 2006 05:05:29 -0000 1.2
+++ gdb/testsuite/gdb.base/prelink.exp 9 Mar 2006 00:07:26 -0000
@@ -103,8 +103,9 @@ gdb_reinitialize_dir $srcdir/$subdir
 gdb_load ${binfile}
 
 set test "prelink"
+global gdb_prompt
 gdb_test_multiple "core-file $objdir/$subdir/prelink.core" "$test" {
-    -re "warning: \.dynamic section.*not at the expected address.*warning: difference.*caused by prelink, adjusting expectations." {
+    -re "warning: \.dynamic section.*not at the expected address.*warning: difference.*caused by prelink, adjusting expectations.*$gdb_prompt $" {
 	pass "$test"
     }
 }

[-- Attachment #3: Type: text/plain, Size: 249 bytes --]


-- 
Alexandre Oliva         http://www.lsd.ic.unicamp.br/~oliva/
Secretary for FSF Latin America        http://www.fsfla.org/
Red Hat Compiler Engineer   aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist  oliva@{lsd.ic.unicamp.br, gnu.org}

Re: Prelink.exp troubles

[Daniel Jacobowitz]

On Wed, Mar 08, 2006 at 09:20:24PM -0300, Alexandre Oliva wrote:
> On Mar  7, 2006, Daniel Jacobowitz <drow@false.org> wrote:
> 
> > On Tue, Mar 07, 2006 at 02:02:40AM -0300, Alexandre Oliva wrote:
> 
> >> Here's what I'm checking in, in the hopes that the archoring you
> >> suggested above is not needed, otherwise I'll take care of it in a
> >> subsequent patch.
> 
> > Please fix the anchoring.
> 
> In the following patch, that I'm checking in as obvious, I fix
> anchoring and also tidy up some code added in the original prelink
> patch, such that it's absolutely obvious that the l_addr field is
> properly initialized in all cases.

Belatedly, thanks for fixing this.

-- 
Daniel Jacobowitz
CodeSourcery