[PATCH][gdb] Fix stepping over fork with follow-fork-mode child and gcc-8

[PATCH][gdb] Fix stepping over fork with follow-fork-mode child and gcc-8

[Tom de Vries]

Hi,

When running test-case gdb.threads/fork-child-threads.exp with gcc-8 instead
of gcc-7, we have:
...
 (gdb) next^M
 [Attaching after Thread 0x7ffff7fae740 (LWP 27574) fork to child process \
   27578]^M
 [New inferior 2 (process 27578)]^M
 [Detaching after fork from parent process 27574]^M
 [Inferior 1 (process 27574) detached]^M
 [Thread debugging using libthread_db enabled]^M
 Using host libthread_db library "/lib64/libthread_db.so.1".^M
 [Switching to Thread 0x7ffff7fae740 (LWP 27578)]^M
-main () at src/gdb/testsuite/gdb.threads/fork-child-threads.c:41^M
+main () at src/gdb/testsuite/gdb.threads/fork-child-threads.c:34^M
-41            i = pthread_create (&thread, NULL, start, NULL);^M
+34        switch (fork ())^M
-(gdb) PASS: gdb.threads/fork-child-threads.exp: next over fork
+(gdb) FAIL: gdb.threads/fork-child-threads.exp: next over fork
...

This is due to the fact that gcc-8 generates more precise line info, making
the instruction after the call to fork a "recommended breakpoint location".
However, it is a bug because next is supposed to move to the next source
line.

The problem is that in process_event_stop_test we hit this code:
...
  if ((ecs->event_thread->suspend.stop_pc == stop_pc_sal.pc)
      && (ecs->event_thread->current_line != stop_pc_sal.line
	  || ecs->event_thread->current_symtab != stop_pc_sal.symtab))
    {
      if (stop_pc_sal.is_stmt)
	{
	  /* We are at the start of a different line.  So stop.  Note that
	     we don't stop if we step into the middle of a different line.
	     That is said to make things like for (;;) statements work
	     better.  */
	  if (debug_infrun)
	    fprintf_unfiltered (gdb_stdlog,
				"infrun: stepped to a different line\n");
	  end_stepping_range (ecs);
	  return;
	}
...
because current_line and current_symtab have initial values:
...
(gdb) p ecs->event_thread->current_line
$8 = 0
(gdb) p ecs->event_thread->current_symtab
$9 = (symtab *) 0x0
...

Fix this in follow_fork by copying current_line and current_symtab from
parent thread to child thread.

Tested on x86_64-linux, with gcc 7.5.0 and gcc 10.0.1.

OK for trunk?

Thanks,
- Tom

[gdb] Fix stepping over fork with follow-fork-mode child and gcc-8

gdb/ChangeLog:

2020-05-07  Tom de Vries  <tdevries@suse.de>

	* infrun.c (follow_fork): Copy current_line and current_symtab to
	child thread.

---
 gdb/infrun.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 0e1ba6986b1..3c6b201a9fc 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -681,6 +681,8 @@ follow_fork ()
   struct breakpoint *exception_resume_breakpoint = NULL;
   CORE_ADDR step_range_start = 0;
   CORE_ADDR step_range_end = 0;
+  int current_line = 0;
+  symtab *current_symtab = NULL;
   struct frame_id step_frame_id = { 0 };
   struct thread_fsm *thread_fsm = NULL;
 
@@ -734,6 +736,8 @@ follow_fork ()
 					 (tp->control.step_resume_breakpoint);
 	    step_range_start = tp->control.step_range_start;
 	    step_range_end = tp->control.step_range_end;
+	    current_line = tp->current_line;
+	    current_symtab = tp->current_symtab;
 	    step_frame_id = tp->control.step_frame_id;
 	    exception_resume_breakpoint
 	      = clone_momentary_breakpoint (tp->control.exception_resume_breakpoint);
@@ -794,6 +798,8 @@ follow_fork ()
 		      = step_resume_breakpoint;
 		    tp->control.step_range_start = step_range_start;
 		    tp->control.step_range_end = step_range_end;
+		    tp->current_line = current_line;
+		    tp->current_symtab = current_symtab;
 		    tp->control.step_frame_id = step_frame_id;
 		    tp->control.exception_resume_breakpoint
 		      = exception_resume_breakpoint;

Re: [PATCH][gdb] Fix stepping over fork with follow-fork-mode child and gcc-8

[Simon Marchi]

On 2020-05-07 1:04 a.m., Tom de Vries wrote:
> Hi,
> 
> When running test-case gdb.threads/fork-child-threads.exp with gcc-8 instead
> of gcc-7, we have:
> ...
>  (gdb) next^M
>  [Attaching after Thread 0x7ffff7fae740 (LWP 27574) fork to child process \
>    27578]^M
>  [New inferior 2 (process 27578)]^M
>  [Detaching after fork from parent process 27574]^M
>  [Inferior 1 (process 27574) detached]^M
>  [Thread debugging using libthread_db enabled]^M
>  Using host libthread_db library "/lib64/libthread_db.so.1".^M
>  [Switching to Thread 0x7ffff7fae740 (LWP 27578)]^M
> -main () at src/gdb/testsuite/gdb.threads/fork-child-threads.c:41^M
> +main () at src/gdb/testsuite/gdb.threads/fork-child-threads.c:34^M
> -41            i = pthread_create (&thread, NULL, start, NULL);^M
> +34        switch (fork ())^M
> -(gdb) PASS: gdb.threads/fork-child-threads.exp: next over fork
> +(gdb) FAIL: gdb.threads/fork-child-threads.exp: next over fork
> ...
> 
> This is due to the fact that gcc-8 generates more precise line info, making
> the instruction after the call to fork a "recommended breakpoint location".
> However, it is a bug because next is supposed to move to the next source
> line.
> 
> The problem is that in process_event_stop_test we hit this code:
> ...
>   if ((ecs->event_thread->suspend.stop_pc == stop_pc_sal.pc)
>       && (ecs->event_thread->current_line != stop_pc_sal.line
> 	  || ecs->event_thread->current_symtab != stop_pc_sal.symtab))
>     {
>       if (stop_pc_sal.is_stmt)
> 	{
> 	  /* We are at the start of a different line.  So stop.  Note that
> 	     we don't stop if we step into the middle of a different line.
> 	     That is said to make things like for (;;) statements work
> 	     better.  */
> 	  if (debug_infrun)
> 	    fprintf_unfiltered (gdb_stdlog,
> 				"infrun: stepped to a different line\n");
> 	  end_stepping_range (ecs);
> 	  return;
> 	}
> ...
> because current_line and current_symtab have initial values:
> ...
> (gdb) p ecs->event_thread->current_line
> $8 = 0
> (gdb) p ecs->event_thread->current_symtab
> $9 = (symtab *) 0x0
> ...
> 
> Fix this in follow_fork by copying current_line and current_symtab from
> parent thread to child thread.
> 
> Tested on x86_64-linux, with gcc 7.5.0 and gcc 10.0.1.
> 
> OK for trunk?

Hi Tom,

The change makes sense to me, although I don't know this code in depth (things
related to lines and SaL).  But I confirm that it fixes on my machine the same
FAIL that you see.

Simon

Re: [PATCH][gdb] Fix stepping over fork with follow-fork-mode child and gcc-8

[Simon Marchi]

On 2020-05-07 1:04 a.m., Tom de Vries wrote:
> Hi,
> 
> When running test-case gdb.threads/fork-child-threads.exp with gcc-8 instead
> of gcc-7, we have:
> ...
>  (gdb) next^M
>  [Attaching after Thread 0x7ffff7fae740 (LWP 27574) fork to child process \
>    27578]^M
>  [New inferior 2 (process 27578)]^M
>  [Detaching after fork from parent process 27574]^M
>  [Inferior 1 (process 27574) detached]^M
>  [Thread debugging using libthread_db enabled]^M
>  Using host libthread_db library "/lib64/libthread_db.so.1".^M
>  [Switching to Thread 0x7ffff7fae740 (LWP 27578)]^M
> -main () at src/gdb/testsuite/gdb.threads/fork-child-threads.c:41^M
> +main () at src/gdb/testsuite/gdb.threads/fork-child-threads.c:34^M
> -41            i = pthread_create (&thread, NULL, start, NULL);^M
> +34        switch (fork ())^M
> -(gdb) PASS: gdb.threads/fork-child-threads.exp: next over fork
> +(gdb) FAIL: gdb.threads/fork-child-threads.exp: next over fork
> ...
> 
> This is due to the fact that gcc-8 generates more precise line info, making
> the instruction after the call to fork a "recommended breakpoint location".
> However, it is a bug because next is supposed to move to the next source
> line.
> 
> The problem is that in process_event_stop_test we hit this code:
> ...
>   if ((ecs->event_thread->suspend.stop_pc == stop_pc_sal.pc)
>       && (ecs->event_thread->current_line != stop_pc_sal.line
> 	  || ecs->event_thread->current_symtab != stop_pc_sal.symtab))
>     {
>       if (stop_pc_sal.is_stmt)
> 	{
> 	  /* We are at the start of a different line.  So stop.  Note that
> 	     we don't stop if we step into the middle of a different line.
> 	     That is said to make things like for (;;) statements work
> 	     better.  */
> 	  if (debug_infrun)
> 	    fprintf_unfiltered (gdb_stdlog,
> 				"infrun: stepped to a different line\n");
> 	  end_stepping_range (ecs);
> 	  return;
> 	}
> ...
> because current_line and current_symtab have initial values:
> ...
> (gdb) p ecs->event_thread->current_line
> $8 = 0
> (gdb) p ecs->event_thread->current_symtab
> $9 = (symtab *) 0x0
> ...
> 
> Fix this in follow_fork by copying current_line and current_symtab from
> parent thread to child thread.
> 
> Tested on x86_64-linux, with gcc 7.5.0 and gcc 10.0.1.
> 
> OK for trunk?

Hi Tom,

I'm not

Re: [PATCH][gdb] Fix stepping over fork with follow-fork-mode child and gcc-8

[Tom de Vries]

On 08-05-2020 17:16, Simon Marchi wrote:
> On 2020-05-07 1:04 a.m., Tom de Vries wrote:
>> Hi,
>>
>> When running test-case gdb.threads/fork-child-threads.exp with gcc-8 instead
>> of gcc-7, we have:
>> ...
>>  (gdb) next^M
>>  [Attaching after Thread 0x7ffff7fae740 (LWP 27574) fork to child process \
>>    27578]^M
>>  [New inferior 2 (process 27578)]^M
>>  [Detaching after fork from parent process 27574]^M
>>  [Inferior 1 (process 27574) detached]^M
>>  [Thread debugging using libthread_db enabled]^M
>>  Using host libthread_db library "/lib64/libthread_db.so.1".^M
>>  [Switching to Thread 0x7ffff7fae740 (LWP 27578)]^M
>> -main () at src/gdb/testsuite/gdb.threads/fork-child-threads.c:41^M
>> +main () at src/gdb/testsuite/gdb.threads/fork-child-threads.c:34^M
>> -41            i = pthread_create (&thread, NULL, start, NULL);^M
>> +34        switch (fork ())^M
>> -(gdb) PASS: gdb.threads/fork-child-threads.exp: next over fork
>> +(gdb) FAIL: gdb.threads/fork-child-threads.exp: next over fork
>> ...
>>
>> This is due to the fact that gcc-8 generates more precise line info, making
>> the instruction after the call to fork a "recommended breakpoint location".
>> However, it is a bug because next is supposed to move to the next source
>> line.
>>
>> The problem is that in process_event_stop_test we hit this code:
>> ...
>>   if ((ecs->event_thread->suspend.stop_pc == stop_pc_sal.pc)
>>       && (ecs->event_thread->current_line != stop_pc_sal.line
>> 	  || ecs->event_thread->current_symtab != stop_pc_sal.symtab))
>>     {
>>       if (stop_pc_sal.is_stmt)
>> 	{
>> 	  /* We are at the start of a different line.  So stop.  Note that
>> 	     we don't stop if we step into the middle of a different line.
>> 	     That is said to make things like for (;;) statements work
>> 	     better.  */
>> 	  if (debug_infrun)
>> 	    fprintf_unfiltered (gdb_stdlog,
>> 				"infrun: stepped to a different line\n");
>> 	  end_stepping_range (ecs);
>> 	  return;
>> 	}
>> ...
>> because current_line and current_symtab have initial values:
>> ...
>> (gdb) p ecs->event_thread->current_line
>> $8 = 0
>> (gdb) p ecs->event_thread->current_symtab
>> $9 = (symtab *) 0x0
>> ...
>>
>> Fix this in follow_fork by copying current_line and current_symtab from
>> parent thread to child thread.
>>
>> Tested on x86_64-linux, with gcc 7.5.0 and gcc 10.0.1.
>>
>> OK for trunk?
> 
> Hi Tom,
> 
> The change makes sense to me, although I don't know this code in depth (things
> related to lines and SaL).  But I confirm that it fixes on my machine the same
> FAIL that you see.

Thanks for the review, committed.

- Tom