This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH, gdbsim] Avoid silly crash when no binary is loaded
- From: Pedro Alves <palves at redhat dot com>
- To: lgustavo at codesourcery dot com
- Cc: "'gdb-patches at sourceware dot org'" <gdb-patches at sourceware dot org>
- Date: Wed, 19 Jun 2013 15:10:44 +0100
- Subject: Re: [PATCH, gdbsim] Avoid silly crash when no binary is loaded
- References: <51C0C7E3 dot 1030603 at codesourcery dot com> <51C193AE dot 7010608 at redhat dot com> <51C19FF0 dot 8000005 at codesourcery dot com>
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