Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934448AbbEMRd3 (ORCPT ); Wed, 13 May 2015 13:33:29 -0400 Received: from st11p01mm-asmtp001.mac.com ([17.172.204.239]:57181 "EHLO st11p01mm-asmtp001.mac.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751441AbbEMRd0 convert rfc822-to-8bit (ORCPT ); Wed, 13 May 2015 13:33:26 -0400 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:5.14.151,1.0.33,0.0.0000 definitions=2015-05-13_06:2015-05-13,2015-05-13,1970-01-01 signatures=0 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 suspectscore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=7.0.1-1412110000 definitions=main-1505130127 Content-type: text/plain; charset=us-ascii MIME-version: 1.0 (Mac OS X Mail 7.3 \(1878.6\)) Subject: Re: [tip:x86/build] x86/build: Remove -Wno-sign-compare From: Louis Langholtz In-reply-to: <20150513102257.GB4318@gmail.com> Date: Wed, 13 May 2015 11:33:20 -0600 Cc: Valdis.Kletnieks@vt.edu, Linus Torvalds , Peter Anvin , Brian Gerst , Thomas Gleixner , Borislav Petkov , Peter Zijlstra , Andy Lutomirski , Denys Vlasenko , linux-kernel@vger.kernel.org Content-transfer-encoding: 8BIT Message-id: References: <20150512084415.GA6535@gmail.com> <156879.1431503013@turing-police.cc.vt.edu> <20150513102257.GB4318@gmail.com> To: Ingo Molnar X-Mailer: Apple Mail (2.1878.6) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4014 Lines: 91 On May 13, 2015, at 4:22 AM, Ingo Molnar wrote: > * Valdis.Kletnieks@vt.edu wrote: > >> On Tue, 12 May 2015 10:44:15 +0200, Ingo Molnar said: >> >>> ... >>> Before I pushed out this -Wno-sign-compare change I made sure there >>> are no extra warnings generated on the 8 key configs I monitor >>> explicitly: [def|allno|allyes|allmod]config[64|32]. >> >> It makes my config totally explode on next-20150512 I note that not all architectures add the no-sign-compare option to their builds. Specifically (according to grep of arch/*/Makefile), s390, sparc, and x86 do add this option while none of the others do (well alpha may but not in that same Makefile). Are warnings about sign-comparisons an architecture specific issue? I can imagine that for some architectures the sign comparison is a bigger concern than for others. I can also imagine that this could be architecture independent enough to warrant being considered an option for all the kernel builds. Do devs for the other architectures just live with all these warnings? Or 'clean' them up? >> >> % gcc --version >> gcc (GCC) 5.1.1 20150422 (Red Hat 5.1.1-1) >> >> % grep "warning: comparison between signed and unsigned integer expressions" build.default | wc -l >> 64148 >> >> A *lot*of them appear to be exploding inside likely(). Here's all the >> exploding for *one file*... > A lot of the explosion appears to be due to the sign compare issue being in code that's inline or macro code in header files that get included in multiple C files. (Is that what you're saying in your next sentence?) > That's a prerelease of GCC, right? So I think GCC gets constant > propagation from various builtins wrong or so. GCC seems to issue a warning on every use of code; not just the first. I won't call that wrong. It does decrease the signal-to-noise ratio however and in that sense I do find it less desirable. OTOH, that does help to prioritize include files that are more responsible for sign-compare issues. > > But ... the output looks horrible, and for years we didn't have the > warnings, still after reintroducing it we didn't get any new warnings > about truly bogus code. I'd hate to have to go through every sign compare that was flagged to see whether the mixed sign-unsigned comparison actually introduced a potential real problem. OTOH, there's a lot of places as we're seeing where the comparison occurs and a lot of these places don't appear to have defensive code to handle a potentially real comparison flaw. > So it does not seem to have much utility, and seems to be horribly > broken on fresh GCC. I use gcc 4.9.2 and I see the above described explosion (so not just gcc 5.1.1+). > > I'll remove the commit. > > Thanks, > > Ingo When I submitted the patch that I did (that addresses sign comparisons in the arch/x86/include/asm/uaccess.h file) that appears to have started this thread, I considered submitting the change to the Makefile (like you made). While I'd love to see this change *someday*, I decided against it for now because it seemed like this would introduce a more significant change that requires a lot more analysis that probably isn't worth the trouble. Individual developers can remove the option and address flagged code in the meantime without bothering more people that aren't interested in potential sign comparison issues in other people's code. I believe Linus brought up the concern too about how sign-unsigned comparisons might get addressed. I think there's sometimes a clear-cut correct "fix" but often there isn't. So I could see a lot of patches coming in to quiet the compiler that also caused more problems like hiding problems that would be better addressed more comprehensively. Lou-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/