Add a 'starti' command.

  This works like 'start' but it stops at the first instruction rather than
the first line in main().  This is useful if one wants to single step
through runtime linker startup.

gdb/ChangeLog:

	* NEWS (Changes since GDB 8.0): Add starti.
	* infcmd.c (enum run_break): New.
	(run_command_1): Insert temporary breakpoint at current PC for
	FIRST_INSTR case.
	(run_command): Use enum run_break.
	(start_command): Likewise.
	(starti_command): New function.
	(_initialize_infcmd): Add starti command.

gdb/doc/ChangeLog:

	* gdb.texinfo (Starting your Program): Add description of
	starti command.  Mention starti command as an alternative for
	debugging the elaboration phase.
---
 gdb/ChangeLog       | 11 +++++++++++
 gdb/NEWS            |  3 +++
 gdb/doc/ChangeLog   |  6 ++++++
 gdb/doc/gdb.texinfo | 18 ++++++++++++++----
 gdb/infcmd.c        | 34 ++++++++++++++++++++++++++++++----
 5 files changed, 64 insertions(+), 8 deletions(-)
  

> From: John Baldwin <jhb@FreeBSD.org>
> Date: Tue, 29 Aug 2017 15:54:57 -0700
> 
> This works like 'start' but it stops at the first instruction rather than
> the first line in main().  This is useful if one wants to single step
> through runtime linker startup.
> 
> gdb/ChangeLog:
> 
> 	* NEWS (Changes since GDB 8.0): Add starti.
> 	* infcmd.c (enum run_break): New.
> 	(run_command_1): Insert temporary breakpoint at current PC for
> 	FIRST_INSTR case.
> 	(run_command): Use enum run_break.
> 	(start_command): Likewise.
> 	(starti_command): New function.
> 	(_initialize_infcmd): Add starti command.
> 
> gdb/doc/ChangeLog:
> 
> 	* gdb.texinfo (Starting your Program): Add description of
> 	starti command.  Mention starti command as an alternative for
> 	debugging the elaboration phase.

OK for the documentation parts.

Thanks.
  

Hi,

I've looked over this patch, and I have a few small requests. [I am not a global maintainer and cannot officially approve your patch.]

On 08/29/2017 03:54 PM, John Baldwin wrote:
> This works like 'start' but it stops at the first instruction rather than
> the first line in main().  This is useful if one wants to single step
> through runtime linker startup.

I like this!

> diff --git a/gdb/infcmd.c b/gdb/infcmd.c
> index bd9ead8a45..5e9173ff44 100644
> --- a/gdb/infcmd.c
> +++ b/gdb/infcmd.c
> @@ -520,12 +520,14 @@ prepare_execution_command (struct target_ops *target, int background)
>      }
>  }
>  
> +enum run_break { NONE, MAIN, FIRST_INSTR };
> +

This does not conform to our coding standard. We typically require a comment before the enum explaining its usage/purpose and additional comments explaining what each member means. Something like:

/* Determines how run_command_1 will behave.  */

enum run_break
{
  /* Run the inferior.  */
   NONE,

   /* Run the inferior, stopping at the first instruction of main.  */ 
   MAIN,

   // ...
};

While I would rather see these options be a little more succinctly named, such as RUN_NONE, RUN_STOP_MAIN, RUN_STOP_MAIN_FIRST_INSN, I'm not going to ask for that change. But please do consider it.

>  /* Implement the "run" command.  If TBREAK_AT_MAIN is set, then insert
>     a temporary breakpoint at the begining of the main program before
>     running the program.  */
>  
>  static void
> -run_command_1 (char *args, int from_tty, int tbreak_at_main)
> +run_command_1 (char *args, int from_tty, enum run_break run_break)

The comment to this function needs updating: it still refers to the (removed) argument TBREAK_AT_MAIN and does not mention RUN_BREAK.

>  {
>    const char *exec_file;
>    struct cleanup *old_chain;
> @@ -632,6 +634,14 @@ run_command_1 (char *args, int from_tty, int tbreak_at_main)
>       has done its thing; now we are setting up the running program.  */
>    post_create_inferior (&current_target, 0);
>  
> +  if (run_break == FIRST_INSTR)
> +    {
> +      gdb::unique_xmalloc_ptr<char> loc
> +	(xstrdup (("*" + std::to_string (regcache_read_pc
> +					 (get_current_regcache ()))).c_str ()));
> +      tbreak_command (loc.get (), 0);
> +    }
> +

I understand why you did it this way, but I really think using create_breakpoint directly is a better option instead of routing through an existing CLI command (tbreak_command).

Something along the lines of:

      CORE_ADDR insn = regcache_read_pc (get_current_regcache ());
      event_location_up loc = new_address_location (insn, NULL, 0);

      create_breakpoint (get_current_arch (), loc.get (),
			 NULL /* cond_string  */, -1 /* thread  */,
			 NULL /* extra_string */, 0 /* parse_extra */,
			 1 /* tempflag  */, bp_breakpoint,
			 0 /* ignore_count  */,
			 AUTO_BOOLEAN_FALSE /* pending_break_support  */,
			 &bkpt_breakpoint_ops, from_tty, 1 /* enabled  */,
			 0 /* internal  */, 0 /* flags  */);

>    /* Start the target running.  Do not use -1 continuation as it would skip
>       breakpoint right at the entry point.  */
>    proceed (regcache_read_pc (get_current_regcache ()), GDB_SIGNAL_0);
> @@ -660,7 +670,17 @@ start_command (char *args, int from_tty)
>      error (_("No symbol table loaded.  Use the \"file\" command."));
>  
>    /* Run the program until reaching the main procedure...  */
> -  run_command_1 (args, from_tty, 1);
> +  run_command_1 (args, from_tty, MAIN);
> +}
> +
> +/* Start the execution of the program until the first instruction.  */
> +
> +static void
> +starti_command (char *args, int from_tty)
> +{
> +
> +  /* Run the program until reaching the first instruction...  */
> +  run_command_1 (args, from_tty, FIRST_INSTR);
>  } 

I'm all for commenting code, but the comment describing the function is sufficient. :-)

>  static int
> @@ -3415,6 +3435,12 @@ You may specify arguments to give to your program, just as with the\n\
>  \"run\" command."));
>    set_cmd_completer (c, filename_completer);
>  
> +  c = add_com ("starti", class_run, starti_command, _("\
> +Start the debugged program stopping at the first instruction.\n\
> +You may specify arguments to give to your program, just as with the\n\
> +\"run\" command."));
> +  set_cmd_completer (c, filename_completer);
> +
>    add_com ("interrupt", class_run, interrupt_command,
>  	   _("Interrupt the execution of the debugged program.\n\
>  If non-stop mode is enabled, interrupt only the current thread,\n\
> 

I would take the common start/starti text and put it into a macro similar to the way the breakpoint commands are setup.

Alas the last thing missing is a test case for this. It doesn't have to be anything fancy, just check that with no breakpoints installed, starti will actually cause the inferior to stop somewhere/anywhere. [I don't think we can generically do much better than this, e.g., testing if we actually do stop at the first insn.]

Keith
  

Hi John,

On 08/30/2017 12:54 AM, John Baldwin wrote:
> This works like 'start' but it stops at the first instruction rather than
> the first line in main().  This is useful if one wants to single step
> through runtime linker startup.

I like the idea.  I actually once wrote a patch quite similar to this.
I had called the command "create", inspired by "target_create_inferior".
Is there a reason to actually set a breakpoint at the first instruction and
run to it, actually?  My old prototype just created the inferior and
didn't resume it all, see:

 https://github.com/palves/gdb/commits/create_command

though maybe going through normal_stop may be a good idea.

I agree with Keith - this should really have some tests.

For example:

- write a global constructor that sets a flag, and then check
  that the flag is still clear when we're still at the entry point.
  This can be either a C++ test or a C test using
  __attribute__ ((constructor))-

- After creating the inferior, check that you can manually set
  a break on main, and continue to it.

- Try backtrace, and check that only one frame comes
  out.  That may expose buggy unwinders that don't stop
  unwinding at the entry point currently, but then that
  should be fixed anyway, since users will run into that
  too.

Thanks,
Pedro Alves
  

On Thursday, August 31, 2017 11:51:33 PM Pedro Alves wrote:
> Hi John,
> 
> On 08/30/2017 12:54 AM, John Baldwin wrote:
> > This works like 'start' but it stops at the first instruction rather than
> > the first line in main().  This is useful if one wants to single step
> > through runtime linker startup.
> 
> I like the idea.  I actually once wrote a patch quite similar to this.
> I had called the command "create", inspired by "target_create_inferior".
> Is there a reason to actually set a breakpoint at the first instruction and
> run to it, actually?  My old prototype just created the inferior and
> didn't resume it all, see:
> 
>  https://github.com/palves/gdb/commits/create_command
> 
> though maybe going through normal_stop may be a good idea.

I had initially tried something like this (but without the print... line),
but GDB hung for me without printing a prompt and accepting more commands.
I assumed that there was some additional handling in proceed() that needed
to be done.  I would probably rather take this approach if it can be made
to work though.
 
> I agree with Keith - this should really have some tests.
> 
> For example:
> 
> - write a global constructor that sets a flag, and then check
>   that the flag is still clear when we're still at the entry point.
>   This can be either a C++ test or a C test using
>   __attribute__ ((constructor))-
> 
> - After creating the inferior, check that you can manually set
>   a break on main, and continue to it.
> 
> - Try backtrace, and check that only one frame comes
>   out.  That may expose buggy unwinders that don't stop
>   unwinding at the entry point currently, but then that
>   should be fixed anyway, since users will run into that
>   too.

Hmmm, these are a bit more interesting than the very simple test I've added
so far.  I'm not very fluent in writing tests yet, but I'll see what I can
come up with.
  

On Thursday, August 31, 2017 11:51:33 PM Pedro Alves wrote:
> Hi John,
> 
> On 08/30/2017 12:54 AM, John Baldwin wrote:
> > This works like 'start' but it stops at the first instruction rather than
> > the first line in main().  This is useful if one wants to single step
> > through runtime linker startup.
> 
> I like the idea.  I actually once wrote a patch quite similar to this.
> I had called the command "create", inspired by "target_create_inferior".
> Is there a reason to actually set a breakpoint at the first instruction and
> run to it, actually?  My old prototype just created the inferior and
> didn't resume it all, see:
> 
>  https://github.com/palves/gdb/commits/create_command
> 
> though maybe going through normal_stop may be a good idea.

I tried this today and ended up with gdb hung in poll() but not printing a
prompt or accepting commands still, so I've left it as a breakpoint.

> I agree with Keith - this should really have some tests.
> 
> For example:
> 
> - write a global constructor that sets a flag, and then check
>   that the flag is still clear when we're still at the entry point.
>   This can be either a C++ test or a C test using
>   __attribute__ ((constructor))-
> 
> - After creating the inferior, check that you can manually set
>   a break on main, and continue to it.

I've created a test which does these two (will send a v2 in a minute).

> - Try backtrace, and check that only one frame comes
>   out.  That may expose buggy unwinders that don't stop
>   unwinding at the entry point currently, but then that
>   should be fixed anyway, since users will run into that
>   too.

This one didn't work out for me on either FreeBSD or a CentOS 7 VM.  I
know for the FreeBSD case the initial entry point in the runtime linker
doesn't have any CFI directives that would aid in unwinding, and that
might be true for ld.so on Linux as well.

FreeBSD:

(gdb) starti
Starting program: /bin/ls 
Temporary breakpoint 1 at 0x800609650: file /usr/src/libexec/rtld-elf/amd64/rtld_start.S, line 33.

Temporary breakpoint 1, .rtld_start ()
    at /usr/src/libexec/rtld-elf/amd64/rtld_start.S:33
33              xorq    %rbp,%rbp               # Clear frame pointer for good form
(gdb) where
#0  .rtld_start () at /usr/src/libexec/rtld-elf/amd64/rtld_start.S:33
#1  0x0000000000000001 in ?? ()
#2  0x00007fffffffe648 in ?? ()
#3  0x0000000000000000 in ?? ()

CentOS 7:

(gdb) starti
Starting program: /usr/bin/ls 
Temporary breakpoint 1 at 0x7ffff7ddd170

Temporary breakpoint 1, 0x00007ffff7ddd170 in _start ()
   from /lib64/ld-linux-x86-64.so.2
(gdb) where
#0  0x00007ffff7ddd170 in _start () from /lib64/ld-linux-x86-64.so.2
#1  0x0000000000000001 in ?? ()
#2  0x00007fffffffe6ac in ?? ()
#3  0x0000000000000000 in ?? ()
  

Hello John,

On 2 September 2017 at 00:42, John Baldwin <jhb@freebsd.org> wrote:
> On Thursday, August 31, 2017 11:51:33 PM Pedro Alves wrote:
>> Hi John,
>>
>> On 08/30/2017 12:54 AM, John Baldwin wrote:
>> > This works like 'start' but it stops at the first instruction rather than
>> > the first line in main().  This is useful if one wants to single step
>> > through runtime linker startup.
>>
>> I like the idea.  I actually once wrote a patch quite similar to this.
>> I had called the command "create", inspired by "target_create_inferior".
>> Is there a reason to actually set a breakpoint at the first instruction and
>> run to it, actually?  My old prototype just created the inferior and
>> didn't resume it all, see:
>>
>>  https://github.com/palves/gdb/commits/create_command
>>
>> though maybe going through normal_stop may be a good idea.
>
> I tried this today and ended up with gdb hung in poll() but not printing a
> prompt or accepting commands still, so I've left it as a breakpoint.
>
Dunno if this helps, but you might want to check what happens inside GDB when
you start an inferior as

gdb -ex 'b *0' -ex run /path/to/program

In this case GDB fails to set the breakpoint (due to invalid access to
address 0x0),
and the `run` command stops exactly at the first instruction. If we
get `starti` to work
using a similar mechanism, I think it'd be just enough on the
implementation side,
no need for any "normal" breakpoints.

>> I agree with Keith - this should really have some tests.
>>
>> For example:
>>
>> - write a global constructor that sets a flag, and then check
>>   that the flag is still clear when we're still at the entry point.
>>   This can be either a C++ test or a C test using
>>   __attribute__ ((constructor))-
>>
>> - After creating the inferior, check that you can manually set
>>   a break on main, and continue to it.
>
> I've created a test which does these two (will send a v2 in a minute).
>
>> - Try backtrace, and check that only one frame comes
>>   out.  That may expose buggy unwinders that don't stop
>>   unwinding at the entry point currently, but then that
>>   should be fixed anyway, since users will run into that
>>   too.
>
> This one didn't work out for me on either FreeBSD or a CentOS 7 VM.  I
> know for the FreeBSD case the initial entry point in the runtime linker
> doesn't have any CFI directives that would aid in unwinding, and that
> might be true for ld.so on Linux as well.
>
> FreeBSD:
>
> (gdb) starti
> Starting program: /bin/ls
> Temporary breakpoint 1 at 0x800609650: file /usr/src/libexec/rtld-elf/amd64/rtld_start.S, line 33.
>
> Temporary breakpoint 1, .rtld_start ()
>     at /usr/src/libexec/rtld-elf/amd64/rtld_start.S:33
> 33              xorq    %rbp,%rbp               # Clear frame pointer for good form
> (gdb) where
> #0  .rtld_start () at /usr/src/libexec/rtld-elf/amd64/rtld_start.S:33
> #1  0x0000000000000001 in ?? ()
> #2  0x00007fffffffe648 in ?? ()
> #3  0x0000000000000000 in ?? ()
>
> CentOS 7:
>
> (gdb) starti
> Starting program: /usr/bin/ls
> Temporary breakpoint 1 at 0x7ffff7ddd170
>
> Temporary breakpoint 1, 0x00007ffff7ddd170 in _start ()
>    from /lib64/ld-linux-x86-64.so.2
> (gdb) where
> #0  0x00007ffff7ddd170 in _start () from /lib64/ld-linux-x86-64.so.2
> #1  0x0000000000000001 in ?? ()
> #2  0x00007fffffffe6ac in ?? ()
> #3  0x0000000000000000 in ?? ()
>
> --
> John Baldwin

Regards,
Ruslan
  

On 09/01/2017 10:42 PM, John Baldwin wrote:
> On Thursday, August 31, 2017 11:51:33 PM Pedro Alves wrote:
>> Hi John,
>>
>> On 08/30/2017 12:54 AM, John Baldwin wrote:
>>> This works like 'start' but it stops at the first instruction rather than
>>> the first line in main().  This is useful if one wants to single step
>>> through runtime linker startup.
>>
>> I like the idea.  I actually once wrote a patch quite similar to this.
>> I had called the command "create", inspired by "target_create_inferior".
>> Is there a reason to actually set a breakpoint at the first instruction and
>> run to it, actually?  My old prototype just created the inferior and
>> didn't resume it all, see:
>>
>>  https://github.com/palves/gdb/commits/create_command
>>
>> though maybe going through normal_stop may be a good idea.

I should expand on this sentence above.  I was thinking of thinks like,
should a user-defined hook-stop run in this case?  Probably.  Going
via "normal_stop" makes sure that is handled.  My old prototype would
skip any hook-stop (and I don't recall whether I did that on purpose.)

This suggests to me that it'd be good to have a test making sure
we either run the hook-stop or not, whatever we decide should happen.

> 
> I tried this today and ended up with gdb hung in poll() but not printing a
> prompt or accepting commands still, so I've left it as a breakpoint.

Another option is still go via process/normal_stop, but make sure that
the thread won't really be run by queing a pending status.  Like so:

  if (/* starti mode*/)
    {
      // queue a pending event instead of setting a breakpoint at "*$pc".
      thread_info *thr = inferior_thread ();
      thr->suspend.waitstatus_pending_p = 1;
      thr->suspend.waitstatus.kind = TARGET_WAITKIND_STOPPED;
      thr->suspend.waitstatus.value.sig = GDB_SIGNAL_0;
    }

One difference this makes is that this way the inferior doesn't really
ever get a chance to run.  If a signal in nostop+pass state is queued
between creating the process and running to the breakpoint at "*$pc",
the signal handler (if any), runs.  With the pending event approach,
it won't.

> 
>> I agree with Keith - this should really have some tests.
>>
>> For example:
>>
>> - write a global constructor that sets a flag, and then check
>>   that the flag is still clear when we're still at the entry point.
>>   This can be either a C++ test or a C test using
>>   __attribute__ ((constructor))-
>>
>> - After creating the inferior, check that you can manually set
>>   a break on main, and continue to it.
> 
> I've created a test which does these two (will send a v2 in a minute).
> 
>> - Try backtrace, and check that only one frame comes
>>   out.  That may expose buggy unwinders that don't stop
>>   unwinding at the entry point currently, but then that
>>   should be fixed anyway, since users will run into that
>>   too.
> 
> This one didn't work out for me on either FreeBSD or a CentOS 7 VM.  I
> know for the FreeBSD case the initial entry point in the runtime linker
> doesn't have any CFI directives that would aid in unwinding, and that
> might be true for ld.so on Linux as well.
> 
> FreeBSD:
> 
> (gdb) starti
> Starting program: /bin/ls 
> Temporary breakpoint 1 at 0x800609650: file /usr/src/libexec/rtld-elf/amd64/rtld_start.S, line 33.
> 
> Temporary breakpoint 1, .rtld_start ()
>     at /usr/src/libexec/rtld-elf/amd64/rtld_start.S:33
> 33              xorq    %rbp,%rbp               # Clear frame pointer for good form
> (gdb) where
> #0  .rtld_start () at /usr/src/libexec/rtld-elf/amd64/rtld_start.S:33
> #1  0x0000000000000001 in ?? ()
> #2  0x00007fffffffe648 in ?? ()
> #3  0x0000000000000000 in ?? ()
> 
> CentOS 7:
> 
> (gdb) starti
> Starting program: /usr/bin/ls 
> Temporary breakpoint 1 at 0x7ffff7ddd170
> 
> Temporary breakpoint 1, 0x00007ffff7ddd170 in _start ()
>    from /lib64/ld-linux-x86-64.so.2
> (gdb) where
> #0  0x00007ffff7ddd170 in _start () from /lib64/ld-linux-x86-64.so.2
> #1  0x0000000000000001 in ?? ()
> #2  0x00007fffffffe6ac in ?? ()
> #3  0x0000000000000000 in ?? ()

Curious.  For the case of entry points that miss CFI, we have fallback code to
stop unwinding at the entry point's frame by default ("set backtrace past-entry",
frame.c:inside_entry_func), but in this case, we're still in the
dynamic loader, not at the program's entry point yet so that hack^Wsafety-net doesn't
kick in.

Thanks,
Pedro Alves
  

On Monday, September 04, 2017 11:57:26 AM Pedro Alves wrote:
> 
> On 09/01/2017 10:42 PM, John Baldwin wrote:
> > On Thursday, August 31, 2017 11:51:33 PM Pedro Alves wrote:
> >> Hi John,
> >>
> >> On 08/30/2017 12:54 AM, John Baldwin wrote:
> >>> This works like 'start' but it stops at the first instruction rather than
> >>> the first line in main().  This is useful if one wants to single step
> >>> through runtime linker startup.
> >>
> >> I like the idea.  I actually once wrote a patch quite similar to this.
> >> I had called the command "create", inspired by "target_create_inferior".
> >> Is there a reason to actually set a breakpoint at the first instruction and
> >> run to it, actually?  My old prototype just created the inferior and
> >> didn't resume it all, see:
> >>
> >>  https://github.com/palves/gdb/commits/create_command
> >>
> >> though maybe going through normal_stop may be a good idea.
> 
> I should expand on this sentence above.  I was thinking of thinks like,
> should a user-defined hook-stop run in this case?  Probably.  Going
> via "normal_stop" makes sure that is handled.  My old prototype would
> skip any hook-stop (and I don't recall whether I did that on purpose.)
> 
> This suggests to me that it'd be good to have a test making sure
> we either run the hook-stop or not, whatever we decide should happen.

Ok, I included hook-stop in my test.

> > I tried this today and ended up with gdb hung in poll() but not printing a
> > prompt or accepting commands still, so I've left it as a breakpoint.
> 
> Another option is still go via process/normal_stop, but make sure that
> the thread won't really be run by queing a pending status.  Like so:
> 
>   if (/* starti mode*/)
>     {
>       // queue a pending event instead of setting a breakpoint at "*$pc".
>       thread_info *thr = inferior_thread ();
>       thr->suspend.waitstatus_pending_p = 1;
>       thr->suspend.waitstatus.kind = TARGET_WAITKIND_STOPPED;
>       thr->suspend.waitstatus.value.sig = GDB_SIGNAL_0;
>     }

Yes, I like this and have adopted it.

> One difference this makes is that this way the inferior doesn't really
> ever get a chance to run.  If a signal in nostop+pass state is queued
> between creating the process and running to the breakpoint at "*$pc",
> the signal handler (if any), runs.  With the pending event approach,
> it won't.

Technically I don't think a program can register a signal handler without
executing an instruction, but I think the above approach is cleaner
regardless.