RE: [PATCH v3 3/4] gdb/breakpoint: add a 'force_condition' parameter to 'create_breakpoint'

Aktemur, Tankut Baris via Gdb-patches
2021-04-21T13:29:51Z

On Wednesday, April 21, 2021 3:19 PM, Simon Marchi wrote:
> On 2021-04-21 8:17 a.m., Tankut Baris Aktemur wrote:> The 'create_breakpoint' function takes
> a 'parse_extra' argument that
> > determines whether the condition, thread, and force-condition
> > specifiers should be parsed from the extra string or be used from the
> > function arguments. However, for the case when 'parse_extra' is
> > false, there is no way to pass the force-condition specifier. This
> > patch adds it as a new argument.
> >
> > Also, in the case when parse_extra is false, the current behavior is
> > as if the condition is being forced. This is a bug. The default
> > behavior should reject the breakpoint. See below for a demo of this
> > incorrect behavior. (The MI command '-break-insert' uses the
> > 'create_breakpoint' function with parse_extra=0.)
> >
> > $ gdb -q --interpreter=mi3 /tmp/simple
> > =thread-group-added,id="i1"
> > =cmd-param-changed,param="history save",value="on"
> > =cmd-param-changed,param="auto-load safe-path",value="/"
> > ~"Reading symbols from /tmp/simple...\n"
> > (gdb)
> > -break-insert -c junk -f main
> > &"warning: failed to validate condition at location 1, disabling:\n "
> > &"No symbol \"junk\" in current context.\n"
> >
> ^done,bkpt={number="1",type="breakpoint",disp="keep",enabled="y",addr="<MULTIPLE>",cond="jun
> k",times="0",original-
> location="main",locations=[{number="1.1",enabled="N",addr="0x000000000000114e",func="main",f
> ile="/tmp/simple.c",fullname="/tmp/simple.c",line="2",thread-groups=["i1"]}]}
> > (gdb)
> > break main if junk
> > &"break main if junk\n"
> > &"No symbol \"junk\" in current context.\n"
> > ^error,msg="No symbol \"junk\" in current context."
> > (gdb)
> > break main -force-condition if junk
> > &"break main -force-condition if junk\n"
> > ~"Note: breakpoint 1 also set at pc 0x114e.\n"
> > &"warning: failed to validate condition at location 1, disabling:\n "
> > &"No symbol \"junk\" in current context.\n"
> > ~"Breakpoint 2 at 0x114e: file /tmp/simple.c, line 2.\n"
> > =breakpoint-
> created,bkpt={number="2",type="breakpoint",disp="keep",enabled="y",addr="<MULTIPLE>",cond="j
> unk",times="0",original-
> location="main",locations=[{number="2.1",enabled="N",addr="0x000000000000114e",func="main",f
> ile="/tmp/simple.c",fullname="/tmp/simple.c",line="2",thread-groups=["i1"]}]}
> > ^done
> > (gdb)
> >
> > After applying this patch, we get the behavior below:
> >
> > (gdb)
> > -break-insert -c junk -f main
> > ^error,msg="No symbol \"junk\" in current context."
> >
> > This restores the behavior that is present in the existing releases.
> >
> > gdb/ChangeLog:
> > 2021-04-06 Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
> >
> > * breakpoint.h (create_breakpoint): Add a new parameter,
> > 'force_condition'.
> > * breakpoint.c (create_breakpoint): Use the 'force_condition'
> > argument when 'parse_extra' is false to check if the condition
> > is invalid at all of the breakpoint locations.
> > Update the users below.
> > (break_command_1)
> > (dprintf_command)
> > (trace_command)
> > (ftrace_command)
> > (strace_command)
> > (create_tracepoint_from_upload): Update.
> > * guile/scm-breakpoint.c (gdbscm_register_breakpoint_x): Update.
> > * mi/mi-cmd-break.c (mi_cmd_break_insert_1): Update.
> > * python/py-breakpoint.c (bppy_init): Update.
> > * python/py-finishbreakpoint.c (bpfinishpy_init): Update.
> >
> > gdb/testsuite/ChangeLog:
> > 2021-04-06 Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
> >
> > * gdb.mi/mi-break.exp: Extend with checks for invalid breakpoint
> > conditions.
>
> Up to this patch, it all LGTM and I think it could be merged right away.
>
> Thanks,
>
> Simon

Just to make sure, did you mean "up to and including this patch"?

-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