Date: Sat, 26 May 2018 10:09:57 +0200 From: Oliver Pinter <oliver.pinter@hardenedbsd.org> To: Poul-Henning Kamp <phk@phk.freebsd.dk> Cc: "arch@freebsd.org" <arch@freebsd.org> Subject: Re: To assert() or not to assert(), that is not really a question... Message-ID: <CAPQ4ffuvhTR61tNEERV4mrsTHTcin532GwLXMw6N_0JPB9o5Pw@mail.gmail.com> In-Reply-To: <4514.1527319154@critter.freebsd.dk> References: <4514.1527319154@critter.freebsd.dk>
next in thread | previous in thread | raw e-mail | index | archive | help
On Saturday, May 26, 2018, Poul-Henning Kamp <phk@phk.freebsd.dk> wrote: > When I started writing Varnish Cache, on of the first decision was > that I would pepper the source code with asserts even "pointless" > ones, and I did to the order of 10% of all source file lines, and > it is probably the best decision I have ever made. > > The primary effects of all the asserts is that we get crash-dumps > from the earliest possible point in time the trouble could be > spotted. For a process which can rutinely have several thousands > threads, that is what makes debugging possible in the first place. > > The secondary effect is that the sheer number of asserts means > that crashes can almost always be isolated to a very small stretch > of code based on the assert location. > > The one benefit from all the asserts I had not anticipated is that > they almost always prevent program bugs from turning into information > leakage vulnerabilities: We stop before we send wrong data out. > > The biggest impact of all the asserts however, is that Varnish Cache > went 11 years while moving a very large fraction of all HTTP traffic > on the net, without a major security issue. > > When we finally got our first big CVE, it was not a remote excution, > an information leak or anything horrible bad: It was a "harmless" > DoS caused by a wrong assert test, but a DoS is still pretty bad > news when so much traffic goes through Varnish. > > I think FreeBSD needs to learn from Varnish Cache's experience: > We should have far more asserts in FreeBSD. > > We already have a "private" assert facility in the kernel, and > people should simply use it more. This private assert facility exists in the kernel, but used only in a limited scope. It's only used in -CURRENT. It would be a really big step forward, if we were enable them for -STABLE branches too, because these beaches changes significantly too without enabled KASSERT. > > But in userland our asserts come from <assert.h>, and that is a > problem. > > The main trouble with using assert(3) is that random people > illadvisedly set NDEBUG expecting their code to run faster as a > result. > > It does not. > > Almost all the asserts in Varnish happens on values already in CPU > registers and/or cache and I have never been able to credibly measure > a performance impact from the asserts in Varnish. > > Besides, many of the asserts never make it to the CPU, modern > compilers have strong analysis which can see that the can never > trigger, so they are removed in optimization. > > We cannot change <assert.h> to get rid of the NDEBUG mistake, and > in a more abstract line of thought we probably should not even use > <assert.h> for FreeBSD source code - it sort of belongs to the users. > > I suggest and strongly urge that we add a set of userland assert-macros > which are never compiled out, for use *only* in FreeBSD userland > source code, and that we sprinkle them liberally, in particular > anything setuid etc. > > In Varnish Cache we have four "kinds" of asserts, which allows us > to communicate crucial information in the message to the users. I > don't know if a similar subdivision of asserts would make sense for > FreeBSD, but I mention it here as inspiration: > > 1. "Regular asserts" - things which are just plain wrong, which > probably means we have a genuine bug somewhere. Examples could > be null pointers where previous checks should have ensured this > not be so. Also error situations for which there is no saner > handling that killing the projcess. > > 2. "xxx asserts" - situations which should (almost) never happen, > and for which we could do more productive error handling, but > where the seldomness of the condition makes it a bad idea (ie: > untested code) and a bad investment of our developer time to do > so. Disk full is a good example for Varnish. > > 3. "wrong asserts" - Internal state is messed up, program flow > has taken a "impossible" branch. A good example is the > default branch of a switch on a finite input set. > > 4. "Incomplete asserts" - Code we have not written yet, extension > points not open for business yet, very strange corner-cases > belived to be impossible, but not proven to be so yet. > > Poul-Henning > > -- > Poul-Henning Kamp | UNIX since Zilog Zeus 3.20 > phk@FreeBSD.ORG | TCP/IP since RFC 956 > FreeBSD committer | BSD since 4.3-tahoe > Never attribute to malice what can adequately be explained by incompetence. > _______________________________________________ > freebsd-arch@freebsd.org mailing list > https://lists.freebsd.org/mailman/listinfo/freebsd-arch > To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org" >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAPQ4ffuvhTR61tNEERV4mrsTHTcin532GwLXMw6N_0JPB9o5Pw>