Re: [patch] validate binary before use

Jan Kratochvil
2013-04-03T18:09:32Z

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