Re: [PATCH v4 1/2] gdb/breakpoint: disable a bp location if condition is invalid at that location

[Simon Marchi <simark@simark.ca>]

On 2020-10-13 8:25 a.m., Tankut Baris Aktemur via Gdb-patches wrote:
> Currently, for a conditional breakpoint, GDB checks if the condition
> can be evaluated in the context of the first symtab and line (SAL).
> In case of an error, defining the conditional breakpoint is aborted.
> This prevents having a conditional breakpoint whose condition may
> actually be meaningful for some of the location contexts.  This patch
> makes it possible to define conditional BPs by checking all location
> contexts.  If the condition is meaningful for even one context, the
> breakpoint is defined.  The locations for which the condition gives
> errors are disabled.
>
> The bp_location struct is introduced a new field, 'disabled_by_cond'.
> This field denotes whether the location is disabled automatically
> because the condition was non-evaluatable.  Disabled-by-cond locations
> cannot be enabled by the user.  But locations that are not
> disabled-by-cond can be enabled/disabled by the user manually as
> before.
>
> For a concrete example, consider 3 contexts of a function 'func'.
>
>   class Base
>   {
>   public:
>     int b = 20;
>
>     void func () {}
>   };
>
>   class A : public Base
>   {
>   public:
>     int a = 10;
>
>     void func () {}
>   };
>
>   class C : public Base
>   {
>   public:
>     int c = 30;
>
>     void func () {}
>   };
>
> Note that
>
> * the variable 'a' is defined only in the context of A::func.
> * the variable 'c' is defined only in the context of C::func.
> * the variable 'b' is defined in all the three contexts.
>
> With the existing GDB, it's not possible to define a conditional
> breakpoint at 'func' if the condition refers to 'a' or 'c':
>
>   (gdb) break func if a == 10
>   No symbol "a" in current context.
>   (gdb) break func if c == 30
>   No symbol "c" in current context.
>   (gdb) info breakpoints
>   No breakpoints or watchpoints.
>
> With this patch, it becomes possible:
>
>   (gdb) break func if a == 10
>   warning: failed to validate condition at location 1, disabling:
>     No symbol "a" in current context.
>   warning: failed to validate condition at location 3, disabling:
>     No symbol "a" in current context.
>   Breakpoint 1 at 0x11b6: func. (3 locations)
>   (gdb) break func if c == 30
>   Note: breakpoint 1 also set at pc 0x11ce.
>   Note: breakpoint 1 also set at pc 0x11c2.
>   Note: breakpoint 1 also set at pc 0x11b6.
>   warning: failed to validate condition at location 1, disabling:
>     No symbol "c" in current context.
>   warning: failed to validate condition at location 2, disabling:
>     No symbol "c" in current context.
>   Breakpoint 2 at 0x11b6: func. (3 locations)
>   (gdb) info breakpoints
>   Num     Type           Disp Enb Address            What
>   1       breakpoint     keep y   <MULTIPLE>
>           stop only if a == 10
>   1.1                         N*  0x00000000000011b6 in Base::func() at condbreak-multi-context.cc:23
>   1.2                         y   0x00000000000011c2 in A::func() at condbreak-multi-context.cc:31
>   1.3                         N*  0x00000000000011ce in C::func() at condbreak-multi-context.cc:39
>   2       breakpoint     keep y   <MULTIPLE>
>           stop only if c == 30
>   2.1                         N*  0x00000000000011b6 in Base::func() at condbreak-multi-context.cc:23
>   2.2                         N*  0x00000000000011c2 in A::func() at condbreak-multi-context.cc:31
>   2.3                         y   0x00000000000011ce in C::func() at condbreak-multi-context.cc:39
>   (*): Breakpoint condition is invalid at this location.
>
> Here, uppercase 'N' denotes that the location is disabled because of
> the invalid condition, as mentioned with a footnote in the legend of
> the table.  Locations that are disabled by the user are still denoted
> with lowercase 'n'.  Executing the code hits the breakpoints 1.2 and
> 2.3 as expected.
>
> Defining a condition on an unconditional breakpoint gives the same
> behavior above:
>
>   (gdb) break func
>   Breakpoint 1 at 0x11b6: func. (3 locations)
>   (gdb) cond 1 a == 10
>   warning: failed to validate condition at location 1.1, disabling:
>     No symbol "a" in current context.
>   warning: failed to validate condition at location 1.3, disabling:
>     No symbol "a" in current context.
>   (gdb) info breakpoints
>   Num     Type           Disp Enb Address            What
>   1       breakpoint     keep y   <MULTIPLE>
>           stop only if a == 10
>   1.1                         N*  0x00000000000011b6 in Base::func() at condbreak-multi-context.cc:23
>   1.2                         y   0x00000000000011c2 in A::func() at condbreak-multi-context.cc:31
>   1.3                         N*  0x00000000000011ce in C::func() at condbreak-multi-context.cc:39
>   (*): Breakpoint condition is invalid at this location.
>
> Locations that are disabled because of a condition cannot be enabled
> by the user:
>
>   ...
>   (gdb) enable 1.1
>   Breakpoint 1's condition is invalid at location 1, cannot enable.
>
> Resetting the condition enables the locations back:
>
>   ...
>   (gdb) cond 1
>   Breakpoint 1's condition is now valid at location 1, enabling.
>   Breakpoint 1's condition is now valid at location 3, enabling.
>   Breakpoint 1 now unconditional.
>   (gdb) info breakpoints
>   Num     Type           Disp Enb Address            What
>   1       breakpoint     keep y   <MULTIPLE>
>   1.1                         y   0x00000000000011b6 in Base::func() at condbreak-multi-context.cc:23
>   1.2                         y   0x00000000000011c2 in A::func() at condbreak-multi-context.cc:31
>   1.3                         y   0x00000000000011ce in C::func() at condbreak-multi-context.cc:39
>
> If a location is disabled by the user, a condition can still be defined
> but the location will remain disabled even if the condition is meaningful
> for the disabled location:
>
>   ...
>   (gdb) disable 1.2
>   (gdb) cond 1 a == 10
>   warning: failed to validate condition at location 1.1, disabling:
>     No symbol "a" in current context.
>   warning: failed to validate condition at location 1.3, disabling:
>     No symbol "a" in current context.
>   (gdb) info breakpoints
>   Num     Type           Disp Enb Address            What
>   1       breakpoint     keep y   <MULTIPLE>
>           stop only if a == 10
>   1.1                         N*  0x00000000000011b6 in Base::func() at condbreak-multi-context.cc:23
>   1.2                         n   0x00000000000011c2 in A::func() at condbreak-multi-context.cc:31
>   1.3                         N*  0x00000000000011ce in C::func() at condbreak-multi-context.cc:39
>   (*): Breakpoint condition is invalid at this location.
>
> The condition of a breakpoint can be changed.  Locations'
> enable/disable states are updated accordingly.
>
>   ...
>   (gdb) cond 1 c == 30
>   warning: failed to validate condition at location 1.1, disabling:
>     No symbol "c" in current context.
>   Breakpoint 1's condition is now valid at location 3, enabling.
>   (gdb) info breakpoints
>   Num     Type           Disp Enb Address            What
>   1       breakpoint     keep y   <MULTIPLE>
>           stop only if c == 30
>   1.1                         N*  0x00000000000011b6 in Base::func() at condbreak-multi-context.cc:23
>   1.2                         N*  0x00000000000011c2 in A::func() at condbreak-multi-context.cc:31
>   1.3                         y   0x00000000000011ce in C::func() at condbreak-multi-context.cc:39
>   (*): Breakpoint condition is invalid at this location.
>
>   (gdb) cond 1 b == 20
>   Breakpoint 1's condition is now valid at location 1, enabling.
>   (gdb) info breakpoints
>   Num     Type           Disp Enb Address            What
>   1       breakpoint     keep y   <MULTIPLE>
>           stop only if b == 20
>   1.1                         y   0x00000000000011b6 in Base::func() at condbreak-multi-context.cc:23
>   1.2                         n   0x00000000000011c2 in A::func() at condbreak-multi-context.cc:31
>   1.3                         y   0x00000000000011ce in C::func() at condbreak-multi-context.cc:39
>   # Note that location 1.2 was disabled by the user previously.
>
> If the condition expression is bad for all the locations, it will be
> rejected.
>
>   (gdb) cond 1 garbage
>   No symbol "garbage" in current context.
>
> For conditions that are invalid or valid for all the locations of a
> breakpoint, the existing behavior is preserved.
>
> Regression-tested on X86_64 Linux.

Hi Baris,

Since I didn't have any major issue with the previous version, I just
took a quick look at this one, and I think it's fine.

Simon