From patchwork Fri Nov 29 11:38:11 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 36379 Received: (qmail 44584 invoked by alias); 29 Nov 2019 11:38:23 -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 44568 invoked by uid 89); 29 Nov 2019 11:38:23 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-18.5 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_PASS autolearn=ham version=3.3.1 spammy= X-HELO: us-smtp-delivery-1.mimecast.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1575027499; 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=7rZ1AFY+oJCSGHYdnjMU4/nOIYUXOkZOCNa71vB4QFU=; b=NzMEfAVd/kGFg5NbJBP1LvUqCWruPCikm/SUZmY40i9+FG5SQVtfNqrVKH5iqBBL+/oK3+ ilfUEDQlONa8Eo5gfdZZP9BGBYh6dFulvDdWiddedEQljW+yYH7ksJubRWzTssftyrIXVz Yjczf2rUYJTaMqbT90bk4O6afVDS3Mk= From: Florian Weimer To: Jonathan Wakely Cc: kamlesh kumar , libc-alpha@sourceware.org, Andreas Schwab , Brooks Moses Subject: Re: [PATCH] For Adding clang check References: <20191128184733.27377-1-kamleshbhalui@gmail.com> <87tv6n7s0t.fsf@mid.deneb.enyo.de> <87a78fay23.fsf@mid.deneb.enyo.de> <20191129105759.GX11522@redhat.com> Date: Fri, 29 Nov 2019 12:38:11 +0100 In-Reply-To: <20191129105759.GX11522@redhat.com> (Jonathan Wakely's message of "Fri, 29 Nov 2019 10:57:59 +0000") Message-ID: <87fti63obg.fsf@oldenburg2.str.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.2 (gnu/linux) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 * Jonathan Wakely: > On 29/11/19 09:25 +0100, Florian Weimer wrote: >>* kamlesh kumar: >> >>> It fixes this. >>> https://bugs.llvm.org/show_bug.cgi?id=44169 >> >>What's the rationale for the condition? What's special about Clang >>3.5? >> >>My understanding is that a compiler needs support for asm aliases >>*and* the C++ library headers need to be compatible. Is there a way >>to determine if libc++ is compatible? > > Libc++ already checks for this macro in its wrapper: > https://github.com/llvm/llvm-project/blob/master/libcxx/include/string.h#L62 > > If the __CORRECT_ISO_CPP_STRING_H_PROTO macro is *not* defined after > doing #include_next (to get the libc header) then libc++ > makes use of a Clang extension to declare new overloads: > https://github.com/llvm/llvm-project/blob/master/libcxx/include/string.h#L70 > > The _LIBCPP_PREFERRED_OVERLOAD macro is defined as: > # define _LIBCPP_PREFERRED_OVERLOAD __attribute__ ((__enable_if__(true, ""))) > > That Clang-only attribute means the compiler will use the new > overloads in preference to the libc strchr. A non-standard extension > is needed because according to the C++ rules the new overloads should > be ambiguous with the one that was declared by libc's . > > >>For libstdc++ with GCC, the >>compiler version check covers libstdc++ implicity, but that does not >>apply to Clang, or libc++ with either compiler. > > Libstdc++ with GCC already works. > > Libstdc++ with Clang needs this patch. > > If I'm reading the libc++ code right ... > > Libc++ with GCC should already work, because the __GNUC_PREREQ will > pass and libc++ is already aware of the existence and effects of the > __CORRECT_ISO_CPP_STRING_H_PROTO macro. (I doubt libc++ works with > ancient GCC versions, but if it does they'll get the wrong signatures > ... well tough luck, use a newer GCC). > > Libc++ with Clang doesn't need this patch, because it uses the Clang > extension, but after this patch it would no longer need to use the > extension. The right signatures would be declared by glibc. > > So of the four combinations, two already work and are unaffected by > this patch. One already works and is affected, but not in a way users > will notice (the correct signatures are already there, the patch just > changes whether they come from glibc or libc++). And one doesn't > currently work but is fixed by the patch. > > I think the patch is right. Thanks. I filed: I tried to distill the discussion into the patch below. Florian 8<------------------------------------------------------------------8< From: Kamlesh Kumar Subject: : Define __CORRECT_ISO_CPP_STRING_H_PROTO for Clang [BZ #25232] Without the asm redirects, strchr et al. are not const-correct. libc++ has a wrapper header that works with and without __CORRECT_ISO_CPP_STRING_H_PROTO (using a Clang extension). But when Clang is used with libstdc++ or just C headers, the overloaded functions with the correct types are not declared. This change does not impact current GCC (with libstdc++ or libc++). ----- string/string.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/string/string.h b/string/string.h index 73c22a535a..faf997b972 100644 --- a/string/string.h +++ b/string/string.h @@ -33,7 +33,8 @@ __BEGIN_DECLS #include /* Tell the caller that we provide correct C++ prototypes. */ -#if defined __cplusplus && __GNUC_PREREQ (4, 4) +#if defined __cplusplus && (__GNUC_PREREQ (4, 4) \ + || __glibc_clang_prereq (3, 5)) # define __CORRECT_ISO_CPP_STRING_H_PROTO #endif