From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-vs1-xe2a.google.com (mail-vs1-xe2a.google.com [IPv6:2607:f8b0:4864:20::e2a]) by sourceware.org (Postfix) with ESMTPS id CC7A238582A9 for ; Mon, 1 Aug 2022 20:04:43 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org CC7A238582A9 Received: by mail-vs1-xe2a.google.com with SMTP id 125so12446833vsx.7 for ; Mon, 01 Aug 2022 13:04:43 -0700 (PDT) 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:references:from:organization:in-reply-to :content-transfer-encoding; bh=qcCQd7RhiyQrtvcYLU6IrLhGo9MyB7onDFkJxgdoA4Y=; b=gRpm3WBr7jwUt7ZadsoS/GRJ3QOcHusOF1iyiVUTWDp3P+QuUH4LGBNlhldBwirJz/ /2VqDG6WeYo7h4Zj1ET3URz8lKnWgUL2SkyvFNmCcsSi6ySGNLcgPgs5CgiapEnSUYjN qdy/6JsqDvd0y4DrFtUHwtBUQXTCEvsP/RTf6FWujd99F23PyqidHm4vnl6gH9P390KR aSQHNUyCTzP+qnaYm/Th1IDORv6KyNOkHtH5gZqslRneDylAni+sM53bv6sl/HYBTcJR 9mnK1rUIc8UUizU9dXWrmfmuyW5FvmVWN/vIfSl44mdXqD9Ytw4VsVD6vbRuyOVcWw3C z9ew== X-Gm-Message-State: AJIora8bmQODXO5qK/Rfhis9ZAwi4W2DOLmZmNukdXzZ3uWAAo7VS5IY TE3L1dgFhQ0YsTvShZB1SCjKlA== X-Google-Smtp-Source: AGRyM1uQhP3ZePt5SOmka0UOom7d21MASPK8f2tueTwgAP3fKfsRvlhvVlnrjF640XPjfspkQQPyFA== X-Received: by 2002:a05:6102:1517:b0:358:4b7a:70d4 with SMTP id f23-20020a056102151700b003584b7a70d4mr7094577vsv.19.1659384283033; Mon, 01 Aug 2022 13:04:43 -0700 (PDT) Received: from ?IPV6:2804:431:c7cb:1e34:a813:f55c:8e00:64cd? ([2804:431:c7cb:1e34:a813:f55c:8e00:64cd]) by smtp.gmail.com with ESMTPSA id r1-20020a67cd81000000b00386bc1fbfb9sm1301125vsl.17.2022.08.01.13.04.41 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 01 Aug 2022 13:04:42 -0700 (PDT) Message-ID: Date: Mon, 1 Aug 2022 17:04:40 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.1.0 Subject: Re: [PATCH 3/3] assert: Do not use stderr in libc-internal assert Content-Language: en-US To: Florian Weimer , libc-alpha@sourceware.org References: <32183839aed39d547a7ebbd50bbcd5575746a233.1659350650.git.fweimer@redhat.com> From: Adhemerval Zanella Netto Organization: Linaro In-Reply-To: <32183839aed39d547a7ebbd50bbcd5575746a233.1659350650.git.fweimer@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 01 Aug 2022 20:04:46 -0000 On 01/08/22 07:45, Florian Weimer via Libc-alpha wrote: > Redirect internal assertion failures to __libc_assert_fail, based on > based on __libc_message, which writes directly to STDERR_FILENO > and calls abort. Also disable message translation and reword the > error message slightly (adjusting stdlib/tst-bz20544 accordingly). > > As a result of these changes, malloc no longer needs its own > redefinition of __assert_fail. > > __libc_assert_fail needs to be stubbed out during rtld dependency > analysis because the rtld rebuilds turn __libc_assert_fail into > __assert_fail, which is unconditionally provided by elf/dl-minimal.c. > > This change is not possible for the public assert macro and its > __assert_fail function because POSIX requires that the diagnostic > is written to stderr. LGTM, thanks. Reviewed-by: Adhemerval Zanella > --- > assert/Makefile | 7 ++++++- > assert/__libc_assert_fail.c | 33 +++++++++++++++++++++++++++++++++ > assert/assert.c | 1 - > elf/Makefile | 1 + > include/assert.h | 12 +++++++++--- > malloc/malloc.c | 16 ---------------- > stdlib/tst-bz20544.c | 2 +- > 7 files changed, 50 insertions(+), 22 deletions(-) > create mode 100644 assert/__libc_assert_fail.c > > diff --git a/assert/Makefile b/assert/Makefile > index f7cf8e9b77..c905845a43 100644 > --- a/assert/Makefile > +++ b/assert/Makefile > @@ -24,7 +24,12 @@ include ../Makeconfig > > headers := assert.h > > -routines := assert assert-perr __assert > +routines := \ > + __assert \ > + __libc_assert_fail \ > + assert \ > + assert-perr \ > + # routines > tests := test-assert test-assert-perr tst-assert-c++ tst-assert-g++ > > ifeq ($(have-cxx-thread_local),yes) Ok. > diff --git a/assert/__libc_assert_fail.c b/assert/__libc_assert_fail.c > new file mode 100644 > index 0000000000..149d5feae1 > --- /dev/null > +++ b/assert/__libc_assert_fail.c > @@ -0,0 +1,33 @@ > +/* libc-internal assert that calls __libc_message. > + Copyright (C) 2022 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + . */ > + > +#include <_itoa.h> > +#include > +#include > +#include > + > +void > +__libc_assert_fail (const char *assertion, const char *file, unsigned int line, > + const char *function) > +{ > + char linebuf[INT_BUFSIZE_BOUND (unsigned int)]; > + array_end (linebuf)[-1] = '\0'; > + char *linestr = _itoa_word (line, array_end (linebuf) - 1, 10, 0); > + __libc_message ("Fatal glibc error: %s:%s (%s): assertion failed: %s\n", > + file, linestr, function, assertion); > +} Ok. > diff --git a/assert/assert.c b/assert/assert.c > index 133a183bc3..564ae28a2b 100644 > --- a/assert/assert.c > +++ b/assert/assert.c > @@ -101,4 +101,3 @@ __assert_fail (const char *assertion, const char *file, unsigned int line, > __assert_fail_base (_("%s%s%s:%u: %s%sAssertion `%s' failed.\n%n"), > assertion, file, line, function); > } > -hidden_def(__assert_fail) Ok. > diff --git a/elf/Makefile b/elf/Makefile > index fd77d0c7c8..3386f0ce77 100644 > --- a/elf/Makefile > +++ b/elf/Makefile > @@ -1283,6 +1283,7 @@ $(objpfx)dl-allobjs.os: $(all-rtld-routines:%=$(objpfx)%.os) > rtld-stubbed-symbols = \ > __GI___pthread_disable_asynccancel \ > __GI___pthread_enable_asynccancel \ > + __libc_assert_fail \ > __pthread_disable_asynccancel \ > __pthread_enable_asynccancel \ > calloc \ Ok. > diff --git a/include/assert.h b/include/assert.h > index 61cc8aa22f..c812808f9b 100644 > --- a/include/assert.h > +++ b/include/assert.h > @@ -20,8 +20,14 @@ extern void __assert_fail_base (const char *fmt, const char *assertion, > const char *function) > __THROW __attribute__ ((__noreturn__)) attribute_hidden; > > -# if IS_IN (libc) || (IS_IN (rtld) && !defined NO_RTLD_HIDDEN) > -hidden_proto (__assert_fail) > -hidden_proto (__assert_perror_fail) > +rtld_hidden_proto (__assert_fail) > +rtld_hidden_proto (__assert_perror_fail) > +libc_hidden_proto (__assert_perror_fail) > + > + > +# if IS_IN (libc) > +/* Redirect to the internal version which does not use stderr. */ > +extern _Noreturn __typeof (__assert_fail) __libc_assert_fail attribute_hidden; > +# define __assert_fail __libc_assert_fail > # endif > #endif Ok. > diff --git a/malloc/malloc.c b/malloc/malloc.c > index f3320d2663..6f9c3b59b9 100644 > --- a/malloc/malloc.c > +++ b/malloc/malloc.c > @@ -287,22 +287,6 @@ > #define MALLOC_DEBUG 0 > #endif > > -#if IS_IN (libc) > -#ifndef NDEBUG > -# define __assert_fail(assertion, file, line, function) \ > - __malloc_assert(assertion, file, line, function) > - > -_Noreturn static void > -__malloc_assert (const char *assertion, const char *file, unsigned int line, > - const char *function) > -{ > - __libc_message ("Fatal glibc error: malloc assertion failure in %s: %s\n", > - function, assertion); > - __builtin_unreachable (); > -} > -#endif > -#endif > - > #if USE_TCACHE > /* We want 64 entries. This is an arbitrary limit, which tunables can reduce. */ > # define TCACHE_MAX_BINS 64 Ok. > diff --git a/stdlib/tst-bz20544.c b/stdlib/tst-bz20544.c > index 411cd3f3ba..7cc236a1b1 100644 > --- a/stdlib/tst-bz20544.c > +++ b/stdlib/tst-bz20544.c > @@ -78,7 +78,7 @@ test_bz20544_cxa_at_quick_exit (void *closure) > static void > test_one_fn (void (*test_fn) (void *)) > { > - const char expected_error[] = "Assertion `func != NULL' failed.\n"; > + const char expected_error[] = "assertion failed: func != NULL\n"; > struct support_capture_subprocess result; > result = support_capture_subprocess (test_fn, NULL); > support_capture_subprocess_check (&result, "bz20544", -SIGABRT, Ok.