From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from arjuna.pair.com (arjuna.pair.com [209.68.5.131]) by sourceware.org (Postfix) with ESMTPS id 8B1EE385DC00 for ; Wed, 1 Apr 2020 23:57:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 8B1EE385DC00 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=bitrange.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=hp@bitrange.com Received: by arjuna.pair.com (Postfix, from userid 3006) id D68808A5FF; Wed, 1 Apr 2020 19:57:32 -0400 (EDT) Received: from localhost (localhost [127.0.0.1]) by arjuna.pair.com (Postfix) with ESMTP id D5AEC8A3A5; Wed, 1 Apr 2020 19:57:32 -0400 (EDT) Date: Wed, 1 Apr 2020 19:57:32 -0400 (EDT) From: Hans-Peter Nilsson X-X-Sender: hp@arjuna.pair.com To: =?UTF-8?Q?Martin_Li=C5=A1ka?= cc: GCC Patches , binutils@sourceware.org Subject: Re: [PATCH] PR lto/94249: Correct endianness detection with the __BYTE_ORDER macro In-Reply-To: <5d567802-b94a-7078-7165-e5b2eadf122e@suse.cz> Message-ID: References: <20200323103505.GF2156@tucnak> <6313e487-6dbb-ac17-4160-4ac600af40be@suse.cz> <7369b1aa-be0d-92cc-4f81-1612f101e2e8@suse.cz> <3786da05-1530-38c5-e9e2-cd69418cd42a@suse.cz> <5b27738a-9885-9906-0c93-888daf4a066f@suse.cz> <20200324083109.GP2156@tucnak> <20200324091805.GQ2156@tucnak> <5d567802-b94a-7078-7165-e5b2eadf122e@suse.cz> User-Agent: Alpine 2.20.16 (BSF 172 2016-09-29) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-11.6 required=5.0 tests=BAYES_00, GIT_PATCH_2, GIT_PATCH_3, KAM_DMARC_STATUS, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=unavailable autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 01 Apr 2020 23:57:35 -0000 On Wed, 1 Apr 2020, Martin Li?ka wrote: > On 4/1/20 7:01 AM, Hans-Peter Nilsson wrote: > > On Tue, 31 Mar 2020, Maciej W. Rozycki wrote: > > > Correct an issue with GCC commit 906b3eb9df6c ("Improve endianess > > > detection.") and fix a typo in the __BYTE_ORDER fallback macro check > > > that causes compilation errors like: > > > > > > .../include/plugin-api.h:162:2: error: #error "Could not detect > > > architecture endianess" > > > > > > on systems that do not provide the __BYTE_ORDER__ macro. > > > > > Index: binutils/include/plugin-api.h > > > =================================================================== > > > --- binutils.orig/include/plugin-api.h > > > +++ binutils/include/plugin-api.h > > > @@ -51,7 +51,7 @@ > > > /* Older GCC releases (<4.6.0) can make detection from glibc macros. */ > > > #if defined(__GLIBC__) || defined(__GNU_LIBRARY__) || > > > defined(__ANDROID__) > > > #include > > > -#ifdef _BYTE_ORDER > > > +#ifdef __BYTE_ORDER > > > #if __BYTE_ORDER == __LITTLE_ENDIAN > > > #define PLUGIN_LITTLE_ENDIAN 1 > > > #elif __BYTE_ORDER == __BIG_ENDIAN > > > > FWIW, I was about to commit that as obvious, also the bignum.h > > inclusion thing! > > > > The only question being, how the typo passed any kind of testing > > in the first place... > > Because I don't have a legacy system with an ancient glibc version. Before anyone starts doing this too: NO. That's just not valid procedure and not a valid excuse. Please test your patches more carefully. *Ask* for it to be tested if you can't find a way to test it on your own. I see you didn't even say that you didn't test those lines, when the patch was submitted. > Note that testing matrix for such a change is massive, including such > exotic targets like SUN, minix, Windows, ... Not necessary, you just have to cover those lines for *one* host system. Such situations can usually even be worked around to make sure those lines are tested at least once, something like "to test this on my current glibc system, I temporarily edited plugins-api.h, putting an #undef __BYTE_ORDER" just after the 'Older GCC' comment". Another solution would be to disable plugins for such legacy systems. > > No actually, there's also the question > > why the plugin-API needs to bother with host endianness. It's > > not like endians are going to be different between plugins and > > gcc on host. > > No, the plugin endianess matches with a host compiler endianess. If that's true then it could be just written and read as-is, unless you do different pickling on the way in, but if so, that could be fixed cleaner than writing differently than reading. ...oh, I see there's a hack; there's an assumption that there was padding with the "old" API and abusing that to add fields for a "new" API, and using the endianness to indicate the location of the padding. Ew. I'm not going to step closer than that. BTW, was this old/new plugin-API support tested *cross*-versions? brgds, H-P