Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 22 Aug 2018 22:19:43 +0000 (UTC)
From:      Conrad Meyer <cem@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r338214 - in head/sys: conf kern sys
Message-ID:  <201808222219.w7MMJhpl049457@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: cem
Date: Wed Aug 22 22:19:42 2018
New Revision: 338214
URL: https://svnweb.freebsd.org/changeset/base/338214

Log:
  KASSERT: Make runtime optionality optional
  
  Add an option, KASSERT_PANIC_OPTIONAL, that allows runtime KASSERT()
  behavior changes.  When this option is not enabled, code that allows
  KASSERTs to become optional is not enabled, and all violated assertions
  cause termination.
  
  The runtime KASSERT behavior was added in r243980.
  
  One important distinction here is that panic has __dead2
  ("attribute((noreturn))"), while kassert_panic does not.  Static analyzers
  like Coverity understand __dead2.  Without it, KASSERTs go misunderstood,
  resulting in many false positives that result from violation of program
  invariants.
  
  Reviewed by:	jhb, jtl, np, vangyzen
  Relnotes:	yes
  Sponsored by:	Dell EMC Isilon
  Differential Revision:	https://reviews.freebsd.org/D16835

Modified:
  head/sys/conf/NOTES
  head/sys/conf/options
  head/sys/kern/kern_shutdown.c
  head/sys/sys/systm.h

Modified: head/sys/conf/NOTES
==============================================================================
--- head/sys/conf/NOTES	Wed Aug 22 21:23:32 2018	(r338213)
+++ head/sys/conf/NOTES	Wed Aug 22 22:19:42 2018	(r338214)
@@ -554,6 +554,14 @@ options 	INVARIANTS
 options 	INVARIANT_SUPPORT
 
 #
+# The KASSERT_PANIC_OPTIONAL option allows kasserts to fire without
+# necessarily inducing a panic.  Panic is the default behavior, but
+# runtime options can configure it either entirely off, or off with a
+# limit.
+#
+options 	KASSERT_PANIC_OPTIONAL
+
+#
 # The DIAGNOSTIC option is used to enable extra debugging information
 # from some parts of the kernel.  As this makes everything more noisy,
 # it is disabled by default.

Modified: head/sys/conf/options
==============================================================================
--- head/sys/conf/options	Wed Aug 22 21:23:32 2018	(r338213)
+++ head/sys/conf/options	Wed Aug 22 22:19:42 2018	(r338214)
@@ -605,6 +605,7 @@ DFLTPHYS		opt_global.h
 DIAGNOSTIC		opt_global.h
 INVARIANT_SUPPORT	opt_global.h
 INVARIANTS		opt_global.h
+KASSERT_PANIC_OPTIONAL	opt_global.h
 MAXCPU			opt_global.h
 MAXMEMDOM		opt_global.h
 MAXPHYS			opt_global.h

Modified: head/sys/kern/kern_shutdown.c
==============================================================================
--- head/sys/kern/kern_shutdown.c	Wed Aug 22 21:23:32 2018	(r338213)
+++ head/sys/kern/kern_shutdown.c	Wed Aug 22 22:19:42 2018	(r338214)
@@ -652,40 +652,47 @@ static int kassert_warnings = 0;
 
 SYSCTL_NODE(_debug, OID_AUTO, kassert, CTLFLAG_RW, NULL, "kassert options");
 
-SYSCTL_INT(_debug_kassert, OID_AUTO, warn_only, CTLFLAG_RWTUN,
+#ifdef KASSERT_PANIC_OPTIONAL
+#define KASSERT_RWTUN	CTLFLAG_RWTUN
+#else
+#define KASSERT_RWTUN	CTLFLAG_RDTUN
+#endif
+
+SYSCTL_INT(_debug_kassert, OID_AUTO, warn_only, KASSERT_RWTUN,
     &kassert_warn_only, 0,
-    "KASSERT triggers a panic (1) or just a warning (0)");
+    "KASSERT triggers a panic (0) or just a warning (1)");
 
 #ifdef KDB
-SYSCTL_INT(_debug_kassert, OID_AUTO, do_kdb, CTLFLAG_RWTUN,
+SYSCTL_INT(_debug_kassert, OID_AUTO, do_kdb, KASSERT_RWTUN,
     &kassert_do_kdb, 0, "KASSERT will enter the debugger");
 #endif
 
 #ifdef KTR
-SYSCTL_UINT(_debug_kassert, OID_AUTO, do_ktr, CTLFLAG_RWTUN,
+SYSCTL_UINT(_debug_kassert, OID_AUTO, do_ktr, KASSERT_RWTUN,
     &kassert_do_ktr, 0,
     "KASSERT does a KTR, set this to the KTRMASK you want");
 #endif
 
-SYSCTL_INT(_debug_kassert, OID_AUTO, do_log, CTLFLAG_RWTUN,
+SYSCTL_INT(_debug_kassert, OID_AUTO, do_log, KASSERT_RWTUN,
     &kassert_do_log, 0,
     "If warn_only is enabled, log (1) or do not log (0) assertion violations");
 
-SYSCTL_INT(_debug_kassert, OID_AUTO, warnings, CTLFLAG_RWTUN,
+SYSCTL_INT(_debug_kassert, OID_AUTO, warnings, KASSERT_RWTUN,
     &kassert_warnings, 0, "number of KASSERTs that have been triggered");
 
-SYSCTL_INT(_debug_kassert, OID_AUTO, log_panic_at, CTLFLAG_RWTUN,
+SYSCTL_INT(_debug_kassert, OID_AUTO, log_panic_at, KASSERT_RWTUN,
     &kassert_log_panic_at, 0, "max number of KASSERTS before we will panic");
 
-SYSCTL_INT(_debug_kassert, OID_AUTO, log_pps_limit, CTLFLAG_RWTUN,
+SYSCTL_INT(_debug_kassert, OID_AUTO, log_pps_limit, KASSERT_RWTUN,
     &kassert_log_pps_limit, 0, "limit number of log messages per second");
 
-SYSCTL_INT(_debug_kassert, OID_AUTO, log_mute_at, CTLFLAG_RWTUN,
+SYSCTL_INT(_debug_kassert, OID_AUTO, log_mute_at, KASSERT_RWTUN,
     &kassert_log_mute_at, 0, "max number of KASSERTS to log");
 
-SYSCTL_INT(_debug_kassert, OID_AUTO, suppress_in_panic, CTLFLAG_RWTUN,
+SYSCTL_INT(_debug_kassert, OID_AUTO, suppress_in_panic, KASSERT_RWTUN,
     &kassert_suppress_in_panic, 0,
     "KASSERTs will be suppressed while handling a panic");
+#undef KASSERT_RWTUN
 
 static int kassert_sysctl_kassert(SYSCTL_HANDLER_ARGS);
 
@@ -709,6 +716,7 @@ kassert_sysctl_kassert(SYSCTL_HANDLER_ARGS)
 	return (0);
 }
 
+#ifdef KASSERT_PANIC_OPTIONAL
 /*
  * Called by KASSERT, this decides if we will panic
  * or if we will log via printf and/or ktr.
@@ -774,6 +782,7 @@ kassert_panic(const char *fmt, ...)
 #endif
 	atomic_add_int(&kassert_warnings, 1);
 }
+#endif /* KASSERT_PANIC_OPTIONAL */
 #endif
 
 /*

Modified: head/sys/sys/systm.h
==============================================================================
--- head/sys/sys/systm.h	Wed Aug 22 21:23:32 2018	(r338213)
+++ head/sys/sys/systm.h	Wed Aug 22 22:19:42 2018	(r338214)
@@ -80,9 +80,22 @@ extern int vm_guest;		/* Running as virtual machine gu
 enum VM_GUEST { VM_GUEST_NO = 0, VM_GUEST_VM, VM_GUEST_XEN, VM_GUEST_HV,
 		VM_GUEST_VMWARE, VM_GUEST_KVM, VM_GUEST_BHYVE, VM_LAST };
 
+/*
+ * These functions need to be declared before the KASSERT macro is invoked in
+ * !KASSERT_PANIC_OPTIONAL builds, so their declarations are sort of out of
+ * place compared to other function definitions in this header.  On the other
+ * hand, this header is a bit disorganized anyway.
+ */
+void	panic(const char *, ...) __dead2 __printflike(1, 2);
+void	vpanic(const char *, __va_list) __dead2 __printflike(1, 0);
+
 #if defined(WITNESS) || defined(INVARIANT_SUPPORT)
+#ifdef KASSERT_PANIC_OPTIONAL
 void	kassert_panic(const char *fmt, ...)  __printflike(1, 2);
+#else
+#define kassert_panic	panic
 #endif
+#endif
 
 #ifdef	INVARIANTS		/* The option is always available */
 #define	KASSERT(exp,msg) do {						\
@@ -212,9 +225,6 @@ void	*phashinit(int count, struct malloc_type *type, u
 void	*phashinit_flags(int count, struct malloc_type *type, u_long *nentries,
     int flags);
 void	g_waitidle(void);
-
-void	panic(const char *, ...) __dead2 __printflike(1, 2);
-void	vpanic(const char *, __va_list) __dead2 __printflike(1, 0);
 
 void	cpu_boot(int);
 void	cpu_flush_dcache(void *, size_t);



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201808222219.w7MMJhpl049457>