From patchwork Tue Dec 3 22:01:44 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Carlos O'Donell X-Patchwork-Id: 36486 Received: (qmail 54742 invoked by alias); 3 Dec 2019 22:01:51 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 54718 invoked by uid 89); 3 Dec 2019 22:01:50 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-17.1 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT autolearn=ham version=3.3.1 spammy=H*f:sk:875zixq, H*i:sk:875zixq, H*MI:sk:875zixq, HX-Languages-Length:2437 X-HELO: us-smtp-1.mimecast.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1575410508; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=TExjbtbvQG3QccK9IcqFzGxTY0RVM3o3O+DRuHZbjwk=; b=Vq08XnWxDf6gvJi3+Crb/diKRQvn2mpVAmsffZ0+HMRuULvZXCF0nyQvMJL1ZG+MFJMqYw g78wALcio5jdqP2ir+Z787vMfJjL37hFNF5Ws4rKwLkbz3ykM/7bbzrhz040CsJV7+0gR4 +acKm4ANOqjcrd6/OAM8zmwf9oGeI3A= Return-Path: Subject: [PATCH v2] Fix build with -march=486 -Os -DNDEBUG (Bug 25240) To: Florian Weimer Cc: libc-alpha References: <9e41bfdd-c2e6-9fcd-9576-d485aba0d401@redhat.com> <875zixqe7d.fsf@oldenburg2.str.redhat.com> From: Carlos O'Donell Message-ID: <7f4d3a93-e140-5125-5247-52b86dcb6247@redhat.com> Date: Tue, 3 Dec 2019 17:01:44 -0500 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: <875zixqe7d.fsf@oldenburg2.str.redhat.com> X-Mimecast-Spam-Score: 0 On 12/3/19 4:41 PM, Florian Weimer wrote: > * Carlos O'Donell: > >> While reviewing bug 25240 I found that -march=486 -Os doesn't build >> with gcc 9.2.1 because of this problem. Fixing is straight forward, >> but I'm not sure if undefining NDEBUG like this is the best solution. >> I wonder if there isn't a pragma we might use here? Thoughts? > > -march=i486? Sorry, yes. > Where does this NDEBUG come from, exactly? Sorry this is confusing, let me rewrite the commit message since I see what I wrote was confusing. In this case it comes directly from CFLAGS, and CXXFLAGS, and CPPFLAGS used to build glibc for as small a size as possible. Neither the compiler nor the headers inject it for -Os, rather I do in order to get as small a glibc as possible. I wrote the commit message rather vaguely and in the abstract sense that anything might inject a -DNDEBUG, but let me be concrete here. Since we don't build Fedora with -DNDEBUG anymore we don't see these issues, but -marhc=i486 -Os -DNDEBUG builds do (small size). How is this v2 with a better commit message? 8< --- 8< --- 8< In a -march=486 -Os build gcc 9.2.1 issues the following error: tst-assert-c++.cc: In function ‘int do_test()’: tst-assert-c++.cc:66:12: error: unused variable ‘value’ [-Werror=unused-variable] 66 | no_int value; | ^~~~~ tst-assert-c++.cc:71:18: error: unused variable ‘value’ [-Werror=unused-variable] 71 | bool_and_int value; | ^~~~~ The assert has been disabled by building glibc with CFLAGS, CXXFLAGS, and CPPFLAGS with -DNDEBUG which removes the assert (minimum size) and leaves the value unused. We never want the assert disabled because that's the point of the test, so we undefine NDEBUG before including assert.h to ensure that we get assert defined correctly. --- assert/tst-assert-c++.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/assert/tst-assert-c++.cc b/assert/tst-assert-c++.cc index 41cb487512..a175f5e961 100644 --- a/assert/tst-assert-c++.cc +++ b/assert/tst-assert-c++.cc @@ -16,6 +16,9 @@ License along with the GNU C Library; if not, see . */ +/* We do not want the compiler or any other pre-included header from + removing the assert we want to test, so undefine NDEBUG right now. */ +#undef NDEBUG #include /* The C++ standard requires that if the assert argument is a constant