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

Simon Marchi via Gdb-patches
2021-04-21T13:18:57Z

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="junk",times="0",original-location="main",locations=[{number="1.1",enabled="N",addr="0x000000000000114e",func="main",file="/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="junk",times="0",original-location="main",locations=[{number="2.1",enabled="N",addr="0x000000000000114e",func="main",file="/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