2007-08-20 20:09:23

by Dave Jones

[permalink] [raw]
Subject: Re: [POWERPC] Fix for assembler -g

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 <[email protected]>
> AuthorDate: Sat Aug 11 09:03:11 2007 +1000
> Committer: Paul Mackerras <[email protected]>
> 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 <[email protected]>
> Signed-off-by: Paul Mackerras <[email protected]>

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


2007-08-20 20:18:48

by Roland McGrath

[permalink] [raw]
Subject: Re: [POWERPC] Fix for assembler -g

> 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)


2007-08-20 20:28:07

by Dave Jones

[permalink] [raw]
Subject: Re: [POWERPC] Fix for assembler -g

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

2007-08-20 21:28:51

by Segher Boessenkool

[permalink] [raw]
Subject: Re: [POWERPC] Fix for assembler -g

> 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

2007-08-20 21:30:10

by Segher Boessenkool

[permalink] [raw]
Subject: Re: [POWERPC] Fix for assembler -g

> 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

2007-08-20 21:33:17

by Roland McGrath

[permalink] [raw]
Subject: Re: [POWERPC] Fix for assembler -g

> 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

2007-08-20 21:36:36

by Roland McGrath

[permalink] [raw]
Subject: Re: [POWERPC] Fix for assembler -g

> > 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

2007-08-20 22:26:56

by Segher Boessenkool

[permalink] [raw]
Subject: Re: [POWERPC] Fix for assembler -g

>>> 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

2007-08-20 22:29:27

by Segher Boessenkool

[permalink] [raw]
Subject: Re: [POWERPC] Fix for assembler -g

>> 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

2007-08-20 23:15:11

by Paul Mackerras

[permalink] [raw]
Subject: Re: [POWERPC] Fix for assembler -g

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.

2007-08-25 13:01:00

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [POWERPC] Fix for assembler -g

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

2007-08-25 13:57:06

by Segher Boessenkool

[permalink] [raw]
Subject: Re: [POWERPC] Fix for assembler -g

>>>> 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