Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1751792AbdG0Qsv (ORCPT <rfc822;w@1wt.eu>);
        Thu, 27 Jul 2017 12:48:51 -0400
Received: from mail-io0-f194.google.com ([209.85.223.194]:34782 "EHLO
        mail-io0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1751461AbdG0Qsu (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Thu, 27 Jul 2017 12:48:50 -0400
Message-ID: <1501174127.26391.1.camel@gmail.com>
Subject: Re: [PATCH] fortify: Use WARN instead of BUG for now
From: Daniel Micay <danielmicay@gmail.com>
To: kbuild test robot <lkp@intel.com>, Kees Cook <keescook@chromium.org>
Cc: kbuild-all@01.org, Andrew Morton <akpm@linux-foundation.org>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Dan Williams <dan.j.williams@intel.com>,
        Mika Westerberg <mika.westerberg@linux.intel.com>,
        Al Viro <viro@zeniv.linux.org.uk>, David Howells <dhowells@redhat.com>,
        Heikki Krogerus <heikki.krogerus@linux.intel.com>,
        Bjorn Helgaas <bhelgaas@google.com>, Arnd Bergmann <arnd@arndb.de>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Mauro Carvalho Chehab <mchehab@kernel.org>,
        linux-kernel@vger.kernel.org
Date: Thu, 27 Jul 2017 12:48:47 -0400
In-Reply-To: <201707271305.eiZ8s96C%fengguang.wu@intel.com>
References: <201707271305.eiZ8s96C%fengguang.wu@intel.com>
Content-Type: text/plain; charset="UTF-8"
X-Mailer: Evolution 3.24.4 
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 1141
Lines: 36

I think the 'else' added in the proposed patch makes it too complicated
for GCC to optimize out the __attribute__((error)) checks before they're
considered to be errors. It's not needed so it's probably best to just
avoid doing something like that. The runtime checks can't get false
positives from overly complex code but the compile-time ones depend on
GCC being able to reliably optimize them out.

This might be easier for GCC:

if (__builtin_constant_p(size) && condition_a) {
    compiletimeerror();
}

if (__builtin_constant_p(size) && condition_b) {
    compiletimeerror();
}

than the current:

if (__builtin_constant_p(size)) {
    if (condition_a) {
        compiletimeerror();
    }

    if (condition_b) {
        compiletimeerror();
    }
}

but it hasn't had a false positive like that with the current code.

Removing __noreturn is making the inline code more complex from GCC's
perspective too, but hopefully it's neither reducing coverage (i.e. not
making it less able to resolve __builtin_object_size - intuitively it
shouldn't impact it much but you never know) or making GCC unable to
deal with the compile-time checks.