From patchwork Mon Oct 4 18:42:23 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Sebor X-Patchwork-Id: 45795 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 23D143858414 for ; Mon, 4 Oct 2021 18:42:55 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 23D143858414 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1633372975; bh=1HBxepre+4IoPOtKgH4CgLlmL9/cAVQIVYJVIQQiqxQ=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=Y9+lcojDoeqfQ2ofWuheF7MAItrWjUbpefRQrChFJZznhV7dftB90fgBclDqX6YBU EQQD2QC3+esLKIwZDzXA4ANqv4+B0DVqsjQ16Mc0dhVlGdAnZAGWSjdnjsQFkIyeWL TqGNz+73yIpSLw9azjlJeLcileYp37jHlajTBd9s= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-ot1-x332.google.com (mail-ot1-x332.google.com [IPv6:2607:f8b0:4864:20::332]) by sourceware.org (Postfix) with ESMTPS id B693C3858D28 for ; Mon, 4 Oct 2021 18:42:25 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org B693C3858D28 Received: by mail-ot1-x332.google.com with SMTP id x33-20020a9d37a4000000b0054733a85462so22813921otb.10 for ; Mon, 04 Oct 2021 11:42:25 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:to:from:subject:cc:message-id:date:user-agent :mime-version:content-language; bh=1HBxepre+4IoPOtKgH4CgLlmL9/cAVQIVYJVIQQiqxQ=; b=8AQ6olYkUwjYG53/mGk6Y2rGj+YO0VAOMOPEwF1v38v4dqzk31f1F42m6cnC+FIuO+ 9nE5qwdxDxoYlMd8MGpyiQXoB2VVnw8QbOgRi7jPuY76ZOGsl+EmnuulGtJCV6k2dNz6 WraZiE4bqOr1jGhH6lrI/7/jc+a4bldj3k5wGuDZCC7TgKbsPxn+gY1G3oMRNqM8dXx+ USu41nasqzNnyfWJ1ltgzUXTrtmh+zPEGXGXv04byUGsmF9Lzw8VKdKwIBIEK8rNNZSK 6phR8mYQyVokmbutmK2fgP/s2p2InsZkmPSXYqYgZv//AF3J08mx8FXq+eZih2klBb2m elPQ== X-Gm-Message-State: AOAM533MKfbKKrAlyRkJQpnKAezyPw453NI6FUWUCQYYbF/oro+45xQz U6XlDT39elmrBPQF0K8x0w19afb8u0I= X-Google-Smtp-Source: ABdhPJxmr2lQqGGptztkHVF23xlbenqb7ZZDImfa9c33E0ep44mjCGvJURodBR5G2fqwrsaqdMdmRg== X-Received: by 2002:a9d:490:: with SMTP id 16mr10718873otm.184.1633372944912; Mon, 04 Oct 2021 11:42:24 -0700 (PDT) Received: from [192.168.0.41] (97-118-96-133.hlrn.qwest.net. [97.118.96.133]) by smtp.gmail.com with ESMTPSA id k4sm2840922oic.48.2021.10.04.11.42.24 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 04 Oct 2021 11:42:24 -0700 (PDT) To: "Joseph S. Myers" , Jason Merrill Subject: [PATCH] restore ancient -Waddress for weak symbols [PR33925] Message-ID: <679cf10f-e16a-e318-0e82-820efb109d0f@gmail.com> Date: Mon, 4 Oct 2021 12:42:23 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 MIME-Version: 1.0 Content-Language: en-US X-Spam-Status: No, score=-10.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Martin Sebor via Gcc-patches From: Martin Sebor Reply-To: Martin Sebor Cc: gcc-patches Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" While resolving the recent -Waddress enhancement request (PR PR102103) I came across a 2007 problem report about GCC 4 having stopped warning for using the address of inline functions in equality comparisons with null. With inline functions being commonplace in C++ this seems like an important use case for the warning. The change that resulted in suppressing the warning in these cases was introduced inadvertently in a fix for PR 22252. To restore the warning, the attached patch enhances the decl_with_nonnull_addr_p() function to return true also for weak symbols for which a definition has been provided. Tested on x86_64-linux and by comparing the GCC output for new test cases to Clang's which diagnoses all but one instance of these cases with either -Wtautological-pointer-compare or -Wpointer-bool-conversion, depending on context. The one case where Clang doesn't issue a warning but GCC with the patch does is for a symbol explicitly declared with attribute weak for which a definition has been provided. I believe the address of such symbols is necessarily nonnull and so issuing the warning is helpful (both GCC and Clang fold such comparisons to a constant). Martin PR c/33925 - missing -Waddress with the address of an inline function gcc/c-family/ChangeLog: PR c/33925 * c-common.c (decl_with_nonnull_addr_p): Also return true for weak symbols for which a definition has been provided. gcc/testsuite/ChangeLog: PR c/33925 * c-c++-common/Waddress-5.c: New test. * g++.dg/warn/Waddress-7.C: New test. diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c index 9d19e352725..ba02d3981c0 100644 --- a/gcc/c-family/c-common.c +++ b/gcc/c-family/c-common.c @@ -3395,7 +3395,8 @@ c_wrap_maybe_const (tree expr, bool non_const) /* Return whether EXPR is a declaration whose address can never be NULL. The address of the first struct member could be NULL only if it were - accessed through a NULL pointer, and such an access would be invalid. */ + accessed through a NULL pointer, and such an access would be invalid. + The address of a weak symbol may be null unless it has a definition. */ bool decl_with_nonnull_addr_p (const_tree expr) @@ -3404,7 +3405,9 @@ decl_with_nonnull_addr_p (const_tree expr) && (TREE_CODE (expr) == FIELD_DECL || TREE_CODE (expr) == PARM_DECL || TREE_CODE (expr) == LABEL_DECL - || !DECL_WEAK (expr))); + || !DECL_WEAK (expr) + || (DECL_INITIAL (expr) + && DECL_INITIAL (expr) != error_mark_node))); } /* Prepare expr to be an argument of a TRUTH_NOT_EXPR, diff --git a/gcc/testsuite/c-c++-common/Waddress-5.c b/gcc/testsuite/c-c++-common/Waddress-5.c new file mode 100644 index 00000000000..fa6658fa133 --- /dev/null +++ b/gcc/testsuite/c-c++-common/Waddress-5.c @@ -0,0 +1,133 @@ +/* PR c/33925 - missing -Waddress with the address of an inline function + { dg-do compile } + { dg-options "-Wall" } + { dg-require-weak "" } */ + +extern inline int eifn (void); +extern inline int eifn_def (void) { return 0; } + +static inline int sifn (void); +static inline int sifn_def (void) { return 0; } + +inline int ifn (void); +inline int ifn_def (void) { return 0; } + +extern __attribute__ ((weak)) int ewfn (void); +extern __attribute__ ((weak)) int ewfn_def (void) { return 0; } + +__attribute__ ((weak)) int wfn (void); +__attribute__ ((weak)) int wfn_def (void) { return 0; } + +static __attribute__((weakref ("ewfn"))) int swrfn; + +void test_function_eqz (int *p) +{ + *p++ = eifn == 0; // { dg-warning "-Waddress" } + *p++ = eifn_def == 0; // { dg-warning "-Waddress" } + *p++ = sifn == 0; // { dg-warning "-Waddress" } + *p++ = sifn_def == 0; // { dg-warning "-Waddress" } + *p++ = ifn == 0; // { dg-warning "-Waddress" } + *p++ = ifn_def == 0; // { dg-warning "-Waddress" } + *p++ = ewfn == 0; + *p++ = ewfn_def == 0; // { dg-warning "-Waddress" } + *p++ = wfn == 0; + *p++ = wfn_def == 0; // { dg-warning "-Waddress" } + *p++ = swrfn == 0; +} + + +int test_function_if (int i) +{ + if (eifn) // { dg-warning "-Waddress" } + i++; + if (eifn_def) // { dg-warning "-Waddress" } + i++; + if (sifn) // { dg-warning "-Waddress" } + i++; + if (sifn_def) // { dg-warning "-Waddress" } + i++; + if (ifn) // { dg-warning "-Waddress" } + i++; + if (ifn_def) // { dg-warning "-Waddress" } + i++; + if (ewfn) + i++; + if (ewfn_def) // { dg-warning "-Waddress" } + i++; + if (wfn) + i++; + if(wfn_def) // { dg-warning "-Waddress" } + i++; + if (swrfn) + i++; + return i; +} + + +extern int ei; +extern int ei_def = 1; + +static int si; +static int si_def = 1; + +int i; +int i_def = 1; + +extern __attribute__ ((weak)) int ewi; +extern __attribute__ ((weak)) int ewi_def = 1; + +__attribute__ ((weak)) int wi; +__attribute__ ((weak)) int wi_def = 1; + +static __attribute__((weakref ("ewi"))) int swri; + +void test_scalar (int *p) +{ + *p++ = &ei == 0; // { dg-warning "-Waddress" } + *p++ = &ei_def == 0; // { dg-warning "-Waddress" } + *p++ = &si == 0; // { dg-warning "-Waddress" } + *p++ = &si_def == 0; // { dg-warning "-Waddress" } + *p++ = &i == 0; // { dg-warning "-Waddress" } + *p++ = &i_def == 0; // { dg-warning "-Waddress" } + *p++ = &ewi == 0; + *p++ = &ewi_def == 0; // { dg-warning "-Waddress" } + *p++ = &wi == 0; + *p++ = &wi_def == 0; // { dg-warning "-Waddress" } + *p++ = &swri == 0; +} + + +extern int eia[]; +extern int eia_def[] = { 1 }; + +static int sia[1]; +static int sia_def[1] = { 1 }; + +int ia[1]; +int ia_def[] = { 1 }; + +extern __attribute__ ((weak)) int ewia[]; +extern __attribute__ ((weak)) int ewia_def[] = { 1 }; + +__attribute__ ((weak)) int wia[1]; +__attribute__ ((weak)) int wia_def[] = { 1 }; + +static __attribute__((weakref ("ewia"))) int swria[1]; + +void test_array (int *p) +{ + *p++ = eia == 0; // { dg-warning "-Waddress" } + *p++ = eia_def == 0; // { dg-warning "-Waddress" } + *p++ = sia == 0; // { dg-warning "-Waddress" } + *p++ = sia_def == 0; // { dg-warning "-Waddress" } + *p++ = ia == 0; // { dg-warning "-Waddress" } + *p++ = ia_def == 0; // { dg-warning "-Waddress" } + *p++ = ewia == 0; + *p++ = ewia_def == 0; // { dg-warning "-Waddress" } + *p++ = wia == 0; + *p++ = wia_def == 0; // { dg-warning "-Waddress" } + *p++ = swria == 0; +} + +/* { dg-prune-output "never defined" } + { dg-prune-output "initialized and declared 'extern'" } */ diff --git a/gcc/testsuite/g++.dg/warn/Waddress-7.C b/gcc/testsuite/g++.dg/warn/Waddress-7.C new file mode 100644 index 00000000000..efdafa50cd1 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Waddress-7.C @@ -0,0 +1,76 @@ +/* PR c/33925 - missing -Waddress with the address of an inline function + { dg-do compile } + { dg-options "-Wall" } */ + +struct A +{ + int mf (); + int mf_def () { return 0; } + + static int smf (); + static int smf_def () { return 0; } + + int mi; + static int smi; + static int smi_def; + + __attribute__ ((weak)) static int wsmi; + __attribute__ ((weak)) static int wsmi_def; + + int mia[]; + static int smia[]; + static int smia_def[]; + + __attribute__ ((weak)) static int wsmia[]; + __attribute__ ((weak)) static int wsmia_def[]; + + void test_waddress (bool*); +}; + + +/* __attribute__ ((weak)) static */ int A::smi_def = 0; +/* __attribute__ ((weak)) static */ int A::smia_def[] = { 0 }; + +/* __attribute__ ((weak)) static */ int A::wsmi_def = 0; +/* __attribute__ ((weak)) static */ int A::wsmia_def[] = { 0 }; + + + +void A::test_waddress (bool *p) +{ + if (mf) // { dg-error "cannot convert" } + p++; + if (mf_def) // { dg-error "cannot convert" } + p++; + + if (smf) // { dg-warning "-Waddress" } + p++; + if (smf_def) // { dg-warning "-Waddress" } + p++; + + if (&mi) // { dg-warning "-Waddress" } + p++; + if (&smi) // { dg-warning "-Waddress" } + p++; + if (&smi_def) // { dg-warning "-Waddress" } + p++; + + if (&wsmi) + p++; + + if (&wsmi_def) // { dg-warning "-Waddress" } + p++; + + if (mia) // { dg-warning "-Waddress" } + p++; + if (smia) // { dg-warning "-Waddress" } + p++; + if (smia_def) // { dg-warning "-Waddress" } + p++; + + if (wsmia) + p++; + + if (wsmia_def) // { dg-warning "-Waddress" } + p++; +}