This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA] Async mode fixes.


On Saturday 08 March 2008 01:18:47 Nick Roberts wrote:
>  > Presently, gdb's async mode is not very healthy, as follows:
>  > 
>  > 	# of expected passes            10129
>  > 	# of unexpected failures        537
>  > 	# of unexpected successes       2
>  > 	# of expected failures          40
>  > 	# of known failures             48
>  > 	# of unresolved testcases       301
>  > 	# of untested testcases         10
>  > 	# of unsupported tests          63
>  > 
>  > The attached patch improves that, bringing the results to:
>  > 
>  > 	# of expected passes            10722
>  > 	# of unexpected failures        39
>  > 	# of unexpected successes       2
>  > 	# of expected failures          41
>  > 	# of known failures             51
>  > 	# of untested testcases         11
>  > 	# of unsupported tests          64
>  > 
>  > where only few failures are actually specific to async mode. There
>  > are no regressions in non-async mode, on x86.
> 
> Like my patch, IMO this one looks a bit like a dog's breakfast.  

I have only a cat, so I'm afraid I don't know what a dog's breakfast look like :-/

> It's got some 
> good ideas, though, and has certainly increased my understanding of the
> asynchronous code.  Perhaps a combination of the two patches would create
> something useful.

You patch has both general async changes, and Linux native async support.
I've looked at your general async changes, but as you might notice, the
changes in my patch are more extensive -- because I wanted to fix all
failures. The Linux part is independent, and better be addressed separately --
otherwise, we won't be able to easily say if a test failure is a problem with
general code, or with Linux support. 
 
> It may reduce the failures, but I suspect that's partly because it's not really
> running asynchronously.  I don't understand how it really could without adding
> another file handler for inferior events.  That doesn't mean it can't, just
> that I don't understand how it could.

Well, "target async" exists and has existed for a long time. I presume that when
it was added, it fully worked. However, as it was not routinely tested, it has
regressed to its current state.

It surely *is* running asynchronously -- say, when I do -exec-continue, then
gdb does indeed respond to -exec-interrupt. The corresponding event handlers
indeed run, and the processing goes via async code paths. 

>  > The patch has comments whenever appropriate, but some points need explicit
>  > clarification.
>  > 
>  > To recap, current gdb, when operating with async-enabled target, allows
>  > two modes for all commands that run target. The fully async mode, requested
>  > by adding "&" to the command (say, "next &"), makes the target run, gives
>  > you a prompt and allows you to type further commands. If no "&" is added,
>  > then a sync execution is simulated -- whereby the prompt is suppressed
>  > and GDB does not handle any events on stdin. It is my understanding that
>  > we cannot kill this simulated sync mode because for console GDB,
>  > simulated sync mode is the only way we can allow the debugged program to
>  > read from stdin.
> 
> I think sync mode is also needed for command files. 

Possibly -- except there's no prompt in the middle of execution of command file
anyway.

> Also if you look at 
> top.c, only a few commands, e.g. pwd, can execute while the target is running.

Yes, and in MI, only "exec-interrupt" is allowed. Anyway, clearly, async mode
where one can use a total of 5 commands, and not very interesting ones,
is not very attractive, so that list will will grow.

>  >               (I haven't checked if the interaction with debugged program
>  > indeed works, with Nick's linux async patch). This simulated sync mode 
>  > is implemented by the sync_execution variable, and the 
>  > async_enable_stdin and async_enable_stdout functions.
>  >
>  > This issue of simulate sync is not relevant to MI -- for MI, gdb stdin
>  > is generally a pipe, not a terminal, used only for reading 
>  > commands from frontend and gdb has no way to route input to the application.
> 
> Emacs uses a tty for MI when one is available.

Can you clarify? Why don't you use external tty (set with the 'tty' command)?

>  > Another important detail of async mode are continuations -- when we resume
>  > the target, we setup a function to be called when the target eventually
>  > stops. The relevant functions are add_continuation and do_all_continuations.
> 
> There's another continuation in the mi directory:

Err, I'm giving the functions to set and execute continuations, I'm not 
trying to list all existing continuations.

>   else if (target_can_async_p ())
>     {
>       add_continuation (mi_interpreter_exec_continuation, NULL);
>     }
> 
> You don't appear to have changed that.  This is where I have issue with the
> current `async' mode: it doesn't appear to use this continuation.

Are you sure? I get the following with my patch:

	(gdb)
	-interpreter-exec console continue
	~"Continuing.\n"
	^running
	*stopped,reason="breakpoint-hit",bkptno="1",thread-id="1",.....

That continuation is called, even in pristine GDB. However, in pristine GDB,
inferior_event_handler first calls do_all_continuation and then calls
complete_execution, which sets target_executing to 0. So, 
mi_interpreter_exec_continuation thinks that the target is still executing, and
does not print anything.

In my patch, target_executing is set to 0 right away, and we get the above output.

> 
>  > The important problems with the current code are:
>  > 
>  > 1. For MI mode, when we call a CLI command, in mi_cmd_interpreter_exec,
>  > we set sync_execution to 1 and then set it back to 0.  This is just bogus --
>  > setting that variable without also disabling stdin bring the entire
>  > system in half-consistent state. Further, if the CLI command throws
>  > an exception, we fail to restore sync_execution. I just removed that code.
> 
> I think that _may_ be there because mi_cmd_interpreter_exec can process more
> than one CLI command at a time:
> 
> -interpreter-exec console pwd dir
> ~"Working directory /home/nickrob.\n"
> ~"Source directories searched: $cdir:$cwd\n"
> ^done
> (gdb) 
> 
> Perhaps that should be changed to just allow one command.

Oh, I think we indeed better allow just one command. However, even if
we wanted to allow several commands, setting sync_executing would be wrong:
1. It should only be set if stdin is really disabled -- otherwise 
async_enable_stdin can just crash, thinking that stdin is disbled and trying
to pop prompt that was never pushed.
2. For a command that actually runs the target, executing of several command
is broken already. Say, "-interpreter-exec console next next" will
try to execute second next before we got an event indicating that the first
next is done. sync_execution does not cause us to wait for event in
any way, it merely makes it impossible for user to type next command.

- Volodya


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]