From owner-freebsd-arch@FreeBSD.ORG Wed Feb 13 16:00:02 2013 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 815A1E05; Wed, 13 Feb 2013 16:00:02 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from bigwig.baldwin.cx (bigknife-pt.tunnel.tserv9.chi1.ipv6.he.net [IPv6:2001:470:1f10:75::2]) by mx1.freebsd.org (Postfix) with ESMTP id 59A77C04; Wed, 13 Feb 2013 16:00:02 +0000 (UTC) Received: from pakbsde14.localnet (unknown [38.105.238.108]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id CD55DB96E; Wed, 13 Feb 2013 11:00:01 -0500 (EST) From: John Baldwin To: Kirk McKusick Subject: Re: Proposal: Unify printing the function name in panic messages() Date: Wed, 13 Feb 2013 10:38:56 -0500 User-Agent: KMail/1.13.5 (FreeBSD/8.2-CBSD-20110714-p25; KDE/4.5.5; amd64; ; ) References: <201302120134.r1C1Ycfh026347@chez.mckusick.com> In-Reply-To: <201302120134.r1C1Ycfh026347@chez.mckusick.com> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Message-Id: <201302131038.57250.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Wed, 13 Feb 2013 11:00:01 -0500 (EST) Cc: Adrian Chadd , Christoph Mallon , Andriy Gapon , freebsd-arch@freebsd.org X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 13 Feb 2013 16:00:02 -0000 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