Re: [POWERPC] Fix for assembler -g

Re: [POWERPC] Fix for assembler -g

[Dave Jones]

On Sat, Aug 18, 2007 at 07:59:19PM +0000, Linux Kernel wrote:
 > Gitweb:     http://git.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=55a910a81d0c3014abc20b9efa73c595b3e68339
 > Commit:     55a910a81d0c3014abc20b9efa73c595b3e68339
 > Parent:     aa1cf632bd6f998cb4567ccf1a9d2e5daaa9fb44
 > Author:     Roland McGrath <roland@redhat.com>
 > AuthorDate: Sat Aug 11 09:03:11 2007 +1000
 > Committer:  Paul Mackerras <paulus@samba.org>
 > CommitDate: Wed Aug 15 15:12:50 2007 +1000
 > 
 >     [POWERPC] Fix for assembler -g
 >     
 >     ppc64 does the unusual thing of using #include on a compiler-generated
 >     assembly file (lparmap.s) from an assembly source file (head_64.S).
 >     This runs afoul of my recent patch to pass -gdwarf2 to the assembler
 >     under CONFIG_DEBUG_INFO.  This patch avoids the problem by disabling
 >     DWARF generation (-g0) when producing lparmap.s.
 >     
 >     Signed-off-by: Roland McGrath <roland@redhat.com>
 >     Signed-off-by: Paul Mackerras <paulus@samba.org>

I'm still seeing failures with this.
http://koji.fedoraproject.org/koji/getfile?taskID=110854&name=build.log
shows..

lparmap.c: Assembler messages:
lparmap.c:84: Error: file number 1 already allocated
make[1]: *** [arch/powerpc/kernel/head_64.o] Error 1

Anything I can provide to help diagnose this?

	Dave

-- 
http://www.codemonkey.org.uk

Re: [POWERPC] Fix for assembler -g

[Roland McGrath]

> I'm still seeing failures with this.
> http://koji.fedoraproject.org/koji/getfile?taskID=110854&name=build.log
> shows..
> 
> lparmap.c: Assembler messages:
> lparmap.c:84: Error: file number 1 already allocated
> make[1]: *** [arch/powerpc/kernel/head_64.o] Error 1
> 
> Anything I can provide to help diagnose this?

Hmm.  Check the V=1 make output to see that the lparmap.c really got the -g0.
The log says it didn't.  Oops!  It looks like the patch that got committed is
the one that sets CFLAGS_lparmap.s, but really it needs to set
CFLAGS_lparmap.o instead (go kbuild).  Did I post the wrong one?  (It's only
one letter different.)  Sorry about that!  (I committed the right one to
Fedora, which you'd think would help Dave, but no.)


Thanks,
Roland


--- linux-2.6/arch/powerpc/kernel/Makefile
+++ linux-2.6/arch/powerpc/kernel/Makefile
@@ -81,6 +81,7 @@ obj-y				+= iomap.o
 endif
 
 ifeq ($(CONFIG_PPC_ISERIES),y)
+CFLAGS_lparmap.o		+= -g0
 extra-y += lparmap.s
 $(obj)/head_64.o:	$(obj)/lparmap.s
 AFLAGS_head_64.o += -I$(obj)

Re: [POWERPC] Fix for assembler -g

[Dave Jones]

On Mon, Aug 20, 2007 at 01:18:18PM -0700, Roland McGrath wrote:
 > > I'm still seeing failures with this.
 > > http://koji.fedoraproject.org/koji/getfile?taskID=110854&name=build.log
 > > shows..
 > > 
 > > lparmap.c: Assembler messages:
 > > lparmap.c:84: Error: file number 1 already allocated
 > > make[1]: *** [arch/powerpc/kernel/head_64.o] Error 1
 > > 
 > > Anything I can provide to help diagnose this?
 > 
 > Hmm.  Check the V=1 make output to see that the lparmap.c really got the -g0.
 > The log says it didn't.  Oops!  It looks like the patch that got committed is
 > the one that sets CFLAGS_lparmap.s, but really it needs to set
 > CFLAGS_lparmap.o instead (go kbuild).  Did I post the wrong one?  (It's only
 > one letter different.)  Sorry about that!  (I committed the right one to
 > Fedora, which you'd think would help Dave, but no.)
 
Ahh, my eyes missed that the one upstream was subtley different, so I
dropped the one you committed to Fedora without trying it.
In a way though, good thing, or mainline would have continued to be
broken :)

	Dave

-- 
http://www.codemonkey.org.uk

Re: [POWERPC] Fix for assembler -g

[Segher Boessenkool]

> Hmm.  Check the V=1 make output to see that the lparmap.c really got 
> the -g0.
> The log says it didn't.  Oops!  It looks like the patch that got 
> committed is
> the one that sets CFLAGS_lparmap.s, but really it needs to set
> CFLAGS_lparmap.o instead (go kbuild).  Did I post the wrong one?  
> (It's only
> one letter different.)  Sorry about that!

But there is no lparmap.o!  lparmap.s is the generated file.

Maybe the best fix is

	$(obj)/lparmap.s: CFLAGS += -g0

This whole, erm, "cleverness" will go away soon fwiw.


Segher

Re: [POWERPC] Fix for assembler -g

[Segher Boessenkool]

> In a way though, good thing, or mainline would have continued to be
> broken :)

What do I need to do to see this bug, btw?  A special config
symbol perhaps?


Segher

Re: [POWERPC] Fix for assembler -g

[Roland McGrath]

> But there is no lparmap.o!  lparmap.s is the generated file.

Yeah, tell that to scripts/Makefile.lib:

	_c_flags       = $(CFLAGS) $(EXTRA_CFLAGS) $(CFLAGS_$(basetarget).o)

What would do what a person expects is $(CFLAGS_$(@F)), I think.

> Maybe the best fix is
> 
> 	$(obj)/lparmap.s: CFLAGS += -g0

Whatever works, I don't care.


Thanks,
Roland

Re: [POWERPC] Fix for assembler -g

[Roland McGrath]

> > In a way though, good thing, or mainline would have continued to be
> > broken :)
> 
> What do I need to do to see this bug, btw?  A special config
> symbol perhaps?

CONFIG_DEBUG_INFO=y
and pass-g-to-assembler-under-config_debug_info.patch from -mm


Thanks,
Roland

Re: [POWERPC] Fix for assembler -g

[Segher Boessenkool]

>>> In a way though, good thing, or mainline would have continued to be
>>> broken :)
>>
>> What do I need to do to see this bug, btw?  A special config
>> symbol perhaps?
>
> CONFIG_DEBUG_INFO=y
> and pass-g-to-assembler-under-config_debug_info.patch from -mm

Ah, an -mm patch.  Gotcha.

The fix most likely won't be necessary anymore for .24, since all
of lparmap.s will be gone -- see patch earlier today to linuxppc-dev.
Good riddance :-)


Segher

Re: [POWERPC] Fix for assembler -g

[Segher Boessenkool]

>> But there is no lparmap.o!  lparmap.s is the generated file.
>
> Yeah, tell that to scripts/Makefile.lib:
>
> 	_c_flags       = $(CFLAGS) $(EXTRA_CFLAGS) $(CFLAGS_$(basetarget).o)
>
> What would do what a person expects is $(CFLAGS_$(@F)), I think.

Looks good to me.  Sam?  We wanted to set CFLAGS_lparmap.s .


Segher

Re: [POWERPC] Fix for assembler -g

[Paul Mackerras]

Roland McGrath writes:

> Hmm.  Check the V=1 make output to see that the lparmap.c really got the -g0.
> The log says it didn't.  Oops!  It looks like the patch that got committed is
> the one that sets CFLAGS_lparmap.s, but really it needs to set
> CFLAGS_lparmap.o instead (go kbuild).  Did I post the wrong one?  (It's only
> one letter different.)  Sorry about that!  (I committed the right one to
> Fedora, which you'd think would help Dave, but no.)

Stephen Rothwell has posted a patch which makes this all moot by
eliminating the #include of the .s file.  I'm going to put that in my
tree shortly.

Paul.

Re: [POWERPC] Fix for assembler -g

[Sam Ravnborg]

On Tue, Aug 21, 2007 at 12:29:08AM +0200, Segher Boessenkool wrote:
> >>But there is no lparmap.o!  lparmap.s is the generated file.
> >
> >Yeah, tell that to scripts/Makefile.lib:
> >
> >	_c_flags       = $(CFLAGS) $(EXTRA_CFLAGS) $(CFLAGS_$(basetarget).o)
> >
> >What would do what a person expects is $(CFLAGS_$(@F)), I think.
> 
> Looks good to me.  Sam?  We wanted to set CFLAGS_lparmap.s .

To avoid confusion (in most cases) setting CFLAGS_file.o
does the expected thing in case on .o, .s, .lst and .i targets.
So the general and easy to remember rule is to set CFLAGS_file.o
and then kbuild takes care of the rest.

I assume you already did so and it solved your problem - no?

	Sam

Re: [POWERPC] Fix for assembler -g

[Segher Boessenkool]

>>>> But there is no lparmap.o!  lparmap.s is the generated file.
>>>
>>> Yeah, tell that to scripts/Makefile.lib:
>>>
>>> 	_c_flags       = $(CFLAGS) $(EXTRA_CFLAGS) $(CFLAGS_$(basetarget).o)
>>>
>>> What would do what a person expects is $(CFLAGS_$(@F)), I think.
>>
>> Looks good to me.  Sam?  We wanted to set CFLAGS_lparmap.s .
>
> To avoid confusion (in most cases) setting CFLAGS_file.o
> does the expected thing in case on .o, .s, .lst and .i targets.
> So the general and easy to remember rule is to set CFLAGS_file.o
> and then kbuild takes care of the rest.

Yeah, that makes sense in the "normal" case.  In this case, the
generated .s file is actually used in the build process though,
so it was a bit confusing.

> I assume you already did so and it solved your problem - no?

Sure, it was just a question "is this the right thing or not".
In any case, the problematic thing will be removed completely
here :-)

Thanks for the explanation, it all makes sense now,


Segher