[PATCH, gdbsim] Avoid silly crash when no binary is loaded

[PATCH, gdbsim] Avoid silly crash when no binary is loaded

[Luis Machado]

[-- Attachment #1: Type: text/plain, Size: 302 bytes --]

Hi,

This patch prevents the long-standing crash scenario where we start 
gdbsim and "run" without any binaries. Warnings are issued, but those 
don't prevent the simulator from proceeding with garbage data.

Replacing those warnings with error calls seems to be the most 
appropriate here.

Ok?

Luis

[-- Attachment #2: sim_crash.diff --]
[-- Type: text/x-patch, Size: 764 bytes --]

2013-06-18  Luis Machado  <lgustavo@codesourcery.com>

	* remote-sim.c (gdbsim_create_inferior): Replace warnings with
	errors when no program is loaded.

diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c
index fda3735..c04ce01 100644
--- a/gdb/remote-sim.c
+++ b/gdb/remote-sim.c
@@ -612,9 +612,9 @@ gdbsim_create_inferior (struct target_ops *target, char *exec_file, char *args,
   char *arg_buf, **argv;
 
   if (exec_file == 0 || exec_bfd == 0)
-    warning (_("No executable file specified."));
+    error (_("No executable file specified."));
   if (!sim_data->program_loaded)
-    warning (_("No program loaded."));
+    error (_("No program loaded."));
 
   if (remote_debug)
     printf_filtered ("gdbsim_create_inferior: exec_file \"%s\", args \"%s\"\n",

Re: [PATCH, gdbsim] Avoid silly crash when no binary is loaded

[Pedro Alves]

On 06/18/2013 09:49 PM, Luis Machado wrote:
> Hi,
> 
> This patch prevents the long-standing crash scenario where we start 
> gdbsim and "run" without any binaries. Warnings are issued, but those 
> don't prevent the simulator from proceeding with garbage data.

Which sim and backtrace?  I suspect this to be sim/arch dependent.

> 
> Replacing those warnings with error calls seems to be the most 
> appropriate here.

Well, the code seems to have been written like that for a reason.

Real boards can be powered on with no real program in memory
too...

>    if (exec_file == 0 || exec_bfd == 0)
> -    warning (_("No executable file specified."));
> +    error (_("No executable file specified."));
>    if (!sim_data->program_loaded)
> -    warning (_("No program loaded."));
> +    error (_("No program loaded."));
>

There's code just below that does:

>    if (remote_debug)
>      printf_filtered ("gdbsim_create_inferior: exec_file \"%s\", args \"%s\"\n",
...
>  if (exec_file != NULL)
>    {
>      len = strlen (exec_file) + 1 + strlen (args) + 1 + /*slop */ 10;
>      arg_buf = (char *) alloca (len);
>      arg_buf[0] = '\0';
>      strcat (arg_buf, exec_file);
>      strcat (arg_buf, " ");
>      strcat (arg_buf, args);
>      argv = gdb_buildargv (arg_buf);
>      make_cleanup_freeargv (argv);
>    }
>  else
>    argv = NULL;

So if we error out, then these NULL checks are now dead.

And e.g., the bfin sim, at sim/bfin/interp.c:sim_create_inferior
allows NULL exec_bfd:

SIM_RC
sim_create_inferior (SIM_DESC sd, struct bfd *abfd,
		     char **argv, char **env)
{
  SIM_CPU *cpu = STATE_CPU (sd, 0);
  SIM_ADDR addr;

  /* Set the PC.  */
  if (abfd != NULL)
    addr = bfd_get_start_address (abfd);
  else
    addr = 0;
  sim_pc_set (cpu, addr);

-- 
Pedro Alves

Re: [PATCH, gdbsim] Avoid silly crash when no binary is loaded

[Luis Machado]

Hi,

On 06/19/2013 08:19 AM, Pedro Alves wrote:
> On 06/18/2013 09:49 PM, Luis Machado wrote:
>> Hi,
>>
>> This patch prevents the long-standing crash scenario where we start
>> gdbsim and "run" without any binaries. Warnings are issued, but those
>> don't prevent the simulator from proceeding with garbage data.
>
> Which sim and backtrace?  I suspect this to be sim/arch dependent.

This is arm. Other simulators (mips and powerpc) have different 
behaviors. No crashes, but they go all over the place in terms of messages.

I'm questioning the use case of attempting to let the simulator go 
without loading any image to it. If it is useful, then we should state 
that and make it stop crashing.

There is already a barrier, see 
remote-sim.c:gdbsim_xfer_inferior_memory. The same message will be 
displayed there with an error.

   if (!sim_data->program_loaded)
     error (_("No program loaded."));

So, in a way, we're already preventing this scenario later on. If we 
want to keep the old behavior, for whatever old reason that may be, i'm 
ok with it.

#0  0x00000000006a0580 in ARMul_SetPC (state=0x0, value=0) at 
../../../gdb-head/sim/arm/armsupp.c:83
#1  0x0000000000690cef in sim_create_inferior (sd=0x1, abfd=0x0, 
argv=0x0, env=0xc21d90) at ../../../gdb-head/sim/arm/wrapper.c:249
#2  0x0000000000456a93 in gdbsim_create_inferior (target=0xb58100 
<gdbsim_ops>, exec_file=0x0, args=0xc39df0 "", env=0xc21d90, from_tty=1) 
at ../../gdb-head/gdb/remote-sim.c:646
#3  0x0000000000575b09 in target_create_inferior (exec_file=0x0, 
args=0xc39df0 "", env=0xc21d90, from_tty=1) at 
../../gdb-head/gdb/target.c:508
#4  0x000000000052a78a in run_command_1 (args=0x0, from_tty=1, 
tbreak_at_main=0) at ../../gdb-head/gdb/infcmd.c:585
#5  0x000000000052a885 in run_command (args=0x0, from_tty=1) at 
../../gdb-head/gdb/infcmd.c:617
#6  0x0000000000459993 in do_cfunc (c=0xbe3f80, args=0x0, from_tty=1) at 
../../gdb-head/gdb/cli/cli-decode.c:113
#7  0x000000000045cab7 in cmd_func (cmd=0xbe3f80, args=0x0, from_tty=1) 
at ../../gdb-head/gdb/cli/cli-decode.c:1888
#8  0x000000000064e735 in execute_command (p=0xb606c3 "", from_tty=1) at 
../../gdb-head/gdb/top.c:489
#9  0x0000000000555327 in command_handler (command=0xb606c0 "") at 
../../gdb-head/gdb/event-top.c:433
#10 0x000000000055590a in command_line_handler (rl=0xc2d7c0 "run") at 
../../gdb-head/gdb/event-top.c:631
#11 0x00000000006e60da in rl_callback_read_char () at 
../../gdb-head/readline/callback.c:220
#12 0x0000000000554e51 in rl_callback_read_char_wrapper 
(client_data=0x0) at ../../gdb-head/gdb/event-top.c:164
#13 0x000000000055523e in stdin_event_handler (error=0, client_data=0x0) 
at ../../gdb-head/gdb/event-top.c:373
#14 0x0000000000553dd6 in handle_file_event (data=...) at 
../../gdb-head/gdb/event-loop.c:768
#15 0x000000000055327f in process_event () at 
../../gdb-head/gdb/event-loop.c:342
#16 0x0000000000553346 in gdb_do_one_event () at 
../../gdb-head/gdb/event-loop.c:406
#17 0x0000000000553397 in start_event_loop () at 
../../gdb-head/gdb/event-loop.c:431
#18 0x0000000000554e7b in cli_command_loop () at 
../../gdb-head/gdb/event-top.c:177
#19 0x000000000054b7db in current_interp_command_loop () at 
../../gdb-head/gdb/interps.c:331
#20 0x000000000054c280 in captured_command_loop (data=0x0) at 
../../gdb-head/gdb/main.c:260
#21 0x0000000000549c1a in catch_errors (func=0x54c265 
<captured_command_loop>, func_args=0x0, errstring=0x810024 "", mask=6) 
at ../../gdb-head/gdb/exceptions.c:546
#22 0x000000000054d6c4 in captured_main (data=0x7fffffffe080) at 
../../gdb-head/gdb/main.c:1055
#23 0x0000000000549c1a in catch_errors (func=0x54c51c <captured_main>, 
func_args=0x7fffffffe080, errstring=0x810024 "", mask=6) at 
../../gdb-head/gdb/exceptions.c:546
#24 0x000000000054d6fa in gdb_main (args=0x7fffffffe080) at 
../../gdb-head/gdb/main.c:1064
#25 0x000000000040602a in main (argc=1, argv=0x7fffffffe188) at 
../../gdb-head/gdb/gdb.c:34

>>
>> Replacing those warnings with error calls seems to be the most
>> appropriate here.
>
> Well, the code seems to have been written like that for a reason.
>
> Real boards can be powered on with no real program in memory
> too...
>

Of course. The question is if there is any useful use case of letting 
the simulator run without any images loaded.

>>     if (exec_file == 0 || exec_bfd == 0)
>> -    warning (_("No executable file specified."));
>> +    error (_("No executable file specified."));
>>     if (!sim_data->program_loaded)
>> -    warning (_("No program loaded."));
>> +    error (_("No program loaded."));
>>
>
> There's code just below that does:
>
>>     if (remote_debug)
>>       printf_filtered ("gdbsim_create_inferior: exec_file \"%s\", args \"%s\"\n",
> ...
>>   if (exec_file != NULL)
>>     {
>>       len = strlen (exec_file) + 1 + strlen (args) + 1 + /*slop */ 10;
>>       arg_buf = (char *) alloca (len);
>>       arg_buf[0] = '\0';
>>       strcat (arg_buf, exec_file);
>>       strcat (arg_buf, " ");
>>       strcat (arg_buf, args);
>>       argv = gdb_buildargv (arg_buf);
>>       make_cleanup_freeargv (argv);
>>     }
>>   else
>>     argv = NULL;
>
> So if we error out, then these NULL checks are now dead.
>

Right. This may turn to be dead code and may need removal.

> And e.g., the bfin sim, at sim/bfin/interp.c:sim_create_inferior
> allows NULL exec_bfd:
>
> SIM_RC
> sim_create_inferior (SIM_DESC sd, struct bfd *abfd,
> 		     char **argv, char **env)
> {
>    SIM_CPU *cpu = STATE_CPU (sd, 0);
>    SIM_ADDR addr;
>
>    /* Set the PC.  */
>    if (abfd != NULL)
>      addr = bfd_get_start_address (abfd);
>    else
>      addr = 0;
>    sim_pc_set (cpu, addr);
>

Is there a good reason why bfin would allow things to proceed without 
any image? It doesn't even run past that point really.

All i see, for whatever operation, is "No memory".

ppc gives me "No program loaded", "The program is not being run" and 
"The program has no registers now"

mips says "sim_monitor: unhandled reason = 0, pc = 0xbfc00000", then 
falls into the old "Cannot execute this command while the selected 
thread is running" or "sim-events.c:231: assertion failed - 
events->resume_wallclock == 0".

If running, and by that i mean issuing run/start/continue/step commands, 
the simulators with no image is a valid use case, then sounds like 
steering the arm simulator to just do more or less what the other 
simulators do is the right thing.

If the use case is not useful at all, i think we should just wipe it out 
rather than preserve some old unclear feature.

Then again, these simulators are old and not used that often.

Luis

Re: [PATCH, gdbsim] Avoid silly crash when no binary is loaded

[Pedro Alves]

On 06/19/2013 01:11 PM, Luis Machado wrote:
> Hi,
> 
> On 06/19/2013 08:19 AM, Pedro Alves wrote:
>> On 06/18/2013 09:49 PM, Luis Machado wrote:
>>> Hi,
>>>
>>> This patch prevents the long-standing crash scenario where we start
>>> gdbsim and "run" without any binaries. Warnings are issued, but those
>>> don't prevent the simulator from proceeding with garbage data.
>>
>> Which sim and backtrace?  I suspect this to be sim/arch dependent.
> 
> This is arm. Other simulators (mips and powerpc) have different 
> behaviors. No crashes, but they go all over the place in terms of messages.
> 
> I'm questioning the use case of attempting to let the simulator go 
> without loading any image to it. If it is useful, then we should state 
> that and make it stop crashing.

I don't really know.  All I see is that from the code at it was
supported at least at some point.

> 
> There is already a barrier, see 
> remote-sim.c:gdbsim_xfer_inferior_memory. The same message will be 
> displayed there with an error.
> 
>    if (!sim_data->program_loaded)
>      error (_("No program loaded."));
> 
> So, in a way, we're already preventing this scenario later on. If we 
> want to keep the old behavior, for whatever old reason that may be, i'm 
> ok with it.
> 
> #0  0x00000000006a0580 in ARMul_SetPC (state=0x0, value=0) at 
> ../../../gdb-head/sim/arm/armsupp.c:83

Curious.  'state' is initialized by the ARM sim's 'init' function in
the same file, and init is called only by sim_write, sim_read,
sim_store_register and sim_fetch_register.  'init' ends up
getting called by "load", through sim_load ->  sim_load_file -> sim_write.

> #1  0x0000000000690cef in sim_create_inferior (sd=0x1, abfd=0x0, 
> argv=0x0, env=0xc21d90) at ../../../gdb-head/sim/arm/wrapper.c:249
> #2  0x0000000000456a93 in gdbsim_create_inferior (target=0xb58100 
> <gdbsim_ops>, exec_file=0x0, args=0xc39df0 "", env=0xc21d90, from_tty=1) 
> at ../../gdb-head/gdb/remote-sim.c:646


>>>
>>> Replacing those warnings with error calls seems to be the most
>>> appropriate here.
>>
>> Well, the code seems to have been written like that for a reason.
>>
>> Real boards can be powered on with no real program in memory
>> too...
>>
> 
> Of course. The question is if there is any useful use case of letting 
> the simulator run without any images loaded.

I'll leave that up to Mike.

> 
>>>     if (exec_file == 0 || exec_bfd == 0)
>>> -    warning (_("No executable file specified."));
>>> +    error (_("No executable file specified."));
>>>     if (!sim_data->program_loaded)
>>> -    warning (_("No program loaded."));
>>> +    error (_("No program loaded."));
>>>
>>
>> There's code just below that does:
>>
>>>     if (remote_debug)
>>>       printf_filtered ("gdbsim_create_inferior: exec_file \"%s\", args \"%s\"\n",
>> ...
>>>   if (exec_file != NULL)
>>>     {
>>>       len = strlen (exec_file) + 1 + strlen (args) + 1 + /*slop */ 10;
>>>       arg_buf = (char *) alloca (len);
>>>       arg_buf[0] = '\0';
>>>       strcat (arg_buf, exec_file);
>>>       strcat (arg_buf, " ");
>>>       strcat (arg_buf, args);
>>>       argv = gdb_buildargv (arg_buf);
>>>       make_cleanup_freeargv (argv);
>>>     }
>>>   else
>>>     argv = NULL;
>>
>> So if we error out, then these NULL checks are now dead.
>>
> 
> Right. This may turn to be dead code and may need removal.

I have no doubt it ends up as dead code.  ;-)  The patch just
looks obviously incomplete as is, and that prompted my reply.

> Is there a good reason why bfin would allow things to proceed without 
> any image? It doesn't even run past that point really.
> 
> All i see, for whatever operation, is "No memory".

Leaving to Mike.  I just picked bfin because it's a maintained sim.

> ppc gives me "No program loaded", "The program is not being run" and 
> "The program has no registers now"
> 
> mips says "sim_monitor: unhandled reason = 0, pc = 0xbfc00000", then 
> falls into the old "Cannot execute this command while the selected 
> thread is running" or "sim-events.c:231: assertion failed - 
> events->resume_wallclock == 0".
> 
> If running, and by that i mean issuing run/start/continue/step commands, 
> the simulators with no image is a valid use case, then sounds like 
> steering the arm simulator to just do more or less what the other 
> simulators do is the right thing.
> 
> If the use case is not useful at all, i think we should just wipe it out 
> rather than preserve some old unclear feature.

Thank you -- all this analysis is much clearer and a stronger
rationale than the original "silly", or just calling out
that things seem appropriate with no backing.  ;-)

> Then again, these simulators are old and not used that often.

-- 
Pedro Alves

Re: [PATCH, gdbsim] Avoid silly crash when no binary is loaded

[Luis Machado]

On 06/19/2013 11:10 AM, Pedro Alves wrote:
> On 06/19/2013 01:11 PM, Luis Machado wrote:
>> Hi,
>>
>> On 06/19/2013 08:19 AM, Pedro Alves wrote:
>>> On 06/18/2013 09:49 PM, Luis Machado wrote:
>>>> Hi,
>>>>
>>>> This patch prevents the long-standing crash scenario where we start
>>>> gdbsim and "run" without any binaries. Warnings are issued, but those
>>>> don't prevent the simulator from proceeding with garbage data.
>>>
>>> Which sim and backtrace?  I suspect this to be sim/arch dependent.
>>
>> This is arm. Other simulators (mips and powerpc) have different
>> behaviors. No crashes, but they go all over the place in terms of messages.
>>
>> I'm questioning the use case of attempting to let the simulator go
>> without loading any image to it. If it is useful, then we should state
>> that and make it stop crashing.
>
> I don't really know.  All I see is that from the code at it was
> supported at least at some point.
>
>>
>> There is already a barrier, see
>> remote-sim.c:gdbsim_xfer_inferior_memory. The same message will be
>> displayed there with an error.
>>
>>     if (!sim_data->program_loaded)
>>       error (_("No program loaded."));
>>
>> So, in a way, we're already preventing this scenario later on. If we
>> want to keep the old behavior, for whatever old reason that may be, i'm
>> ok with it.
>>
>> #0  0x00000000006a0580 in ARMul_SetPC (state=0x0, value=0) at
>> ../../../gdb-head/sim/arm/armsupp.c:83
>
> Curious.  'state' is initialized by the ARM sim's 'init' function in
> the same file, and init is called only by sim_write, sim_read,
> sim_store_register and sim_fetch_register.  'init' ends up
> getting called by "load", through sim_load ->  sim_load_file -> sim_write.
>
>> #1  0x0000000000690cef in sim_create_inferior (sd=0x1, abfd=0x0,
>> argv=0x0, env=0xc21d90) at ../../../gdb-head/sim/arm/wrapper.c:249
>> #2  0x0000000000456a93 in gdbsim_create_inferior (target=0xb58100
>> <gdbsim_ops>, exec_file=0x0, args=0xc39df0 "", env=0xc21d90, from_tty=1)
>> at ../../gdb-head/gdb/remote-sim.c:646
>
>
>>>>
>>>> Replacing those warnings with error calls seems to be the most
>>>> appropriate here.
>>>
>>> Well, the code seems to have been written like that for a reason.
>>>
>>> Real boards can be powered on with no real program in memory
>>> too...
>>>
>>
>> Of course. The question is if there is any useful use case of letting
>> the simulator run without any images loaded.
>
> I'll leave that up to Mike.
>
>>
>>>>      if (exec_file == 0 || exec_bfd == 0)
>>>> -    warning (_("No executable file specified."));
>>>> +    error (_("No executable file specified."));
>>>>      if (!sim_data->program_loaded)
>>>> -    warning (_("No program loaded."));
>>>> +    error (_("No program loaded."));
>>>>
>>>
>>> There's code just below that does:
>>>
>>>>      if (remote_debug)
>>>>        printf_filtered ("gdbsim_create_inferior: exec_file \"%s\", args \"%s\"\n",
>>> ...
>>>>    if (exec_file != NULL)
>>>>      {
>>>>        len = strlen (exec_file) + 1 + strlen (args) + 1 + /*slop */ 10;
>>>>        arg_buf = (char *) alloca (len);
>>>>        arg_buf[0] = '\0';
>>>>        strcat (arg_buf, exec_file);
>>>>        strcat (arg_buf, " ");
>>>>        strcat (arg_buf, args);
>>>>        argv = gdb_buildargv (arg_buf);
>>>>        make_cleanup_freeargv (argv);
>>>>      }
>>>>    else
>>>>      argv = NULL;
>>>
>>> So if we error out, then these NULL checks are now dead.
>>>
>>
>> Right. This may turn to be dead code and may need removal.
>
> I have no doubt it ends up as dead code.  ;-)  The patch just
> looks obviously incomplete as is, and that prompted my reply.
>
>> Is there a good reason why bfin would allow things to proceed without
>> any image? It doesn't even run past that point really.
>>
>> All i see, for whatever operation, is "No memory".
>
> Leaving to Mike.  I just picked bfin because it's a maintained sim.
>
>> ppc gives me "No program loaded", "The program is not being run" and
>> "The program has no registers now"
>>
>> mips says "sim_monitor: unhandled reason = 0, pc = 0xbfc00000", then
>> falls into the old "Cannot execute this command while the selected
>> thread is running" or "sim-events.c:231: assertion failed -
>> events->resume_wallclock == 0".
>>
>> If running, and by that i mean issuing run/start/continue/step commands,
>> the simulators with no image is a valid use case, then sounds like
>> steering the arm simulator to just do more or less what the other
>> simulators do is the right thing.
>>
>> If the use case is not useful at all, i think we should just wipe it out
>> rather than preserve some old unclear feature.
>
> Thank you -- all this analysis is much clearer and a stronger
> rationale than the original "silly", or just calling out
> that things seem appropriate with no backing.  ;-)
>

Ok. That's good. :-)

I'll wait for Mike's feedback before attempting any other changes for 
this particular issue.

Luis

Re: [PATCH, gdbsim] Avoid silly crash when no binary is loaded

[Luis Machado]

Thought it would be best to cc Mike.

On 06/19/2013 12:15 PM, Luis Machado wrote:
> On 06/19/2013 11:10 AM, Pedro Alves wrote:
>> On 06/19/2013 01:11 PM, Luis Machado wrote:
>>> Hi,
>>>
>>> On 06/19/2013 08:19 AM, Pedro Alves wrote:
>>>> On 06/18/2013 09:49 PM, Luis Machado wrote:
>>>>> Hi,
>>>>>
>>>>> This patch prevents the long-standing crash scenario where we start
>>>>> gdbsim and "run" without any binaries. Warnings are issued, but those
>>>>> don't prevent the simulator from proceeding with garbage data.
>>>>
>>>> Which sim and backtrace?  I suspect this to be sim/arch dependent.
>>>
>>> This is arm. Other simulators (mips and powerpc) have different
>>> behaviors. No crashes, but they go all over the place in terms of
>>> messages.
>>>
>>> I'm questioning the use case of attempting to let the simulator go
>>> without loading any image to it. If it is useful, then we should state
>>> that and make it stop crashing.
>>
>> I don't really know.  All I see is that from the code at it was
>> supported at least at some point.
>>
>>>
>>> There is already a barrier, see
>>> remote-sim.c:gdbsim_xfer_inferior_memory. The same message will be
>>> displayed there with an error.
>>>
>>>     if (!sim_data->program_loaded)
>>>       error (_("No program loaded."));
>>>
>>> So, in a way, we're already preventing this scenario later on. If we
>>> want to keep the old behavior, for whatever old reason that may be, i'm
>>> ok with it.
>>>
>>> #0  0x00000000006a0580 in ARMul_SetPC (state=0x0, value=0) at
>>> ../../../gdb-head/sim/arm/armsupp.c:83
>>
>> Curious.  'state' is initialized by the ARM sim's 'init' function in
>> the same file, and init is called only by sim_write, sim_read,
>> sim_store_register and sim_fetch_register.  'init' ends up
>> getting called by "load", through sim_load ->  sim_load_file ->
>> sim_write.
>>
>>> #1  0x0000000000690cef in sim_create_inferior (sd=0x1, abfd=0x0,
>>> argv=0x0, env=0xc21d90) at ../../../gdb-head/sim/arm/wrapper.c:249
>>> #2  0x0000000000456a93 in gdbsim_create_inferior (target=0xb58100
>>> <gdbsim_ops>, exec_file=0x0, args=0xc39df0 "", env=0xc21d90, from_tty=1)
>>> at ../../gdb-head/gdb/remote-sim.c:646
>>
>>
>>>>>
>>>>> Replacing those warnings with error calls seems to be the most
>>>>> appropriate here.
>>>>
>>>> Well, the code seems to have been written like that for a reason.
>>>>
>>>> Real boards can be powered on with no real program in memory
>>>> too...
>>>>
>>>
>>> Of course. The question is if there is any useful use case of letting
>>> the simulator run without any images loaded.
>>
>> I'll leave that up to Mike.
>>
>>>
>>>>>      if (exec_file == 0 || exec_bfd == 0)
>>>>> -    warning (_("No executable file specified."));
>>>>> +    error (_("No executable file specified."));
>>>>>      if (!sim_data->program_loaded)
>>>>> -    warning (_("No program loaded."));
>>>>> +    error (_("No program loaded."));
>>>>>
>>>>
>>>> There's code just below that does:
>>>>
>>>>>      if (remote_debug)
>>>>>        printf_filtered ("gdbsim_create_inferior: exec_file \"%s\",
>>>>> args \"%s\"\n",
>>>> ...
>>>>>    if (exec_file != NULL)
>>>>>      {
>>>>>        len = strlen (exec_file) + 1 + strlen (args) + 1 + /*slop */
>>>>> 10;
>>>>>        arg_buf = (char *) alloca (len);
>>>>>        arg_buf[0] = '\0';
>>>>>        strcat (arg_buf, exec_file);
>>>>>        strcat (arg_buf, " ");
>>>>>        strcat (arg_buf, args);
>>>>>        argv = gdb_buildargv (arg_buf);
>>>>>        make_cleanup_freeargv (argv);
>>>>>      }
>>>>>    else
>>>>>      argv = NULL;
>>>>
>>>> So if we error out, then these NULL checks are now dead.
>>>>
>>>
>>> Right. This may turn to be dead code and may need removal.
>>
>> I have no doubt it ends up as dead code.  ;-)  The patch just
>> looks obviously incomplete as is, and that prompted my reply.
>>
>>> Is there a good reason why bfin would allow things to proceed without
>>> any image? It doesn't even run past that point really.
>>>
>>> All i see, for whatever operation, is "No memory".
>>
>> Leaving to Mike.  I just picked bfin because it's a maintained sim.
>>
>>> ppc gives me "No program loaded", "The program is not being run" and
>>> "The program has no registers now"
>>>
>>> mips says "sim_monitor: unhandled reason = 0, pc = 0xbfc00000", then
>>> falls into the old "Cannot execute this command while the selected
>>> thread is running" or "sim-events.c:231: assertion failed -
>>> events->resume_wallclock == 0".
>>>
>>> If running, and by that i mean issuing run/start/continue/step commands,
>>> the simulators with no image is a valid use case, then sounds like
>>> steering the arm simulator to just do more or less what the other
>>> simulators do is the right thing.
>>>
>>> If the use case is not useful at all, i think we should just wipe it out
>>> rather than preserve some old unclear feature.
>>
>> Thank you -- all this analysis is much clearer and a stronger
>> rationale than the original "silly", or just calling out
>> that things seem appropriate with no backing.  ;-)
>>
>
> Ok. That's good. :-)
>
> I'll wait for Mike's feedback before attempting any other changes for
> this particular issue.
>
> Luis
>
>

Re: [PATCH, gdbsim] Avoid silly crash when no binary is loaded

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 2858 bytes --]

On Wednesday 19 June 2013 08:11:28 Luis Machado wrote:
> On 06/19/2013 08:19 AM, Pedro Alves wrote:
> > SIM_RC
> > sim_create_inferior (SIM_DESC sd, struct bfd *abfd,
> > 		     char **argv, char **env)
> > {
> >    SIM_CPU *cpu = STATE_CPU (sd, 0);
> >    SIM_ADDR addr;
> >    
> >    /* Set the PC.  */
> >    if (abfd != NULL)
> >      addr = bfd_get_start_address (abfd);
> >    else
> >      addr = 0;
> >    sim_pc_set (cpu, addr);
> 
> Is there a good reason why bfin would allow things to proceed without
> any image? It doesn't even run past that point really.

because it'd segfault otherwise ;)
$ ./gdb/gdb -q
(gdb) target sim
Connected to the simulator.
(gdb) run
Starting program:  
warning: No executable file specified.
warning: No program loaded.
Segmentation fault (core dumped)

> If running, and by that i mean issuing run/start/continue/step commands,
> the simulators with no image is a valid use case, then sounds like
> steering the arm simulator to just do more or less what the other
> simulators do is the right thing.

well, there's a bit more nuance than that.  consider the operating environment 
(literally, --environment operating).  when you connect to that, it's like you 
took jtag and connected it to a cpu fresh out of reset.  sure, there's no 
"program" loaded in its memory, but there's still plenty you can do to the 
device like poke memory and see how it reacts -- whether it be external SDRAM, 
or a parallel flash, or on-chip L1/L2, or memory mapped registers for the 
peripherals, or an async memory bus, or an on-chip boot/ROM (which has both 
code & data).  maybe i'm an odd ball, but i find this scenario great for both 
testing, development (like writing new sim device models), and one-off checks.  
the fact that i need to compile & link a dummy program just to load it into 
the sim so i can start poking around is obnoxious.  similarly, when i have 
just a .bin of raw code/data (e.g. `objcopy -O binary foo.elf foo.bin` which 
is common in the embedded world), how am i going to get that into the 
simulator ?  when i'm connected to jtag, i could use "load" & friends to copy 
it to memory, set the pc, and let it run.  with the sim, i need to create a 
dummy .s that does .incbin "foo.bin" and compile+link that.

now, when you talk about the other environments (like the virtual or user), 
what you want makes a bit more sense as there's not a whole lot you could 
reasonably do.  but i don't think we should head in a direction that moves 
even farther away from what i desire above for the operating environment.  
maybe there's some middle ground ?

> Then again, these simulators are old and not used that often.

that depends on the specific sim.  i take umbrage to the idea that the bfin sim 
is old and not used that often ;).
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH, gdbsim] Avoid silly crash when no binary is loaded

[Pedro Alves]

On 06/20/2013 06:50 PM, Mike Frysinger wrote:

> well, there's a bit more nuance than that.  consider the operating environment 
> (literally, --environment operating).  when you connect to that, it's like you 
> took jtag and connected it to a cpu fresh out of reset.  sure, there's no 
> "program" loaded in its memory, but there's still plenty you can do to the 
> device like poke memory and see how it reacts -- whether it be external SDRAM, 
> or a parallel flash, or on-chip L1/L2, or memory mapped registers for the 
> peripherals, or an async memory bus, or an on-chip boot/ROM (which has both 
> code & data).  maybe i'm an odd ball, but i find this scenario great for both 
> testing, development (like writing new sim device models), and one-off checks.  
> the fact that i need to compile & link a dummy program just to load it into 
> the sim so i can start poking around is obnoxious.  similarly, when i have 
> just a .bin of raw code/data (e.g. `objcopy -O binary foo.elf foo.bin` which 
> is common in the embedded world), how am i going to get that into the 
> simulator ?  when i'm connected to jtag, i could use "load" & friends to copy 
> it to memory, set the pc, and let it run.  with the sim, i need to create a 
> dummy .s that does .incbin "foo.bin" and compile+link that.

This makes a lot of sense to me.  I fully agree.

Thanks,
-- 
Pedro Alves

Re: [PATCH, gdbsim] Avoid silly crash when no binary is loaded

[Stan Shebs]

On 6/20/13 10:50 AM, Mike Frysinger wrote:
> On Wednesday 19 June 2013 08:11:28 Luis Machado wrote:
>> On 06/19/2013 08:19 AM, Pedro Alves wrote:
>>> SIM_RC
>>> sim_create_inferior (SIM_DESC sd, struct bfd *abfd,
>>> 		     char **argv, char **env)
>>> {
>>>    SIM_CPU *cpu = STATE_CPU (sd, 0);
>>>    SIM_ADDR addr;
>>>    
>>>    /* Set the PC.  */
>>>    if (abfd != NULL)
>>>      addr = bfd_get_start_address (abfd);
>>>    else
>>>      addr = 0;
>>>    sim_pc_set (cpu, addr);
>>
>> Is there a good reason why bfin would allow things to proceed without
>> any image? It doesn't even run past that point really.
> 
> because it'd segfault otherwise ;)
> $ ./gdb/gdb -q
> (gdb) target sim
> Connected to the simulator.
> (gdb) run
> Starting program:  
> warning: No executable file specified.
> warning: No program loaded.
> Segmentation fault (core dumped)
> 
>> If running, and by that i mean issuing run/start/continue/step commands,
>> the simulators with no image is a valid use case, then sounds like
>> steering the arm simulator to just do more or less what the other
>> simulators do is the right thing.
> 
> well, there's a bit more nuance than that.  consider the operating environment 
> (literally, --environment operating).  when you connect to that, it's like you 
> took jtag and connected it to a cpu fresh out of reset.  sure, there's no 
> "program" loaded in its memory, but there's still plenty you can do to the 
> device like poke memory and see how it reacts -- whether it be external SDRAM, 
> or a parallel flash, or on-chip L1/L2, or memory mapped registers for the 
> peripherals, or an async memory bus, or an on-chip boot/ROM (which has both 
> code & data).  maybe i'm an odd ball, but i find this scenario great for both 
> testing, development (like writing new sim device models), and one-off checks.  
> the fact that i need to compile & link a dummy program just to load it into 
> the sim so i can start poking around is obnoxious.  similarly, when i have 
> just a .bin of raw code/data (e.g. `objcopy -O binary foo.elf foo.bin` which 
> is common in the embedded world), how am i going to get that into the 
> simulator ?  when i'm connected to jtag, i could use "load" & friends to copy 
> it to memory, set the pc, and let it run.  with the sim, i need to create a 
> dummy .s that does .incbin "foo.bin" and compile+link that.

I agree that having this sort of environment is useful.

The "exec creates the simulation" model is the original one, introduced
by Steve Chamberlain in 1992 or so.  It actually predates my time at
Cygnus, so I don't have any direct knowledge of the decision process,
but I remember having to consider how to keep sims from pre-allocating a
too-large address space for the hosts of the time; using the exec to
size memory was a convenient way to ensure enough space was allocated,
and avoided having to come up with a fancy allocate-on-demand scheme
(which could have been a problem for simulation speed, seems quaint now
but was also a concern at the time).

> now, when you talk about the other environments (like the virtual or user), 
> what you want makes a bit more sense as there's not a whole lot you could 
> reasonably do.  but i don't think we should head in a direction that moves 
> even farther away from what i desire above for the operating environment.  
> maybe there's some middle ground ?

At the risk of stealth trolling via Nth reply to a patch :-) , I'd like
to understand why we're continuing to maintain our own sims when qemu
exists.  Qemu has infrastructure for all the device/board simulation, it
supports a variety of architectures, and it gets a lot of use, including
for GDB testing.

I'm not saying we should start deleting code en masse, but perhaps we
can articulate a rule for relying on qemu vs not, and then that informs
us where to expend effort in our sources.

Stan
stan@codesourcery.com

Re: [PATCH, gdbsim] Avoid silly crash when no binary is loaded

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 3848 bytes --]

On Thursday 20 June 2013 16:45:03 Stan Shebs wrote:
> The "exec creates the simulation" model is the original one, introduced
> by Steve Chamberlain in 1992 or so.  It actually predates my time at
> Cygnus, so I don't have any direct knowledge of the decision process,
> but I remember having to consider how to keep sims from pre-allocating a
> too-large address space for the hosts of the time; using the exec to
> size memory was a convenient way to ensure enough space was allocated,
> and avoided having to come up with a fancy allocate-on-demand scheme
> (which could have been a problem for simulation speed, seems quaint now
> but was also a concern at the time).

you've shown your hand!  my knowledge of the sim is really only from reading 
the code and a bit of the mailing list.  i started hacking around ~Jan 2010, 
so i have no historical background which sometimes would be useful.  without 
any real documents or design info, i can only at best infer.  at least the 
files in common/ tend to be decently documented, so if you read the various 
headers enough, you can start to piece it together.  but maybe i'll hit you up 
with random questions :).

> > now, when you talk about the other environments (like the virtual or
> > user), what you want makes a bit more sense as there's not a whole lot
> > you could reasonably do.  but i don't think we should head in a
> > direction that moves even farther away from what i desire above for the
> > operating environment. maybe there's some middle ground ?
> 
> At the risk of stealth trolling via Nth reply to a patch :-) , I'd like
> to understand why we're continuing to maintain our own sims when qemu
> exists.  Qemu has infrastructure for all the device/board simulation, it
> supports a variety of architectures, and it gets a lot of use, including
> for GDB testing.
> 
> I'm not saying we should start deleting code en masse, but perhaps we
> can articulate a rule for relying on qemu vs not, and then that informs
> us where to expend effort in our sources.

a fair point.  having ported both the GNU/sim and QEMU to support the Blackfin 
architecture, there is certainly overlap.  but i find their ultimate goals to 
be a bit at odds (or at least, how i perceive things).  personally, i find the 
level of tracing & gdb integration in the GNU/sim to be a lot better and 
stable than QEMU (just a mini example, but if you connect to QEMU's gdb 
backend, the SOB will eat a cpu because it sets the socket to nonblocking and 
then does read() in a loop).  the GNU/sim is also a much simpler design (QEMU 
really likes to segfault when something goes wrong) which lends itself to 
debugging.  certainly if i want speed or a "complete" Linux/user shim, i reach 
for QEMU.  but if i want to actually debug/trace some code, especially when it 
comes to something like u-boot or kernel testing, i reach for the GNU/sim.

as i've been trying to bring up the QEMU Blackfin/system port, i don't want to 
rewrite all the lovely device models i've already put together for the 
GNU/sim.  so been trying to ponder some way of architecting things so i can 
copy & paste the core model files between the two.  hopefully the QEMU peeps 
aren't as angry when it comes to HAL shims like the Linux kernel community.

fwiw, i wrote a paper with a friend of mine on the topic of simulation 
focusing with a GNU/sim focus at OSL a few years ago:
https://docs.google.com/document/d/1YE1ma7X-974-4GAcq1hg2Pom646yofRd6Fowol0BDC0/edit

i also did a presentation that broke down my experience between QEMU & the 
GNU/sim and some of the tradeoffs at LSM:
http://wh0rd.org/~vapier/lsm-2012-sim.pdf

i think i'll try and give those presentations again as OSL/LSM weren't exactly 
large.  maybe SCALE or something.  i'm just lazy :/.
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH, gdbsim] Avoid silly crash when no binary is loaded

[Pedro Alves]

On 06/20/2013 10:33 PM, Mike Frysinger wrote:

> fwiw, i wrote a paper with a friend of mine on the topic of simulation 
> focusing with a GNU/sim focus at OSL a few years ago:
> https://docs.google.com/document/d/1YE1ma7X-974-4GAcq1hg2Pom646yofRd6Fowol0BDC0/edit
> 
> i also did a presentation that broke down my experience between QEMU & the 
> GNU/sim and some of the tradeoffs at LSM:
> http://wh0rd.org/~vapier/lsm-2012-sim.pdf

Interesting!  I've now created:

 http://sourceware.org/gdb/wiki/ReferenceMaterial

(linked from the front wiki page)

... and added links to these.

Everyone, please feel free to add to that page.

-- 
Pedro Alves