From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x102d.google.com (mail-pj1-x102d.google.com [IPv6:2607:f8b0:4864:20::102d]) by sourceware.org (Postfix) with ESMTPS id 220313858D28 for ; Sat, 23 Oct 2021 23:06:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 220313858D28 Received: by mail-pj1-x102d.google.com with SMTP id pi19-20020a17090b1e5300b0019fdd3557d3so5571739pjb.5 for ; Sat, 23 Oct 2021 16:06:21 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language; bh=aZeAxI7w2lSkDgyD+zGV+Lza6+OMpklvUHI2ggmu+gs=; b=zcPFGliypXasJp+5eU9Nh7JIkWMq5LcL1n+no4zbq7hgAZjBYQqllI7QdabtiLeM4f E1g25Q3P/ejE2tKTmSr1Hj4xXPdaXUUpte1hFYPkZ8/3umpbp1Nqv2njRvs3T/D1yOIO ijMa+uZeEUMNHRAxJdJKXLZJie/3wvw6yC9pOkPIGm4/Xo4754aBsm8EqhgDIA4/8ayh AJL54cFR5lQSxcmnt59BFTNH73DY/MPvxAxqjoriBguHnEchh5T6Cq+s6RwM7/X+fg8P Ez2/apwgkHCTVvveLJ4kWcRoeflwb+x1m95ND/F9A//JpMfWvPQ0UQbJthPUO/2iuhWM /+Qw== X-Gm-Message-State: AOAM532Hq23aN1eTfGztBaiTTR/LArdN4PJwq2K+3uOzqfvFhyV4n9CM XaZHlttGU4ICbSl0iM9nL1rJu3XHIkQ= X-Google-Smtp-Source: ABdhPJxk2BScss2VwNVACyL7S4xHSZ1C1RQeOSGB6YGNmrkGHPnX3ZBpVOTOTEh9m+4/iWkcu48IYA== X-Received: by 2002:a17:902:ecc6:b0:140:1917:7290 with SMTP id a6-20020a170902ecc600b0014019177290mr7695280plh.70.1635030379797; Sat, 23 Oct 2021 16:06:19 -0700 (PDT) Received: from [192.168.0.41] (184-96-250-116.hlrn.qwest.net. [184.96.250.116]) by smtp.gmail.com with ESMTPSA id h22sm13976258pfh.85.2021.10.23.16.06.17 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 23 Oct 2021 16:06:19 -0700 (PDT) Subject: Re: [PATCH] restore ancient -Waddress for weak symbols [PR33925] To: Jason Merrill , "Joseph S. Myers" Cc: gcc-patches References: <679cf10f-e16a-e318-0e82-820efb109d0f@gmail.com> From: Martin Sebor Message-ID: <24cd9565-b127-6534-d98e-7482b3dc082f@gmail.com> Date: Sat, 23 Oct 2021 17:06:14 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/mixed; boundary="------------2918A1B59CCE82BD42601F55" Content-Language: en-US X-Spam-Status: No, score=-10.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, NICE_REPLY_A, 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-List-Received-Date: Sat, 23 Oct 2021 23:06:24 -0000 This is a multi-part message in MIME format. --------------2918A1B59CCE82BD42601F55 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit On 10/4/21 3:37 PM, Jason Merrill wrote: > On 10/4/21 14:42, Martin Sebor wrote: >> 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. > > I think you probably want to merge this function with > fold-const.c:maybe_nonzero_address, which already handles more cases. maybe_nonzero_address() doesn't behave quite like decl_with_nonnull_addr_p() expects and I'm reluctant to muck around with the former too much since it's used for codegen, while the latter just for warnings. (There is even a case where the functions don't behave the same, and would result in different warnings between C and C++ without some extra help.) So in the attached revision I just have maybe_nonzero_address() call decl_with_nonnull_addr_p() and then refine the failing (or uncertain) cases separately, with some overlap between them. Since I worked on this someone complained that some instances of the warning newly enhanced under PR102103 aren't suppresed in code resulting from macro expansion. Since it's trivial, I include the fix for that report in this patch as well. Tested on x86_64-linux. Martin --------------2918A1B59CCE82BD42601F55 Content-Type: text/x-patch; charset=UTF-8; name="gcc-33925.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="gcc-33925.diff" Restore ancient -Waddress for weak symbols [PR33925]. Resolves: PR c/33925 - gcc -Waddress lost some useful warnings PR c/102867 - -Waddress from macro expansion in readelf.c gcc/c-family/ChangeLog: PR c++/33925 PR c/102867 * c-common.c (decl_with_nonnull_addr_p): Call maybe_nonzero_address and improve handling tof defined symbols. gcc/c/ChangeLog: PR c++/33925 PR c/102867 * c-typeck.c (maybe_warn_for_null_address): Suppress warnings for code resulting from macro expansion. gcc/cp/ChangeLog: PR c++/33925 PR c/102867 * typeck.c (warn_for_null_address): Suppress warnings for code resulting from macro expansion. gcc/ChangeLog: PR c++/33925 PR c/102867 * doc/invoke.texi (-Waddress): Update. * fold-const.c (maybe_nonzero_address): Declare extern. Simplify for readability. * tree.h (maybe_nonzero_address): Declare. gcc/testsuite/ChangeLog: PR c++/33925 PR c/102867 * g++.dg/warn/Walways-true-2.C: Adjust to avoid a valid warning. * c-c++-common/Waddress-5.c: New test. * c-c++-common/Waddress-6.c: New test. * g++.dg/warn/Waddress-7.C: New test. * g++.dg/warn/Walways-true-2.C: Adjust to avoid a valid warning. * gcc.dg/weak/weak-3.c: Expect a warning. diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c index 32c7e3e8972..71cc74ac63d 100644 --- a/gcc/c-family/c-common.c +++ b/gcc/c-family/c-common.c @@ -3395,16 +3395,46 @@ 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) { - return (DECL_P (expr) - && (TREE_CODE (expr) == FIELD_DECL - || TREE_CODE (expr) == PARM_DECL - || TREE_CODE (expr) == LABEL_DECL - || !DECL_WEAK (expr))); + if (maybe_nonzero_address (const_cast (expr)) > 0) + return true; + + if (!DECL_P (expr)) + return false; + + if (TREE_CODE (expr) == FIELD_DECL + || TREE_CODE (expr) == PARM_DECL + || TREE_CODE (expr) == LABEL_DECL) + return true; + + if (!VAR_OR_FUNCTION_DECL_P (expr)) + return false; + + if (!DECL_WEAK (expr)) + /* Ordinary (non-weak) symbols have nonnull addresses. */ + return true; + + if (DECL_INITIAL (expr) && DECL_INITIAL (expr) != error_mark_node) + /* Initialized weak symbols have nonnull addresses. */ + return true; + + if (DECL_EXTERNAL (expr) || !TREE_STATIC (expr)) + /* Uninitialized extern weak symbols and weak symbols with no + allocated stroage might have a null address. */ + return false; + + tree attribs = DECL_ATTRIBUTES (expr); + if (lookup_attribute ("weakref", attribs)) + /* Weakref symbols might have a null address unless their referent + is known not to. Don't bother following weakref targets here. */ + return false; + + return true; } /* Prepare expr to be an argument of a TRUTH_NOT_EXPR, diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c index 0aac978c02e..0ceedfa7b04 100644 --- a/gcc/c/c-typeck.c +++ b/gcc/c/c-typeck.c @@ -11571,7 +11571,10 @@ build_vec_cmp (tree_code code, tree type, static void maybe_warn_for_null_address (location_t loc, tree op, tree_code code) { - if (!warn_address || warning_suppressed_p (op, OPT_Waddress)) + /* Prevent warnings issued for macro expansion. */ + if (!warn_address + || warning_suppressed_p (op, OPT_Waddress) + || from_macro_expansion_at (loc)) return; if (TREE_CODE (op) == NOP_EXPR) diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c index ab0f9da2552..ae40e27e1d5 100644 --- a/gcc/cp/typeck.c +++ b/gcc/cp/typeck.c @@ -4597,9 +4597,11 @@ build_vec_cmp (tree_code code, tree type, static void warn_for_null_address (location_t location, tree op, tsubst_flags_t complain) { + /* Prevent warnings issued for macro expansion. */ if (!warn_address || (complain & tf_warning) == 0 || c_inhibit_evaluation_warnings != 0 + || from_macro_expansion_at (location) || warning_suppressed_p (op, OPT_Waddress)) return; diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 71992b8c597..99dfce7201a 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -8603,6 +8603,8 @@ suppressed by casting the pointer operand to an integer type such as @code{inptr_t} or @code{uinptr_t}. Comparisons against string literals result in unspecified behavior and are not portable, and suggest the intent was to call @code{strcmp}. +The warning is suppressed if the suspicious expression is the result +of macro expansion. @option{-Waddress} warning is enabled by @option{-Wall}. @item -Wno-address-of-packed-member diff --git a/gcc/fold-const.c b/gcc/fold-const.c index ff23f12f33c..4a173d278fb 100644 --- a/gcc/fold-const.c +++ b/gcc/fold-const.c @@ -9900,18 +9900,20 @@ pointer_may_wrap_p (tree base, tree offset, poly_int64 bitpos) symbol), and a negative integer when the symbol is not yet in the symbol table and so whether or not its address is zero is unknown. For function local objects always return positive integer. */ -static int +int maybe_nonzero_address (tree decl) { - if (DECL_P (decl) && decl_in_symtab_p (decl)) + if (!DECL_P (decl)) + return -1; + + if (decl_in_symtab_p (decl)) if (struct symtab_node *symbol = symtab_node::get_create (decl)) return symbol->nonzero_address (); /* Function local objects are never NULL. */ - if (DECL_P (decl) - && (DECL_CONTEXT (decl) + if (DECL_CONTEXT (decl) && TREE_CODE (DECL_CONTEXT (decl)) == FUNCTION_DECL - && auto_var_in_fn_p (decl, DECL_CONTEXT (decl)))) + && auto_var_in_fn_p (decl, DECL_CONTEXT (decl))) return 1; return -1; 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..5d63c7dae7c --- /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); + +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; // declaration (may be null) +extern __attribute__ ((weak)) int ewi_def = 1; + +__attribute__ ((weak)) int wi; // definition (cannot be bull) +__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; // { dg-warning "-Waddress" } + *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]; // definition (cannot be null) +__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; // { dg-warning "-Waddress" } + *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/c-c++-common/Waddress-6.c b/gcc/testsuite/c-c++-common/Waddress-6.c new file mode 100644 index 00000000000..e66e6e4a0b0 --- /dev/null +++ b/gcc/testsuite/c-c++-common/Waddress-6.c @@ -0,0 +1,32 @@ +/* PR c/102867 - -Waddress from macro expansion in readelf.c + { dg-do compile } + { dg-options "-Wall" } */ + +#define F(x) ((&x) != 0) + +int warn_nomacro (int *p, int i) +{ + return &p[i] != 0; // { dg-warning "-Waddress" } +} + +int nowarn_macro_expansion (int *p, int i) +{ + // Verify that -Waddress isn't issued for code from macro expansion. + return F (p[i]); // { dg-bogus "-Waddress" } +} + +#define G(x, i) ((&x) + i) + +int warn_function_macro_expansion (int *p, int i) +{ + /* Verify that -Waddress is issued for code involving macro expansion + where the comparison takes place outside the macro. */ + return G (*p, i) != 0; // { dg-warning "-Waddress" } +} + +#define malloc __builtin_malloc + +int warn_object_macro_expansion (int *p, int i) +{ + return malloc != 0; // { dg-warning "-Waddress" } +} 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++; +} diff --git a/gcc/testsuite/g++.dg/warn/Walways-true-2.C b/gcc/testsuite/g++.dg/warn/Walways-true-2.C index 29a80e5c113..e951e95b336 100644 --- a/gcc/testsuite/g++.dg/warn/Walways-true-2.C +++ b/gcc/testsuite/g++.dg/warn/Walways-true-2.C @@ -9,7 +9,7 @@ extern int foo (int) __attribute__ ((weak)); -int i __attribute__ ((weak)); +extern int i __attribute__ ((weak)); void bar (int a) diff --git a/gcc/testsuite/gcc.dg/Walways-true-2.c b/gcc/testsuite/gcc.dg/Walways-true-2.c index 7f0bb7b10b9..ae3262b6876 100644 --- a/gcc/testsuite/gcc.dg/Walways-true-2.c +++ b/gcc/testsuite/gcc.dg/Walways-true-2.c @@ -9,7 +9,7 @@ extern int foo (int) __attribute__ ((weak)); -int i __attribute__ ((weak)); +extern int i __attribute__ ((weak)); void bar (int a) diff --git a/gcc/testsuite/gcc.dg/weak/weak-3.c b/gcc/testsuite/gcc.dg/weak/weak-3.c index a719848c935..ac85f42b34d 100644 --- a/gcc/testsuite/gcc.dg/weak/weak-3.c +++ b/gcc/testsuite/gcc.dg/weak/weak-3.c @@ -68,7 +68,9 @@ void * ffoox1g (void) { return (void *)0; } extern void * ffoo1g (void) __attribute__((weak, alias ("ffoox1g"))); void * foo1g (void) { - if (ffoo1g) + /* ffoo1g is a weak alias for a symbol defined in this file, expect + a -Waddress for the test (which is folded to true). */ + if (ffoo1g) // { dg-warning "-Waddress" } ffoo1g (); return 0; } --------------2918A1B59CCE82BD42601F55--