Re: svn commit: r213744 - head/bin/sh

2010-10-13 Thread John Baldwin
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

2010-10-13 Thread David O'Brien
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

2010-10-13 Thread David O'Brien
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

2010-10-12 Thread David E. O'Brien
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

2010-10-12 Thread Bruce Evans

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

2010-10-12 Thread David O'Brien
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

2010-10-12 Thread Bruce Evans

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