Date: Wed, 13 Feb 2013 10:38:56 -0500 From: John Baldwin <jhb@freebsd.org> To: Kirk McKusick <mckusick@mckusick.com> Cc: Adrian Chadd <adrian@freebsd.org>, Christoph Mallon <christoph.mallon@gmx.de>, Andriy Gapon <avg@freebsd.org>, freebsd-arch@freebsd.org Subject: Re: Proposal: Unify printing the function name in panic messages() Message-ID: <201302131038.57250.jhb@freebsd.org> In-Reply-To: <201302120134.r1C1Ycfh026347@chez.mckusick.com> References: <201302120134.r1C1Ycfh026347@chez.mckusick.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201302131038.57250.jhb>