From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2800 invoked by alias); 8 Oct 2017 16:38:57 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 2790 invoked by uid 89); 8 Oct 2017 16:38:56 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=no version=3.3.2 spammy= X-HELO: mail-oi0-f50.google.com Received: from mail-oi0-f50.google.com (HELO mail-oi0-f50.google.com) (209.85.218.50) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 08 Oct 2017 16:38:55 +0000 Received: by mail-oi0-f50.google.com with SMTP id n82so29020885oig.3 for ; Sun, 08 Oct 2017 09:38:55 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:from:date:message-id:subject:to:cc; bh=vinEPe8vjfPtSm/epcpvpgdhn/0/ViOsVpVzVScnwLI=; b=L/UqWypHxlEy0dFoMpitZmpY4PYa0uHaUuM/rGa6V6uZ2X9G0HxiJr1QfabnaeZwH2 tFUZ3h3reDJDbD8jLT6fXeEvX2MMVs+4Htd7TkUmujSg94pskCWIJU0cCoOGS80LnB5M Bl9OvqRuTUWGaxoaJwBrUJ0IgBTGpgMNz8zqtmseBn2nl7hkdh1LH8C/flq9r4wGTohp LNttkh1xkxLkkq+UX/BuoyBeg/uZN7DNP2zfo1WTIAHkjkckJDaTvqPwLWmBoA7eczMU K6gDSXSEf1SMaylq5TjmFnB2oHpcUGXTVqK04rsa8Tyh9tMtUrO2z6FRfG8bZYfhxbSU 6SEg== X-Gm-Message-State: AMCzsaUI4zj0NoYIpx4mD/nyo5lON+DqLS8uEh8OQ+OG/0hQypsTHtv3 AK9mTg68amDPVnr8WwWzrmoQCHehYxlXDaRmFcZ7/g== X-Google-Smtp-Source: AOwi7QDPYNBGTwawGaZGGc3oyPKpmxpljv5GCKM4eMdTecqcdrZLhFWwRhj2VFgBwZspiIYaUnuCJV9JkOzhF6Xi7iU= X-Received: by 10.202.242.195 with SMTP id q186mr3878600oih.206.1507480733323; Sun, 08 Oct 2017 09:38:53 -0700 (PDT) MIME-Version: 1.0 Received: by 10.157.54.244 with HTTP; Sun, 8 Oct 2017 09:38:32 -0700 (PDT) From: Eric Gallager Date: Sun, 08 Oct 2017 16:47:00 -0000 Message-ID: Subject: Re: [RFA] [PATCH] Add a warning for invalid function casts To: Martin Sebor Cc: Bernd Edlinger , "gcc-patches@gcc.gnu.org" Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2017-10/txt/msg00444.txt.bz2 On Fri, Oct 6, 2017 at 4:37 PM, Martin Sebor wrote: > On 10/06/2017 12:06 PM, Bernd Edlinger wrote: >> >> On 10/06/17 17:43, Martin Sebor wrote: >>> >>> On 10/06/2017 07:25 AM, Bernd Edlinger wrote: >>>> >>>> On 10/05/17 18:16, Martin Sebor wrote: >>>>> >>>>> In my (very quick) tests the warning appears to trigger on all >>>>> strictly incompatible conversions, even if they are otherwise >>>>> benign, such as: >>>>> >>>>> int f (const int*); >>>>> typedef int F (int*); >>>>> >>>>> F* pf1 = f; // -Wincompatible-pointer-types >>>>> F* pf2 = (F*)f; // -Wcast-function-type >>>>> >>>>> Likewise by: >>>>> >>>>> int f (signed char); >>>>> typedef int F (unsigned char); >>>>> >>>>> F* pf = (F*)f; // -Wcast-function-type >>>>> >>>>> I'd expect these conversions to be useful and would view warning >>>>> for them with -Wextra as excessive. In fact, I'm not sure I see >>>>> the benefit of warning on these casts under any circumstances. >>>>> >>>> >>>> Well, while the first example should be safe, >>>> the second one is probably not safe: >>>> >>>> Because the signed and unsigned char are promoted to int, >>>> by the ABI but one is in the range -128..127 while the >>>> other is in the range 0..255, right? >>> >>> >>> Right. The cast is always safe but whether or not a call to such >>> a function through the incompatible pointer is also safe depends >>> on the definition of the function (and on the caller). If the >>> function uses just the low bits of the argument then it's most >>> likely fine for any argument. If the caller only calls it with >>> values in the 7-bit range (e.g., the ASCII subset) then it's also >>> fine. Otherwise there's the potential for the problem you pointed >>> out. (Similarly, if in the first example I gave the cast added >>> constness to the argument rather than removing it and the function >>> modified the pointed-to object calling it through the incompatible >>> pointer on a constant object would also be unsafe.) >>> >>> Another class of cases to consider are casts between functions >>> taking pointers to different but related structs. Code like this >>> could be written to mimic C++ calling a base class function on >>> a derived object. >>> >>> struct Base { ... }; >>> struct Derived { Base b; ... }; >>> >>> typedef void F (Derived*); >>> >>> void foo (Base*); >>> >>> F* pf = (F*)foo; >>> >> >> Hmm, yes. >> >> I start to believe, that this warning should treat all pointers >> as equivalent, but everything else need to be of the same type. >> That would at least cover the majority of all "safe" use cases. > > > Perhaps basing the warning on some form of structural equivalence > between function arguments might be useful. For instance, in > ILP32, casting between 'int foo (int)' and 'long foo (long)' and > calling the function is probably generally considered safe (even > though it's undefined by the language) and works as people expect > so avoiding the warning there would help keep the false positive > rate down. (Something like this might also work for the kernel > alias macros.) > > Similarly, casting between a function that returns a scalar smaller > than int and int and then calling it is probably safe (or maybe > even long, depending on the ABI). > > Casting a function returning a value to one returning void and > calling it through the result should always be safe. > > I would also suggest to consider disregarding qualifiers as those > are often among the reasons for intentional casts (e.g., when > mixing a legacy API and a more modern const-correct one). > > Casts are also not uncommon between variadic and ordinary function > types so some heuristic might be appropriate there. > >> >> And I need a way to by-pass the warning with a generic function >> pointer type. uintptr_t is not the right choice, as you pointed >> out already. >> >> But I also see problems with "int (*) ()" as a escape mechanism >> because this declaration creates a warning in C with >> -Wstrict-prototypes, and in C++ this syntax means "int (*) (void)" >> while the C++ type "int (*) (...)" is rejected by the C front end. > > > I wouldn't consider it a problem if the suppression mechanism were > different between languages. Most such casts are going to be in > source files (as opposed to C headers) so it should be fine to use > each language's unique form of function without a prototype. > > Martin Some codebases attempt to be compilable as both C and C++, whether it be by using -Wc++-compat, or having options to compile with either the C compiler or C++ compiler. In such cases, I'd prefer to keep the amounts of "#ifdef __cplusplus" required to a minimum; a suppression mechanic that works the same between languages would be much preferred.