From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id AFC19385B83B for ; Wed, 17 Nov 2021 18:31:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org AFC19385B83B Received: from mail-qt1-f197.google.com (mail-qt1-f197.google.com [209.85.160.197]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-398-2gIgOUVUMv-D4hgjhhTXEA-1; Wed, 17 Nov 2021 13:31:26 -0500 X-MC-Unique: 2gIgOUVUMv-D4hgjhhTXEA-1 Received: by mail-qt1-f197.google.com with SMTP id u14-20020a05622a198e00b002b2f35a6dcfso2557385qtc.21 for ; Wed, 17 Nov 2021 10:31:26 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=94PNcVJMmec5xV3CLfJ1S1LWtSMAHbjuaVUi1jySc9Y=; b=aVD0psLY0r+rALAA143M0lNl7mcXqW0JzmJmU0uAdXYFjxqCW3HzJ0PldIx3Ju+joJ sACBD1KMA+FEmdIIO5aZQdBphDYRqtiMVmlv5BrwfvBSNjf7RkkP8w7/aYmjijSvKkAx ot4QggunowrUK+XtgMD+0Ngj02VPYWhrDdAY5bLDM5OYz9Yltju3HUuW4VNBf0Gb0ypP 19sz7oqLvyQPb12aHzbaUZWFyyj75O/fbZMopyyeBoUg6x+m6ySQfaHCjdDYiUiYxfet vDhR87W778pMI4txxr+KVYLiQzl/SQm3+LlveUz6FXXfa3Kp2OaZDoUoWPhUAjEkMMmY 7Q9w== X-Gm-Message-State: AOAM53345KcGGXYnnEy4jnHj28DnMwz5jNon54UQp+LoMcKm2Rz0UYpJ 6iknLdjdturln5gs0K9rMuBd/1Ybtm2AS4Lrbq0WbYZnhgARHWMHDBwqxhyi0w+SGyk59zggx6A owR2uudMgHjOPdoRNkw== X-Received: by 2002:a05:620a:298a:: with SMTP id r10mr14945546qkp.447.1637173885682; Wed, 17 Nov 2021 10:31:25 -0800 (PST) X-Google-Smtp-Source: ABdhPJw6t2rwvzdQ5VkvSUogc/6tX+EJoBH8qnU68/iM329tUIjVQzGaM6rIuNBgzbnfvvfYJbDLZg== X-Received: by 2002:a05:620a:298a:: with SMTP id r10mr14945516qkp.447.1637173885390; Wed, 17 Nov 2021 10:31:25 -0800 (PST) Received: from [192.168.1.149] (130-44-159-43.s15913.c3-0.arl-cbr1.sbo-arl.ma.cable.rcncustomer.com. [130.44.159.43]) by smtp.gmail.com with ESMTPSA id x17sm348636qta.66.2021.11.17.10.31.24 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 17 Nov 2021 10:31:24 -0800 (PST) Message-ID: Date: Wed, 17 Nov 2021 13:31:24 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.3.1 Subject: Re: [PATCH] restore ancient -Waddress for weak symbols [PR33925] To: Martin Sebor , "Joseph S. Myers" Cc: gcc-patches References: <679cf10f-e16a-e318-0e82-820efb109d0f@gmail.com> <24cd9565-b127-6534-d98e-7482b3dc082f@gmail.com> <75dfaeb9-b6ee-6405-69e7-72ed32ff07ae@redhat.com> From: Jason Merrill In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-6.2 required=5.0 tests=BAYES_00, BODY_8BITS, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, 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: Wed, 17 Nov 2021 18:31:29 -0000 On 11/16/21 20:11, Martin Sebor wrote: > On 11/16/21 1:23 PM, Jason Merrill wrote: >> On 10/23/21 19:06, Martin Sebor wrote: >>> 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. >> >>> +       allocated stroage might have a null address.  */ >> >> typo. >> >> OK with that fixed. > > After retesting the patch before committing I noticed it triggers > a regression in weak/weak-3.c that I missed the first time around. > Here's the test case: > > extern void * ffoo1f (void); > void * foo1f (void) > { >   if (ffoo1f) /* { dg-warning "-Waddress" } */ >     ffoo1f (); >   return 0; > } > > void * ffoox1f (void) { return (void *)0; } > extern void * ffoo1f (void)  __attribute__((weak, alias ("ffoox1f"))); > > The unexpected error is: > > a.c: At top level: > a.c:1:15: error: ‘ffoo1f’ declared weak after being used >     1 | extern void * ffoo1f (void); >       |               ^~~~~~ > > The error is caused by the new call to maybe_nonzero_address() > made from decl_with_nonnull_addr_p().  The call registers > the symbol as used. > > So unless the error is desirable for this case I think it's > best to go back to the originally proposed solution.  I attach > it for reference and will plan to commit it tomorrow unless I > hear otherwise. Hmm, the error seems correct to me: we tested whether the address is nonzero in the dg-warning line, and presumably evaluating that test could depend on the absence of weak. > PS I don't know enough about the logic behind issuing this error > in other situations to tell for sure that it's wrong in this one > but I see no difference in the emitted code for a case in the same > test that declares the alias first, before taking its address and > that's accepted and this one.  I also checked that both Clang and > ICC accept the code either way, so I'm inclined to think the error > would be a bug.