sim: don't ignore SIG_TRAP

Information
Submitter:	Steve Reinhardt
Repository:	gem5
Branch:	default
Bugs:
Depends On:
Reviewers
Groups:	Default
People:

Description

Changeset 11062:fbce13fdad11
---------------------------
sim: don't ignore SIG_TRAP

By ignoring SIG_TRAP, using --debug-break <N> when not connected to
a debugger becomes a no-op. Apparently this was intended to be a
feature, though the rationale is not clear.

If we don't ignore SIG_TRAP, then using --debug-break <N> when not
connected to a debugger causes the simulation process to terminate
at tick N. This is occasionally useful, e.g., if you just want to
collect a trace for a specific window of execution then you can combine
this with --debug-start to do exactly that.

In addition to not ignoring the signal, this patch also updates
the --debug-break help message and deletes a handful of unprotected
calls to Debug::breakpoint() that relied on the prior behavior.

Testing Done

Ship It!

I think this makes sense, but I think this might breaks some of the assumptions of the Debug::breakpoint() call. This call seems to be sprinkled across NSGigE and Sinic devices. It seems like the author of those devices assumed that Debug::breakpoint() would be a no-op if a debugger isn't connected. Do we need to get rid of those calls before this change goes in? Or is the assumption that those devices are broken and no longer in use?

Steve Reinhardt Sept. 1, 2015, 9:09 a.m. (Sept. 1, 2015, 9:09 a.m.)

Good catch... I hadn't noticed that. There are a few calls outside of those two devices as well: there's a debugbreak pseudo-inst, a BreakPCEvent, and some pretty idiosyncratic usage in base/statistics.cc and arch/alpha/ev5.cc.

My inclination is to get rid of all the unprotected calls, but leave the ones that require some intention on the part of the user (the pseudo-inst and the BreakPCEvent). I'd also consider getting rid of the kill() call in Debug::breakpoint and requiring the user to manually set a breakpoint there from gdb (you could even put it in your .gdbinit) if we're concerned about things like the pseudo-inst inadvertently killing the process, but I'd like to know whether that's actually an issue before we make that change.

Andreas Sandberg Sept. 1, 2015, 1:07 p.m. (Sept. 1, 2015, 1:07 p.m.)

I like the idea of getting rid of the unprotected calls to Debug::breakpoint, I really can't see any other sensible use of it. I'm not sure if getting rid of the kill() call is necessary if you remove the unguarded calls. If you do get rid of it, consider adding a inform_once (if there is such a thing) to tell the user that the function was triggered.

Change Summary:

Extended prior patch to (1) delete the calls to Debug::breakpoint() Andreas pointed out and (2) update --debug-break help message.

Diff:

Revision 2 (+5 -16)

Show changes

	src/arch/alpha/ev5.cc
	src/dev/net/ns_gige.cc
	src/dev/net/sinic.cc
	src/python/m5/main.py
	src/sim/init_signals.cc

Description:

~		Changeset 11062:fbce13fdad11
~
~		sim: don't ignore SIG_TRAP
	~	Changeset 11062:fbce13fdad11
	~	---------------------------
	~	sim: don't ignore SIG_TRAP

		By ignoring SIG_TRAP, using --debug-break <N> when not connected to
		a debugger becomes a no-op. Apparently this was intended to be a
		feature, though the rationale is not clear.

		If we don't ignore SIG_TRAP, then using --debug-break <N> when not
		connected to a debugger causes the simulation process to terminate
		at tick N. This is occasionally useful, e.g., if you just want to
		collect a trace for a specific window of execution then you can combine
		this with --debug-start to do exactly that.
	+
	+	In addition to not ignoring the signal, this patch also updates
	+	the --debug-break help message and deletes a handful of unprotected
	+	calls to Debug::breakpoint() that relied on the prior behavior.

Ship It!

You have a pending review.

Review Board 2.0.15

This change has been marked as submitted.

Screenshots

Files

Change Summary:

Diff:

Description:

Changeset 11062:fbce13fdad11

Status: Closed (submitted)