RE: Proposal: scheduler-locking during inferior calls

["Saiapova, Natalia" <natalia dot saiapova at intel dot com>]

> -----Original Message-----
> 
> Dear All,
> 
> In multithreaded program, when several threads are stopped at a breakpoint and
> scheduler-locking mode is not 'on', there might be unexpected thread switches
> during an inferior call evaluation. If such thread switching occurs,
> the evaluation of the inferior call is abandoned:
> 
> ~~~
> (gdb) run
> Thread 1 "a.out" hit Breakpoint 2, main._omp_fn.0(void) () at multi-omp.cpp:18
> 18                                         std::cout << omp_get_thread_num();
> (gdb) call do_something()
> [Switching to Thread 0x7ffff6327700 (LWP 32498)]
> 
> Thread 3 "a.out" hit Breakpoint 2, main._omp_fn.0(void) () at multi-omp.cpp:18
> 18                                         std::cout << omp_get_thread_num();
> The program stopped in another thread while making a function call from GDB.
> Evaluation of the expression containing the function
> (do_something()) will be abandoned.
> When the function is done executing, GDB will silently stop.
> ~~~
> 
> The similar problem happens with stepping commands, however, for these we have
> 'step' scheduler-locking mode. We could add a new scheduler locking mode that
> will handle inferior calls. But it is highly likely, that this mode would
> be used together with 'step'. If we blend step locking into the new mode,
> one might also want to include the replay, and then the number of possible
> combinations explodes. It does not seem to be flexible and scalable solution.
> And if the new mode locks only inferior calls, it will not be user-friendly,
> since then a user will be forced to switch modes during the debugging.
> 
> I would like to propose a change of existing scheduler-locking mechanism, which
> will give a user fine-granular control over the scheduler lock. The user will
> define his or her own mode based on options that he or she could set
> independently:
> 
> ~~~
> (gdb) set scheduler-locking step on
> (gdb) set scheduler-locking eval on
> (gdb) set scheduler-locking replay on
> (gdb) set scheduler-locking run off
> (gdb) show scheduler-locking
> Scheduler-locking mode:
> step on
> eval on
> replay on
> run off
> ~~~
> 
> Here 'eval' represents inferior calls, and 'run' represents 'continue' and
> 'finish' commands.
> 
> However, these will not be backward compatible with the current 'set/show
> scheduler-locking' commands. As I am not sure, that pros of this change will
> outweigh the backward incompatibility, I have added a command
> 'set scheduler-locking-eval' as a work-around. It toggles scheduler locking
> for inferior calls and coexists with current scheduler-locking modes
> (so it could be used together with or without step or replay modes).
> 
> I would appreciate other opinions about this problem and its possible solutions.

Currently, if we want to execute an inferior call in multithreaded program, 
we have to switch scheduler-locking to 'on' before the call, and then, switch it
back to 'step' or 'replay' after the call.

I could think about three options how we could improve scheduler locking for this case:
add a new mode, add a new set/show command (example from my first email), or
modify the existing locking mechanism (the proposed solution).

The easiest solution would be a creation of a new mode (that was actually 
my first attempt).  The mode was called 'step+' and covered all three options:
'step', 'replay', and 'inferior calls'.  But I believe, that was not 
very user-friendly.  This mode did not fit well to the current list: it not 
only added a new locking option, but also included already existing ones.

The second option is only a temporary workaround for the problem (I attached the example
to my first email).

The third option is not backward compatible, however, I believe, it would make
the scheduler locking UI more transparent to a user.  As far as I could see, 
current scheduler-locking modes do not contradict with each other, so if the 
interface allowed it, they could be mixed.  From the code perspective, 
the proposed change should not be big and will involve only infrun.c:

1. Instead of storing the current mode in 'scheduler_mode' variable, we will store 
a structure of all options
~~~
static struct { 
  int replay = 1;
  int step = 0;
  int eval = 0;
  int run = 0;
} scheduler_locking;
~~~
2. Modify 'schedlock_applies' function;
3. Modify user_visible_resume_ptid (possibly, also use 'schedlock_applies' there;
4. Modify 'clear_proceed_status' to use 'scheduler_locking.replay'.

If the proposed change sounds sensible I could create a patch.

--Natalia
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928