Bug 27528 - [4.2/4.3 Regression] compiling linux kernels 2.6.16.14/15 2.6.17-rc3 on powerpc (7450) get error on long exixting code
Summary: [4.2/4.3 Regression] compiling linux kernels 2.6.16.14/15 2.6.17-rc3 on power...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 4.2.0
: P3 normal
Target Milestone: 4.2.0
Assignee: rsandifo@gcc.gnu.org
URL: http://gcc.gnu.org/ml/gcc-patches/200...
Keywords: patch
: 29611 (view as bug list)
Depends on:
Blocks:
 
Reported: 2006-05-10 02:43 UTC by Ray Malitzke
Modified: 2007-01-10 19:14 UTC (History)
4 users (show)

See Also:
Host:
Target: powerpc-*-*
Build:
Known to work: 3.4.6 4.1.2
Known to fail:
Last reconfirmed: 2006-11-03 13:30:24


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ray Malitzke 2006-05-10 02:43:53 UTC
"main_rmg" Invoking command

gcc -v -save-temps -m32 -Wp,-MD,init/.main.o.d  -nostdinc -isystem /usr/lib/gcc/powerpc-unknown-linux-gnu/4.2.0/include -D__KERNEL__ -Iinclude  -include include/linux/autoconf.h -Iarch/powerpc -Iarch/powerpc/include -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -O2 -fomit-frame-pointer -msoft-float -pipe -Iarch/powerpc -ffixed-r2 -mmultiple -mno-altivec -funit-at-a-time -mstring -mcpu=powerpc -Wa,-maltivec -Wdeclaration-after-statement -Wno-pointer-sign    -D"KBUILD_STR(s)=#s" -D"KBUILD_BASENAME=KBUILD_STR(main)"  -D"KBUILD_MODNAME=KBUILD_STR(main)" -c -o init/main.o init/main.c


"main_rmg.out"

gcc: warning: -pipe ignored because -save-temps specified
Using built-in specs.
Target: powerpc-unknown-linux-gnu
Configured with: ../gcc-4.2.0/configure --prefix=/usr --enable-version-specific-runtime-libs --with-java-home=/usr --infodir=/usr/share/gcc-42 --datadir=/usr/share/gcc-42 --mandir=/usr/share/gcc-42 --program-suffix=-42 --enable-shared --enable-gomp --enable-mudflap --enable-libgfortran --enable-threads=posix --enable-__cxa_atexit --enable-libgcc-math --disable-checking --disable-multilib --disable-nls --disable-werror --with-gnu-ld --with-mpfr-dir=/src/src/mpfr-2.2.0 --with-mpfr=/usr/lib --with-gmp-dir=/src/src/gmp-4.2 --with-gmp=/usr/lib --enable-languages=c,c++ --with-cpu=7450 --enable-clocale=gnu
Thread model: posix
gcc version 4.2.0 20060509 (experimental)
 /usr/libexec/gcc/powerpc-unknown-linux-gnu/4.2.0/cc1 -E -quiet -nostdinc -v -Iinclude -Iarch/powerpc -Iarch/powerpc/include -Iarch/powerpc -D__unix__ -D__gnu_linux__ -D__linux__ -Dunix -D__unix -Dlinux -D__linux -Asystem=linux -Asystem=unix -Asystem=posix -D__KERNEL__ -DKBUILD_STR(s)=#s -DKBUILD_BASENAME=KBUILD_STR(main) -DKBUILD_MODNAME=KBUILD_STR(main) -isystem /usr/lib/gcc/powerpc-unknown-linux-gnu/4.2.0/include -include include/linux/autoconf.h -MD init/.main.o.d init/main.c -m32 -msoft-float -mmultiple -mno-altivec -mstring -mcpu=powerpc -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs -Wdeclaration-after-statement -Wno-pointer-sign -fno-strict-aliasing -fno-common -fomit-frame-pointer -ffixed-r2 -funit-at-a-time -O2 -fpch-preprocess -o main.i
ignoring duplicate directory "arch/powerpc"
#include "..." search starts here:
#include <...> search starts here:
 include
 arch/powerpc
 arch/powerpc/include
 /usr/lib/gcc/powerpc-unknown-linux-gnu/4.2.0/include
End of search list.
 /usr/libexec/gcc/powerpc-unknown-linux-gnu/4.2.0/cc1 -fpreprocessed main.i -quiet -dumpbase main.c -m32 -msoft-float -mmultiple -mno-altivec -mstring -mcpu=powerpc -auxbase-strip init/main.o -O2 -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs -Wdeclaration-after-statement -Wno-pointer-sign -version -fno-strict-aliasing -fno-common -fomit-frame-pointer -ffixed-r2 -funit-at-a-time -o main.s
GNU C version 4.2.0 20060509 (experimental) (powerpc-unknown-linux-gnu)
	compiled by GNU C version 4.2.0 20060509 (experimental).
GGC heuristics: --param ggc-min-expand=81 --param ggc-min-heapsize=96676
Compiler executable checksum: 1ddc21b7b24d4a7d358320168400dad6
init/main.c: In function 'unknown_bootoption':
init/main.c:235: warning: asm operand 2 probably doesn't match constraints
init/main.c:235: error: impossible constraint in 'asm'


"from_main_i" I believthe rest of main.i adds little but could send it if requested.

static int __attribute__ ((__section__ (".init.text"))) unknown_bootoption(char *param, char *val)
{

 if (val) {

  if (val == param+strlen(param)+1)
   val[-1] = '=';
  else if (val == param+strlen(param)+2) {
   val[-2] = '=';
   memmove(val-1, val, strlen(val)+1);
   val--;
  } else
   do { __asm__ __volatile__( "1:	twi 31,0,0\n" ".section __bug_table,\"a\"\n" "\t"".long" " ""	1b,%0,%1,%2\n" ".previous" : : "i" (235), "i" ("init/main.c"), "i" ((__func__))); } while (0);
 }


 if (obsolete_checksetup(param))
  return 0;



"main_c.offending" producing "from_main_i"

/*
 * Unknown boot options get handed to init, unless they look like
 * failed parameters
 */
static int __init unknown_bootoption(char *param, char *val)
{
	/* Change NUL term back to "=", to make "param" the whole string. */
	if (val) {
		/* param=val or param="val"? */
		if (val == param+strlen(param)+1)
			val[-1] = '=';
		else if (val == param+strlen(param)+2) {
			val[-2] = '=';
			memmove(val-1, val, strlen(val)+1);
			val--;
		} else
			BUG();
	}



"powerpc_BUG_def"  defining BUG()

#define BUG() do {							 \
	__asm__ __volatile__(						 \
		"1:	twi 31,0,0\n"					 \
		".section __bug_table,\"a\"\n"				 \
		"\t"PPC_LONG"	1b,%0,%1,%2\n"				 \
		".previous"						 \
		: : "i" (__LINE__), "i" (__FILE__), "i" (__FUNCTION__)); \
} while (0)
Comment 1 Ray Malitzke 2006-05-10 03:04:56 UTC
There are similar problems with other kernel modules that did not occur before.
It looks like the asm expansion causes problems with some rs6000 work done by David Edelsohn. Will be glad to assist in solving this hopefully minor item.

Had a Hell of a time with the known to work fail fields. There should be a way to qualify current versus twoo weeks before
Comment 2 Andrew Pinski 2006-05-10 04:03:48 UTC
"i" (__FILE__), "i" (__FUNCTION__)

The second makes less sense but both of them are the cause.
Comment 3 Andrew Pinski 2006-05-10 05:24:04 UTC
Reduced testcase:
int f(void)
{
  asm ("%0" : : "i"("Hi"));
}


I don't think this is valid code:
`i'
An immediate integer operand (one with constant value) is allowed. This includes symbolic constants whose values will be known only at assembly time or later.

--------
Because "Hi" is not a symbolic constant really.  Also -funit-at-a-time is needed to reproduce this issue.

Also this is not related to any change David did.
Comment 4 Andrew Pinski 2006-05-10 05:30:51 UTC
This is not a bug and here is why:
Section anchors is turned on so that "Hi" is turned into a symbol offset of an anchor so it is no longer a symbolic constant.  This never really would have worked in anyways in the normal time.
Comment 5 Ray Malitzke 2006-05-10 14:43:48 UTC
To A Pinski
While I am _not_ a C lawyer, the following seems pertinent:

1 __FUNCTION__  is _not_ a predefined macro. However __func__ a predefined identifier and I will take this up with the kernel people. However, even changing__FUNCTION__ to __func__ still produces an error. Let the language lawyer sort this out.

2 Taking __FUNCTION__ entirely out of the original Macro Definition and using all of the kernel paraphernalia produces valid code. Out of that context I can not get even __FILE__ to work properly; only __line__

3 Your "Hi" misses the point  because it is certainly not a predefined macro and not even a predefined identifier. Therefore the comparison seems invalid to me.

I am reopening this because I believe that the raised by "__func__" should be addressed. Also it is not the first time that the kernel people found ways to get GCC closer to the standards.
Comment 6 David Edelsohn 2006-05-10 14:56:23 UTC
The section anchors feature does not like "__FUNCTION__" or "__func__" as an inlined asm argument.

Also, "some rs6000 work" is not very informative or useful.
Comment 7 Andrew Pinski 2006-05-10 15:06:11 UTC
(In reply to comment #5)
> To A Pinski
> While I am _not_ a C lawyer, the following seems pertinent:
Why do you think this is related to the C standard?

> 
> 1 __FUNCTION__  is _not_ a predefined macro. However __func__ a predefined
> identifier and I will take this up with the kernel people. However, even
> changing__FUNCTION__ to __func__ still produces an error. Let the language
> lawyer sort this out.

So what?

 
> 2 Taking __FUNCTION__ entirely out of the original Macro Definition and using
> all of the kernel paraphernalia produces valid code. Out of that context I can
> not get even __FILE__ to work properly; only __line__
> 
> 3 Your "Hi" misses the point  because it is certainly not a predefined macro
> and not even a predefined identifier. Therefore the comparison seems invalid to
> me.

HUH?  Look at how __FILE__ gets subsituted.
> 
> I am reopening this because I believe that the raised by "__func__" should be
> addressed. Also it is not the first time that the kernel people found ways to
> get GCC closer to the standards.

__func__ is a static variable and that is how the C standard defines it.  It will be put into the section anchor.
You will also have issues with:

static int t;

void f(void)
{
  asm ("" : : "I"(&t));
}

=====
But all of that is undefined as explained by how the documentation is defined.
Comment 8 Ray Malitzke 2006-05-10 20:17:34 UTC
Well Fellas: Either have the Steering Committee revise the
Invitation to participate in testing; quoted iselectively below.
Or,have a member from the Steering Committe ask me to refrain
from further participation. I, for one, am no longer willing
to be at the receiving end of snide comments from people who
can not admit that, volunteering or not, are talking through 
their hats.

I am over 70 years of age and have come to my expertise (which
is not in compilers) the hard way. I started with plugboards
and worked my way up to real time assembly systems programming
(telephone Central Office switches and dispatching systems).
The proof of any programming effort is for the hardware to
generate the right output to control other hardware or to
be comprehensible by human beings.   

Now to the specifics:

>  This page describes regular efforts to test GCC thoroughly,
>  plus ideas for additional testing by volunteers who have
>  machine cycles to spare.

You might not believe it, but I have a router-firewall, a 
MAC (dual 800 Mhz G4's), a Pentium 3 server (four 550 Mhz
Pentium 3 with 2Gbyte error correcting memory an 130 Giga 
byes of SCSI) assorted other machines all running with
software entirely compiled with gcc-4.1 and gcc-4.2. I only
report to gcc.gnu.org what I consider important problems.
In this specific case I just eliminated the __FUNCTION__
part from bug.h (asm-powerpc) and I am writing this with
kernel-2.6.17-rc3 as compiled by the current gcc-4.2 on
the MAC.
 
>  Perform regular builds and testing of current GCC sources
>  that are not already being reported regularly.

Compiling everything but Ada and running the full test
suite now takes 8.5 hours on a ~800 Mhz pentium 3. There
is realy no publicly available Ada source. I, personally
do not care for Java, but some source packages require
it and sun is apparently no coming out with new releases for 
the powerpc series (competition for server sales) I check
the test suite output to see if that particular gcc is to
be my current production compiler for either pentium3 or
powerpc.

>  If the operating system kernel you use is normally compiled
>  with GCC, try building it with the current sources;
>  such as a release branch, use the newly built kernel for
>  running further GCC tests.

I am a user of compilers (not only gcc) and not a compiler
builder. The four or five problems I reported (and caused 
changes in gcc) sofar were not evidenced by the test suite.
 
>  Build and test applications that are important to you;
>  investigate and report any problems you find.
  
>  Build and test packages that are normally available on
>  your platform and for which you have access to source.a

I have about 40 Gigabyte of source code (no games, no IRC,
no themes or their engines, no music or downloading software,
as little Java as possible, and no Pascal).  This is really
software for workstation use. 

Regarding __func__ C99 declares it a "predefined identifier"
and "Like keywords, predefined identifiers must no be
defined by programmers." I am not asking that it becomes
part of GCC. however it should made clear that it and 
certainly __FUNCTION__ are no longer supported.

Regarding __LINE__, __FILE__, __DATE__, etc are required
as per Standard C. Again, the GCC community can state
that it is no honoring that particular requiremnt. However,
the GCC comunity can not unilaterally abrogate that
requirement. The trigraphs come to mind. These are 
probably matters for the Steering Committee.
Comment 9 Andrew Pinski 2006-05-10 20:41:49 UTC
This has nothing to do with the C standard or even standard code.  This is inline-asm constraint which is failing and the inline-asm in the code is wrong.

This is a bug in the code so report it to Linux instead of here.
Comment 10 Hans-Peter Nilsson 2006-06-14 15:08:34 UTC
I can't help but thinking the code is valid and that this is a valid bug.
Arguably, it *might* be hard to fix, and we'll have to cop out and adjust the
documentation instead.
I mean, the "i" constraint purpose is documented as:
"this includes symbolic constants whose values will be known only at assembly
time or later."  In this case, "later" is link-time.  In some cases, including
variables, the "later" would be dynamic-link-time which will be a bit beyond
reach; but as long as GCC generates an internal label, it should work with no
warnings.  See also PR19708 and 17346.  If the C standard defines something
other than programmer sense for "symbolic constant", that isn't as interesting;
we're already in asm-land and are supposed to be useful, not pedantic.  Taking
the address of a constant string is useful...

And oh, I almost forgot to mention that I get the warning with 3.2 for
cris-axis-linux-gnu too.  Yes, in the kernel, for the same use. (Sigh.)
I'm a bit behind on gcc work so I don't know if I get it for 4.2 as well.
I assigned the PR to me to avoid the feeling of adding to peoples workload,
but I guess I'll have to hack trees and that might take a while.
Comment 11 Ray Malitzke 2006-06-15 03:03:52 UTC
Hans-Peter!

Thanks for shedding _some_ light on this murky corner. Perhaps, the "i" constraint is now really inapropriate. 

First of all, a kernel header appropriately replaces __FUNCTION__ with __func__. Therefore the leftover __FUNCTION__ (probably from non GCC compilers) is not an issue.
Second, __LINE__, __FILE__, and __func__ are all known to the compiler, because the compiler definitely knows what __FILE__ it is compiling and just emits the appropriate string. Same argument line pertains to the __LINE__ in the __FILE__ and the name of the __func__ (function) it is trying to compile. The whole idea
is akin to an "assert". Therefore it is not up to the linker or assembler.
Third, if putting __FILE__ in place of the of either __FUNCTION__ or __func__ it works fine as can be seen in the "-S option" output (e.g. file.s).
Fourth, and this might be mine and others misunderstanding, I had thought that "i" really referred to the assembly file address of the string. This address, naturally is an address subject to relocation by the linker.
To an _old_ assembly language programmer this appears just like a storm in a waterglass. This particular __asm construct is only used for powerpc (and perhaps ppc) It must be left as a holdover for the original AIX compiler or AIX binutils.

In my bugzilla PR to kernel.org I pointed out that knowing the __FILE__ and __LINE__ is really quite sufficient and that perhaps, according to my patch to  
asm-powerpc/bug.h the particular item could be dropped. See bugzilla.kernel.org PR 6533. They have not taken it up as it only pertains to gcc-4.2.0 (experimental) 
Comment 12 Hans-Peter Nilsson 2006-06-15 16:50:11 UTC
In reply to comment #11, "i" *is* an appropriate constraint, if any.
I see the problem with the reduced test-case in comment #3,
so I'm going to limit the scope of my involvement to fixing that.

Hopefully we can leave the discussion on macro expansion for now,
or at least any specific problems with e.g. __FUNCTION__ would be
subject to a separate PR. (Though it's interesting if it works but
__FILE__ doesn't.)

The address of a string constant is for all normal use expressed as a
local label, hence "symbolic constant" when it comes to assembly code.
As a GCC backend guy, I know that "i" is a constraint useful in a GCC
machine description when there's any constant symbol or label, possibly
offset by a numeric constant. (Constraints are used for both machine
descriptions and asms.)

Besides bug.h in asm-ppc and asm-ppc64, I see this construct for
asm-x86_64, asm-alpha, asm-ppc64, asm-i386 too.  Not as not an argument
for correctness, just an observation that contradicts the "only used with
powerpc" statement.  That is, except for use of __FUNCTION__.  Hm.

I think I need reconfirmation that we see the same problem:
Is the code in comment #3 really a reduced test-case, i.e.
does the code in comment #3 cause a warning for the compiler and options
(filename replaced) for which you originally reported the problem?

(If not, I'd like to know for what version it causes the same warning.)
Thanks for your patience.
Comment 13 Ray Malitzke 2006-06-15 22:58:57 UTC
Hans_Peter

Your, not mine, concern seems to be comment 3. For that you have to contact Pinski. I saw a number os inconsitencies in his comments and after the reception got did not want to pursue this further. My problem got solved as follows:

Typescript
for i in `find -name 'bug.h*'` ; do grep -e '\ "i"\ ' $i && echo $i ; done 

		: : "i" (__LINE__), "i" (__FILE__)); \
		: : "r" ((long)(x)), "i" (__LINE__),		\
		    "i" (__FILE__));	\
		    "i" (__LINE__ + BUG_WARNING_TRAP),		\
		    "i" (__FILE__));	\
./asm-powerpc/bug.h


		: : "i" (__LINE__), "i" (__FILE__), "i" (__FUNCTION__)); \
		: : "r" ((long)(x)), "i" (__LINE__),		\
		    "i" (__FILE__), "i" (__FUNCTION__));	\
		    "i" (__LINE__ + BUG_WARNING_TRAP),		\
		    "i" (__FILE__), "i" (__FUNCTION__));	\
./asm-powerpc/bug.h.org


		     "i"(__LINE__), "i" (__FILE__))
./asm-x86_64/bug.h


		       : : "i" (PAL_bugchk), "i"(__LINE__), "i"(__FILE__))
./asm-alpha/bug.h


			 : : "i" (__LINE__), "i" (__FILE__))
./asm-i386/bug.h


	__asm__ __volatile__("break %0" : : "i" (BRK_BUG));		\
./asm-mips/bug.h


The pertinent patch is on file in my PR to bugzilla.kernel.org.

The problem turned up a week or so before I filed PR 27528, and after considerable rs6000 changes made by David Edelsohn. 

To me this is a matter for the GCC community to solve and not for a non compiler expert like myself. I am just a user as defined by the steering committee.

Comment 14 Andrew Pinski 2006-06-15 23:03:41 UTC
Subject: Re:  compiling linux kernels 2.6.16.14/15 2.6.17-rc3 on powerpc (7450) get error on long exixting code

> The problem turned up a week or so before I filed PR 27528, and after
> considerable rs6000 changes made by David Edelsohn. 

The change just exposed a latent bug. I am trying to make sure that
you understand that and not blaming David.

Could someone please CC Richard Sandiford, if he is not already since
-fsection-anchors is his creation.

-- Pinski
Comment 15 Andrew Pinski 2006-10-27 05:23:51 UTC
*** Bug 29611 has been marked as a duplicate of this bug. ***
Comment 16 rsandifo@gcc.gnu.org 2006-11-03 13:23:32 UTC
I'm testing a possible patch.  I didn't want to reassign the bug
to myself in case H-P is still looking at it too.

At this stage I have no idea how successful the patch will be.
It seems to me that, as with most other gcc extensions, the "right"
behaviour isn't really defined here.  The warning and error don't
seem unreasonable from a gcc hacker's perspective, but I can see why
users would expect it to work.  My main fear is that, even if the
patch survives the normal testing (and a kernel build, which I
intend to try too), it could easily break another unstated
assumption in other user code.

Richard
Comment 17 Hans-Peter Nilsson 2006-11-03 13:27:42 UTC
(Re: comment #16)
Thank you, be my guest! :-)
Actually, it's a mistake that I'm still assigned; I should've unassigned myself
after a week with no activity from my side.
Comment 18 rsandifo@gcc.gnu.org 2006-11-03 13:30:24 UTC
OK, I step up to the guillotine...
Comment 19 rsandifo@gcc.gnu.org 2006-11-09 09:34:43 UTC
Patch posted for review.
Comment 20 rsandifo@gcc.gnu.org 2006-11-11 09:47:44 UTC
Subject: Bug 27528

Author: rsandifo
Date: Sat Nov 11 09:47:35 2006
New Revision: 118689

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=118689
Log:
gcc/
	PR middle-end/27528
	* stmt.c (expand_asm_operands): Use EXPAND_INITIALIZER if the
	constraints accept neither registers or memories.

gcc/testsuite/
	PR middle-end/27528
	* gcc.c-torture/compile/pr27528.c: New test.
	* gcc.dg/pr27528.c: Likewise.

Added:
    trunk/gcc/testsuite/gcc.c-torture/compile/pr27528.c
    trunk/gcc/testsuite/gcc.dg/pr27528.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/stmt.c
    trunk/gcc/testsuite/ChangeLog

Comment 21 rsandifo@gcc.gnu.org 2006-11-11 09:53:29 UTC
Subject: Bug 27528

Author: rsandifo
Date: Sat Nov 11 09:53:20 2006
New Revision: 118690

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=118690
Log:
gcc/
	PR middle-end/27528
	* stmt.c (expand_asm_operands): Use EXPAND_INITIALIZER if the
	constraints accept neither registers or memories.

gcc/testsuite/
	PR middle-end/27528
	* gcc.c-torture/compile/pr27528.c: New test.
	* gcc.dg/pr27528.c: Likewise.

Added:
    branches/gcc-4_2-branch/gcc/testsuite/gcc.c-torture/compile/pr27528.c
    branches/gcc-4_2-branch/gcc/testsuite/gcc.dg/pr27528.c
Modified:
    branches/gcc-4_2-branch/gcc/ChangeLog
    branches/gcc-4_2-branch/gcc/stmt.c
    branches/gcc-4_2-branch/gcc/testsuite/ChangeLog

Comment 22 rsandifo@gcc.gnu.org 2006-11-11 09:54:27 UTC
Patch applied to trunk and 4.2.