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

[Tom de Vries <tdevries@suse.de>]

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