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. ;-)