From: Joel Brobecker <brobecker@adacore.com>
To: Jan Kratochvil <jan.kratochvil@redhat.com>
Cc: Tom Tromey <tromey@redhat.com>, gdb-patches@sourceware.org
Subject: Re: [patch] Fix internal error on breaking at a multi-locations caller
Date: Wed, 06 May 2009 16:51:00 -0000 [thread overview]
Message-ID: <20090506165143.GM10734@adacore.com> (raw)
In-Reply-To: <20090501173212.GA31331@host0.dyn.jankratochvil.net>
Hi Jan,
> gdb/
> 2009-05-01 Joel Brobecker <brobecker@adacore.com>
> Jan Kratochvil <jan.kratochvil@redhat.com>
>
> Fix internal error on breaking at a multi-locations caller source line.
> * breakpoint.c (parse_breakpoint_sals): Set EXPLICIT_PC for the `break'
> command with no parameters.
The code part is OK, but I was a little confused by the comment.
I think this is because you're talking about your specific scenario
whereas I'm ready to bet that the issue can appear without doing
an "up" before. My suggestion is to keep it general, but saying
something like: "break" without arguments is equivalent to "break *PC"
where PC is the default_breakpoint_address. So make sure to set
sal.explicit_pc to prevent GDB from trying to expand the list of
sals to include all other instances with the same symtab and line.
> +# PC should not be at the boundary of source lines to make the original bug
> +# exploitable.
> +
> +set test "p/x \$pc"
> +set pc {}
> +gdb_test_multiple $test $test {
> + -re "\\$\[0-9\]+ = (0x\[0-9a-f\]+)\r\n$gdb_prompt $" {
> + set pc $expect_out(1,string)
> + pass $test
> + }
> +}
> +
> +set test "info line"
> +set end {}
> +gdb_test_multiple $test $test {
> + -re "Line \[0-9\]+ of .* starts at address 0x\[0-9a-f\]+.* and ends at (0x\[0-9a-f\]+).*\\.\r\n$gdb_prompt $" {
> + set end $expect_out(1,string)
> + pass $test
> + }
> +}
> +
> +set test "caller line has trailing code"
> +if {$pc != $end} {
> + pass $test
> +} else {
> + fail $test
> +}
I don't think this is right - the fail here is conditional on the code
generated by the compiler on the given platform. If the return address
points at the beginning of the next line of code, then our testcase
won't allow us to test the issue. But there's no real failure
demonstrated at this point.
My feeling on this is that we should just do the testing you do after
without worrying whether the break address is at the beginning of
a line or not. So I'd just ditch the above, and keep the rest of
the testcase as is.
--
Joel
next prev parent reply other threads:[~2009-05-06 16:51 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-03-09 22:17 Jan Kratochvil
2009-04-24 1:01 ` Tom Tromey
2009-04-28 20:32 ` Joel Brobecker
2009-05-01 9:20 ` Jan Kratochvil
2009-05-01 15:48 ` Joel Brobecker
2009-05-01 17:32 ` Jan Kratochvil
2009-05-06 16:51 ` Joel Brobecker [this message]
2009-05-10 18:33 ` Jan Kratochvil
2009-05-11 9:11 ` Joel Brobecker
2009-05-11 15:07 ` Jan Kratochvil
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20090506165143.GM10734@adacore.com \
--to=brobecker@adacore.com \
--cc=gdb-patches@sourceware.org \
--cc=jan.kratochvil@redhat.com \
--cc=tromey@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox