Proposal: Unify printing the function name in panic messages()

[John Baldwin]

On Monday, February 11, 2013 8:34:38 pm Kirk McKusick wrote:
> I have recently gone down several ratholes trying to understand a
> panic message which had the wrong function name (prefix ufs1_ instead
> of ufs2_).  I can definitely say that it matters to me. And I think
> that fixing the problem as Christoph has outlined will be a big
> step in the right direction.
> 
> John, in your code you cannot expect to match the entire panic
> string if it has any % formats in it. So to be useful, you must be
> able to work with a constant subset of the string. The addition of
> other information such as a function name at the start should not
> affect that constant part that you are trying to match.

Actually, that's not quite true.  I store both the "raw" format string
and a generated string in an on-stack buffer in the PANIC_TRY macro
that is compared against.  Some more examples:

static int
test_no_sleeping_recursion(void)
{
	struct timespec before, after;
	int error;

	/* These tests will DROP_GIANT before panic'ing. */
	DROP_GIANT();
	THREAD_NO_SLEEPING();
	PANIC_TRY {
		tsleep(&test_wchan, 0, "sleep", 1);
	} PANIC_CATCH {
		sleepq_release(&test_wchan);
		IGNORE_PANIC("%s: td %p to sleep on wchan %p with sleeping prohibited");
	} PANIC_END;
	THREAD_NO_SLEEPING();
	PANIC_TRY {
		tsleep(&test_wchan, 0, "sleep", 1);
	} PANIC_CATCH {
		sleepq_release(&test_wchan);
		IGNORE_PANIC("%s: td %p to sleep on wchan %p with sleeping prohibited");
	} PANIC_END;
	THREAD_SLEEPING_OK();
	PANIC_TRY {
		tsleep(&test_wchan, 0, "sleep", 1);
	} PANIC_CATCH {
		sleepq_release(&test_wchan);
		IGNORE_PANIC("%s: td %p to sleep on wchan %p with sleeping prohibited");
	} PANIC_END;	
	THREAD_SLEEPING_OK();

	nanouptime(&before);
	error = tsleep(&test_wchan, 0, "sleep", hz);
	nanouptime(&after);
	KASSERT(error == EWOULDBLOCK, ("tsleep did not timeout: %d", error));
	PICKUP_GIANT();
	return (TEST_OK);
}
UNIT_TEST("sleeping disabled recursion", test_no_sleeping_recursion);

(This panic already use __func__ explicitly)

And my unit tests for PANIC_TRY itself:

static int
catch_panics(void)
{

	PANIC_TRY {
		panic("test");
	} PANIC_CATCH {
		IGNORE_PANIC("test");
	} PANIC_END;
	PANIC_TRY {
		panic("test longer message");
	} PANIC_CATCH {
		IGNORE_PANIC_STARTS_WITH("test");
	} PANIC_END;
	PANIC_TRY {
		panic("test %s", "raw message");
	} PANIC_CATCH {
		IGNORE_PANIC("test %s");
	} PANIC_END;
	PANIC_TRY {
		panic("test %s", "formatted message");
	} PANIC_CATCH {
		IGNORE_PANIC("test formatted message");
	} PANIC_END;
	return (TEST_OK);
}
UNIT_TEST("catching panics", catch_panics);

static int
unexpected_panics(void)
{
	
	PANIC_TRY {
		PANIC_TRY {
			panic("test");
		} PANIC_CATCH {
		} PANIC_END;
	} PANIC_CATCH {
		IGNORE_PANIC_STARTS_WITH("Unexpected panic");
	} PANIC_END;
	PANIC_TRY {
		PANIC_TRY {
			panic("foo");
		} PANIC_CATCH {
			IGNORE_PANIC("bar");
			IGNORE_PANIC_STARTS_WITH("b");
		} PANIC_END;
	} PANIC_CATCH {
		IGNORE_PANIC_STARTS_WITH("Unexpected panic");
	} PANIC_END;
	return (TEST_OK);
}
UNIT_TEST("unexpected panics", unexpected_panics);

TESTER_MODULE(panic);

If every panic has a slightly '%s:' at the front that is perhaps
less painful to deal with in the unformatted case.  In the case
that you want a test to catch a formatted message then it's a bit
less obvious you need to prepend the relevant function name.

-- 
John Baldwin