[3/4] print-symbol-loading.exp: Allow libc symbols to be already loaded

Message ID 20210610172641.1052704-4-kevinb@redhat.com
State New
Headers show
Series
  • libthread_db initialization changes related to upcoming glibc-2.34
Related show

Commit Message

Simon Marchi via Gdb-patches June 10, 2021, 5:26 p.m.
One consequence of changing libpthread_name_p() in solib.c to (also)
match libc is that the symbols for libc will now be loaded by
solib_add() in solib.c.  I think this is mostly harmless because
we'll likely want these symbols to be loaded anyway, but it did cause
two failures in gdb.base/print-symbol-loading.exp.

Specifically...

1)

sharedlibrary .*
(gdb) PASS: gdb.base/print-symbol-loading.exp: shlib off: load shared-lib

now looks like this:

sharedlibrary .*
Symbols already loaded for /lib64/libc.so.6
(gdb) PASS: gdb.base/print-symbol-loading.exp: shlib off: load shared-lib

2)

sharedlibrary .*
Loading symbols for shared libraries: .*
(gdb) PASS: gdb.base/print-symbol-loading.exp: shlib brief: load shared-lib

now looks like this:

sharedlibrary .*
Loading symbols for shared libraries: .*
Symbols already loaded for /lib64/libc.so.6
(gdb) PASS: gdb.base/print-symbol-loading.exp: shlib brief: load shared-lib

Fixing case #2 ended up being easier than #1.  #1 had been using
gdb_test_no_output to correctly match this no-output case.  I
ended up replacing it with gdb_test_multiple, matching the exact
expected output for each of the two now acceptable cases.

For case #2, I simply added an optional non-capturing group
for the potential new output.

gdb/testsuite/ChangeLog:

	* gdb.base/print-symbol-loading.exp (proc test_load_shlib):
	Allow "Symbols already loaded for..." messages.
---
 gdb/testsuite/gdb.base/print-symbol-loading.exp | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

-- 
2.31.1

Comments

Simon Marchi via Gdb-patches June 11, 2021, 7:22 p.m. | #1
On 2021-06-10 1:26 p.m., Kevin Buettner via Gdb-patches wrote:
> One consequence of changing libpthread_name_p() in solib.c to (also)

> match libc is that the symbols for libc will now be loaded by

> solib_add() in solib.c.  I think this is mostly harmless because

> we'll likely want these symbols to be loaded anyway, but it did cause

> two failures in gdb.base/print-symbol-loading.exp.

> 

> Specifically...

> 

> 1)

> 

> sharedlibrary .*

> (gdb) PASS: gdb.base/print-symbol-loading.exp: shlib off: load shared-lib

> 

> now looks like this:

> 

> sharedlibrary .*

> Symbols already loaded for /lib64/libc.so.6

> (gdb) PASS: gdb.base/print-symbol-loading.exp: shlib off: load shared-lib

> 

> 2)

> 

> sharedlibrary .*

> Loading symbols for shared libraries: .*

> (gdb) PASS: gdb.base/print-symbol-loading.exp: shlib brief: load shared-lib

> 

> now looks like this:

> 

> sharedlibrary .*

> Loading symbols for shared libraries: .*

> Symbols already loaded for /lib64/libc.so.6

> (gdb) PASS: gdb.base/print-symbol-loading.exp: shlib brief: load shared-lib

> 

> Fixing case #2 ended up being easier than #1.  #1 had been using

> gdb_test_no_output to correctly match this no-output case.  I

> ended up replacing it with gdb_test_multiple, matching the exact

> expected output for each of the two now acceptable cases.

> 

> For case #2, I simply added an optional non-capturing group

> for the potential new output.

> 

> gdb/testsuite/ChangeLog:

> 

> 	* gdb.base/print-symbol-loading.exp (proc test_load_shlib):

> 	Allow "Symbols already loaded for..." messages.

> ---

>  gdb/testsuite/gdb.base/print-symbol-loading.exp | 15 ++++++++++++---

>  1 file changed, 12 insertions(+), 3 deletions(-)

> 

> diff --git a/gdb/testsuite/gdb.base/print-symbol-loading.exp b/gdb/testsuite/gdb.base/print-symbol-loading.exp

> index b8eb1c844bd..6e176de351e 100644

> --- a/gdb/testsuite/gdb.base/print-symbol-loading.exp

> +++ b/gdb/testsuite/gdb.base/print-symbol-loading.exp

> @@ -96,6 +96,7 @@ test_load_core full

>  

>  proc test_load_shlib { print_symbol_loading } {

>      global binfile

> +    global gdb_prompt

>      with_test_prefix "shlib ${print_symbol_loading}" {

>  	clean_restart ${binfile}

>  	gdb_test_no_output "set auto-solib-add off"

> @@ -106,12 +107,20 @@ proc test_load_shlib { print_symbol_loading } {

>  	set test_name "load shared-lib"

>  	switch ${print_symbol_loading} {

>  	    "off" {

> -		gdb_test_no_output "sharedlibrary .*" \

> -		    ${test_name}

> +		set cmd "sharedlibrary .*"

> +		set cmd_regex [string_to_regexp $cmd]

> +		gdb_test_multiple $cmd $test_name {

> +        	    -re "^$cmd_regex\r\n$gdb_prompt $" {

> +			pass $test_name

> +		    }

> +        	    -re "^$cmd_regex\r\nSymbols already loaded for.*?\\/libc\\..*?\r\n$gdb_prompt $" {


I'm just wondering about these regexps:

 - What does the sequence .*? mean in a regexp?
 - Doesn't having .* risk matching multiple lines, including some
   unexpected output that we wouldn't want to match?  Should we use the
   typical [^\r\n]* instead?

Simon
Simon Marchi via Gdb-patches June 11, 2021, 7:33 p.m. | #2
On Fri, 11 Jun 2021 15:22:23 -0400
Simon Marchi <simon.marchi@polymtl.ca> wrote:

> On 2021-06-10 1:26 p.m., Kevin Buettner via Gdb-patches wrote:


> > ---

> >  gdb/testsuite/gdb.base/print-symbol-loading.exp | 15 ++++++++++++---

> >  1 file changed, 12 insertions(+), 3 deletions(-)

> > 

> > diff --git a/gdb/testsuite/gdb.base/print-symbol-loading.exp b/gdb/testsuite/gdb.base/print-symbol-loading.exp

> > index b8eb1c844bd..6e176de351e 100644

> > --- a/gdb/testsuite/gdb.base/print-symbol-loading.exp

> > +++ b/gdb/testsuite/gdb.base/print-symbol-loading.exp

> > @@ -96,6 +96,7 @@ test_load_core full

> >  

> >  proc test_load_shlib { print_symbol_loading } {

> >      global binfile

> > +    global gdb_prompt

> >      with_test_prefix "shlib ${print_symbol_loading}" {

> >  	clean_restart ${binfile}

> >  	gdb_test_no_output "set auto-solib-add off"

> > @@ -106,12 +107,20 @@ proc test_load_shlib { print_symbol_loading } {

> >  	set test_name "load shared-lib"

> >  	switch ${print_symbol_loading} {

> >  	    "off" {

> > -		gdb_test_no_output "sharedlibrary .*" \

> > -		    ${test_name}

> > +		set cmd "sharedlibrary .*"

> > +		set cmd_regex [string_to_regexp $cmd]

> > +		gdb_test_multiple $cmd $test_name {

> > +        	    -re "^$cmd_regex\r\n$gdb_prompt $" {

> > +			pass $test_name

> > +		    }

> > +        	    -re "^$cmd_regex\r\nSymbols already loaded for.*?\\/libc\\..*?\r\n$gdb_prompt $" {  

> 

> I'm just wondering about these regexps:

> 

>  - What does the sequence .*? mean in a regexp?

>  - Doesn't having .* risk matching multiple lines, including some

>    unexpected output that we wouldn't want to match?  Should we use the

>    typical [^\r\n]* instead?


The regexp ".*?" is the non-greedy variation of ".*".  I.e. whereas ".*"
matches as much as possible, ".*?" will match as little as possible.
Therefore, ".*?\r\n" will only match only as much as needed until
the next \r is found; therefore it will not match multiple lines.

Kevin
Simon Marchi via Gdb-patches June 11, 2021, 9:05 p.m. | #3
On Fri, 11 Jun 2021 12:33:33 -0700
Kevin Buettner via Gdb-patches <gdb-patches@sourceware.org> wrote:

> On Fri, 11 Jun 2021 15:22:23 -0400

> Simon Marchi <simon.marchi@polymtl.ca> wrote:

>>		    }

> > > +        	    -re "^$cmd_regex\r\nSymbols already loaded for.*?\\/libc\\..*?\r\n$gdb_prompt $" {    

> > 

> > I'm just wondering about these regexps:

> > 

> >  - What does the sequence .*? mean in a regexp?

> >  - Doesn't having .* risk matching multiple lines, including some

> >    unexpected output that we wouldn't want to match?  Should we use the

> >    typical [^\r\n]* instead?  

> 

> The regexp ".*?" is the non-greedy variation of ".*".  I.e. whereas ".*"

> matches as much as possible, ".*?" will match as little as possible.

> Therefore, ".*?\r\n" will only match only as much as needed until

> the next \r is found; therefore it will not match multiple lines.


After some IRC discussion, we both agreed that it'd be safer to use
[^\r\n]* instead.  So that line left quoted above looks like this now:

		    -re "^$cmd_regex\r\nSymbols already loaded for\[^\r\n\]*\\/libc\\.\[^\r\n\]*\r\n$gdb_prompt $" {

Kevin

Patch

diff --git a/gdb/testsuite/gdb.base/print-symbol-loading.exp b/gdb/testsuite/gdb.base/print-symbol-loading.exp
index b8eb1c844bd..6e176de351e 100644
--- a/gdb/testsuite/gdb.base/print-symbol-loading.exp
+++ b/gdb/testsuite/gdb.base/print-symbol-loading.exp
@@ -96,6 +96,7 @@  test_load_core full
 
 proc test_load_shlib { print_symbol_loading } {
     global binfile
+    global gdb_prompt
     with_test_prefix "shlib ${print_symbol_loading}" {
 	clean_restart ${binfile}
 	gdb_test_no_output "set auto-solib-add off"
@@ -106,12 +107,20 @@  proc test_load_shlib { print_symbol_loading } {
 	set test_name "load shared-lib"
 	switch ${print_symbol_loading} {
 	    "off" {
-		gdb_test_no_output "sharedlibrary .*" \
-		    ${test_name}
+		set cmd "sharedlibrary .*"
+		set cmd_regex [string_to_regexp $cmd]
+		gdb_test_multiple $cmd $test_name {
+        	    -re "^$cmd_regex\r\n$gdb_prompt $" {
+			pass $test_name
+		    }
+        	    -re "^$cmd_regex\r\nSymbols already loaded for.*?\\/libc\\..*?\r\n$gdb_prompt $" {
+			pass $test_name
+		    }
+		}
 	    }
 	    "brief" {
 		gdb_test "sharedlibrary .*" \
-		    "Loading symbols for shared libraries: \\.\\*" \
+		    "Loading symbols for shared libraries: \\.\\*.*?(?:Symbols already loaded for .*?libc)?" \
 		    ${test_name}
 	    }
 	    "full" {