491828 - Debugger functionality is not thread safe / multi-vm aware

Status: VERIFIED FIXED

(Tamarin Graveyard :: Virtual Machine, defect, P2)

Description

[Lars T Hansen]

There is supposed to be one debugger per VM instance, and there can be multiple VM instances in a process, but as it is right now some amount of debugger functionality is hooked into static variables in the Debugger class, at least these:

  static TraceLevel	astrace_console; 
  static TraceLevel	astrace_callback;
  static bool		in_trace;
  static uint64		astraceStartTime;

They are updated from various places in the code and are not protected by locks, so there is at least a thread safety concern; in practice, they should probably be moved into the debugger instance accessed via core->debugger(), and not be static at all.

Comment 1

[Lars T Hansen]

Blocks a P2/Flash10.x bug.

Comment 2

[Lars T Hansen]

Attachment: Patch

A couple of notes:

- The argument from the command line should really be a member of the
  TraceLevel enum sooner, but there's a problem with inline functions
  referencing types not defined yet.  Ed's cleanup will fix that, and
  then we can clean up the argument types later.

- I chose not to use default values for arguments anywhere; I could
  probably argue either way.  Opinions?

- Have not tried to compile with FRR, but if there is code in FRR that
  assigns to the static variables it will need to be rewritten.  FOL.

Comment 3

[Rick Reitmaier]

Comment on attachment 402831 [details] [diff] [review]
Patch

Quite sure that there is FP code that relies on accessing the statics; so yes it will indeed need to be re-written.

Comment 4

[Lars T Hansen]

redux changeset:   2619:99d96e5546a7