Re: svn commit: r213744 - head/bin/sh
On Wednesday, October 13, 2010 12:29:27 am Bruce Evans wrote: On Tue, 12 Oct 2010, David O'Brien wrote: On Wed, Oct 13, 2010 at 02:18:33PM +1100, Bruce Evans wrote: On Tue, 12 Oct 2010, David E. O'Brien wrote: Log: If DEBUG is 3 or greater, disable STATICization of functions. Also correct the documented location of the trace file. Private functions should always be static, which no `#define STATIC static' [..] In theory, the debugging info should make it possible for debuggers to restore the semantics of not-explictly-inline functions by virtualizing them, but gdb's debugging info and/or gdb are too primitive to do this (gdb doesn't allow putting a breakpoint at a deleted static function, This is actually what my motivation is -- trying to set breakpoints and finding GDB was unable to. Of course, debugging and profiling are magic, but I don't want to have to adorn all functions with STATICs and __attributes() (and pragmas for othercc...) to recover historical/normal or variant debugging or profiling of them. I agree, and would not add STATIC's to a program's code that didn't already have them. But in this case we inherited it from 4.4BSD. I'm just making it actually do something other than being a gratuitous spelling change. I believe I've made things more consistent with r213760. Add __noinline or whatever attributes to STATIC (but keep static in it) for the DEBUG = 3 case if you are going that far. __noinline should be a syntax error for variables, so this should also find any STATICs still on variables. The spelling fix of changing STATIC to what it actually means (ASSORTED_HACKS_FOR_DEBUGGING_BUT_NOW_ONLY_FOR_FUNCTIONS) goes too far for me. To be honest, I think changing STATIC is excessive churn. The right fix is to use 'make DEBUG_FLAGS=-g -DDEBUG=2 -fno-inline'. I often use 'make DEBUG_FLAGS=-g -fno-inline' to workaround excessive inlining when debugging. I think all of the STATIC changes should just be backed out. -- John Baldwin ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r213744 - head/bin/sh
On Wed, Oct 13, 2010 at 03:29:27PM +1100, Bruce Evans wrote: Add __noinline or whatever attributes to STATIC (but keep static in it) for the DEBUG = 3 case if you are going that far. __noinline should be a syntax error for variables, so this should also find any STATICs still on variables. The spelling fix of changing STATIC to what it actually means (ASSORTED_HACKS_FOR_DEBUGGING_BUT_NOW_ONLY_FOR_FUNCTIONS) goes too far for me. I've make all uses static. Otherwise we have some of the functions and variable STATIC (if from 4.4BSD), and some others static (added later by FreeBSD). Surely this inconsistency isn't desired. -- -- David (obr...@freebsd.org) ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r213744 - head/bin/sh
On Wed, Oct 13, 2010 at 09:07:59AM -0400, John Baldwin wrote: To be honest, I think changing STATIC is excessive churn. At this point I'm just making everything consistently static so that folks that add to ash won't need to realize that STATIC exists. Some of the folks making various FreeBSD changes did not realize this and created inconsistencies. Surely this inconsistency isn't desired. The right fix is to use 'make DEBUG_FLAGS=-g -DDEBUG=2 -fno-inline'. I often use 'make DEBUG_FLAGS=-g -fno-inline' to workaround excessive inlining when I've added -fno-inline to DEBUG_FLAGS and will leave it up to others if that spelling isn't accepted by clang, Intel C, or what ever compiler someone may want to use. -- -- David (obr...@freebsd.org) ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
svn commit: r213744 - head/bin/sh
Author: obrien Date: Tue Oct 12 19:24:41 2010 New Revision: 213744 URL: http://svn.freebsd.org/changeset/base/213744 Log: If DEBUG is 3 or greater, disable STATICization of functions. Also correct the documented location of the trace file. Modified: head/bin/sh/Makefile head/bin/sh/shell.h head/bin/sh/show.c Modified: head/bin/sh/Makefile == --- head/bin/sh/MakefileTue Oct 12 19:24:29 2010(r213743) +++ head/bin/sh/MakefileTue Oct 12 19:24:41 2010(r213744) @@ -21,7 +21,7 @@ LDADD= -ll -ledit -ltermcap LFLAGS= -8 # 8-bit lex scanner for arithmetic CFLAGS+=-DSHELL -I. -I${.CURDIR} # for debug: -# CFLAGS+= -g -DDEBUG=2 +# CFLAGS+= -g -DDEBUG=3 WARNS?=2 WFORMAT=0 Modified: head/bin/sh/shell.h == --- head/bin/sh/shell.h Tue Oct 12 19:24:29 2010(r213743) +++ head/bin/sh/shell.h Tue Oct 12 19:24:41 2010(r213744) @@ -43,8 +43,9 @@ * JOBS - 1 if you have Berkeley job control, 0 otherwise. * define DEBUG=1 to compile in debugging (set global debug to turn on) * define DEBUG=2 to compile in and turn on debugging. + * define DEBUG=3 to also build all functions as public * - * When debugging is on, debugging info will be written to $HOME/trace and + * When debugging is on, debugging info will be written to ./trace and * a quit signal will generate a core dump. */ @@ -61,7 +62,11 @@ typedef intmax_t arith_t; #definestrtoarith_t(nptr, endptr, base) strtoimax(nptr, endptr, base) typedef void *pointer; +#if DEBUG = 3 +#define STATIC +#else #define STATIC static +#endif #define MKINIT /* empty */ #include sys/cdefs.h Modified: head/bin/sh/show.c == --- head/bin/sh/show.c Tue Oct 12 19:24:29 2010(r213743) +++ head/bin/sh/show.c Tue Oct 12 19:24:41 2010(r213744) @@ -274,7 +274,7 @@ indent(int amount, char *pfx, FILE *fp) FILE *tracefile; -#if DEBUG == 2 +#if DEBUG = 2 int debug = 1; #else int debug = 0; ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r213744 - head/bin/sh
On Tue, 12 Oct 2010, David E. O'Brien wrote: Log: If DEBUG is 3 or greater, disable STATICization of functions. Also correct the documented location of the trace file. Private functions should always be static, which no `#define STATIC static' hack to control this, but there are compiler bugs that result in them being inlined too often. DEBUG=3 also disables staticization of many or all static variables, since STATIC is used for both functions and variables. Variable names might more reasonably be not unique. Modified: head/bin/sh/Makefile == --- head/bin/sh/MakefileTue Oct 12 19:24:29 2010(r213743) +++ head/bin/sh/MakefileTue Oct 12 19:24:41 2010(r213744) @@ -21,7 +21,7 @@ LDADD= -ll -ledit -ltermcap LFLAGS= -8 # 8-bit lex scanner for arithmetic CFLAGS+=-DSHELL -I. -I${.CURDIR} # for debug: -# CFLAGS+= -g -DDEBUG=2 +# CFLAGS+= -g -DDEBUG=3 WARNS?= 2 WFORMAT=0 -O2 and perhaps even -O now does excessive inlining (due to it implying -funit-at-a-time -finline-functions-call-once). This gets in the way of debugging even more than the broken default of -O2 , even with -g. (OTOH, -g is supposed to not change the object code, so it shouldn't undo parts of -O2.) In theory, the debugging info should make it possible for debuggers to restore the semantics of not-explictly-inline functions by virtualizing them, but gdb's debugging info and/or gdb are too primitive to do this (gdb doesn't allow putting a breakpoint at a deleted static function, and at least in FreeBSD, at least on amd64 and i386, gdb makes a mess of even stepping over an explicit inline function -- it doesn't even display the source code for lines that call an inline function (this is even worse than for macros), and thus it doesn't even give a chance of stepping over an inline function using 'n' -- stepping stops in the inline function and displays its lines (except for nested inlines -- then it only displays the leaf lines) (this is better than for macros where you can't see the internals). These bugs are larger for the kernel with primitive instruction-level debuggers like ddb and primitive backtracers that don't understand the debugging info. It can be very hard to see where you are in a large function comprised of other large functions that were inlined just because they are only called once, especially after -O2 reorders everything. These bugs are larger when the inlines are not explicit. You may have made a function separate just for easier debugging, or to get separate profiling info for it... Of course, debugging and profiling are magic, but I don't want to have to adorn all functions with STATICs and __attributes() (and pragmas for othercc...) to recover historical/normal or variant debugging or profiling of them. This already stopped me from adding attributes to inline functions in kernel headers. Not inlining them would be usefulfor re-profiling them to see if they really should be inline, but the control structure for this would be ugly. Bruce ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r213744 - head/bin/sh
On Wed, Oct 13, 2010 at 02:18:33PM +1100, Bruce Evans wrote: On Tue, 12 Oct 2010, David E. O'Brien wrote: Log: If DEBUG is 3 or greater, disable STATICization of functions. Also correct the documented location of the trace file. Private functions should always be static, which no `#define STATIC static' [..] In theory, the debugging info should make it possible for debuggers to restore the semantics of not-explictly-inline functions by virtualizing them, but gdb's debugging info and/or gdb are too primitive to do this (gdb doesn't allow putting a breakpoint at a deleted static function, This is actually what my motivation is -- trying to set breakpoints and finding GDB was unable to. Of course, debugging and profiling are magic, but I don't want to have to adorn all functions with STATICs and __attributes() (and pragmas for othercc...) to recover historical/normal or variant debugging or profiling of them. I agree, and would not add STATIC's to a program's code that didn't already have them. But in this case we inherited it from 4.4BSD. I'm just making it actually do something other than being a gratuitous spelling change. I believe I've made things more consistent with r213760. -- -- David (obr...@freebsd.org) ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r213744 - head/bin/sh
On Tue, 12 Oct 2010, David O'Brien wrote: On Wed, Oct 13, 2010 at 02:18:33PM +1100, Bruce Evans wrote: On Tue, 12 Oct 2010, David E. O'Brien wrote: Log: If DEBUG is 3 or greater, disable STATICization of functions. Also correct the documented location of the trace file. Private functions should always be static, which no `#define STATIC static' [..] In theory, the debugging info should make it possible for debuggers to restore the semantics of not-explictly-inline functions by virtualizing them, but gdb's debugging info and/or gdb are too primitive to do this (gdb doesn't allow putting a breakpoint at a deleted static function, This is actually what my motivation is -- trying to set breakpoints and finding GDB was unable to. Of course, debugging and profiling are magic, but I don't want to have to adorn all functions with STATICs and __attributes() (and pragmas for othercc...) to recover historical/normal or variant debugging or profiling of them. I agree, and would not add STATIC's to a program's code that didn't already have them. But in this case we inherited it from 4.4BSD. I'm just making it actually do something other than being a gratuitous spelling change. I believe I've made things more consistent with r213760. Add __noinline or whatever attributes to STATIC (but keep static in it) for the DEBUG = 3 case if you are going that far. __noinline should be a syntax error for variables, so this should also find any STATICs still on variables. The spelling fix of changing STATIC to what it actually means (ASSORTED_HACKS_FOR_DEBUGGING_BUT_NOW_ONLY_FOR_FUNCTIONS) goes too far for me. Bruce ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org