From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io0-f173.google.com ([209.85.223.173]:33859 "EHLO mail-io0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932401AbeCJQam (ORCPT ); Sat, 10 Mar 2018 11:30:42 -0500 MIME-Version: 1.0 In-Reply-To: References: <20180309200536.GA5670@beast> <20180309160719.154a3158e2d8ee56e43a918f@linux-foundation.org> From: Linus Torvalds Date: Sat, 10 Mar 2018 08:30:40 -0800 Message-ID: Subject: Re: [PATCH v3] kernel.h: Skip single-eval logic on literals in min()/max() To: Kees Cook Cc: Miguel Ojeda , Randy Dunlap , Andrew Morton , linux-kernel , Josh Poimboeuf , Rasmus Villemoes , "Gustavo A. R. Silva" , "Tobin C. Harding" , Steven Rostedt , Jonathan Corbet , Chris Mason , Josef Bacik , David Sterba , "David S. Miller" , Alexey Kuznetsov , Hideaki YOSHIFUJI , Ingo Molnar , Peter Zijlstra , Thomas Gleixner , Masahiro Yamada , Borislav Petkov , Ian Abbott , Sergey Senozhatsky , Petr Mladek , Andy Shevchenko , Pantelis Antoniou , Linux Btrfs , Network Development , Kernel Hardening Content-Type: text/plain; charset="UTF-8" Sender: linux-btrfs-owner@vger.kernel.org List-ID: On Sat, Mar 10, 2018 at 7:33 AM, Kees Cook wrote: > > Alright, I'm giving up on fixing max(). I'll go back to STACK_MAX() or > some other name for the simple macro. Bleh. Oh, and I'm starting to see the real problem. It's not that our current "min/max()" are broiken. It's that "-Wvla" is garbage. Lookie here: int array[(1,2)]; results in gcc saying warning: ISO C90 forbids variable length array ‘array’ [-Wvla] int array[(1,2)]; ^~~ and that error message - and the name of the flag - is obviously pure garbage. What is *actually* going on is that ISO C90 requires an array size to be not a constant value, but a constant *expression*. Those are two different things. A constant expression has little to do with "compile-time constant". It's a more restricted form of it, and has actual syntax requirements. A comma expression is not a constant expression, for example, which was why I tested this. So "-Wvla" is garbage, with a misleading name, and a misleading warning string. It has nothing to do with "variable length" and whether the compiler can figure it out at build time, and everything to do with a _syntax_ rule. Linus From mboxrd@z Thu Jan 1 00:00:00 1970 From: Linus Torvalds Subject: Re: [PATCH v3] kernel.h: Skip single-eval logic on literals in min()/max() Date: Sat, 10 Mar 2018 08:30:40 -0800 Message-ID: References: <20180309200536.GA5670@beast> <20180309160719.154a3158e2d8ee56e43a918f@linux-foundation.org> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Cc: Miguel Ojeda , Randy Dunlap , Andrew Morton , linux-kernel , Josh Poimboeuf , Rasmus Villemoes , "Gustavo A. R. Silva" , "Tobin C. Harding" , Steven Rostedt , Jonathan Corbet , Chris Mason , Josef Bacik , David Sterba , "David S. Miller" , Alexey Kuznetsov , Hideaki YOSHIFUJI , Ingo Molnar , Peter Zijlstra , Thomas Gleixner , Masahiro Yamada Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On Sat, Mar 10, 2018 at 7:33 AM, Kees Cook wrote: > > Alright, I'm giving up on fixing max(). I'll go back to STACK_MAX() or > some other name for the simple macro. Bleh. Oh, and I'm starting to see the real problem. It's not that our current "min/max()" are broiken. It's that "-Wvla" is gar= bage. Lookie here: int array[(1,2)]; results in gcc saying warning: ISO C90 forbids variable length array =E2=80=98array=E2=80=99= [-Wvla] int array[(1,2)]; ^~~ and that error message - and the name of the flag - is obviously pure garba= ge. What is *actually* going on is that ISO C90 requires an array size to be not a constant value, but a constant *expression*. Those are two different things. A constant expression has little to do with "compile-time constant". It's a more restricted form of it, and has actual syntax requirements. A comma expression is not a constant expression, for example, which was why I tested this. So "-Wvla" is garbage, with a misleading name, and a misleading warning string. It has nothing to do with "variable length" and whether the compiler can figure it out at build time, and everything to do with a _syntax_ rule. Linus