RE: [PATCH v3 4/4] gdb/mi: add a '--force-condition' flag to the '-break-insert' cmd

Aktemur, Tankut Baris via Gdb-patches
2021-04-21T13:49:58Z

On Wednesday, April 21, 2021 3:37 PM, Simon Marchi wrote:
> > diff --git a/gdb/testsuite/gdb.mi/mi-break.exp b/gdb/testsuite/gdb.mi/mi-break.exp
> > index e46a6cc0ec6..5f1c6810389 100644
> > --- a/gdb/testsuite/gdb.mi/mi-break.exp
> > +++ b/gdb/testsuite/gdb.mi/mi-break.exp
> > @@ -408,6 +408,30 @@ proc_with_prefix test_explicit_breakpoints {} {
> > ".*Source filename requires function, label, or line offset.*"
> > }
> >
> > +# Test forcing an invalid condition.
> > +
> > +proc_with_prefix test_forced_conditions {} {
> > + set warning ".*warning: failed to validate condition .* disabling.*"
> > +
> > + # Define a plain breakpoint first, and a condition later.
> > + mi_create_breakpoint "callme" "define a bp" ""
> > + mi_gdb_test "-break-condition --force \$bpnum bad" \
> > + "${warning}\\^done" \
> > + "invalid condition is forced"
>
> This literally sends
>
> -break-condition --force $bpnum bad
>
> But I don't understand what $bpnum refers to, can you explain? I'm a bit lost.

It's GDB's internal variable that refers to the last-defined breakpoint's number.
It was my attempt to avoid hard-coding the breakpoint number.

> > +
> > + set loc [mi_make_breakpoint_loc -enabled "N"]
> > + set args [list -cond "bad" -locations "\\\[$loc\\\]"]
> > + set bp [eval mi_make_breakpoint_multi $args]
> > +
> > + mi_gdb_test "-break-insert -c bad --force-condition callme" \
> > + "${warning}\\^done,$bp" \
> > + "breakpoint with forced condition"
>
> 👍 (are we allowed emojis here?)
>
> > +
> > + mi_gdb_test "-dprintf-insert -c bad --force-condition callme 123" \
> > + "${warning}\\^done,$bp" \
> > + "dprintf with forced condition"
>
> It's really just a nit because it doesn't affect the test, but a dprintf
> would normally take a format string, so I guess you meant:
>
> -dprintf-insert -c bad --force-condition callme "%d" 123
>
> Or something like that.

I meant "123" as a dummy format string that takes no parameters.
I'll convert it to `"%d" 123` to avoid confusion.

> So the patch pretty much LGTM, except the --force vs -force issue
> mentioned in my other message.
>
> Simon

Thanks, I'll work on that part and send a revision.

-Baris

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928