This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch] validate binary before use
- From: Aleksandar Ristovski <aristovski at qnx dot com>
- To: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- Cc: "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>
- Date: Tue, 2 Apr 2013 13:18:09 -0400
- Subject: Re: [patch] validate binary before use
- References: <20121227211328 dot GA5739 at host2 dot jankratochvil dot net> <50DCBBD1 dot 7000707 at qnx dot com> <5107F591 dot 304 at qnx dot com> <20130131063518 dot GA3027 at host2 dot jankratochvil dot net> <510A7EB0 dot 90702 at qnx dot com> <51278A2A dot 9000802 at qnx dot com> <512E42D1 dot 3040101 at qnx dot com> <514C58B2 dot 6090701 at qnx dot com> <20130328183727 dot GA14798 at host2 dot jankratochvil dot net> <515B0632 dot 1040502 at qnx dot com> <20130402165306 dot GA9479 at host2 dot jankratochvil dot net>
On 13-04-02 12:53 PM, Jan Kratochvil wrote:
Hi Aleksandar,
just some obvious issues of the testsuite first:
On Tue, 02 Apr 2013 18:24:18 +0200, Aleksandar Ristovski wrote:
+ send_gdb "set verbose 1\n"
Never (only in some exceptional cases) use send_gdb, it creates races wrt
syncing on end of the commands. Use gdb_test or gdb_test_no_output.
[AR] I very much dislike using gdb_test unless I actually am doing a
test. Otherwise, we end up with testcases that tend to have 30-40
passes but only 2-3 relevant. Thus, when these 2-3 relevant ones
start to FAIL it is easy to neglect that due to false cozy feeling
that, well, *most* are still passing.
* Even a single PASS->FAIL can be a serious GDB regression.
* There are still many racy testcases (with "random" results).
* Therefore comparing any PASS/FAIL counts is irrelevant, only diff matters.
Besides that send_gdb really does not work, it does not read the "(gdb) "
response will confuse the later first test which does wait for a response.
[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
gdb_run_cmd { ${binlibfiledirrun} }
gdb_expect {
If you do not like trivial testcase names then just use:
gdb_test_no_output "command" ""
or
gdb_test "command" "response" ""
GDB testsuite handles testcase name "" by omitting it from the output.
+ send_gdb "tbreak ${srcfile}:${bp_location}\n"
Do not use send_gdb and there is gdb_breakpoint function.
[AR] I am not testing setting breakpoints. I do not want these to
show up as PASS-es. These passes are irrelevant. The assumption is
that breakpoints do work; there are other tests for breakponts.
gdb_breakpoint does not produce any PASS message when it succeeds.
But it will FAIL if a problem occured.
+ send_gdb "run\r\n"
Use runto_main. And check its result code.
[AR] The same. I am not testing run to main. I am testing this
particular feature. There are other tests that test runto_main.
Again, successful runto_main does not produce any PASS message.
[AR]
It does produce PASS (see gdb.exp, lines 476-488), 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.
+ gdb_test "info sharedlibrary ${solibfile}" \
+ ".*From.*To.*Syms.*Read.*Shared.*\r\n.*${symsloaded}.*" \
^^
BTW leading .* is excessive, gdb_test regex does not have anchored its start.
[AR] ok.
+ "Symbols for ${solibfile} loaded: expected '${symsloaded}'"
Protect ${symsloaded} by [string_to_regexp $string] as user
may have regex-unsafe characters there.
[AR] symsloaded is argument passed to solib_matching_test, and the
test is the only user. Ther eare no other users, and the string may
contain only 'Yes' or 'No'.
OK, I did not notice, I agree string_to_regexp is not needed there.
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? I will augument regex to explicitly
look for ${solibfile}
@@ -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,
Aleksandar