Bug 16506

I get no such warnings or errors.
Did you download the 2.6.35 tarball file?
Maybe it is corrupted (but unzipping should have caught that).
Or maybe the source files themselves are mangled.

Can you try it again?  Download a new tarball file and unzip it,
rebuild it?

Redownloaded the tarball (.bz2 version).  It compared identically (via "diff") to what I already had.  Re-untarred it anyway (no errors).  I still get the same compile-time errors.

I was able to do everything else I normally do (compile and install modules, and install updated kernel headers).  However, this still blocks creation of an installable bootable image (bzImage or vmlinuz).

I'm not familiar with the assembler macro language used, but one possibility that occurs is:  Is macro nesting (one defined inside another) permitted?  If not, that would explain the error.  The assembler version I have:

GNU assembler version 2.15.92.0.2 (i486-slackware-linux) using BFD version 2.15.92.0.2 20040927

Binutils 2.12 is indicated as the minimum version supported per Documentation/Changes.

I messed with various things for a couple of hours today and found a path that finally allowed me to compile without error, but I still get the warnings.

Upgraded binutils to the most current release (2.20.51.0.10), and the assembler error disappeared.  Therefore, file "Documentation/Changes" needs to be updated to a new minimum version for binutils as 2.15 didn't cut it, so 2.12, currently listed, definently won't.  DOCUMENTATION UPDATE NEEDED.

I'm leaving the bug report open, but changing to P5 (enhancement priority) to address the excessively repeated warning that is also reported (file "cpufeature.h" line 315 really needs to be fixed).  I have not yet run the new kernel.

What version of gcc are you using, too?

On 08/05/2010 02:01 PM, bugzilla-daemon@bugzilla.kernel.org wrote:
> https://bugzilla.kernel.org/show_bug.cgi?id=16506
> 
> 
> H. Peter Anvin <hpa@zytor.com> changed:
> 
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>                  CC|                            |hpa@zytor.com
> 
> 
> 
> 
> --- Comment #4 from H. Peter Anvin <hpa@zytor.com>  2010-08-05 21:01:11 ---
> What version of gcc are you using, too?
> 

Luca,

This is your code (atomic64_386_32.S).  This is confirmed to break with
binutils 2.14.90, which is recent enough that we *might* still care
about it.  In particular, this is the $v stuff which really seems
somewhat unnecessary to me (you pass it in and out to a bunch of places
which then don't use it... other than that it seems just like a
superfluous shorthand, although it does somewhat enhance readability.

The earliest gcc we still support, at least for x86, is 3.4, which came
out when 2.14.90 was still current (although 2.15 came out just
after)... so I'm not entirely opposed to bumping the binutils
requirement, but to do it for this particular code seems a bit weak.

	-hpa

Created attachment 27359 [details]
Patch that refactors the code, making it work on binutils 2.12

It seems that the problem is that old versions of binutils don't like setting a symbol to a register.

I refactored the code to use the C preprocessor instead, and I believe also made it slightly clearer (it's now obvious what "v" is, and the ending macros now have more intuitive names).

The binary is unchanged, and it assembles with binutils 2.12.

Please let me know whether the patch is fine.

Sorry, patch is missing the signoff: just add the obvious
Signed-off-by: Luca Barbieri <luca@luca-barbieri.com>

By the way, I'm not sure it's a good idea to support such old binutils.

For instance, requiring a recent binutils would allow to remove the handrolled assembler in arch/x86/include/asm/inst.h for AES-NI and others.

In this case, however, I believe my patch makes the code slightly more readable anyway, and so would be desirable even if binutils wasn't an issue.

Created attachment 27360 [details]
Add signoff and cosmetic changes to previous patch

New version of the patch with the signoff and minor whitespace cosmetic improvements.

Created attachment 27376 [details]
Kernel config that cause error

This error is also present on binutils-2.16.1 (or possible on even some newer versions).
After patching - there is other fault:

  AS      arch/x86/lib/atomic64_386_32.o
arch/x86/lib/atomic64_386_32.S: Assembler messages:
arch/x86/lib/atomic64_386_32.S:54: Error: Macro with this name was already defined
arch/x86/lib/atomic64_386_32.S:61: Error: Macro with this name was already defined
arch/x86/lib/atomic64_386_32.S:70: Error: Macro with this name was already defined
arch/x86/lib/atomic64_386_32.S:77: Error: Macro with this name was already defined
arch/x86/lib/atomic64_386_32.S:86: Error: Macro with this name was already defined
arch/x86/lib/atomic64_386_32.S:93: Error: Macro with this name was already defined
arch/x86/lib/atomic64_386_32.S:105: Error: Macro with this name was already defined
arch/x86/lib/atomic64_386_32.S:112: Error: Macro with this name was already defined
arch/x86/lib/atomic64_386_32.S:123: Error: Macro with this name was already defined
arch/x86/lib/atomic64_386_32.S:130: Error: Macro with this name was already defined
arch/x86/lib/atomic64_386_32.S:141: Error: Macro with this name was already defined
arch/x86/lib/atomic64_386_32.S:163: Error: Macro with this name was already defined
arch/x86/lib/atomic64_386_32.S:184: Error: Macro with this name was already defined

For i686 - all is built OK.

Created attachment 27377 [details]
New version that fixes reported issue with binutils 2.16

Right, it doesn't work with binutils 2.16.

And if I define the macro with a lowercase name instead of an uppercase one, the problem magically disappears (no idea why, anyone knows?).

Here is a patch that does that and assembled with my built copy of binutils 2.16

Thanks. Now all is OK.

Please note there are still warnings that need to be dealt with too.
Only the fatal compile error was fixed so far.

The patch was merged in mainline (2.6.36-rc1) and marked for stable. 

commit 30246557a06bb20618bed906a06d1e1e0faa8bb4
Author: Luca Barbieri <luca@luca-barbieri.com>
Date:   Fri Aug 6 04:04:38 2010 +0200

    x86, asm: Refactor atomic64_386_32.S to support old binutils and be cleaner
    
    The old code didn't work on binutils 2.12 because setting a symbol to
    a register apparently requires a fairly recent version.
    
    This commit refactors the code to use the C preprocessor instead, and
    in the process makes the whole code a bit easier to understand.
    
    The object code produced is unchanged as expected.
    
    This fixes kernel bugzilla 16506.
    
    Reported-by: Dieter Stussy <kd6lvw+software@kd6lvw.ampr.org>
    Signed-off-by: Luca Barbieri <luca@luca-barbieri.com>
    Signed-off-by: H. Peter Anvin <hpa@zytor.com>
    Cc: <stable@kernel.org> 2.6.35
    LKML-Reference: <tip-*@git.kernel.org>

Sorry, I overlooked your comment about the warnings. 

Do they happen also on soon-to-be 2.6.36? 

This one changes some macro names and could fix the warnings:

commit 417484d47e115774745ef025bce712a102b6f86f
Author: Luca Barbieri <luca@luca-barbieri.com>
Date:   Thu Aug 12 07:00:35 2010 -0700

    x86, asm: Use a lower case name for the end macro in atomic64_386_32.S
    
    Use a lowercase name for the end macro, which somehow fixes a binutils 2.16
    problem.
    
    Signed-off-by: Luca Barbieri <luca@luca-barbieri.com>
    LKML-Reference: <tip-30246557a06bb20618bed906a06d1e1e0faa8bb4@git.kernel.org>
    Signed-off-by: H. Peter Anvin <hpa@zytor.com>


If not, please shout.

I don't actively participate in kernel development, so I don't test candidate kernels.  However, the warnings are still present in 2.6.35.4 which has this patch applied to fix the error.  Since the cpufeature.h:315 warning appears for about 75% of the kernel files, it is damn annoying.  The patch committed so far doesn't even touch the files where any of the warnings appear.  The warnings at processor.h:506 and processor.h:533 are probably the same bad coding style as the cpufeature.h since the warning message is the same.

(Yes, I still see the warnings with 2.6.35.4).

Now I know what warnings you were talking about, sorry. 

I'm reopening the bugreport. Mabye someone of the maintainers can comment on whether and when these warnings are going to be fixed.

Btw, can you attach a compressed copy of the build log? 

I've seen fixes to some of your warnings above on lkml. 

But if the excessive warnings don't have a single cause and instead are just differences in what earlier gcc versions warned about and what modern gcc is warning about, new warnings are likely to pop up while squashing the old warnings. ( hydra-like )

If each and every of your warning has another cause, it is unlikely that we will be able to ever fix that in this bug report.

arch/x86/include/asm/cpufeature.h is included (whether directly or indirectly) in many files.  It's the SAME WARNING REPEATED FOR ALMOST EVERY FILE that is compiled via gcc due to inclusion.

Thanks for highlighting the important part. But note that UPPERCASE is often considered shouting. 

Sadly, I can't reproduce these warnings and thus can not fix it for you. I guess it is the same for many kernel developers.

Also please understand that when if and how a warning triggers not only depends on the gcc and toolchain version (yes they get cleverer and sometimes newer gcc can suppress false positives that older gcc versions warned about) but also on the architecture (64bit vs 32bit) and the specific .config.

It's a toolchain issue; it only identifies a specific version of binutils (2.16 I believe), and it should be resolved by cherry-picking 417484d47e115774745ef025bce712a102b6f86f.  This is clearly a binutils bug, and 417484d47e115774745ef025bce712a102b6f86f works around it.

It would be good to get this verified.

(In reply to comment #20)
> Sadly, I can't reproduce these warnings and thus can not fix it for you. I
> guess it is the same for many kernel developers.

If you can't reproduce it by installing gcc-3.3.3 + old binutils - you can just get one of toolchains from router distro - for ex., one from LEAF aka Bering-uClibc v4 (from http://leaf.cvs.sourceforge.net/viewvc/leaf/src/bering-uclibc4/buildtool/?view=tar ); v4 (now still in pre-beta stage) has 2.6.35.2 kernel.

P.S. Can these warnings be related with bug 16633 (kernel that compiled with gcc-3.3.3 has broken wireless, and compiled with gcc-4.4.3 with same config works OK) ?

Or would it be possible for you to try one of the following kernels? 
$git tag --contains 417484d4
v2.6.36-rc1
v2.6.36-rc2
v2.6.36-rc3

They all contain the fix for the issue and if one of you verified that this is indeed the case Greg could apply them to the 2.6.35.y stable series.

In 2.6.36-rc3 still present warnings about cpufeature.h:

/var/testpoint/LEAF/buildtool/source/linux/linux-2.6.36-rc3/arch/x86/include/asm/cpufeature.h:326: warning: asm operand 1 probably doesn't match constraints

And there is other warnings:

arch/x86/lib/inat-tables.c:1002: warning: duplicate `const'
arch/x86/lib/inat-tables.c:1023: warning: duplicate `const'
arch/x86/lib/inat-tables.c:989: warning: duplicate `const'
crypto/async_tx/async_tx.c:235: warning: `chan' might be used uninitialized in this function
drivers/acpi/apei/erst.c:273: warning: cast to pointer from integer of different size
drivers/acpi/apei/erst.c:274: warning: cast to pointer from integer of different size
drivers/char/keyboard.c:719: warning: comparison is always true due to limited range of data type
drivers/md/dm.c:1145: warning: statement with no effect
drivers/md/dm.c:1173: warning: statement with no effect
drivers/md/dm-table.c:1120: warning: statement with no effect
drivers/md/dm-table.c:1126: warning: statement with no effect
drivers/net/wireless/ath/ath9k/mac.h:725: warning: parameter has incomplete type
drivers/net/wireless/iwlwifi/iwl-3945.c:1072: warning: suggest explicit braces to avoid ambiguous `else'
drivers/net/wireless/mwl8k.c:3403: warning: `ap_legacy_rates' might be used uninitialized in this function
drivers/net/wireless/zd1211rw/zd_usb.c:1079: warning: `bulk_out_ep' might be used uninitialized in this function
drivers/pcmcia/cs.c:849: warning: `pcmcia_socket_dev_resume' defined but not used
drivers/scsi/constants.c:1407: warning: zero-length printf format string
drivers/scsi/constants.c:1413: warning: zero-length printf format string
drivers/scsi/constants.c:1456: warning: zero-length printf format string
drivers/scsi/sd.c:2628: warning: zero-length printf format string
drivers/scsi/sd.c:2630: warning: zero-length printf format string
drivers/scsi/sd.c:2636: warning: zero-length printf format string
drivers/ssb/driver_chipcommon_pmu.c:420: warning: `updown_tab_size' might be used uninitialized in this function
drivers/ssb/driver_chipcommon_pmu.c:422: warning: `depend_tab_size' might be used uninitialized in this function
drivers/usb/host/xhci-ring.c:1719: warning: `event_trb' might be used uninitialized in this function
fs/autofs4/root.c:31: warning: `autofs4_root_compat_ioctl' declared `static' but never defined
fs/fuse/dev.c:1357: warning: `total_len' might be used uninitialized in this function
include/linux/spinlock.h:379: warning: asm operand 0 probably doesn't match constraints
net/ipv4/inet_connection_sock.c:95: warning: `head' might be used uninitialized in this function
net/mac80211/tx.c:1702: warning: unused variable `mesh_hdr'
net/netfilter/nf_conntrack_sip.c:1379: warning: `ret' might be used uninitialized in this function
net/packet/af_packet.c:1107: warning: `vnet_hdr_len' might be used uninitialized in this function
net/sctp/socket.c:5412: warning: `head' might be used uninitialized in this function
net/wireless/nl80211.c:776: warning: `rdev' might be used uninitialized in this function
net/wireless/nl80211.c:779: warning: `result' might be used uninitialized in this function
sound/pci/hda/patch_conexant.c:1263: warning: statement with no effect
sound/pci/hda/patch_conexant.c:2082: warning: statement with no effect
sound/pci/hda/patch_conexant.c:3192: warning: statement with no effect
/var/testpoint/LEAF/buildtool/source/linux/linux-2.6.36-rc3/arch/x86/include
/var/testpoint/LEAF/buildtool/source/linux/linux-2.6.36-rc3/arch/x86/include/asm/processor.h:502: warning: asm operand 0 probably doesn't match constraints
/var/testpoint/LEAF/buildtool/source/linux/linux-2.6.36-rc3/arch/x86/include/asm/processor.h:529: warning: asm operand 0 probably doesn't match constraints

OK, I assume the kernel compiles in spite of these warnings, doesn't it?

Please reopen if it doesn't compile, closing as fixed in mainline.

(In reply to comment #25)
> OK, I assume the kernel compiles in spite of these warnings, doesn't it?

It's compiled, but lot of warnings isn't good. Warning about asm/cpufeature.h is still present in almost all compiled files. And I suspect that 2.6.36-rcX also will have broken wireless when compiled with old gcc.

Broken wireless is definitely a  must fix. See trick for easy build-warning bisecting in the referenced bug.

Please note that gcc-3.3.3 is known broken on x86; gcc-3.4 is the oldest version which is known to *not* be broken.

Ok, I'll try to upgrade GCC - but it'll cause much headache - there are too much hacks for gcc to work in buildenv sandbox (there are hundreds of lines in makefile that starts configuration and compilation of gcc); I hope that 3.4 hasn't much difference comparable to 3.3 and migration will not be a nightmare.
This gcc version was used in this distro (LEAF) for years, and it compiles all successfully.
Warning in cpufeature.h is caused by this code:

static __always_inline __pure bool __static_cpu_has(u16 bit)
{
#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 5)
                asm goto("1: jmp %l[t_no]\n"
                         "2:\n"
                         ".section .altinstructions,\"a\"\n"
                         _ASM_ALIGN "\n"
                         _ASM_PTR "1b\n"
                         _ASM_PTR "0\n"         /* no replacement */
                         " .word %P0\n"         /* feature bit */
                         " .byte 2b - 1b\n"     /* source len */
                         " .byte 0\n"           /* replacement len */
                         ".previous\n"
                         /* skipping size check since replacement size = 0 */
                         : : "i" (bit) : : t_no);
                return true;
        t_no:
                return false;
#else
                u8 flag;
                /* Open-coded due to __stringify() in ALTERNATIVE() */
                asm volatile("1: movb $0,%0\n"
                             "2:\n"
                             ".section .altinstructions,\"a\"\n"
                             _ASM_ALIGN "\n"
                             _ASM_PTR "1b\n"
                             _ASM_PTR "3f\n"
                             " .word %P1\n"             /* feature bit */
                             " .byte 2b - 1b\n"         /* source len */
                             " .byte 4f - 3f\n"         /* replacement len */
                             ".previous\n"
                             ".section .discard,\"aw\",@progbits\n"
                             " .byte 0xff + (4f-3f) - (2b-1b)\n" /* size check */
                             ".previous\n"
                             ".section .altinstr_replacement,\"ax\"\n"
                             "3: movb $1,%0\n"
                             "4:\n"
                             ".previous\n"
                             : "=qm" (flag) : "i" (bit));
                return flag;
#endif
}

#if __GNUC__ >= 4
#define static_cpu_has(bit)                                     \
(                                                               \
        __builtin_constant_p(boot_cpu_has(bit)) ?               \
                boot_cpu_has(bit) :                             \
        __builtin_constant_p(bit) ?                             \
                __static_cpu_has(bit) :                         \
                boot_cpu_has(bit)                               \
)
#else
/*
 * gcc 3.x is too stupid to do the static test; fall back to dynamic.
 */
#define static_cpu_has(bit) boot_cpu_has(bit)
#endif

gcc-3.3.3 can't parse __static_cpu_has code. And as I saw by quick code search, it's used only in cpufeature.h - so can be declared (and compiled) conditionally. And as I saw in google, somebody already proposes patch here: http://us.generation-nt.com/answer/patch-x86-cpufeature-suppress-compiler-warning-gcc-3-x-help-198700871.html - but it looks that patch still isn't in main tree...

At this point I'm officially proposing that we drop support for gcc < 4.1 in the x86 kernel, unless someone has a very strong use case.

1)  Since it is a warning, not an error, and the code still compiles and works, it would be inappropriate to raise the minimum version.

2)  A patch exists.  Mr. Anvin: You've seen it, because you replied to it.
http://us.generation-nt.com/answer/patch-x86-cpufeature-suppress-compiler-warning-gcc-3-x-help-198700871.html

Just because you've kept up with upgrading your compiler doesn't mean that everyone has.  As long as support of the older versions are as trivial as indicated in these patches, there's no justification for raising the minimum version.  That should only be done where there is no backwards compatible way of expressing current code, which isn't the case here.

"That should only be done where there is no backwards compatible way
of expressing current code, which isn't the case here."

That is your opinion.  You're not the one who has to deal with the constant stream of bug reports whenever ancient versions of gcc miscompiles new code.  The support burden is getting severe and the payback is getting to be minute.

There is a difference between "new code" and rewriting existing code (for reasons other than errors), such as the error that this bug report started with.

I concede that even computer languages evolve, but any code that is error and warning free that through rewriting introduces a warning is suspect.

The developers who modify this code use newer compilers than you do and
therefore they don't see the warnings that you're seeing.  You basically
demand that people test-build their code with your compiler just to make
sure that _you_ won't see any new warnings.  That appears to be an
excessive demand to me.

I don't care what compiler version with which they develop and test their code.  However, I do care when they say that a particular minimum version should be used and that code FAILS to work with that minimum version (especially when alternative coding performing the same function does work).  Just because they decide to adopt a particular style is not sufficient reason by itself to raise the minimum version floor.  Raising the floor is acceptable only when the construct will not work with the [prior] minimum version regardless of how it's coded, i.e. the floor is raised only when necessary, not willy-nilly.  I do demand that the software compile and work based on the minimum compiler version stated; nothing more.  Such was not met here (cf. comment #2 regarding the binutils package).

As for me personally, the latest compiler (GCC 4.5.1) source does not yield a stable compiler on my system.  There are various places where it is broken, and in one place, there appears to be a hardcoded reference to a prior version (4.2.0) - which itself is poor coding.  I skipped past that particular release, but regardless, no compiler should have symbol references to prior versions, especially those that were never installed on the system in question.

> I do demand that the software compile and work based on the minimum compiler
> version stated; nothing more.  
> Such was not met here (cf. comment #2 regarding the binutils package).

Kernel developers develop the kernel, not gcc. They rely on gcc to be backwards compatible. 

I agree that it would be nice if more users tested the lower bound of the toolchain, but this mostly depends on the distributions. When they upgrade their toolchains, we loose testers without knowing it. 

Pro-actively bumping up the requirements is a controversial issue which is not done light-heartedly and so we rely on people (like you) who go through the pain of reporting and debugging their breakages. This is much appreciated!

If you have suggestions for an easy way to improve the situation I assume everybody would be glad. 

Regards,
Flo

(In reply to comment #34)
> The developers who modify this code use newer compilers than you do and
> therefore they don't see the warnings that you're seeing.  You basically
> demand that people test-build their code with your compiler just to make
> sure that _you_ won't see any new warnings.  That appears to be an
> excessive demand to me.

Indeed. You have to remember, that gcc _progresses_. So warnings on older compiler are _actively_ uninteresting. 

(Yeah, I know, the assumption beeing that gcc actually progresses and not regresses.)