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 <endian.h>
> > > -#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
