From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qt1-x82d.google.com (mail-qt1-x82d.google.com [IPv6:2607:f8b0:4864:20::82d]) by sourceware.org (Postfix) with ESMTPS id AEC6A3858402 for ; Tue, 2 Nov 2021 18:51:18 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org AEC6A3858402 Received: by mail-qt1-x82d.google.com with SMTP id s1so111994qta.13 for ; Tue, 02 Nov 2021 11:51:18 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:subject:to:cc:references:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=WPZiUVku6WMowTEJaZ3YHBDw1jISoZy6bl9IzA0tSJU=; b=0Qj+2LXd/GgZQirLBeNzsdwImppf6yVtiTKyzrzrkYoFiiWr00tzkd1gQrS1yz8LlZ JePXIemThZ5GM7VccxqzDDhbXcI1rI1cEuDrjDznFvZEvQsAUv2DaRn3BOjTv0ASdSZF xouHVvjKtWoj9uptGgdE7j8LtIqirZIPW+DJos+YdYp/avVw7YUEZQcAVNEB5Q86Iy9u oAiiaO9a4idJ2RzXkW9GPyIMrWdAf77Ko6FVZPKGkNJ7nQYta8AFeJ9ebb2IPFSdWCyd QupxvKFmhu2Vkx5SfriFAG1FpZvDzDtKHbQAxqQqV1J3vsV9qF3LS9HUkzipuN/wjjKV iQNg== X-Gm-Message-State: AOAM530ndMOJ3R6u0Rlbh+JRLvcGFfyuB+TOx5AZhMJ4BUAqf8WilD4d cnZ9nJ4fBf9KEl/OOpOi2kjm7BJ9gDk= X-Google-Smtp-Source: ABdhPJwaEspDUeenxYgN+Y1bMqtn7v1Pu6ANNeMcUJCtYRaLZyzR3WBVqdIpV9GvEN2SEjanhLtjzw== X-Received: by 2002:a05:622a:349:: with SMTP id r9mr40528524qtw.57.1635879078219; Tue, 02 Nov 2021 11:51:18 -0700 (PDT) Received: from [192.168.0.41] (184-96-250-76.hlrn.qwest.net. [184.96.250.76]) by smtp.gmail.com with ESMTPSA id v12sm9775960qkp.96.2021.11.02.11.51.17 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 02 Nov 2021 11:51:17 -0700 (PDT) From: Martin Sebor Subject: Re: [PATCH] restore ancient -Waddress for weak symbols [PR33925] To: Eric Gallager Cc: "Joseph S. Myers" , Jason Merrill , gcc-patches References: <679cf10f-e16a-e318-0e82-820efb109d0f@gmail.com> Message-ID: Date: Tue, 2 Nov 2021 12:51:16 -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: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-5.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, 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: Tue, 02 Nov 2021 18:51:20 -0000 On 10/4/21 4:39 PM, Eric Gallager wrote: > On Mon, Oct 4, 2021 at 2:43 PM Martin Sebor via Gcc-patches > 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. >> >> 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. > > Would it make sense to use the same names as clang's flags here, too, > instead of dumping them all under -Waddress? I think the additional > granularity could be helpful for people who only want some warnings, > but not others. In general I agree. In this case I'm not sure. The options that control these warnings in neither compiler make perfect sense to me. Here's a breakdown of the cases: Clang GCC array == array -Wtautological-compare -Warray-compare &decl == null -Wtautological-pointer-compare -Waddress &decl1 == &decl2 N/A N/A GCC has recently diverged from Clang by introducing the new -Warray-compare option, and we don't have -Wtautological-pointer-compare. So while I think it makes sense to use the same names for new features as those they are controlled by in Clang, the argument to do the same for simple enhancements to existing features is quite a bit less compelling. We'd likely end up diagnosing different subsets of the same problem under different options. Martin > >> 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