Date: Wed, 29 Nov 2017 04:04:49 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Edward Tomasz Napierala <trasz@freebsd.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r326314 - in head/sys: ddb kern sys Message-ID: <20171129030035.W2283@besplex.bde.org> In-Reply-To: <201711281253.vASCrtlB071488@repo.freebsd.org> References: <201711281253.vASCrtlB071488@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 28 Nov 2017, Edward Tomasz Napierala wrote: > Log: > Make kdb_reenter() silent when explicitly called from db_error(). > This removes the useless backtrace on various ddb(4) user errors. > > Reviewed by: jhb@ > Obtained from: CheriBSD > MFC after: 2 weeks > Sponsored by: DARPA, AFRL > Differential Revision: https://reviews.freebsd.org/D13212 This doesn't fix the spam for user errors that cause a trap, which are very common (for mistyping memory addresses). ddb sets up trap handlers using setjmp, and actually does this correctly in many cases, but when a trap occurs the error handling is: trap -> kdb_reenter (print spam here) -> ddb (print error message here) The second error message is useful iff the trap handler was set up correctly (longjmp then goes back to a place that knows the precise context so it is easy to print a relevant error message). I forget if this error message is printed using db_error() which would repeat the spam, but it probably is in correct cases -- ddb should return to the main loop using longjmp, but it never calls longjmp directly. Instead it goes by db_error() to kdb which does the longjmp. So the correct error handling (still badly implemented) is now: setjmp in main loop setjmp in command, perhaps for memory access that might trap trap call kdb print spam (remove this for correctness) longjmp back to command back in command after longmp call db_error print good error message call kdb_reenter print spam (remove this for correctness) longjmp back to main loop back in main loop after longjmp IIRC. the jmp_buf for kdn_reenter() is global, so this must be switched to set up context-sensitive error handling. > Modified: head/sys/kern/subr_kdb.c > ============================================================================== > --- head/sys/kern/subr_kdb.c Tue Nov 28 11:06:17 2017 (r326313) > +++ head/sys/kern/subr_kdb.c Tue Nov 28 12:53:55 2017 (r326314) > @@ -509,6 +509,17 @@ kdb_reenter(void) > /* NOTREACHED */ > } > > +void > +kdb_reenter_silent(void) > +{ > + > + if (!kdb_active || kdb_jmpbufp == NULL) > + return; > + > + longjmp(kdb_jmpbufp, 1); > + /* NOTREACHED */ This duplicates style bugs (extra blank line and lint support that even lint doesn't need) from kdb_reenter(). I use the following fix of disabling the spam in all cases (unless reenabled in all cases using ddb): X Index: subr_kdb.c X =================================================================== X --- subr_kdb.c (revision 325403) X +++ subr_kdb.c (working copy) X @@ -1,3 +1,7 @@ X +volatile int bde_ipi = 0; X + X +static volatile int bde_kdb_wantspam = 0; X + X /*- X * Copyright (c) 2004 The FreeBSD Project X * All rights reserved. X @@ -501,7 +508,9 @@ X if (!kdb_active || kdb_jmpbufp == NULL) X return; X X +if (bde_kdb_wantspam & 1) X printf("KDB: reentering\n"); X +if (bde_kdb_wantspam & 2) X kdb_backtrace(); X longjmp(kdb_jmpbufp, 1); X /* NOTREACHED */ kib is the author of the verbose messages, and he didn't like just removing them. He agreed to putting them under a sysctl (or perhaps just a variable). Later, I found a case where the verbose messages are actually useful. I forget what this was. Probably some bug that I put in ddb. ddb is broken if it doesn't set up a special trap handler using setjmp() before doing anything that might trap. So we can detect most unexpected traps by checking the jmp_buf: - remove above complications and just longjmp() to the global jmp_buf in db_error(). This global jmp_buf is owned by ddb. Only the pointer to it is owned by kdb. So it is not a layering violationg to use it directly. - in kdb_reenter(), somehow determine if the jmp_buf is the top-level one. Traps are not expected in this case, so print the spam. Otherwise, assume that ddb set up jmp_buf for handling the trap and just longjmp(). to it. The spam also affects gdb in a worse way, and the above doesn't fix it. gdb also never calls longjmp() directly. It only calls kdb_reenter() directly after receiving ^C when switching back to ddb fails. Then it prints "using longjmp, hope it works!", but actually uses kdb_reenter() which prints spam and actually uses longjmp(). The spam goes to the console which is a logically different device to the gdb device, so might be invisible if the device is physical difference or mess up the gdb device if the devices are physically the same. The error messages have the same problem. It is the case where switching back to ddb doesn't work that we prefer to see the messages about this failure on the gdb device. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20171129030035.W2283>