From nobody Thu Apr 18 18:19:01 2024 X-Original-To: dev-commits-src-branches@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4VL5dL3bp9z5HKqy; Thu, 18 Apr 2024 18:19:02 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4VL5dL16H4z4237; Thu, 18 Apr 2024 18:19:02 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1713464342; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=2ipSIoL0WULuyGDyZKMOUCwl8dzBudK/Zo6HIe+wpD4=; b=F3YjNWNCnma1fw4kgT5cvlfFBlfEfSiw8NhYcgP1wVQcSwsrvgMFtjKNphLzKvxNKuK6kF sS8wzjJPerJ3QrdkyUQm7avolfrQUpSOs/hUPjZ4jCeoE5HeJwRc1d4Vc8mHVL38kTqh8d zd1t96L/Vo2X3J3tkfpHwi+SsMmGrhaE1KjinexjIJOOB/GAeDxVxMWI01OZNUqOBTY/r6 GzgFkiSIm0VlP/8InlJ61BgYg/+Z5mCV4j6UebMZWD4HPuRc23AzNxYgPa3+zz8dEdGIwZ a7i+t0SFLl4PnRQ3uAw10DOvzZv/J9Ixpk7rkzpDf5BzZogpQUXgsocwPxQa4A== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1713464342; a=rsa-sha256; cv=none; b=yxbGiWA3jlBLzke/njo900hEEAWzY0yjhvOZ4dE6Q47h2CvJLSSanniz9QqRCKdi6Uqr6J Hf6xd1XAOsLMIWD3ZxcR/cYGcm2JcGjNN1xCM7Yy16TRp7x4IzPKur/Xsi6m7l1UrpDoY8 49mdoSZw1yY5lIJz5Xi6HzmNBAH7+8jcfMTgssv9j7ESNrm2Jmi2dXuM/tBFO/yRI/Ln2r qln7nHdOeyahUneHws+ztHC2uHhymDU+6jkkn8v/6/bi4mDtZmo5exqcKOPGcV2Ry/7qol OtNTYIl9nuO3p+qmRTa+Sm+11sze8+UUYWqDwQluO1TXghbKhibanLvxqe2Gww== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1713464342; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=2ipSIoL0WULuyGDyZKMOUCwl8dzBudK/Zo6HIe+wpD4=; b=CbRL0segKDlDgy4lGHoLqwFcs+OU/8ptzHkE3qVeLBYnQQuMbHMltW+QMTldekYn5OUpc7 Rx+YCW7do5IEgfZ7xqMHDyp4wMM6c6yBHVw8Dx5YvjFxqYwjJWKL8mzKiuL5dYTXgX76Qp SN4rAKAzcxMm1IStcSNqTjELg1zf11G5os1aZxRS6mNjB6sO/pQOYjGZ47Z/17FqMxFXIs Dxi2dxaCzm2tDoUjzK0LfE6Z+yTDd/QKfvvrrluruyzpdMwklTqARzXEXxkZaRzVRYkdgf C6ScG7vEzMDFOCcU5NcaqjPBPcYhxtRd/aQqI67udqK8/D8x/aGsjbl9V5ulAA== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4VL5dL0W9XzKk6; Thu, 18 Apr 2024 18:19:02 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.17.1/8.17.1) with ESMTP id 43IIJ1ht017515; Thu, 18 Apr 2024 18:19:01 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 43IIJ12S017512; Thu, 18 Apr 2024 18:19:01 GMT (envelope-from git) Date: Thu, 18 Apr 2024 18:19:01 GMT Message-Id: <202404181819.43IIJ12S017512@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Mitchell Horne Subject: git: 929d8ef0f71e - stable/14 - KASSERT(9): add assertion message guidelines List-Id: Commits to the stable branches of the FreeBSD src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-branches List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-branches@freebsd.org Sender: owner-dev-commits-src-branches@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: mhorne X-Git-Repository: src X-Git-Refname: refs/heads/stable/14 X-Git-Reftype: branch X-Git-Commit: 929d8ef0f71e347ab5921179f2efecb53565c5bd Auto-Submitted: auto-generated The branch stable/14 has been updated by mhorne: URL: https://cgit.FreeBSD.org/src/commit/?id=929d8ef0f71e347ab5921179f2efecb53565c5bd commit 929d8ef0f71e347ab5921179f2efecb53565c5bd Author: Mitchell Horne AuthorDate: 2024-03-21 14:54:49 +0000 Commit: Mitchell Horne CommitDate: 2024-04-18 18:17:52 +0000 KASSERT(9): add assertion message guidelines Add some text describing how to create useful assertion messages. Improve and add to the EXAMPLES. See the discussion prompting this on -hackers: https://mail-archive.freebsd.org/cgi/mid.cgi?57o4rnnq-013s-3nsn-59n5-4ssn1pq81s94 Reviewed by: emaste Discussed with: imp, bz MFC after: 1 week Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D44434 (cherry picked from commit 83a426d13a6a384e63e75d8252c03dd40af3817e) --- share/man/man9/KASSERT.9 | 72 ++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 67 insertions(+), 5 deletions(-) diff --git a/share/man/man9/KASSERT.9 b/share/man/man9/KASSERT.9 index 1d28ce80c9fb..8d9dd98014a8 100644 --- a/share/man/man9/KASSERT.9 +++ b/share/man/man9/KASSERT.9 @@ -2,7 +2,7 @@ .\" .\" Copyright (c) 2000 Jonathan M. Bresler .\" All rights reserved. -.\" Copyright (c) 2023 The FreeBSD Foundation +.\" Copyright (c) 2023-2024 The FreeBSD Foundation .\" .\" Portions of this documentation were written by Mitchell Horne .\" under sponsorship from the FreeBSD Foundation. @@ -29,7 +29,7 @@ .\" (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF .\" THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. .\" -.Dd March 16, 2023 +.Dd March 19, 2024 .Dt KASSERT 9 .Os .Sh NAME @@ -93,8 +93,37 @@ The macro (read as: "must-pass") is a convenience wrapper around .Fn KASSERT -that automatically generates a sensible assertion message including file and -line information. +that automatically generates a simple assertion message including file and line +information. +.Ss Assertion Guidelines +When adding new assertions, keep in mind their primary purpose: to aid in +identifying and debugging of complex error conditions. +.Pp +The panic messages resulting from assertion failures should be useful without +the resulting kernel dump; the message may be included in a bug report, and +should contain the relevant information needed to discern how the assertion was +violated. +This is especially important when the error condition is difficult or +impossible for the developer to reproduce locally. +.Pp +Therefore, assertions should adhere to the following guidelines: +.Bl -enum +.It +Whenever possible, the value of a runtime variable checked by an assertion +condition should appear in its message. +.It +Unrelated conditions must appear in separate assertions. +.It +Multiple related conditions should be distinguishable (e.g. by value), or split +into separate assertions. +.It +When in doubt, print more information, not less. +.El +.Pp +Combined, this gives greater clarity into the exact cause of an assertion +panic; see +.Sx EXAMPLES +below. .Sh EXAMPLES A hypothetical .Vt struct foo @@ -106,11 +135,19 @@ foo_dealloc(struct foo *fp) { KASSERT((fp->foo_flags & FOO_ACTIVE) == 0, - ("%s: fp %p is still active", __func__, fp)); + ("%s: fp %p is still active, flags=%x", __func__, fp, + fp->foo_flags)); ... } .Ed .Pp +This assertion provides the full flag set for the object, as well as the memory +pointer, which may be used by a debugger to examine the object in detail +.Po +for example with a 'show foo' command in +.Xr ddb 4 +.Pc . +.Pp The assertion .Bd -literal -offset indent MPASS(td == curthread); @@ -121,6 +158,31 @@ message: .Bd -literal -offset indent panic: Assertion td == curthread failed at foo.c:87 .Ed +.Pp +This is a simple condition, and the message provides enough information to +investigate the failure. +.Pp +The assertion +.Bd -literal -offset indent +MPASS(td == curthread && (sz >= SIZE_MIN && sz <= SIZE_MAX)); +.Ed +.Pp +is +.Em NOT +useful enough. +The message doesn't indicate which part of the assertion was violated, nor +does it report the value of +.Dv sz , +which may be critical to understanding +.Em why +the assertion failed. +.Pp +According to the guidelines above, this would be correctly expressed as: +.Bd -literal -offset indent +MPASS(td == curthread); +KASSERT(sz >= SIZE_MIN && sz <= SIZE_MAX, + ("invalid size argument: %u", sz)); +.Ed .Sh SEE ALSO .Xr panic 9 .Sh AUTHORS