Fix gdb.base/watch-vfork.exp: Watchpoint triggers after vfork (sw) (timeout) with Linux 2.6.32 and older version
Commit Message
Hi Pedro,
Thanks for your help.
On 05/29/14 03:19, Pedro Alves wrote:
> On 04/03/2014 09:12 AM, Hui Zhu wrote:
>> Got gdb.base/watch-vfork.exp: Watchpoint triggers after vfork (sw)
>> (timeout) with Linux 2.6.32 and older version.
>>
>> The rootcause is after the test use "set can-use-hw-watchpoints 0" let GDB
>> doesn't use hardware breakpoint and set a watchpoint on "global", GDB
>> continue will keep single step inside function "vfork".
>> The Linux 2.6.32 and older version doesn't have commit
>> 6580807da14c423f0d0a708108e6df6ebc8bc83d (get more info please goto
>> http://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/?id=6580807da14c423f0d0a708108e6df6ebc8bc83d).
>> When the function "vfork" do syscall, the single step flag TIF_SINGLESTEP
>> will copy to child process.
>> Then GDB detach it, child process and parent process will be hanged.
>>
>> So I make a patch that do a single step before detach. Then TIF_SINGLESTEP
>> of child process in old Linux kernel will be cleared before detach.
>> Child process in new Linux kernel will not be affected by this single step.
>>
>> The patch was tested and pass regression in new linux
>> kernel (3.13.6-200.fc20.x86_64) and old Linux kernel (2.6.32-38-server).
>>
>> Please help me review it.
>
> Thanks.
>
>> 2014-04-03 Hui Zhu <hui@codesourcery.com>
>>
>> * linux-nat.c (linux_child_follow_fork): do a single step before
>> detach.
>>
>> --- a/gdb/linux-nat.c
>> +++ b/gdb/linux-nat.c
>> @@ -442,6 +442,26 @@ holding the child stopped. Try \"set de
>>
>> if (linux_nat_prepare_to_resume != NULL)
>> linux_nat_prepare_to_resume (child_lp);
>> +
>> + /* When debug a inferior in the architecture that support
>> + hardware single step and the Linux kernel without commit
>> + 6580807da14c423f0d0a708108e6df6ebc8bc83d, the vfork child
>> + process will starts with TIF_SINGLESTEP/X86_EFLAGS_TF bits
>> + if the parent process has it.
>> + So let child process do a single step under GDB control
>> + before detach it to remove this flags. */
>
> From the kernel patch's looks, this doesn't sound like architecture
> specific, otherwise I'd suggest clearing TF instead.
>
> So it sounds like a good solution.
>
> I suggested this updated comment, copy/edited a bit from yours:
>
> /* When debugging an inferior in an architecture that supports
> hardware single stepping on a kernel without commit
> 6580807da14c423f0d0a708108e6df6ebc8bc83d, the vfork child
> process starts with the TIF_SINGLESTEP/X86_EFLAGS_TF bits
> set if the parent process had them set.
> To work around this, single step the child process
> once before detaching to clear the flags. */
>
Updated.
>> +
>> + if (!gdbarch_software_single_step_p (target_thread_architecture
>> + (child_lp->ptid)))
>> + {
>> + int status;
>> +
>> + if (ptrace (PTRACE_SINGLESTEP, child_pid, 0, 0) < 0)
>> + perror_with_name (_("Couldn't do single step"));
>> + if (my_waitpid (child_pid, &status, 0) < 0)
>> + perror_with_name (_("Couldn't wait vfork process"));
>
> If the child gets a signal here, we should pass it on to the child.
>
>> + }
>> +
>> ptrace (PTRACE_DETACH, child_pid, 0, 0);
>
> That is:
>
> ptrace (PTRACE_DETACH, child_pid, 0, WSTOPSIG (status));
>
Fixed.
> And I think we should disable all ptrace options in the child
> before stepping it, in case some event is reported right
> at that point, and we mishandle it. Otherwise we'd need to
> make sure we didn't get an extended wait status before passing
> it on. But disabling events is just safer.
>
> There's a linux_enable_event_reporting function in common/linux-ptrace.c.
> Add a linux_disable_event_reporting counterpart, and call that.
>
Added a new function linux_disable_event_reporting and call it in the
part before let child do single step.
>>
>> do_cleanups (old_chain);
>>
>
This is the new patch for the issue. Please help me review it.
Best,
Hui
2014-06-05 Hui Zhu <hui@codesourcery.com>
* common/linux-ptrace.c (linux_disable_event_reporting): New.
* common/linux-ptrace.h (linux_disable_event_reporting): New extern.
* linux-nat.c (linux_child_follow_fork): do a single step before
detach
Comments
On 06/05/2014 08:47 AM, Hui Zhu wrote:
>
> 2014-06-05 Hui Zhu <hui@codesourcery.com>
>
> * common/linux-ptrace.c (linux_disable_event_reporting): New.
> * common/linux-ptrace.h (linux_disable_event_reporting): New extern.
You're not adding an extern to an existing function, but adding new declaration
that happens to declare a function with extern linkage.
> * linux-nat.c (linux_child_follow_fork): do a single step before
> detach
Capitalization, and full stop both missing. Write instead:
* common/linux-ptrace.c (linux_disable_event_reporting): New function.
* common/linux-ptrace.h (linux_disable_event_reporting): New declaration.
* linux-nat.c (linux_child_follow_fork): Do a single step before
detach.
> + if (ptrace (PTRACE_SINGLESTEP, child_pid, 0, 0) < 0)
> + perror_with_name (_("Couldn't do single step"));
> + if (my_waitpid (child_pid, &status, 0) < 0)
> + perror_with_name (_("Couldn't wait vfork process"));
> + }
> +
> + ptrace (PTRACE_DETACH, child_pid, 0, WSTOPSIG (status));
The child could have exited with that single-step. So:
if (WIFSTOPPED (status))
ptrace (PTRACE_DETACH, child_pid, 0, WSTOPSIG (status));
OK with that change.
On 06/05/14 16:43, Pedro Alves wrote:
> On 06/05/2014 08:47 AM, Hui Zhu wrote:
>
>>
>> 2014-06-05 Hui Zhu <hui@codesourcery.com>
>>
>> * common/linux-ptrace.c (linux_disable_event_reporting): New.
>> * common/linux-ptrace.h (linux_disable_event_reporting): New extern.
>
> You're not adding an extern to an existing function, but adding new declaration
> that happens to declare a function with extern linkage.
>
>> * linux-nat.c (linux_child_follow_fork): do a single step before
>> detach
>
> Capitalization, and full stop both missing. Write instead:
>
> * common/linux-ptrace.c (linux_disable_event_reporting): New function.
> * common/linux-ptrace.h (linux_disable_event_reporting): New declaration.
> * linux-nat.c (linux_child_follow_fork): Do a single step before
> detach.
Updated.
>
>> + if (ptrace (PTRACE_SINGLESTEP, child_pid, 0, 0) < 0)
>> + perror_with_name (_("Couldn't do single step"));
>> + if (my_waitpid (child_pid, &status, 0) < 0)
>> + perror_with_name (_("Couldn't wait vfork process"));
>> + }
>> +
>> + ptrace (PTRACE_DETACH, child_pid, 0, WSTOPSIG (status));
>
> The child could have exited with that single-step. So:
>
> if (WIFSTOPPED (status))
> ptrace (PTRACE_DETACH, child_pid, 0, WSTOPSIG (status));
Updated.
>
> OK with that change.
>
Thanks for your help. Committed as
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=c077881afaedb9b74063bee992b3e472b4b6e9ca
Best,
Hui
@@ -476,6 +476,15 @@ linux_enable_event_reporting (pid_t pid)
(PTRACE_TYPE_ARG4) (uintptr_t) current_ptrace_options);
}
+/* Disable reporting of all currently supported ptrace events. */
+
+void
+linux_disable_event_reporting (pid_t pid)
+{
+ /* Set the options. */
+ ptrace (PTRACE_SETOPTIONS, pid, (PTRACE_TYPE_ARG3) 0, 0);
+}
+
/* Returns non-zero if PTRACE_OPTIONS is contained within
CURRENT_PTRACE_OPTIONS, therefore supported. Returns 0
otherwise. */
@@ -86,6 +86,7 @@ struct buffer;
extern void linux_ptrace_attach_fail_reason (pid_t pid, struct buffer
*buffer);
extern void linux_ptrace_init_warnings (void);
extern void linux_enable_event_reporting (pid_t pid);
+extern void linux_disable_event_reporting (pid_t pid);
extern int linux_supports_tracefork (void);
extern int linux_supports_traceclone (void);
extern int linux_supports_tracevforkdone (void);
@@ -414,6 +414,7 @@ holding the child stopped. Try \"set de
if (detach_fork)
{
struct cleanup *old_chain;
+ int status = 0;
/* Before detaching from the child, remove all breakpoints
from it. If we forked, then this has already been taken
@@ -447,7 +448,28 @@ holding the child stopped. Try \"set de
if (linux_nat_prepare_to_resume != NULL)
linux_nat_prepare_to_resume (child_lp);
- ptrace (PTRACE_DETACH, child_pid, 0, 0);
+
+ /* When debugging an inferior in an architecture that supports
+ hardware single stepping on a kernel without commit
+ 6580807da14c423f0d0a708108e6df6ebc8bc83d, the vfork child
+ process starts with the TIF_SINGLESTEP/X86_EFLAGS_TF bits
+ set if the parent process had them set.
+ To work around this, single step the child process
+ once before detaching to clear the flags. */
+
+ if (!gdbarch_software_single_step_p (target_thread_architecture
+ (child_lp->ptid)))
+ {
+ int status;
+
+ linux_disable_event_reporting (child_pid);
+ if (ptrace (PTRACE_SINGLESTEP, child_pid, 0, 0) < 0)
+ perror_with_name (_("Couldn't do single step"));
+ if (my_waitpid (child_pid, &status, 0) < 0)
+ perror_with_name (_("Couldn't wait vfork process"));
+ }
+
+ ptrace (PTRACE_DETACH, child_pid, 0, WSTOPSIG (status));
do_cleanups (old_chain);
}