Re: [patch] validate binary before use

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

On Tue, 02 Apr 2013 19:18:09 +0200, Aleksandar Ristovski wrote:
> [AR]
> Ok, it's been a while since I looked at gdb.exp closely, and
> 'no-message' was added since. Changed the test as so:
> 
> @@ -93,15 +93,7 @@ proc solib_matching_test { solibfile symsloaded msg } {
> 
>    set bp_location [gdb_get_line_number "set breakpoint 1 here"]
> 
> -  send_gdb "tbreak ${srcfile}:${bp_location}\n"
> -  gdb_expect {
> -    -re "Temporary breakpoint.*${gdb_prompt} $" {
> -    }
> -    default {
> -      untested "${msg} - Failed to set temp. breakpoint at ${bp_location}"
> -      return -1
> -    }
> -  }
> +  gdb_breakpoint ${srcfile}:${bp_location} temporary no-message

"no-message" should not be used there, there should be just:
    gdb_breakpoint ${srcfile}:${bp_location} temporary

Copy-pasting here the comment of gdb_break from lib/gdb.exp:

# Note: The handling of message vs no-message is messed up, but it's based
# on historical usage.  By default this function does not print passes,
# only fails.
# no-message: turns off printing of fails (and passes, but they're already off)
# message: turns on printing of passes (and fails, but they're already on)

I have even verified now the behavior really fully corresponds to this comment.

You do not want to suppress FAIL if ${srcfile}:${bp_location} is not found.


> >Again, successful runto_main does not produce any PASS message.
> 
> 
> [AR]
> It does produce PASS (see gdb.exp, lines 476-488),

It does not, runto has the same parameters and behavior as gdb_breakpoint.

On those lines is:
            if { $print_pass } {
                pass $test_name
            }
 
But $print_pass is true only if "message" parameter was specified, "message"
parameter should not be specified.


> but this comment is not relevant as test does not use 'run' any more, it
> uses gdb_run_cmd; it does not need to stop at main, just run to the line it
> wants.

There is now:

+  gdb_run_cmd { ${binlibfiledirrun} }
+  gdb_expect {
+    -re "set breakpoint 1 here.*${gdb_prompt} $" {
+    }
+    default {
+      untested "${msg} - Failed to hit breakpoint at ${bp_location}"
+      return -1
+    }
+  }

Never use gdb_expect, there is gdb_test_multiple for such cases which
automatically handles various errors, that "default" case is not needed there.

But here even gdb_test_multiple is not needed, use just:

gdb_run_cmd { ${binlibfiledirrun} }
gdb_test "" "set breakpoint 1 here.*" ""

First empty parameter of gdb_test does not send anything, even no newline.


> >But when you expect only one shared library make the expectation explicit,
> >both for a single line and for ${binlibfilebase}.
> >
> >   gdb_test "info sharedlibrary ${solibfile}" \
> >     "From\[^\r\n\]*To\[^\r\n\]*Syms\[^\r\n\]*Read\[^\r\n\]*Shared\[^\r\n\]*\r\n\[^\r\n\]*${symsloaded}\[^\r\n\]*[string_to_regexp ${binlibfilebase}]" \
> >   (I did not test this regex.)
> >
> >I can very well imagine GDB could print >= 2 lines or a line without
> >${binlibfilebase} there which could make false PASS.
> 
> [AR] How can it print >= 2 lines?

For example when "set auto-solib-add off" starts to behave differently and for
example vDSO get still displayed (if they get displayed) etc.


> I will augument regex to explicitly look for ${solibfile}

This will not work as if there are >= 2 lines ${symsloaded} may match on one
line and ${solibfile} may match on a different line.


> 
> @@ -124,7 +124,7 @@ proc solib_matching_test { solibfile symsloaded msg } {
>    }
> 
>    gdb_test "info sharedlibrary ${solibfile}" \
> -    "From.*To.*Syms.*Read.*Shared.*\r\n.*${symsloaded}.*" \
> +    "From.*To.*Syms.*Read.*Shared.*\r\n.*${symsloaded}.*${solibfile}.*" \
>      "${msg} - Symbols for ${solibfile} loaded: expected '${symsloaded}'"
>    return 0
>  }


Thanks,
Jan