[v2,gdb/breakpoints] Fix gdb.base/scope-hw-watch-disable.exp on arm-linux
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gdb_build--master-arm |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
success
|
Test passed
|
Commit Message
On arm-linux, with test-case gdb.base/scope-hw-watch-disable.exp I run into:
...
(gdb) awatch a^M
Can't set read/access watchpoint when hardware watchpoints are disabled.^M
(gdb) PASS: $exp: unsuccessful attempt to create an access watchpoint
rwatch b^M
Can't set read/access watchpoint when hardware watchpoints are disabled.^M
(gdb) PASS: $exp: unsuccessful attempt to create a read watchpoint
continue^M
Continuing.^M
^M
Program received signal SIGSEGV, Segmentation fault.^M
0xf7ec82c8 in ?? () from /lib/arm-linux-gnueabihf/libc.so.6^M
(gdb) FAIL: $exp: continue until exit
...
Using "maint info break", we can see that the two failed attempts to set a
watchpoint each left behind a stale "watchpoint scope" breakpoint:
...
-5 watchpoint scope del y 0xf7ec569a inf 1
-5.1 y 0xf7ec569a inf 1
stop only in stack frame at 0xfffef4f8
-6 watchpoint scope del y 0xf7ec569a inf 1
-6.1 y 0xf7ec569a inf 1
stop only in stack frame at 0xfffef4f8
...
The SIGSEGV is a consequence of the stale "watchpoint scope" breakpoint: the
same happens if we:
- have can-use-hw-watchpoints == 1,
- set one of the watchpoints, and
- continue to exit.
The problem is missing symbol info on libc which is supposed to tell which
code is thumb. After doing "set arm fallback-mode thumb" the SIGSEGV
disappears.
Extend the test-case to check the "maint info break" command before and after
the two failed attempts, to make sure that we catch the stale
"watchpoint scope" breakpoints also on x86_64-linux.
Fix this in watch_command_1 by moving creation of the "watchpoint scope"
breakpoint after the call to update_watchpoint.
Tested on x86_64-linux.
PR breakpoints/31860
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31860
---
gdb/breakpoint.c | 100 ++++++++++--------
.../gdb.base/scope-hw-watch-disable.exp | 18 ++++
2 files changed, 71 insertions(+), 47 deletions(-)
base-commit: 28c3bf55f0f9aca8619c6d01be34a02a887c5577
Comments
On 8/21/24 13:09, Tom de Vries wrote:
> On arm-linux, with test-case gdb.base/scope-hw-watch-disable.exp I run into:
> ...
> (gdb) awatch a^M
> Can't set read/access watchpoint when hardware watchpoints are disabled.^M
> (gdb) PASS: $exp: unsuccessful attempt to create an access watchpoint
> rwatch b^M
> Can't set read/access watchpoint when hardware watchpoints are disabled.^M
> (gdb) PASS: $exp: unsuccessful attempt to create a read watchpoint
> continue^M
> Continuing.^M
> ^M
> Program received signal SIGSEGV, Segmentation fault.^M
> 0xf7ec82c8 in ?? () from /lib/arm-linux-gnueabihf/libc.so.6^M
> (gdb) FAIL: $exp: continue until exit
> ...
>
> Using "maint info break", we can see that the two failed attempts to set a
> watchpoint each left behind a stale "watchpoint scope" breakpoint:
> ...
> -5 watchpoint scope del y 0xf7ec569a inf 1
> -5.1 y 0xf7ec569a inf 1
> stop only in stack frame at 0xfffef4f8
> -6 watchpoint scope del y 0xf7ec569a inf 1
> -6.1 y 0xf7ec569a inf 1
> stop only in stack frame at 0xfffef4f8
> ...
>
> The SIGSEGV is a consequence of the stale "watchpoint scope" breakpoint: the
> same happens if we:
> - have can-use-hw-watchpoints == 1,
> - set one of the watchpoints, and
> - continue to exit.
> The problem is missing symbol info on libc which is supposed to tell which
> code is thumb. After doing "set arm fallback-mode thumb" the SIGSEGV
> disappears.
>
> Extend the test-case to check the "maint info break" command before and after
> the two failed attempts, to make sure that we catch the stale
> "watchpoint scope" breakpoints also on x86_64-linux.
>
> Fix this in watch_command_1 by moving creation of the "watchpoint scope"
> breakpoint after the call to update_watchpoint.
>
Ping.
Thanks,
- Tom
> Tested on x86_64-linux.
>
> PR breakpoints/31860
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31860
> ---
> gdb/breakpoint.c | 100 ++++++++++--------
> .../gdb.base/scope-hw-watch-disable.exp | 18 ++++
> 2 files changed, 71 insertions(+), 47 deletions(-)
>
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index 17bd627f867..6d8610d14e7 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -10504,45 +10504,6 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
> 'wp_frame'. */
> frame_id watchpoint_frame = get_frame_id (wp_frame);
>
> - /* If the expression is "local", then set up a "watchpoint scope"
> - breakpoint at the point where we've left the scope of the watchpoint
> - expression. Create the scope breakpoint before the watchpoint, so
> - that we will encounter it first in bpstat_stop_status. */
> - if (exp_valid_block != NULL && wp_frame != NULL)
> - {
> - frame_id caller_frame_id = frame_unwind_caller_id (wp_frame);
> -
> - if (frame_id_p (caller_frame_id))
> - {
> - gdbarch *caller_arch = frame_unwind_caller_arch (wp_frame);
> - CORE_ADDR caller_pc = frame_unwind_caller_pc (wp_frame);
> -
> - scope_breakpoint
> - = create_internal_breakpoint (caller_arch, caller_pc,
> - bp_watchpoint_scope);
> -
> - /* create_internal_breakpoint could invalidate WP_FRAME. */
> - wp_frame = NULL;
> -
> - scope_breakpoint->enable_state = bp_enabled;
> -
> - /* Automatically delete the breakpoint when it hits. */
> - scope_breakpoint->disposition = disp_del;
> -
> - /* Only break in the proper frame (help with recursion). */
> - scope_breakpoint->frame_id = caller_frame_id;
> -
> - /* Set the address at which we will stop. */
> - bp_location &loc = scope_breakpoint->first_loc ();
> - loc.gdbarch = caller_arch;
> - loc.requested_address = caller_pc;
> - loc.address
> - = adjust_breakpoint_address (loc.gdbarch, loc.requested_address,
> - scope_breakpoint->type,
> - current_program_space);
> - }
> - }
> -
> /* Now set up the breakpoint. We create all watchpoints as hardware
> watchpoints here even if hardware watchpoints are turned off, a call
> to update_watchpoint later in this function will cause the type to
> @@ -10613,14 +10574,6 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
> w->watchpoint_thread = null_ptid;
> }
>
> - if (scope_breakpoint != NULL)
> - {
> - /* The scope breakpoint is related to the watchpoint. We will
> - need to act on them together. */
> - w->related_breakpoint = scope_breakpoint;
> - scope_breakpoint->related_breakpoint = w.get ();
> - }
> -
> if (!just_location)
> value_free_to_mark (mark);
>
> @@ -10628,7 +10581,60 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
> that should be inserted. */
> update_watchpoint (w.get (), true /* reparse */);
>
> + /* If the expression is "local", then set up a "watchpoint scope"
> + breakpoint at the point where we've left the scope of the watchpoint
> + expression. Create the scope breakpoint before the watchpoint, so
> + that we will encounter it first in bpstat_stop_status. */
> + if (exp_valid_block != nullptr && wp_frame != nullptr)
> + {
> + frame_id caller_frame_id = frame_unwind_caller_id (wp_frame);
> +
> + if (frame_id_p (caller_frame_id))
> + {
> + gdbarch *caller_arch = frame_unwind_caller_arch (wp_frame);
> + CORE_ADDR caller_pc = frame_unwind_caller_pc (wp_frame);
> +
> + scope_breakpoint
> + = create_internal_breakpoint (caller_arch, caller_pc,
> + bp_watchpoint_scope);
> +
> + /* create_internal_breakpoint could invalidate WP_FRAME. */
> + wp_frame = nullptr;
> +
> + scope_breakpoint->enable_state = bp_enabled;
> +
> + /* Automatically delete the breakpoint when it hits. */
> + scope_breakpoint->disposition = disp_del;
> +
> + /* Only break in the proper frame (help with recursion). */
> + scope_breakpoint->frame_id = caller_frame_id;
> +
> + /* Set the address at which we will stop. */
> + bp_location &loc = scope_breakpoint->first_loc ();
> + loc.gdbarch = caller_arch;
> + loc.requested_address = caller_pc;
> + loc.address
> + = adjust_breakpoint_address (loc.gdbarch, loc.requested_address,
> + scope_breakpoint->type,
> + current_program_space);
> + }
> + }
> +
> + if (scope_breakpoint != nullptr)
> + {
> + /* The scope breakpoint is related to the watchpoint. We will
> + need to act on them together. */
> + w->related_breakpoint = scope_breakpoint;
> + scope_breakpoint->related_breakpoint = w.get ();
> + }
> +
> + /* Verify that the scope breakpoint comes before the watchpoint in the
> + breakpoint chain. */
> + gdb_assert (scope_breakpoint == nullptr
> + || &breakpoint_chain.back () == scope_breakpoint);
> + watchpoint *watchpoint_ptr = w.get ();
> install_breakpoint (internal, std::move (w), 1);
> + gdb_assert (&breakpoint_chain.back () == watchpoint_ptr);
> }
>
> /* Return count of debug registers needed to watch the given expression.
> diff --git a/gdb/testsuite/gdb.base/scope-hw-watch-disable.exp b/gdb/testsuite/gdb.base/scope-hw-watch-disable.exp
> index 61137703e5c..29eb682df47 100644
> --- a/gdb/testsuite/gdb.base/scope-hw-watch-disable.exp
> +++ b/gdb/testsuite/gdb.base/scope-hw-watch-disable.exp
> @@ -29,6 +29,15 @@ if {![runto_main]} {
> return -1
> }
>
> +gdb_test_multiple "maint info break" "maint info break before" {
> + -re -wrap "watchpoint.*" {
> + fail $gdb_test_name
> + }
> + -re -wrap "" {
> + pass $gdb_test_name
> + }
> +}
> +
> gdb_test "awatch a" \
> "Can't set read/access watchpoint when hardware watchpoints are disabled." \
> "unsuccessful attempt to create an access watchpoint"
> @@ -36,5 +45,14 @@ gdb_test "rwatch b" \
> "Can't set read/access watchpoint when hardware watchpoints are disabled." \
> "unsuccessful attempt to create a read watchpoint"
>
> +gdb_test_multiple "maint info break" "maint info break after" {
> + -re -wrap "watchpoint.*" {
> + fail $gdb_test_name
> + }
> + -re -wrap "" {
> + pass $gdb_test_name
> + }
> +}
> +
> # The program continues until termination.
> gdb_continue_to_end
>
> base-commit: 28c3bf55f0f9aca8619c6d01be34a02a887c5577
On 9/4/24 09:38, Tom de Vries wrote:
> On 8/21/24 13:09, Tom de Vries wrote:
>> On arm-linux, with test-case gdb.base/scope-hw-watch-disable.exp I run
>> into:
>> ...
>> (gdb) awatch a^M
>> Can't set read/access watchpoint when hardware watchpoints are
>> disabled.^M
>> (gdb) PASS: $exp: unsuccessful attempt to create an access watchpoint
>> rwatch b^M
>> Can't set read/access watchpoint when hardware watchpoints are
>> disabled.^M
>> (gdb) PASS: $exp: unsuccessful attempt to create a read watchpoint
>> continue^M
>> Continuing.^M
>> ^M
>> Program received signal SIGSEGV, Segmentation fault.^M
>> 0xf7ec82c8 in ?? () from /lib/arm-linux-gnueabihf/libc.so.6^M
>> (gdb) FAIL: $exp: continue until exit
>> ...
>>
>> Using "maint info break", we can see that the two failed attempts to
>> set a
>> watchpoint each left behind a stale "watchpoint scope" breakpoint:
>> ...
>> -5 watchpoint scope del y 0xf7ec569a inf 1
>> -5.1 y 0xf7ec569a inf 1
>> stop only in stack frame at 0xfffef4f8
>> -6 watchpoint scope del y 0xf7ec569a inf 1
>> -6.1 y 0xf7ec569a inf 1
>> stop only in stack frame at 0xfffef4f8
>> ...
>>
>> The SIGSEGV is a consequence of the stale "watchpoint scope"
>> breakpoint: the
>> same happens if we:
>> - have can-use-hw-watchpoints == 1,
>> - set one of the watchpoints, and
>> - continue to exit.
>> The problem is missing symbol info on libc which is supposed to tell
>> which
>> code is thumb. After doing "set arm fallback-mode thumb" the SIGSEGV
>> disappears.
>>
>> Extend the test-case to check the "maint info break" command before
>> and after
>> the two failed attempts, to make sure that we catch the stale
>> "watchpoint scope" breakpoints also on x86_64-linux.
>>
>> Fix this in watch_command_1 by moving creation of the "watchpoint scope"
>> breakpoint after the call to update_watchpoint.
>>
>
Ping^2.
Thanks,
- Tom
>> Tested on x86_64-linux.
>>
>> PR breakpoints/31860
>> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31860
>> ---
>> gdb/breakpoint.c | 100 ++++++++++--------
>> .../gdb.base/scope-hw-watch-disable.exp | 18 ++++
>> 2 files changed, 71 insertions(+), 47 deletions(-)
>>
>> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
>> index 17bd627f867..6d8610d14e7 100644
>> --- a/gdb/breakpoint.c
>> +++ b/gdb/breakpoint.c
>> @@ -10504,45 +10504,6 @@ watch_command_1 (const char *arg, int
>> accessflag, int from_tty,
>> 'wp_frame'. */
>> frame_id watchpoint_frame = get_frame_id (wp_frame);
>> - /* If the expression is "local", then set up a "watchpoint scope"
>> - breakpoint at the point where we've left the scope of the
>> watchpoint
>> - expression. Create the scope breakpoint before the watchpoint, so
>> - that we will encounter it first in bpstat_stop_status. */
>> - if (exp_valid_block != NULL && wp_frame != NULL)
>> - {
>> - frame_id caller_frame_id = frame_unwind_caller_id (wp_frame);
>> -
>> - if (frame_id_p (caller_frame_id))
>> - {
>> - gdbarch *caller_arch = frame_unwind_caller_arch (wp_frame);
>> - CORE_ADDR caller_pc = frame_unwind_caller_pc (wp_frame);
>> -
>> - scope_breakpoint
>> - = create_internal_breakpoint (caller_arch, caller_pc,
>> - bp_watchpoint_scope);
>> -
>> - /* create_internal_breakpoint could invalidate WP_FRAME. */
>> - wp_frame = NULL;
>> -
>> - scope_breakpoint->enable_state = bp_enabled;
>> -
>> - /* Automatically delete the breakpoint when it hits. */
>> - scope_breakpoint->disposition = disp_del;
>> -
>> - /* Only break in the proper frame (help with recursion). */
>> - scope_breakpoint->frame_id = caller_frame_id;
>> -
>> - /* Set the address at which we will stop. */
>> - bp_location &loc = scope_breakpoint->first_loc ();
>> - loc.gdbarch = caller_arch;
>> - loc.requested_address = caller_pc;
>> - loc.address
>> - = adjust_breakpoint_address (loc.gdbarch, loc.requested_address,
>> - scope_breakpoint->type,
>> - current_program_space);
>> - }
>> - }
>> -
>> /* Now set up the breakpoint. We create all watchpoints as hardware
>> watchpoints here even if hardware watchpoints are turned off, a
>> call
>> to update_watchpoint later in this function will cause the type to
>> @@ -10613,14 +10574,6 @@ watch_command_1 (const char *arg, int
>> accessflag, int from_tty,
>> w->watchpoint_thread = null_ptid;
>> }
>> - if (scope_breakpoint != NULL)
>> - {
>> - /* The scope breakpoint is related to the watchpoint. We will
>> - need to act on them together. */
>> - w->related_breakpoint = scope_breakpoint;
>> - scope_breakpoint->related_breakpoint = w.get ();
>> - }
>> -
>> if (!just_location)
>> value_free_to_mark (mark);
>> @@ -10628,7 +10581,60 @@ watch_command_1 (const char *arg, int
>> accessflag, int from_tty,
>> that should be inserted. */
>> update_watchpoint (w.get (), true /* reparse */);
>> + /* If the expression is "local", then set up a "watchpoint scope"
>> + breakpoint at the point where we've left the scope of the
>> watchpoint
>> + expression. Create the scope breakpoint before the watchpoint, so
>> + that we will encounter it first in bpstat_stop_status. */
>> + if (exp_valid_block != nullptr && wp_frame != nullptr)
>> + {
>> + frame_id caller_frame_id = frame_unwind_caller_id (wp_frame);
>> +
>> + if (frame_id_p (caller_frame_id))
>> + {
>> + gdbarch *caller_arch = frame_unwind_caller_arch (wp_frame);
>> + CORE_ADDR caller_pc = frame_unwind_caller_pc (wp_frame);
>> +
>> + scope_breakpoint
>> + = create_internal_breakpoint (caller_arch, caller_pc,
>> + bp_watchpoint_scope);
>> +
>> + /* create_internal_breakpoint could invalidate WP_FRAME. */
>> + wp_frame = nullptr;
>> +
>> + scope_breakpoint->enable_state = bp_enabled;
>> +
>> + /* Automatically delete the breakpoint when it hits. */
>> + scope_breakpoint->disposition = disp_del;
>> +
>> + /* Only break in the proper frame (help with recursion). */
>> + scope_breakpoint->frame_id = caller_frame_id;
>> +
>> + /* Set the address at which we will stop. */
>> + bp_location &loc = scope_breakpoint->first_loc ();
>> + loc.gdbarch = caller_arch;
>> + loc.requested_address = caller_pc;
>> + loc.address
>> + = adjust_breakpoint_address (loc.gdbarch, loc.requested_address,
>> + scope_breakpoint->type,
>> + current_program_space);
>> + }
>> + }
>> +
>> + if (scope_breakpoint != nullptr)
>> + {
>> + /* The scope breakpoint is related to the watchpoint. We will
>> + need to act on them together. */
>> + w->related_breakpoint = scope_breakpoint;
>> + scope_breakpoint->related_breakpoint = w.get ();
>> + }
>> +
>> + /* Verify that the scope breakpoint comes before the watchpoint in the
>> + breakpoint chain. */
>> + gdb_assert (scope_breakpoint == nullptr
>> + || &breakpoint_chain.back () == scope_breakpoint);
>> + watchpoint *watchpoint_ptr = w.get ();
>> install_breakpoint (internal, std::move (w), 1);
>> + gdb_assert (&breakpoint_chain.back () == watchpoint_ptr);
>> }
>> /* Return count of debug registers needed to watch the given
>> expression.
>> diff --git a/gdb/testsuite/gdb.base/scope-hw-watch-disable.exp
>> b/gdb/testsuite/gdb.base/scope-hw-watch-disable.exp
>> index 61137703e5c..29eb682df47 100644
>> --- a/gdb/testsuite/gdb.base/scope-hw-watch-disable.exp
>> +++ b/gdb/testsuite/gdb.base/scope-hw-watch-disable.exp
>> @@ -29,6 +29,15 @@ if {![runto_main]} {
>> return -1
>> }
>> +gdb_test_multiple "maint info break" "maint info break before" {
>> + -re -wrap "watchpoint.*" {
>> + fail $gdb_test_name
>> + }
>> + -re -wrap "" {
>> + pass $gdb_test_name
>> + }
>> +}
>> +
>> gdb_test "awatch a" \
>> "Can't set read/access watchpoint when hardware watchpoints are
>> disabled." \
>> "unsuccessful attempt to create an access watchpoint"
>> @@ -36,5 +45,14 @@ gdb_test "rwatch b" \
>> "Can't set read/access watchpoint when hardware watchpoints are
>> disabled." \
>> "unsuccessful attempt to create a read watchpoint"
>> +gdb_test_multiple "maint info break" "maint info break after" {
>> + -re -wrap "watchpoint.*" {
>> + fail $gdb_test_name
>> + }
>> + -re -wrap "" {
>> + pass $gdb_test_name
>> + }
>> +}
>> +
>> # The program continues until termination.
>> gdb_continue_to_end
>>
>> base-commit: 28c3bf55f0f9aca8619c6d01be34a02a887c5577
>
On 9/23/24 10:15, Tom de Vries wrote:
> On 9/4/24 09:38, Tom de Vries wrote:
>> On 8/21/24 13:09, Tom de Vries wrote:
>>> On arm-linux, with test-case gdb.base/scope-hw-watch-disable.exp I
>>> run into:
>>> ...
>>> (gdb) awatch a^M
>>> Can't set read/access watchpoint when hardware watchpoints are
>>> disabled.^M
>>> (gdb) PASS: $exp: unsuccessful attempt to create an access watchpoint
>>> rwatch b^M
>>> Can't set read/access watchpoint when hardware watchpoints are
>>> disabled.^M
>>> (gdb) PASS: $exp: unsuccessful attempt to create a read watchpoint
>>> continue^M
>>> Continuing.^M
>>> ^M
>>> Program received signal SIGSEGV, Segmentation fault.^M
>>> 0xf7ec82c8 in ?? () from /lib/arm-linux-gnueabihf/libc.so.6^M
>>> (gdb) FAIL: $exp: continue until exit
>>> ...
>>>
>>> Using "maint info break", we can see that the two failed attempts to
>>> set a
>>> watchpoint each left behind a stale "watchpoint scope" breakpoint:
>>> ...
>>> -5 watchpoint scope del y 0xf7ec569a inf 1
>>> -5.1 y 0xf7ec569a inf 1
>>> stop only in stack frame at 0xfffef4f8
>>> -6 watchpoint scope del y 0xf7ec569a inf 1
>>> -6.1 y 0xf7ec569a inf 1
>>> stop only in stack frame at 0xfffef4f8
>>> ...
>>>
>>> The SIGSEGV is a consequence of the stale "watchpoint scope"
>>> breakpoint: the
>>> same happens if we:
>>> - have can-use-hw-watchpoints == 1,
>>> - set one of the watchpoints, and
>>> - continue to exit.
>>> The problem is missing symbol info on libc which is supposed to tell
>>> which
>>> code is thumb. After doing "set arm fallback-mode thumb" the SIGSEGV
>>> disappears.
>>>
>>> Extend the test-case to check the "maint info break" command before
>>> and after
>>> the two failed attempts, to make sure that we catch the stale
>>> "watchpoint scope" breakpoints also on x86_64-linux.
>>>
>>> Fix this in watch_command_1 by moving creation of the "watchpoint scope"
>>> breakpoint after the call to update_watchpoint.
>>>
>>
>
> Ping^2.
>
I've pushed this.
Thanks,
- Tom
>>> Tested on x86_64-linux.
>>>
>>> PR breakpoints/31860
>>> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31860
>>> ---
>>> gdb/breakpoint.c | 100 ++++++++++--------
>>> .../gdb.base/scope-hw-watch-disable.exp | 18 ++++
>>> 2 files changed, 71 insertions(+), 47 deletions(-)
>>>
>>> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
>>> index 17bd627f867..6d8610d14e7 100644
>>> --- a/gdb/breakpoint.c
>>> +++ b/gdb/breakpoint.c
>>> @@ -10504,45 +10504,6 @@ watch_command_1 (const char *arg, int
>>> accessflag, int from_tty,
>>> 'wp_frame'. */
>>> frame_id watchpoint_frame = get_frame_id (wp_frame);
>>> - /* If the expression is "local", then set up a "watchpoint scope"
>>> - breakpoint at the point where we've left the scope of the
>>> watchpoint
>>> - expression. Create the scope breakpoint before the watchpoint, so
>>> - that we will encounter it first in bpstat_stop_status. */
>>> - if (exp_valid_block != NULL && wp_frame != NULL)
>>> - {
>>> - frame_id caller_frame_id = frame_unwind_caller_id (wp_frame);
>>> -
>>> - if (frame_id_p (caller_frame_id))
>>> - {
>>> - gdbarch *caller_arch = frame_unwind_caller_arch (wp_frame);
>>> - CORE_ADDR caller_pc = frame_unwind_caller_pc (wp_frame);
>>> -
>>> - scope_breakpoint
>>> - = create_internal_breakpoint (caller_arch, caller_pc,
>>> - bp_watchpoint_scope);
>>> -
>>> - /* create_internal_breakpoint could invalidate WP_FRAME. */
>>> - wp_frame = NULL;
>>> -
>>> - scope_breakpoint->enable_state = bp_enabled;
>>> -
>>> - /* Automatically delete the breakpoint when it hits. */
>>> - scope_breakpoint->disposition = disp_del;
>>> -
>>> - /* Only break in the proper frame (help with recursion). */
>>> - scope_breakpoint->frame_id = caller_frame_id;
>>> -
>>> - /* Set the address at which we will stop. */
>>> - bp_location &loc = scope_breakpoint->first_loc ();
>>> - loc.gdbarch = caller_arch;
>>> - loc.requested_address = caller_pc;
>>> - loc.address
>>> - = adjust_breakpoint_address (loc.gdbarch,
>>> loc.requested_address,
>>> - scope_breakpoint->type,
>>> - current_program_space);
>>> - }
>>> - }
>>> -
>>> /* Now set up the breakpoint. We create all watchpoints as hardware
>>> watchpoints here even if hardware watchpoints are turned off,
>>> a call
>>> to update_watchpoint later in this function will cause the
>>> type to
>>> @@ -10613,14 +10574,6 @@ watch_command_1 (const char *arg, int
>>> accessflag, int from_tty,
>>> w->watchpoint_thread = null_ptid;
>>> }
>>> - if (scope_breakpoint != NULL)
>>> - {
>>> - /* The scope breakpoint is related to the watchpoint. We will
>>> - need to act on them together. */
>>> - w->related_breakpoint = scope_breakpoint;
>>> - scope_breakpoint->related_breakpoint = w.get ();
>>> - }
>>> -
>>> if (!just_location)
>>> value_free_to_mark (mark);
>>> @@ -10628,7 +10581,60 @@ watch_command_1 (const char *arg, int
>>> accessflag, int from_tty,
>>> that should be inserted. */
>>> update_watchpoint (w.get (), true /* reparse */);
>>> + /* If the expression is "local", then set up a "watchpoint scope"
>>> + breakpoint at the point where we've left the scope of the
>>> watchpoint
>>> + expression. Create the scope breakpoint before the watchpoint, so
>>> + that we will encounter it first in bpstat_stop_status. */
>>> + if (exp_valid_block != nullptr && wp_frame != nullptr)
>>> + {
>>> + frame_id caller_frame_id = frame_unwind_caller_id (wp_frame);
>>> +
>>> + if (frame_id_p (caller_frame_id))
>>> + {
>>> + gdbarch *caller_arch = frame_unwind_caller_arch (wp_frame);
>>> + CORE_ADDR caller_pc = frame_unwind_caller_pc (wp_frame);
>>> +
>>> + scope_breakpoint
>>> + = create_internal_breakpoint (caller_arch, caller_pc,
>>> + bp_watchpoint_scope);
>>> +
>>> + /* create_internal_breakpoint could invalidate WP_FRAME. */
>>> + wp_frame = nullptr;
>>> +
>>> + scope_breakpoint->enable_state = bp_enabled;
>>> +
>>> + /* Automatically delete the breakpoint when it hits. */
>>> + scope_breakpoint->disposition = disp_del;
>>> +
>>> + /* Only break in the proper frame (help with recursion). */
>>> + scope_breakpoint->frame_id = caller_frame_id;
>>> +
>>> + /* Set the address at which we will stop. */
>>> + bp_location &loc = scope_breakpoint->first_loc ();
>>> + loc.gdbarch = caller_arch;
>>> + loc.requested_address = caller_pc;
>>> + loc.address
>>> + = adjust_breakpoint_address (loc.gdbarch,
>>> loc.requested_address,
>>> + scope_breakpoint->type,
>>> + current_program_space);
>>> + }
>>> + }
>>> +
>>> + if (scope_breakpoint != nullptr)
>>> + {
>>> + /* The scope breakpoint is related to the watchpoint. We will
>>> + need to act on them together. */
>>> + w->related_breakpoint = scope_breakpoint;
>>> + scope_breakpoint->related_breakpoint = w.get ();
>>> + }
>>> +
>>> + /* Verify that the scope breakpoint comes before the watchpoint in
>>> the
>>> + breakpoint chain. */
>>> + gdb_assert (scope_breakpoint == nullptr
>>> + || &breakpoint_chain.back () == scope_breakpoint);
>>> + watchpoint *watchpoint_ptr = w.get ();
>>> install_breakpoint (internal, std::move (w), 1);
>>> + gdb_assert (&breakpoint_chain.back () == watchpoint_ptr);
>>> }
>>> /* Return count of debug registers needed to watch the given
>>> expression.
>>> diff --git a/gdb/testsuite/gdb.base/scope-hw-watch-disable.exp
>>> b/gdb/testsuite/gdb.base/scope-hw-watch-disable.exp
>>> index 61137703e5c..29eb682df47 100644
>>> --- a/gdb/testsuite/gdb.base/scope-hw-watch-disable.exp
>>> +++ b/gdb/testsuite/gdb.base/scope-hw-watch-disable.exp
>>> @@ -29,6 +29,15 @@ if {![runto_main]} {
>>> return -1
>>> }
>>> +gdb_test_multiple "maint info break" "maint info break before" {
>>> + -re -wrap "watchpoint.*" {
>>> + fail $gdb_test_name
>>> + }
>>> + -re -wrap "" {
>>> + pass $gdb_test_name
>>> + }
>>> +}
>>> +
>>> gdb_test "awatch a" \
>>> "Can't set read/access watchpoint when hardware watchpoints are
>>> disabled." \
>>> "unsuccessful attempt to create an access watchpoint"
>>> @@ -36,5 +45,14 @@ gdb_test "rwatch b" \
>>> "Can't set read/access watchpoint when hardware watchpoints are
>>> disabled." \
>>> "unsuccessful attempt to create a read watchpoint"
>>> +gdb_test_multiple "maint info break" "maint info break after" {
>>> + -re -wrap "watchpoint.*" {
>>> + fail $gdb_test_name
>>> + }
>>> + -re -wrap "" {
>>> + pass $gdb_test_name
>>> + }
>>> +}
>>> +
>>> # The program continues until termination.
>>> gdb_continue_to_end
>>>
>>> base-commit: 28c3bf55f0f9aca8619c6d01be34a02a887c5577
>>
>
On 10/10/24 11:42, Tom de Vries wrote:
> On 9/23/24 10:15, Tom de Vries wrote:
>> On 9/4/24 09:38, Tom de Vries wrote:
>>> On 8/21/24 13:09, Tom de Vries wrote:
>>>> On arm-linux, with test-case gdb.base/scope-hw-watch-disable.exp I run into:
>>>> ...
>>>> (gdb) awatch a^M
>>>> Can't set read/access watchpoint when hardware watchpoints are disabled.^M
>>>> (gdb) PASS: $exp: unsuccessful attempt to create an access watchpoint
>>>> rwatch b^M
>>>> Can't set read/access watchpoint when hardware watchpoints are disabled.^M
>>>> (gdb) PASS: $exp: unsuccessful attempt to create a read watchpoint
>>>> continue^M
>>>> Continuing.^M
>>>> ^M
>>>> Program received signal SIGSEGV, Segmentation fault.^M
>>>> 0xf7ec82c8 in ?? () from /lib/arm-linux-gnueabihf/libc.so.6^M
>>>> (gdb) FAIL: $exp: continue until exit
>>>> ...
>>>>
>>>> Using "maint info break", we can see that the two failed attempts to set a
>>>> watchpoint each left behind a stale "watchpoint scope" breakpoint:
>>>> ...
>>>> -5 watchpoint scope del y 0xf7ec569a inf 1
>>>> -5.1 y 0xf7ec569a inf 1
>>>> stop only in stack frame at 0xfffef4f8
>>>> -6 watchpoint scope del y 0xf7ec569a inf 1
>>>> -6.1 y 0xf7ec569a inf 1
>>>> stop only in stack frame at 0xfffef4f8
>>>> ...
>>>>
>>>> The SIGSEGV is a consequence of the stale "watchpoint scope" breakpoint: the
>>>> same happens if we:
>>>> - have can-use-hw-watchpoints == 1,
>>>> - set one of the watchpoints, and
>>>> - continue to exit.
>>>> The problem is missing symbol info on libc which is supposed to tell which
>>>> code is thumb. After doing "set arm fallback-mode thumb" the SIGSEGV
>>>> disappears.
>>>>
>>>> Extend the test-case to check the "maint info break" command before and after
>>>> the two failed attempts, to make sure that we catch the stale
>>>> "watchpoint scope" breakpoints also on x86_64-linux.
>>>>
>>>> Fix this in watch_command_1 by moving creation of the "watchpoint scope"
>>>> breakpoint after the call to update_watchpoint.
>>>>
>>>
>>
>> Ping^2.
>>
>
> I've pushed this.
>
> Thanks,
> - Tom
>
>>>> Tested on x86_64-linux.
>>>>
>>>> PR breakpoints/31860
>>>> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31860
>>>> ---
>>>> gdb/breakpoint.c | 100 ++++++++++--------
>>>> .../gdb.base/scope-hw-watch-disable.exp | 18 ++++
>>>> 2 files changed, 71 insertions(+), 47 deletions(-)
>>>>
>>>> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
>>>> index 17bd627f867..6d8610d14e7 100644
>>>> --- a/gdb/breakpoint.c
>>>> +++ b/gdb/breakpoint.c
>>>> @@ -10504,45 +10504,6 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
>>>> 'wp_frame'. */
>>>> frame_id watchpoint_frame = get_frame_id (wp_frame);
>>>> - /* If the expression is "local", then set up a "watchpoint scope"
>>>> - breakpoint at the point where we've left the scope of the watchpoint
>>>> - expression. Create the scope breakpoint before the watchpoint, so
>>>> - that we will encounter it first in bpstat_stop_status. */
>>>> - if (exp_valid_block != NULL && wp_frame != NULL)
>>>> - {
>>>> - frame_id caller_frame_id = frame_unwind_caller_id (wp_frame);
>>>> -
>>>> - if (frame_id_p (caller_frame_id))
>>>> - {
>>>> - gdbarch *caller_arch = frame_unwind_caller_arch (wp_frame);
>>>> - CORE_ADDR caller_pc = frame_unwind_caller_pc (wp_frame);
>>>> -
>>>> - scope_breakpoint
>>>> - = create_internal_breakpoint (caller_arch, caller_pc,
>>>> - bp_watchpoint_scope);
>>>> -
>>>> - /* create_internal_breakpoint could invalidate WP_FRAME. */
>>>> - wp_frame = NULL;
>>>> -
>>>> - scope_breakpoint->enable_state = bp_enabled;
>>>> -
>>>> - /* Automatically delete the breakpoint when it hits. */
>>>> - scope_breakpoint->disposition = disp_del;
>>>> -
>>>> - /* Only break in the proper frame (help with recursion). */
>>>> - scope_breakpoint->frame_id = caller_frame_id;
>>>> -
>>>> - /* Set the address at which we will stop. */
>>>> - bp_location &loc = scope_breakpoint->first_loc ();
>>>> - loc.gdbarch = caller_arch;
>>>> - loc.requested_address = caller_pc;
>>>> - loc.address
>>>> - = adjust_breakpoint_address (loc.gdbarch, loc.requested_address,
>>>> - scope_breakpoint->type,
>>>> - current_program_space);
>>>> - }
>>>> - }
>>>> -
>>>> /* Now set up the breakpoint. We create all watchpoints as hardware
>>>> watchpoints here even if hardware watchpoints are turned off, a call
>>>> to update_watchpoint later in this function will cause the type to
>>>> @@ -10613,14 +10574,6 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
>>>> w->watchpoint_thread = null_ptid;
>>>> }
>>>> - if (scope_breakpoint != NULL)
>>>> - {
>>>> - /* The scope breakpoint is related to the watchpoint. We will
>>>> - need to act on them together. */
>>>> - w->related_breakpoint = scope_breakpoint;
>>>> - scope_breakpoint->related_breakpoint = w.get ();
>>>> - }
>>>> -
>>>> if (!just_location)
>>>> value_free_to_mark (mark);
>>>> @@ -10628,7 +10581,60 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
>>>> that should be inserted. */
>>>> update_watchpoint (w.get (), true /* reparse */);
>>>> + /* If the expression is "local", then set up a "watchpoint scope"
>>>> + breakpoint at the point where we've left the scope of the watchpoint
>>>> + expression. Create the scope breakpoint before the watchpoint, so
>>>> + that we will encounter it first in bpstat_stop_status. */
>>>> + if (exp_valid_block != nullptr && wp_frame != nullptr)
>>>> + {
>>>> + frame_id caller_frame_id = frame_unwind_caller_id (wp_frame);
>>>> +
>>>> + if (frame_id_p (caller_frame_id))
>>>> + {
>>>> + gdbarch *caller_arch = frame_unwind_caller_arch (wp_frame);
>>>> + CORE_ADDR caller_pc = frame_unwind_caller_pc (wp_frame);
>>>> +
>>>> + scope_breakpoint
>>>> + = create_internal_breakpoint (caller_arch, caller_pc,
>>>> + bp_watchpoint_scope);
>>>> +
>>>> + /* create_internal_breakpoint could invalidate WP_FRAME. */
>>>> + wp_frame = nullptr;
>>>> +
>>>> + scope_breakpoint->enable_state = bp_enabled;
>>>> +
>>>> + /* Automatically delete the breakpoint when it hits. */
>>>> + scope_breakpoint->disposition = disp_del;
>>>> +
>>>> + /* Only break in the proper frame (help with recursion). */
>>>> + scope_breakpoint->frame_id = caller_frame_id;
>>>> +
>>>> + /* Set the address at which we will stop. */
>>>> + bp_location &loc = scope_breakpoint->first_loc ();
>>>> + loc.gdbarch = caller_arch;
>>>> + loc.requested_address = caller_pc;
>>>> + loc.address
>>>> + = adjust_breakpoint_address (loc.gdbarch, loc.requested_address,
>>>> + scope_breakpoint->type,
>>>> + current_program_space);
>>>> + }
>>>> + }
>>>> +
>>>> + if (scope_breakpoint != nullptr)
>>>> + {
>>>> + /* The scope breakpoint is related to the watchpoint. We will
>>>> + need to act on them together. */
>>>> + w->related_breakpoint = scope_breakpoint;
>>>> + scope_breakpoint->related_breakpoint = w.get ();
>>>> + }
>>>> +
>>>> + /* Verify that the scope breakpoint comes before the watchpoint in the
>>>> + breakpoint chain. */
>>>> + gdb_assert (scope_breakpoint == nullptr
>>>> + || &breakpoint_chain.back () == scope_breakpoint);
>>>> + watchpoint *watchpoint_ptr = w.get ();
>>>> install_breakpoint (internal, std::move (w), 1);
>>>> + gdb_assert (&breakpoint_chain.back () == watchpoint_ptr);
>>>> }
>>>> /* Return count of debug registers needed to watch the given expression.
>>>> diff --git a/gdb/testsuite/gdb.base/scope-hw-watch-disable.exp b/gdb/testsuite/gdb.base/scope-hw-watch-disable.exp
>>>> index 61137703e5c..29eb682df47 100644
>>>> --- a/gdb/testsuite/gdb.base/scope-hw-watch-disable.exp
>>>> +++ b/gdb/testsuite/gdb.base/scope-hw-watch-disable.exp
>>>> @@ -29,6 +29,15 @@ if {![runto_main]} {
>>>> return -1
>>>> }
>>>> +gdb_test_multiple "maint info break" "maint info break before" {
>>>> + -re -wrap "watchpoint.*" {
>>>> + fail $gdb_test_name
>>>> + }
>>>> + -re -wrap "" {
>>>> + pass $gdb_test_name
>>>> + }
>>>> +}
>>>> +
>>>> gdb_test "awatch a" \
>>>> "Can't set read/access watchpoint when hardware watchpoints are disabled." \
>>>> "unsuccessful attempt to create an access watchpoint"
>>>> @@ -36,5 +45,14 @@ gdb_test "rwatch b" \
>>>> "Can't set read/access watchpoint when hardware watchpoints are disabled." \
>>>> "unsuccessful attempt to create a read watchpoint"
>>>> +gdb_test_multiple "maint info break" "maint info break after" {
>>>> + -re -wrap "watchpoint.*" {
>>>> + fail $gdb_test_name
>>>> + }
>>>> + -re -wrap "" {
>>>> + pass $gdb_test_name
>>>> + }
>>>> +}
>>>> +
>>>> # The program continues until termination.
>>>> gdb_continue_to_end
>>>>
>>>> base-commit: 28c3bf55f0f9aca8619c6d01be34a02a887c5577
>>>
>>
>
For the record, it looked OK to me. I remember this one, and I'm glad
that was addressed. Thanks!
@@ -10504,45 +10504,6 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
'wp_frame'. */
frame_id watchpoint_frame = get_frame_id (wp_frame);
- /* If the expression is "local", then set up a "watchpoint scope"
- breakpoint at the point where we've left the scope of the watchpoint
- expression. Create the scope breakpoint before the watchpoint, so
- that we will encounter it first in bpstat_stop_status. */
- if (exp_valid_block != NULL && wp_frame != NULL)
- {
- frame_id caller_frame_id = frame_unwind_caller_id (wp_frame);
-
- if (frame_id_p (caller_frame_id))
- {
- gdbarch *caller_arch = frame_unwind_caller_arch (wp_frame);
- CORE_ADDR caller_pc = frame_unwind_caller_pc (wp_frame);
-
- scope_breakpoint
- = create_internal_breakpoint (caller_arch, caller_pc,
- bp_watchpoint_scope);
-
- /* create_internal_breakpoint could invalidate WP_FRAME. */
- wp_frame = NULL;
-
- scope_breakpoint->enable_state = bp_enabled;
-
- /* Automatically delete the breakpoint when it hits. */
- scope_breakpoint->disposition = disp_del;
-
- /* Only break in the proper frame (help with recursion). */
- scope_breakpoint->frame_id = caller_frame_id;
-
- /* Set the address at which we will stop. */
- bp_location &loc = scope_breakpoint->first_loc ();
- loc.gdbarch = caller_arch;
- loc.requested_address = caller_pc;
- loc.address
- = adjust_breakpoint_address (loc.gdbarch, loc.requested_address,
- scope_breakpoint->type,
- current_program_space);
- }
- }
-
/* Now set up the breakpoint. We create all watchpoints as hardware
watchpoints here even if hardware watchpoints are turned off, a call
to update_watchpoint later in this function will cause the type to
@@ -10613,14 +10574,6 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
w->watchpoint_thread = null_ptid;
}
- if (scope_breakpoint != NULL)
- {
- /* The scope breakpoint is related to the watchpoint. We will
- need to act on them together. */
- w->related_breakpoint = scope_breakpoint;
- scope_breakpoint->related_breakpoint = w.get ();
- }
-
if (!just_location)
value_free_to_mark (mark);
@@ -10628,7 +10581,60 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
that should be inserted. */
update_watchpoint (w.get (), true /* reparse */);
+ /* If the expression is "local", then set up a "watchpoint scope"
+ breakpoint at the point where we've left the scope of the watchpoint
+ expression. Create the scope breakpoint before the watchpoint, so
+ that we will encounter it first in bpstat_stop_status. */
+ if (exp_valid_block != nullptr && wp_frame != nullptr)
+ {
+ frame_id caller_frame_id = frame_unwind_caller_id (wp_frame);
+
+ if (frame_id_p (caller_frame_id))
+ {
+ gdbarch *caller_arch = frame_unwind_caller_arch (wp_frame);
+ CORE_ADDR caller_pc = frame_unwind_caller_pc (wp_frame);
+
+ scope_breakpoint
+ = create_internal_breakpoint (caller_arch, caller_pc,
+ bp_watchpoint_scope);
+
+ /* create_internal_breakpoint could invalidate WP_FRAME. */
+ wp_frame = nullptr;
+
+ scope_breakpoint->enable_state = bp_enabled;
+
+ /* Automatically delete the breakpoint when it hits. */
+ scope_breakpoint->disposition = disp_del;
+
+ /* Only break in the proper frame (help with recursion). */
+ scope_breakpoint->frame_id = caller_frame_id;
+
+ /* Set the address at which we will stop. */
+ bp_location &loc = scope_breakpoint->first_loc ();
+ loc.gdbarch = caller_arch;
+ loc.requested_address = caller_pc;
+ loc.address
+ = adjust_breakpoint_address (loc.gdbarch, loc.requested_address,
+ scope_breakpoint->type,
+ current_program_space);
+ }
+ }
+
+ if (scope_breakpoint != nullptr)
+ {
+ /* The scope breakpoint is related to the watchpoint. We will
+ need to act on them together. */
+ w->related_breakpoint = scope_breakpoint;
+ scope_breakpoint->related_breakpoint = w.get ();
+ }
+
+ /* Verify that the scope breakpoint comes before the watchpoint in the
+ breakpoint chain. */
+ gdb_assert (scope_breakpoint == nullptr
+ || &breakpoint_chain.back () == scope_breakpoint);
+ watchpoint *watchpoint_ptr = w.get ();
install_breakpoint (internal, std::move (w), 1);
+ gdb_assert (&breakpoint_chain.back () == watchpoint_ptr);
}
/* Return count of debug registers needed to watch the given expression.
@@ -29,6 +29,15 @@ if {![runto_main]} {
return -1
}
+gdb_test_multiple "maint info break" "maint info break before" {
+ -re -wrap "watchpoint.*" {
+ fail $gdb_test_name
+ }
+ -re -wrap "" {
+ pass $gdb_test_name
+ }
+}
+
gdb_test "awatch a" \
"Can't set read/access watchpoint when hardware watchpoints are disabled." \
"unsuccessful attempt to create an access watchpoint"
@@ -36,5 +45,14 @@ gdb_test "rwatch b" \
"Can't set read/access watchpoint when hardware watchpoints are disabled." \
"unsuccessful attempt to create a read watchpoint"
+gdb_test_multiple "maint info break" "maint info break after" {
+ -re -wrap "watchpoint.*" {
+ fail $gdb_test_name
+ }
+ -re -wrap "" {
+ pass $gdb_test_name
+ }
+}
+
# The program continues until termination.
gdb_continue_to_end