Re: [PATCH 12/14] FRV: Generate more useful debug info

Re: [PATCH 12/14] FRV: Generate more useful debug info

[Andi Kleen]

dhowells@redhat.com writes:

> diff -uNr /warthog/kernels/linux-2.6.10-rc1-bk10/Makefile linux-2.6.10-rc1-bk10-frv/Makefile
> --- /warthog/kernels/linux-2.6.10-rc1-bk10/Makefile	2004-11-01 11:45:20.000000000 +0000
> +++ linux-2.6.10-rc1-bk10-frv/Makefile	2004-11-01 11:48:36.397037723 +0000
> @@ -497,11 +497,18 @@
>  # Defaults vmlinux but it is usually overriden in the arch makefile
>  all: vmlinux
>  
> +
> +ifdef CONFIG_DEBUG_INFO
> +CFLAGS		+= -g -O1

Please don't do that. At least on i386/x86-64 we want the same
code with debug info as without. Otherwise how would you debug
a problem that only shows up at -O2. 

-Andi

P.S.: And it's quite unfriendly to put subscriber post only
mailing lists into cc of linux kernel threads. Dropped.

Re: [PATCH 12/14] FRV: Generate more useful debug info

[Chris Wedgwood]

On Wed, Nov 03, 2004 at 06:14:41AM +0100, Andi Kleen wrote:

> Please don't do that. At least on i386/x86-64 we want the same
> code with debug info as without. Otherwise how would you debug
> a problem that only shows up at -O2.

this is true for all architectures really (unless there is a platform
where gcc is perfect and completely bug free)

[PATCH 12/14] FRV: Generate more useful debug info

[dhowells]

The attached patch permits the generation of more useful debugging information
by reducing the optimisation level and by telling the assembler to produce
debug info too.

Signed-Off-By: dhowells@redhat.com
---
diffstat frv-debuginfo-2610rc1bk10.diff
 Makefile |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff -uNr /warthog/kernels/linux-2.6.10-rc1-bk10/Makefile linux-2.6.10-rc1-bk10-frv/Makefile
--- /warthog/kernels/linux-2.6.10-rc1-bk10/Makefile	2004-11-01 11:45:20.000000000 +0000
+++ linux-2.6.10-rc1-bk10-frv/Makefile	2004-11-01 11:48:36.397037723 +0000
@@ -497,11 +497,18 @@
 # Defaults vmlinux but it is usually overriden in the arch makefile
 all: vmlinux
 
+
+ifdef CONFIG_DEBUG_INFO
+CFLAGS		+= -g -O1
+AFLAGS		+= -Wa,--gdwarf2
+ASFLAGS		+= -Wa,--gdwarf2
+else
 ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE
 CFLAGS		+= -Os
 else
 CFLAGS		+= -O2
 endif
+endif
 
 #Add align options if CONFIG_CC_* is not equal to 0
 add-align = $(if $(filter-out 0,$($(1))),$(cc-option-align)$(2)=$($(1)))
@@ -516,10 +523,6 @@
 CFLAGS		+= -fomit-frame-pointer
 endif
 
-ifdef CONFIG_DEBUG_INFO
-CFLAGS		+= -g
-endif
-
 include $(srctree)/arch/$(ARCH)/Makefile
 
 # warn about C99 declaration after statement

Re: [PATCH 12/14] FRV: Generate more useful debug info

[Andrew Morton]

dhowells@redhat.com wrote:
>
> The attached patch permits the generation of more useful debugging information
> by reducing the optimisation level and by telling the assembler to produce
> debug info too.

Generates rejects against Sam's tree and appears to be unrelated to FRV,
yes?

I'd prefer that this be worked through Sam's tree as a separate enhancement
please.


+ifdef CONFIG_DEBUG_INFO
+CFLAGS		+= -g -O1
+AFLAGS		+= -Wa,--gdwarf2
+ASFLAGS		+= -Wa,--gdwarf2
+else

Are you sure that all architectures want this?  And that their toolchains
will continue to work correctly?  And that it doesn't break older gcc's and
that kgdb will continue to work correctly, etc?

I'm not...

Re: [PATCH 12/14] FRV: Generate more useful debug info

[David Howells]

> Generates rejects against Sam's tree and appears to be unrelated to FRV,
> yes?

I know not Sam's tree.

It's a generic thing. "gcc -g" does not cause compiled .S files to include
debugging information, and -O1 optimised code is more debuggable than -O2
optimised code.

> Are you sure that all architectures want this?  And that their toolchains
> will continue to work correctly?  And that it doesn't break older gcc's and
> that kgdb will continue to work correctly, etc?

-O1: Dunno; if they don't they're buggy, and if they don't they're buggy.

-Wa,--gdwarf2: No, no and no.

I can move the -Wa flag back into the frv arch if you insist, but moving the
-O1 flag there is more tricky, since it'd then be overridden in master
Makefile.

David

Re: [PATCH 12/14] FRV: Generate more useful debug info

[Linus Torvalds]

On Tue, 2 Nov 2004, David Howells wrote:
> 
> -O1: Dunno; if they don't they're buggy, and if they don't they're buggy.

No. It used to be that inlining was only done with -O2 if I remember 
correctly. The kernel _needed_ to be compiled with -O2.

That may not be true today, but what is true is that -O1 is not a light 
thing to just do.

		Linus 

Re: [PATCH 12/14] FRV: Generate more useful debug info

[Linus Torvalds]

On Tue, 2 Nov 2004, Linus Torvalds wrote:
> 
> That may not be true today, but what is true is that -O1 is not a light 
> thing to just do.

And btw, in some cases the inlining used to be a correcness issue, so no,
just making it be "static inline" doesn't necessarily fix the basic issue. 

Again, whether that is necessarily true today is not something I'm ready 
to guarantee one way or the other, so this is just an observation that 
things can be a lot more subtle than you seem to assume.

		Linus

Re: [PATCH 12/14] FRV: Generate more useful debug info

[Florian Weimer]

* Linus Torvalds:

>> That may not be true today, but what is true is that -O1 is not a light 
>> thing to just do.
>
> And btw, in some cases the inlining used to be a correcness issue, so no,
> just making it be "static inline" doesn't necessarily fix the basic issue. 

But the always_inline attribute hopefully does.

Re: [PATCH 12/14] FRV: Generate more useful debug info

[Linus Torvalds]

On Wed, 3 Nov 2004, Florian Weimer wrote:

> * Linus Torvalds:
> 
> >> That may not be true today, but what is true is that -O1 is not a light 
> >> thing to just do.
> >
> > And btw, in some cases the inlining used to be a correcness issue, so no,
> > just making it be "static inline" doesn't necessarily fix the basic issue. 
> 
> But the always_inline attribute hopefully does.

Not with older compilers. 

		Linus

Re: [PATCH 12/14] FRV: Generate more useful debug info

[David Howells]

> > That may not be true today, but what is true is that -O1 is not a light 
> > thing to just do.
> 
> And btw, in some cases the inlining used to be a correcness issue, so no,
> just making it be "static inline" doesn't necessarily fix the basic issue. 
> 
> Again, whether that is necessarily true today is not something I'm ready 
> to guarantee one way or the other, so this is just an observation that 
> things can be a lot more subtle than you seem to assume.

I've been able to run a range of 2.4 and 2.6 kernels compiled with -O1 and
without any -O flag at all, and with all "extern inline" changed to "static
inline". It doesn't seem to be a problem on i386, x86_64, frv (which I'm
trying to add) and am33 (which I haven't tried to add yet).

When debugging, -O2 makes for a real problem because, amongst other
inconvenient things, it enables the explicit parallelisation features that frv
cpus support.

Would you object to making it possible for the arch to override the
optimisation level when debugging?

David

Re: [PATCH 12/14] FRV: Generate more useful debug info

[Linus Torvalds]

On Wed, 3 Nov 2004, David Howells wrote:
> 
> I've been able to run a range of 2.4 and 2.6 kernels compiled with -O1 and
> without any -O flag at all, and with all "extern inline" changed to "static
> inline". It doesn't seem to be a problem on i386, x86_64, frv (which I'm
> trying to add) and am33 (which I haven't tried to add yet).

sparc64 was one of the things that definitely depended on it. Also, early
init/main.c depended on the stack not getting clobbered by the initial
fork, but that thankfully was cleaned up with the kernel threads
interfaces.

> Would you object to making it possible for the arch to override the
> optimisation level when debugging?

Quite frankly, I'd prefer developers to do it by just editing the 
Makefile, or doing it entirely statically for some architecture. 

For something like FRV in its current stages it simply doesn't _matter_,
but in architectures that get actual real usage, I absolutely hate the
idea of having different optimization options and wildly different code.
It just results in bugs being harder to reproduce.

And for architectures that don't have enough users to matter, arguably you 
shouldn't then need to have a config option that is visible to the rest of 
the world.

		Linus

Re: [PATCH 12/14] FRV: Generate more useful debug info

[Daniel Jacobowitz]

On Tue, Nov 02, 2004 at 11:21:36AM +0000, David Howells wrote:
> 
> > Generates rejects against Sam's tree and appears to be unrelated to FRV,
> > yes?
> 
> I know not Sam's tree.
> 
> It's a generic thing. "gcc -g" does not cause compiled .S files to include
> debugging information, and -O1 optimised code is more debuggable than -O2
> optimised code.

FYI, "gcc -g" _should_ cause .S files to include assembler debugging
information.  If it doesn't, that's a bug in your port.

*asm_debug:
%{gstabs*:--gstabs}%{!gstabs*:%{g*:--gdwarf2}}

-- 
Daniel Jacobowitz

Re: [PATCH 12/14] FRV: Generate more useful debug info

[David Howells]

> FYI, "gcc -g" _should_ cause .S files to include assembler debugging
> information.  If it doesn't, that's a bug in your port.

It appears you may have a point. Running FRV compiler with gcc -g on a .S file
does not pass any sort of -g flag to the assembler. It does with the i386
compiler.

David

Re: [PATCH 12/14] FRV: Generate more useful debug info

[Daniel Jacobowitz]

On Fri, Nov 12, 2004 at 03:15:18PM +0000, David Howells wrote:
> 
> > FYI, "gcc -g" _should_ cause .S files to include assembler debugging
> > information.  If it doesn't, that's a bug in your port.
> 
> It appears you may have a point. Running FRV compiler with gcc -g on a .S file
> does not pass any sort of -g flag to the assembler. It does with the i386
> compiler.

You're probably not selecting the right assembler to feature-test at
build time.  Check these:
     && defined(HAVE_AS_GDWARF2_DEBUG_FLAG) && defined(HAVE_AS_GSTABS_DEBUG_FLAG)

-- 
Daniel Jacobowitz