perl-5.28.0 fails to build on Solaris 10

p5pRT commented 6 years ago

Migrated from rt.perl.org#133495 (status was 'pending release')

Searchable as RT133495$

p5pRT commented 6 years ago

From hako@affrc.go.jp

Created by hako@affrc.go.jp

perl-5.28.0 fails to build on Solaris 10. I send this bug report with an advice from Benny\, gnats-bugs@netbsd.org. Thanks in advance.

Hiroshi Hakoyama

The following reply was made to PR pkg/53568; it has been noted by GNATS.

From: Benny Siegert \bsiegert@gmail\.com To: gnats-bugs@netbsd.org Cc: pkg-manager@netbsd.org\, gnats-admin@netbsd.org\, pkgsrc-bugs@netbsd.org Subject: Re: pkg/53568: perl-5.28.0 fails to build on Solaris 10 Date: Sun\, 2 Sep 2018 16:06:27 +0000

perl-5.28.0 fails to build on Solaris 10. perl-5.26.2 was OK.

... gcc -c -DPERL_CORE -D_REENTRANT -O3 -mcpu=3Dultrasparc3 -mtune=3Dultraspa= rc3 -D_FORTIFY_SOURCE=3D2 -pthread -I/usr/include -fwrapv -fno-strict-alias= ing -pipe -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=3D64= -DPERL_USE_SAFE_PUTENV -O3 -mcpu=3Dultrasparc3 -mtune=3Dultrasparc3 -D_FOR= TIFY_SOURCE=3D2 -pthread -I/usr/include -Wall -Werror=3Ddeclaration-after-s= tatement -Werror=3Dpointer-arith -Wextra -Wc++-compat -Wwrite-strings -fPIC= miniperlmain.c gcc -pthread -L/usr/lib -Wl\,-R/usr/lib -Wl\,-R/usr/pkg/lib -o miniperl op= mini.o perlmini.o gv.o toke.o perly.o pad.o regcomp.o dump.o util.o mg.o r= eentr.o mro_core.o keywords.o hv.o av.o run.o pp_hot.o sv.o pp.o scope.o pp= _ctl.o pp_sys.o doop.o doio.o regexec.o utf8.o taint.o deb.o universal.o gl= obals.o perlio.o perlapi.o numeric.o mathoms.o locale.o pp_pack.o pp_sort.o= caretx.o dquote.o time64.o miniperlmain.o -lm -ldl -lsocket -lnsl -lpthr= ead -lrt LD_LIBRARY_PATH=3D/usr/pkgsrc/lang/perl5/work/perl-5.28.0 ./miniperl -w -= Ilib -Idist/Exporter/lib -MExporter -e '\<?>' || sh -c 'echo >&2 Failed to b= uild miniperl. Please run make minitest; exit 1' /usr/bin/bash: line 1: 28934 Bus Error (core dumped) LD_LIB= RARY_PATH=3D/usr/pkgsrc/lang/perl5/work/perl-5.28.0 ./miniperl -w -Ilib -Id= ist/Exporter/lib -MExporter -e '\<?>' Failed to build miniperl. Please run make minitest *** Error code 1

This sounds like an upstream regression. Please file a bug report upstream (bugs.perl.org) and send a link to this list.

--=20 Benny

Perl Info

``` Flags: category=core severity=medium Site configuration information for perl 5.26.2: Configured by hako at Tue May 1 10:08:44 JST 2018. Summary of my perl5 (revision 5 version 26 subversion 2) configuration: Platform: osname=solaris osvers=2.10 archname=sparc-solaris-thread-multi uname='sunos ec21 5.10 generic_147147-26 sun4u sparc sunw,sun-blade-1000 ' config_args='-sde -Dldflags= -pthread -L/usr/lib -Wl,-R/usr/lib -Wl,-R/usr/pkg/lib -Duseshrplib -Duseithreads -Uusemymalloc' hint=recommended useposix=true d_sigaction=define useithreads=define usemultiplicity=define use64bitint=undef use64bitall=undef uselongdouble=undef usemymalloc=n default_inc_excludes_dot=define bincompat5005=undef Compiler: cc='gcc' ccflags ='-D_REENTRANT -O3 -mcpu=ultrasparc3 -mtune=ultrasparc3 -D_FORTIFY_SOURCE=2 -pthread -I/usr/include -fwrapv -fno-strict-aliasing -pipe -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -DPERL_USE_SAFE_PUTENV' optimize=' -O3 -mcpu=ultrasparc3 -mtune=ultrasparc3 -D_FORTIFY_SOURCE=2 -pthread -I/usr/include' cppflags='-D_REENTRANT -O3 -mcpu=ultrasparc3 -mtune=ultrasparc3 -D_FORTIFY_SOURCE=2 -pthread -I/usr/include -fwrapv -fno-strict-aliasing -pipe -I/usr/local/include' ccversion='' gccversion='4.9.4' gccosandvers='' intsize=4 longsize=4 ptrsize=4 doublesize=8 byteorder=4321 doublekind=4 d_longlong=define longlongsize=8 d_longdbl=define longdblsize=16 longdblkind=2 ivtype='long' ivsize=4 nvtype='double' nvsize=8 Off_t='off_t' lseeksize=8 alignbytes=8 prototype=define Linker and Libraries: ld='gcc' ldflags ='-pthread -L/usr/lib -Wl,-R/usr/lib -Wl,-R/usr/pkg/lib' libpth=/usr/lib /usr/ccs/lib /usr/local/lib libs=-lm -ldl -lsocket -lnsl -lpthread -lrt perllibs=-lm -ldl -lsocket -lnsl -lpthread -lrt libc=/lib/libc.so so=so useshrplib=true libperl=libperl.so gnulibc_version='' Dynamic Linking: dlsrc=dl_dlopen.xs dlext=so d_dlsymun=undef ccdlflags=' -R /usr/pkg/lib/perl5/5.26.0/sparc-solaris-thread-multi/CORE' cccdlflags='-fPIC' lddlflags='-G -L/usr/lib -Wl,-R/usr/lib -Wl,-R/usr/pkg/lib -L/usr/local/lib' @INC for perl 5.26.2: /usr/pkg/lib/perl5/site_perl/5.26.0/sparc-solaris-thread-multi /usr/pkg/lib/perl5/site_perl/5.26.0 /usr/pkg/lib/perl5/vendor_perl/5.26.0/sparc-solaris-thread-multi /usr/pkg/lib/perl5/vendor_perl/5.26.0 /usr/pkg/lib/perl5/5.26.0/sparc-solaris-thread-multi /usr/pkg/lib/perl5/5.26.0 Environment for perl 5.26.2: HOME=/export/home/hako LANG=ja LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/usr/ccs/bin:/usr/pkg/gcc49/bin:/usr/pkg/bin:/usr/pkg/sbin:/usr/local/bin:/usr/local/sbin:/usr/sunvts/bin:/usr/bin:/usr/sbin:/usr/sadm/admin/bin:/usr/sadm/bin:/bin:/sbin:/usr/openwin/bin PERL_BADLANG (unset) SHELL=/usr/bin/tcsh ```

p5pRT commented 6 years ago

From @jkeenan

On Mon\, 03 Sep 2018 00:00:45 GMT\, hako@affrc.go.jp wrote:

This is a bug report for perl from hako@affrc.go.jp\, generated with the help of perlbug 1.40 running under perl 5.26.2.

----------------------------------------------------------------- [Please describe your issue here] perl-5.28.0 fails to build on Solaris 10. I send this bug report with an advice from Benny\, gnats- bugs@netbsd.org. Thanks in advance.

Hiroshi Hakoyama

The following reply was made to PR pkg/53568; it has been noted by GNATS.

From: Benny Siegert \bsiegert@gmail\.com To: gnats-bugs@netbsd.org Cc: pkg-manager@netbsd.org\, gnats-admin@netbsd.org\, pkgsrc- bugs@netbsd.org Subject: Re: pkg/53568: perl-5.28.0 fails to build on Solaris 10 Date: Sun\, 2 Sep 2018 16:06:27 +0000

perl-5.28.0 fails to build on Solaris 10. perl-5.26.2 was OK.

... gcc -c -DPERL_CORE -D_REENTRANT -O3 -mcpu=3Dultrasparc3 -mtune=3Dultraspa= rc3 -D_FORTIFY_SOURCE=3D2 -pthread -I/usr/include -fwrapv -fno-strict- alias= ing -pipe -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=3D64= -DPERL_USE_SAFE_PUTENV -O3 -mcpu=3Dultrasparc3 -mtune=3Dultrasparc3 -D_FOR= TIFY_SOURCE=3D2 -pthread -I/usr/include -Wall -Werror=3Ddeclaration- after-s= tatement -Werror=3Dpointer-arith -Wextra -Wc++-compat -Wwrite-strings -fPIC= miniperlmain.c gcc -pthread -L/usr/lib -Wl\,-R/usr/lib -Wl\,-R/usr/pkg/lib -o miniperl op= mini.o perlmini.o gv.o toke.o perly.o pad.o regcomp.o dump.o util.o mg.o r= eentr.o mro_core.o keywords.o hv.o av.o run.o pp_hot.o sv.o pp.o scope.o pp= _ctl.o pp_sys.o doop.o doio.o regexec.o utf8.o taint.o deb.o universal.o gl= obals.o perlio.o perlapi.o numeric.o mathoms.o locale.o pp_pack.o pp_sort.o= caretx.o dquote.o time64.o miniperlmain.o -lm -ldl -lsocket -lnsl -lpthr= ead -lrt LD_LIBRARY_PATH=3D/usr/pkgsrc/lang/perl5/work/perl-5.28.0 ./miniperl -w -= Ilib -Idist/Exporter/lib -MExporter -e '\<?>' || sh -c 'echo >&2 Failed to b= uild miniperl. Please run make minitest; exit 1' /usr/bin/bash: line 1: 28934 Bus Error (core dumped) LD_LIB= RARY_PATH=3D/usr/pkgsrc/lang/perl5/work/perl-5.28.0 ./miniperl -w -Ilib -Id= ist/Exporter/lib -MExporter -e '\<?>' Failed to build miniperl. Please run make minitest *** Error code 1

This sounds like an upstream regression. Please file a bug report upstream (bugs.perl.org) and send a link to this list.

--=20 Benny

We have to concede that this report is prima facie plausible\, because we don't have any smoke-test reports for Perl on Solaris 2.10 since 2013 -- the perl-5.19.* development cycle (http://perl5.test-smoke.org/search; enter "Solaris" in drop-down for OS and "2.10" for OS version).

We are\, however\, receiving satisfactory smoke-test reports for Solaris 2.11 (same site; clear the OS version drop-down). So to resolve this we'd need access to a Solaris 2.10 machine.

List: any suggestions?

Thank you very much.

--- Site configuration information for perl 5.26.2:

Configured by hako at Tue May 1 10:08:44 JST 2018.

Summary of my perl5 (revision 5 version 26 subversion 2) configuration:

Platform: osname=solaris osvers=2.10 archname=sparc-solaris-thread-multi uname='sunos ec21 5.10 generic_147147-26 sun4u sparc sunw\,sun-blade- 1000 ' config_args='-sde -Dldflags= -pthread -L/usr/lib -Wl\,-R/usr/lib -Wl\,-R/usr/pkg/lib -Duseshrplib -Duseithreads -Uusemymalloc' [snip] Compiler: cc='gcc' ccflags ='-D_REENTRANT -O3 -mcpu=ultrasparc3 -mtune=ultrasparc3 -D_FORTIFY_SOURCE=2 -pthread -I/usr/include -fwrapv -fno-strict- aliasing -pipe -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -DPERL_USE_SAFE_PUTENV' optimize=' -O3 -mcpu=ultrasparc3 -mtune=ultrasparc3 -D_FORTIFY_SOURCE=2 -pthread -I/usr/include' cppflags='-D_REENTRANT -O3 -mcpu=ultrasparc3 -mtune=ultrasparc3 -D_FORTIFY_SOURCE=2 -pthread -I/usr/include -fwrapv -fno-strict- aliasing -pipe -I/usr/local/include' [snip]

-- James E Keenan (jkeenan@cpan.org)

p5pRT commented 6 years ago

The RT System itself - Status changed from 'new' to 'open'

p5pRT commented 6 years ago

From @Leont

On Mon\, Sep 3\, 2018 at 5:34 AM Hiroshi Hakoyama (via RT) \perlbug\-followup@perl\.org wrote:

# New Ticket Created by Hiroshi Hakoyama # Please include the string: [perl #133495] # in the subject line of all future correspondence about this issue. # \<URL: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=133495 >

This is a bug report for perl from hako@affrc.go.jp\, generated with the help of perlbug 1.40 running under perl 5.26.2.

----------------------------------------------------------------- [Please describe your issue here] perl-5.28.0 fails to build on Solaris 10. I send this bug report with an advice from Benny\, gnats-bugs@netbsd.org. Thanks in advance.

Hiroshi Hakoyama

The following reply was made to PR pkg/53568; it has been noted by GNATS.

From: Benny Siegert \bsiegert@gmail\.com To: gnats-bugs@netbsd.org Cc: pkg-manager@netbsd.org\, gnats-admin@netbsd.org\, pkgsrc-bugs@netbsd.org Subject: Re: pkg/53568: perl-5.28.0 fails to build on Solaris 10 Date: Sun\, 2 Sep 2018 16:06:27 +0000

perl-5.28.0 fails to build on Solaris 10. perl-5.26.2 was OK.

... gcc -c -DPERL_CORE -D_REENTRANT -O3 -mcpu=3Dultrasparc3 -mtune=3Dultraspa= rc3 -D_FORTIFY_SOURCE=3D2 -pthread -I/usr/include -fwrapv -fno-strict-alias= ing -pipe -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=3D64= -DPERL_USE_SAFE_PUTENV -O3 -mcpu=3Dultrasparc3 -mtune=3Dultrasparc3 -D_FOR= TIFY_SOURCE=3D2 -pthread -I/usr/include -Wall -Werror=3Ddeclaration-after-s= tatement -Werror=3Dpointer-arith -Wextra -Wc++-compat -Wwrite-strings -fPIC= miniperlmain.c gcc -pthread -L/usr/lib -Wl\,-R/usr/lib -Wl\,-R/usr/pkg/lib -o miniperl op= mini.o perlmini.o gv.o toke.o perly.o pad.o regcomp.o dump.o util.o mg.o r= eentr.o mro_core.o keywords.o hv.o av.o run.o pp_hot.o sv.o pp.o scope.o pp= _ctl.o pp_sys.o doop.o doio.o regexec.o utf8.o taint.o deb.o universal.o gl= obals.o perlio.o perlapi.o numeric.o mathoms.o locale.o pp_pack.o pp_sort.o= caretx.o dquote.o time64.o miniperlmain.o -lm -ldl -lsocket -lnsl -lpthr= ead -lrt LD_LIBRARY_PATH=3D/usr/pkgsrc/lang/perl5/work/perl-5.28.0 ./miniperl -w -= Ilib -Idist/Exporter/lib -MExporter -e '\<?>' || sh -c 'echo >&2 Failed to b= uild miniperl. Please run make minitest; exit 1' /usr/bin/bash: line 1: 28934 Bus Error (core dumped) LD_LIB= RARY_PATH=3D/usr/pkgsrc/lang/perl5/work/perl-5.28.0 ./miniperl -w -Ilib -Id= ist/Exporter/lib -MExporter -e '\<?>' Failed to build miniperl. Please run make minitest *** Error code 1

This sounds like an upstream regression. Please file a bug report upstream (bugs.perl.org) and send a link to this list.

--=20 Benny

Given that that is a SPARC machine\, and you're receiving a SIGBUS\, this is probably an alignment issue (and thus being sparc specific\, not solaris specific).

Can you run that miniperl command (or possibly any small one-liner) in a debugger and give us a backtrace? That should give us much more information on what's happening here.

Leon

p5pRT commented 6 years ago

From hako@affrc.go.jp

I found a workaround: A build with CFLAGS+= -O2 -mcpu=ultrasparc3 -mtune=ultrasparc3 was OK. Optimization level -O3 causes the problem.

Can you run that miniperl command (or possibly any small one-liner) in a debugger and give us a backtrace? That should give us much more information on what's happening here.

Leon

Please see the following result:

# dbx ./miniperl core For information about new features see `help changes' To remove this message\, put `dbxenv suppress_startup_message 7.6' in your .dbxrc Reading miniperl dbx: warning: unknown location expression code (0xf2) dbx: warning: unknown location expression code (0x9e) dbx: warning: unknown location expression code (0x9e) dbx: warning: unknown location expression code (0x9e) dbx: warning: unknown location expression code (0x9e) dbx: warning: unknown location expression code (0x9e) dbx: warning: unknown location expression code (0x9e) dbx: warning: unknown location expression code (0x9e) dbx: warning: unknown location expression code (0x9e) dbx: warning: unknown location expression code (0x9e) dbx: warning: unknown location expression code (0x9e) dbx: warning: unknown location expression code (0x9e) dbx: warning: unknown location expression code (0x9e) dbx: warning: unknown location expression code (0x9e) dbx: warning: unknown location expression code (0x9e) dbx: warning: unknown location expression code (0x9e) dbx: warning: unknown location expression code (0x9e) dbx: warning: unknown location expression code (0x9e) dbx: warning: unknown location expression code (0x9e) dbx: warning: unknown location expression code (0x9e) core file header read successfully Reading ld.so.1 Reading libm.so.2 Reading libdl.so.1 Reading libsocket.so.1 Reading libnsl.so.1 Reading libpthread.so.1 Reading librt.so.1 Reading libssp.so.0.0.0 Reading libc.so.1 Reading libaio.so.1 Reading libmd.so.1 Reading libgcc_s.so.1 Reading libc_psr.so.1 t@1 (l@1) program terminated by signal BUS (invalid address alignment) 0x000ecf6c: Perl_hv_common+0x156c: ld [%g3]\, %o0 (dbx) where
current thread: t@1 =>[1] Perl_hv_common(0x5b65439d\, 0x338818\, 0x0\, 0xffbffaaa\, 0x33d1e8\, 0x0)\, at 0xecf6c [2] Perl_hv_common_key_len(0x29bba8\, 0x338818\, 0xffbffaaa\, 0x19\, 0x8\, 0x0)\, at 0xee2f0 [3] S_init_postdump_symbols(0x29bba8\, 0x338818\, 0xffbffac3\, 0xffbfede0\, 0x19\, 0xffbffaaa)\, at 0x5765c [4] perl_parse(0x0\, 0x334290\, 0x339820\, 0xffbfece0\, 0x1\, 0x338758)\, at 0x5c758 [5] main(0x4\, 0xffbfecd4\, 0xffbfece8\, 0x2959b8\, 0xff170100\, 0x0)\, at 0x1bd9dc (dbx) quit dbx: internal warning: td_ta_clear_event() failed -- debugger service failed dbx: internal warning: td_ta_sync_tracking_enable(0) failed -- debugger service failed

2018/09/03 23:30、Leon Timmermans via RT \perlbug\-followup@perl\.orgのメール:

On Mon\, Sep 3\, 2018 at 5:34 AM Hiroshi Hakoyama (via RT) \perlbug\-followup@perl\.org wrote:

# New Ticket Created by Hiroshi Hakoyama # Please include the string: [perl #133495] # in the subject line of all future correspondence about this issue. # \<URL: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=133495 >

This is a bug report for perl from hako@affrc.go.jp\, generated with the help of perlbug 1.40 running under perl 5.26.2.

----------------------------------------------------------------- [Please describe your issue here] perl-5.28.0 fails to build on Solaris 10. I send this bug report with an advice from Benny\, gnats-bugs@netbsd.org. Thanks in advance.

Hiroshi Hakoyama

The following reply was made to PR pkg/53568; it has been noted by GNATS.

From: Benny Siegert \bsiegert@gmail\.com To: gnats-bugs@netbsd.org Cc: pkg-manager@netbsd.org\, gnats-admin@netbsd.org\, pkgsrc-bugs@netbsd.org Subject: Re: pkg/53568: perl-5.28.0 fails to build on Solaris 10 Date: Sun\, 2 Sep 2018 16:06:27 +0000

perl-5.28.0 fails to build on Solaris 10. perl-5.26.2 was OK.

... gcc -c -DPERL_CORE -D_REENTRANT -O3 -mcpu=3Dultrasparc3 -mtune=3Dultraspa= rc3 -D_FORTIFY_SOURCE=3D2 -pthread -I/usr/include -fwrapv -fno-strict-alias= ing -pipe -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=3D64= -DPERL_USE_SAFE_PUTENV -O3 -mcpu=3Dultrasparc3 -mtune=3Dultrasparc3 -D_FOR= TIFY_SOURCE=3D2 -pthread -I/usr/include -Wall -Werror=3Ddeclaration-after-s= tatement -Werror=3Dpointer-arith -Wextra -Wc++-compat -Wwrite-strings -fPIC= miniperlmain.c gcc -pthread -L/usr/lib -Wl\,-R/usr/lib -Wl\,-R/usr/pkg/lib -o miniperl op= mini.o perlmini.o gv.o toke.o perly.o pad.o regcomp.o dump.o util.o mg.o r= eentr.o mro_core.o keywords.o hv.o av.o run.o pp_hot.o sv.o pp.o scope.o pp= _ctl.o pp_sys.o doop.o doio.o regexec.o utf8.o taint.o deb.o universal.o gl= obals.o perlio.o perlapi.o numeric.o mathoms.o locale.o pp_pack.o pp_sort.o= caretx.o dquote.o time64.o miniperlmain.o -lm -ldl -lsocket -lnsl -lpthr= ead -lrt LD_LIBRARY_PATH=3D/usr/pkgsrc/lang/perl5/work/perl-5.28.0 ./miniperl -w -= Ilib -Idist/Exporter/lib -MExporter -e '\<?>' || sh -c 'echo >&2 Failed to b= uild miniperl. Please run make minitest; exit 1' /usr/bin/bash: line 1: 28934 Bus Error (core dumped) LD_LIB= RARY_PATH=3D/usr/pkgsrc/lang/perl5/work/perl-5.28.0 ./miniperl -w -Ilib -Id= ist/Exporter/lib -MExporter -e '\<?>' Failed to build miniperl. Please run make minitest *** Error code 1

This sounds like an upstream regression. Please file a bug report upstream (bugs.perl.org) and send a link to this list.

--=20 Benny

Given that that is a SPARC machine\, and you're receiving a SIGBUS\, this is probably an alignment issue (and thus being sparc specific\, not solaris specific).

Can you run that miniperl command (or possibly any small one-liner) in a debugger and give us a backtrace? That should give us much more information on what's happening here.

Leon

p5pRT commented 5 years ago

From grobian@gentoo.org

On Mon\, 10 Sep 2018 21:47:05 -0700\, hako@affrc.go.jp wrote:

I found a workaround: A build with CFLAGS+= -O2 -mcpu=ultrasparc3 -mtune=ultrasparc3 was OK. Optimization level -O3 causes the problem.

This didn't work for me (Sun Blade 100\, UltraSPARC IIe 500MHz\, Solaris 10\, 32-bits target)\, so I tried with CFLAGS="-g pipe". This did get me a working interpreter\, so I added back -O2 to CFLAGS leaving in the -g in order to hopefully get a better pointer at where the alignment issue is happening.

Can you run that miniperl command (or possibly any small one-liner) in a debugger and give us a backtrace? That should give us much more information on what's happening here.

Leon

% env LD_LIBRARY_PATH=/gentoo/prefix32/var/tmp/portage/dev-lang/perl-5.28.0/work/perl-5.28.0 gdb --args ./miniperl -w -Ilib -Idist/Exporter/lib -MExporter -e '\<?>' GNU gdb (Gentoo 8.2.1 p1) 8.2.1 Copyright (C) 2018 Free Software Foundation\, Inc. License GPLv3+: GNU GPL version 3 or later \<http://gnu.org/licenses/gpl.html> This is free software: you are free to change and redistribute it. There is NO WARRANTY\, to the extent permitted by law. Type "show copying" and "show warranty" for details. This GDB was configured as "sparc-sun-solaris2.10". Type "show configuration" for configuration details. For bug reporting instructions\, please see: \https://bugs.gentoo.org/. Find the GDB manual and other documentation resources online at: \<http://www.gnu.org/software/gdb/documentation/>.

For help\, type "help". Type "apropos word" to search for commands related to "word"... Reading symbols from ./miniperl...done. (gdb) r Starting program: /gentoo/prefix32/var/tmp/portage/dev-lang/perl-5.28.0/work/perl-5.28.0/miniperl -w -Ilib -Idist/Exporter/lib -MExporter -e \\<\?\> [Thread debugging using libthread_db enabled] [New Thread 1 (LWP 1)]

Thread 2 received signal SIGSEGV\, Segmentation fault. [Switching to Thread 1 (LWP 1)] 0x0004562c in zaphod32_hash_with_state (key_len=29\, key=0x26d036 "dist/Exporter/lib/Exporter.pm"\, state_ch=\) at zaphod32_hash.h:280 280 v1 -= U8TO32_LE(key+0); (gdb) bt #0 0x0004562c in zaphod32_hash_with_state (key_len=29\, key=0x26d036 "dist/Exporter/lib/Exporter.pm"\, state_ch=\) at zaphod32_hash.h:280 #1 Perl_newGP (gv=\) at gv.c:206 #2 0x00046b38 in Perl_gv_init_pvn (gv=0x27bc30\, stash=0x27b5d0\, name=0x27fce2 "Debug"\, len=5\, flags=2) at gv.c:421 #3 0x00047bc0 in Perl_gv_fetchpvn_flags (nambeg=0x27fcd8 "Exporter::Debug"\, full_len=15\, flags=2\, sv_type=\) at gv.c:2454 #4 0x0004a704 in Perl_gv_fetchsv (name=0x27bc20\, flags=2\, sv_type=SVt_PV) at gv.c:1588 #5 0x00060010 in S_pending_ident () at toke.c:9059 #6 Perl_yylex () at toke.c:4847 #7 0x00076414 in Perl_yyparse (gramtype=\) at perly.c:340 #8 0x00118d14 in S_doeval_compile (gimme=\\, outside=\\, seq=\\, hh=0x0) at pp_ctl.c:3501 #9 0x0011ec90 in S_require_file (sv=\) at pp_ctl.c:4321 #10 Perl_pp_require () at pp_ctl.c:4345 #11 0x000d2780 in Perl_runops_standard () at run.c:42 #12 0x0003a7dc in Perl_call_sv (sv=0x27bae0\, flags=\) at perl.c:3021 #13 0x0003dcbc in Perl_call_list (oldscope=2\, paramList=0x266f48) at perl.c:5065 #14 0x00016de4 in S_process_special_blocks (floor=\\, fullname=\\, cv=\\, gv=\) #15 0x000317e0 in Perl_newATTRSUB_x (floor=47\, o=\\, proto=\\, attrs=\\, block=\\, o_is_gv=\) at op.c:10269 #16 0x00035e40 in Perl_utilize (aver=\\, floor=47\, version=\\, idop=\\, arg=\) at op.c:7484 #17 0x00077988 in Perl_yyparse (gramtype=\) at perly.y:329 #18 0x00042930 in S_parse_body (xsinit=\\, env=\) at perl.c:2509 #19 perl_parse (my_perl=\\, xsinit=\\, argc=\\, argv=\\, env=\) at perl.c:1803 #20 0x00012e38 in main (argc=\\, argv=\\, env=\) at miniperlmain.c:127

% gcc --version gcc (Gentoo 8.2.0-r5 p1.6) 8.2.0 Copyright (C) 2018 Free Software Foundation\, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

% ld --version GNU ld (Gentoo 2.29.1 p3) 2.29.1 Copyright (C) 2017 Free Software Foundation\, Inc. This program is free software; you may redistribute it under the terms of the GNU General Public License version 3 or (at your option) a later version. This program has absolutely no warranty.

p5pRT commented 5 years ago

From grobian@gentoo.org

On Wed\, 16 Jan 2019 01:50:32 -0800\, grobian@gentoo.org wrote:

Thread 2 received signal SIGSEGV\, Segmentation fault. [Switching to Thread 1 (LWP 1)] 0x0004562c in zaphod32_hash_with_state (key_len=29\, key=0x26d036 "dist/Exporter/lib/Exporter.pm"\, state_ch=\<optimized out>) at zaphod32_hash.h:280 280 v1 -= U8TO32_LE(key+0);

Sorry I didn't report this at my first message. I just found a bit of time to look into this.

my Configure run somehow found:

try.c: In function 'main': try.c:46:28: warning: format '%x' expects argument of type 'unsigned int'\, but argument 2 has type 'long unsigned int' [-Wformat=] printf("read failed (%x)\n"\, *up); ~^ ~~~ %lx try.c:56:29: warning: format '%x' expects argument of type 'unsigned int'\, but argument 2 has type 'long unsigned int' [-Wformat=] printf("write failed (%x)\n"\, *up); ~^ ~~~ %lx (Testing for character data alignment may crash the test. That's okay.) You can access character data pretty unalignedly.

This is obviously wrong on sparc. The check also fails on 5.26.2 but for some reason no bus-error there. I manually set d_u32align and that made the 5.28.0 build succeed.

p5pRT commented 5 years ago

From @Leont

On Wed\, Jan 16\, 2019 at 3:47 PM Fabian Groffen via RT \perlbug\-followup@perl\.org wrote:

On Wed\, 16 Jan 2019 01:50:32 -0800\, grobian@gentoo.org wrote:

Thread 2 received signal SIGSEGV\, Segmentation fault. [Switching to Thread 1 (LWP 1)] 0x0004562c in zaphod32_hash_with_state (key_len=29\, key=0x26d036 "dist/Exporter/lib/Exporter.pm"\, state_ch=\<optimized out>) at zaphod32_hash.h:280 280 v1 -= U8TO32_LE(key+0);

Sorry I didn't report this at my first message. I just found a bit of time to look into this.

my Configure run somehow found:

try.c: In function 'main': try.c:46:28: warning: format '%x' expects argument of type 'unsigned int'\, but argument 2 has type 'long unsigned int' [-Wformat=] printf("read failed (%x)\n"\, *up); ~^ ~~~ %lx try.c:56:29: warning: format '%x' expects argument of type 'unsigned int'\, but argument 2 has type 'long unsigned int' [-Wformat=] printf("write failed (%x)\n"\, *up); ~^ ~~~ %lx (Testing for character data alignment may crash the test. That's okay.) You can access character data pretty unalignedly.

This is obviously wrong on sparc. The check also fails on 5.26.2 but for some reason no bus-error there. I manually set d_u32align and that made the 5.28.0 build succeed.

That is most helpful. That macro has two implementations\, one for platforms little endian platforms without alignment requirement and one for platforms with requirements. It clearly picks the wrong options here.

It's not just that\, it picks the wrong one (the aligned version) on x64 as well. This appears to be caused by not taking into account 64bitness. That doesn't explain why it thinks it can access those bytes unalignedly on SPARC though. Could an overeager optimizer cause it not to fail? What is the autodetected value of d_u32align when it does work?

Yves added some hashing code in 5.27 that made usage of this macro\, AFAICT it wasn't used anywhere except in an optimization in Digest::MD5. I guess that's why we never noticed something was broken about it.

Leon

p5pRT commented 5 years ago

From grobian@gentoo.org

On Wed\, 16 Jan 2019 18:34:24 -0800\, LeonT wrote:

On Wed\, Jan 16\, 2019 at 3:47 PM Fabian Groffen via RT

This is obviously wrong on sparc. The check also fails on 5.26.2 but for some reason no bus-error there. I manually set d_u32align and that made the 5.28.0 build succeed.

That is most helpful. That macro has two implementations\, one for platforms little endian platforms without alignment requirement and one for platforms with requirements. It clearly picks the wrong options here.

It's not just that\, it picks the wrong one (the aligned version) on x64 as well. This appears to be caused by not taking into account 64bitness. That doesn't explain why it thinks it can access those bytes unalignedly on SPARC though. Could an overeager optimizer cause it not to fail? What is the autodetected value of d_u32align when it does work?

I think that the GCC compiler is actually doing "too smart" things here when optimisations are enabled.

I extracted the code it tries:

% cat unaligned.c #include \<stdio.h> #define I_STDLIB #ifdef I_STDLIB #include \<stdlib.h> #endif #define U32 unsigned long #define BYTEORDER 0x4321 #define U8 unsigned char #include \<signal.h> #ifdef SIGBUS void bletch(int s) { exit(4); } #endif int main() { #if BYTEORDER == 0x1234 || BYTEORDER == 0x4321 volatile U8 buf[8]; volatile U32 *up; int i;

if (sizeof(U32) != 4) { printf("sizeof(U32) is not 4\, but %d\n"\, sizeof(U32)); exit(1); }

fflush(stdout);

#ifdef SIGBUS signal(SIGBUS\, bletch); #endif

buf[0] = 0; buf[1] = 0; buf[2] = 0; buf[3] = 1; buf[4] = 0; buf[5] = 0; buf[6] = 0; buf[7] = 1;

for (i = 0; i \< 4; i++) { up = (U32*)(buf + i); if (! ((*up == 1 \<\< (8*i)) || /* big-endian */ (*up == 1 \<\< (8*(3-i))) /* little-endian */ ) ) { printf("read failed (%x)\n"\, *up); exit(2); } }

/* write test */ for (i = 0; i \< 4; i++) { up = (U32*)(buf + i); *up = 0xBeef; if (*up != 0xBeef) { printf("write failed (%x)\n"\, *up); exit(3); } }

exit(0); #else printf("1\n"); exit(1); #endif return 0; }

Next\, I compile and run this example like Configure does:

$ for flag in "-O2" "-g" ; do for cc in /usr/sfw/bin/gcc gcc-6.4.0 gcc-7.3.0 gcc-8.2.0 ; do $cc --version ; $cc $flag -o unaligned unaligned.c ; ./unaligned ; echo "$cc $flag -> $?" ; done ; done gcc (GCC) 3.4.3 (csl-sol210-3_4-branch+sol_rpath) Copyright (C) 2004 Free Software Foundation\, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

/usr/sfw/bin/gcc -O2 -> 4 gcc-6.4.0 (Gentoo 6.4.0-r2 p1.4) 6.4.0 Copyright (C) 2017 Free Software Foundation\, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

unaligned.c: In function ‘main’: unaligned.c:46:28: warning: format ‘%x’ expects argument of type ‘unsigned int’\, but argument 2 has type ‘long unsigned int’ [-Wformat=] printf("read failed (%x)\n"\, *up); ^ unaligned.c:56:29: warning: format ‘%x’ expects argument of type ‘unsigned int’\, but argument 2 has type ‘long unsigned int’ [-Wformat=] printf("write failed (%x)\n"\, *up); ^ gcc-6.4.0 -O2 -> 4 gcc-7.3.0 (Gentoo 7.3.0-r3 p1.4) 7.3.0 Copyright (C) 2017 Free Software Foundation\, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

unaligned.c: In function ‘main’: unaligned.c:46:28: warning: format ‘%x’ expects argument of type ‘unsigned int’\, but argument 2 has type ‘long unsigned int’ [-Wformat=] printf("read failed (%x)\n"\, *up); ~^ ~~~ %lx unaligned.c:56:29: warning: format ‘%x’ expects argument of type ‘unsigned int’\, but argument 2 has type ‘long unsigned int’ [-Wformat=] printf("write failed (%x)\n"\, *up); ~^ ~~~ %lx gcc-7.3.0 -O2 -> 4 gcc-8.2.0 (Gentoo 8.2.0-r5 p1.6) 8.2.0 Copyright (C) 2018 Free Software Foundation\, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

unaligned.c: In function ‘main’: unaligned.c:46:28: warning: format ‘%x’ expects argument of type ‘unsigned int’\, but argument 2 has type ‘long unsigned int’ [-Wformat=] printf("read failed (%x)\n"\, *up); ~^ ~~~ %lx unaligned.c:56:29: warning: format ‘%x’ expects argument of type ‘unsigned int’\, but argument 2 has type ‘long unsigned int’ [-Wformat=] printf("write failed (%x)\n"\, *up); ~^ ~~~ %lx gcc-8.2.0 -O2 -> 0 gcc (GCC) 3.4.3 (csl-sol210-3_4-branch+sol_rpath) Copyright (C) 2004 Free Software Foundation\, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

/usr/sfw/bin/gcc -g -> 4 gcc-6.4.0 (Gentoo 6.4.0-r2 p1.4) 6.4.0 Copyright (C) 2017 Free Software Foundation\, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

unaligned.c: In function ‘main’: unaligned.c:46:28: warning: format ‘%x’ expects argument of type ‘unsigned int’\, but argument 2 has type ‘long unsigned int’ [-Wformat=] printf("read failed (%x)\n"\, *up); ^ unaligned.c:56:29: warning: format ‘%x’ expects argument of type ‘unsigned int’\, but argument 2 has type ‘long unsigned int’ [-Wformat=] printf("write failed (%x)\n"\, *up); ^ gcc-6.4.0 -g -> 4 gcc-7.3.0 (Gentoo 7.3.0-r3 p1.4) 7.3.0 Copyright (C) 2017 Free Software Foundation\, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

unaligned.c: In function ‘main’: unaligned.c:46:28: warning: format ‘%x’ expects argument of type ‘unsigned int’\, but argument 2 has type ‘long unsigned int’ [-Wformat=] printf("read failed (%x)\n"\, *up); ~^ ~~~ %lx unaligned.c:56:29: warning: format ‘%x’ expects argument of type ‘unsigned int’\, but argument 2 has type ‘long unsigned int’ [-Wformat=] printf("write failed (%x)\n"\, *up); ~^ ~~~ %lx gcc-7.3.0 -g -> 4 gcc-8.2.0 (Gentoo 8.2.0-r5 p1.6) 8.2.0 Copyright (C) 2018 Free Software Foundation\, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

unaligned.c: In function ‘main’: unaligned.c:46:28: warning: format ‘%x’ expects argument of type ‘unsigned int’\, but argument 2 has type ‘long unsigned int’ [-Wformat=] printf("read failed (%x)\n"\, *up); ~^ ~~~ %lx unaligned.c:56:29: warning: format ‘%x’ expects argument of type ‘unsigned int’\, but argument 2 has type ‘long unsigned int’ [-Wformat=] printf("write failed (%x)\n"\, *up); ~^ ~~~ %lx gcc-8.2.0 -g -> 4

So\, basically above list without the barf:

/usr/sfw/bin/gcc -O2 -> 4 gcc-6.4.0 -O2 -> 4 gcc-7.3.0 -O2 -> 4 gcc-8.2.0 -O2 -> 0 /usr/sfw/bin/gcc -g -> 4 gcc-6.4.0 -g -> 4 gcc-7.3.0 -g -> 4 gcc-8.2.0 -g -> 4

Behaviour seems to be pretty much limited to gcc-8.2 while optimising at the moment.

Yves added some hashing code in 5.27 that made usage of this macro\, AFAICT it wasn't used anywhere except in an optimization in Digest::MD5. I guess that's why we never noticed something was broken about it.

Yes\, including the fact that at the time I compiled 5.26.2\, I used GCC-7.3\, which produced the correct result for the unaligned check.

This makes me wonder what the problem of OP is\, though. His env seems to suggest using GCC-4.9\, which I don't have anymore for verification of the results.

p5pRT commented 5 years ago

From @Leont

On Thu\, Jan 17\, 2019 at 9:26 AM Fabian Groffen via RT \perlbug\-followup@perl\.org wrote:

I think that the GCC compiler is actually doing "too smart" things here when optimisations are enabled.

/usr/sfw/bin/gcc -O2 -> 4 gcc-6.4.0 -O2 -> 4 gcc-7.3.0 -O2 -> 4 gcc-8.2.0 -O2 -> 0 /usr/sfw/bin/gcc -g -> 4 gcc-6.4.0 -g -> 4 gcc-7.3.0 -g -> 4 gcc-8.2.0 -g -> 4

Behaviour seems to be pretty much limited to gcc-8.2 while optimising at the moment.

Yeah. I guess that means we need a better test that confuses the compiler a little bit more. One would expect volatile to take care of that but apparently not; The C standard is notoriously fuzzy on volatiles (C99 6.7.6).

What happens if you put the array outside the function?

Yes\, including the fact that at the time I compiled 5.26.2\, I used GCC-7.3\, which produced the correct result for the unaligned check.

This makes me wonder what the problem of OP is\, though. His env seems to suggest using GCC-4.9\, which I don't have anymore for verification of the results.

They did use -O3\, so I guess that always enabled that optimization.

Leon

p5pRT commented 5 years ago

From grobian@gentoo.org

On Thu\, 17 Jan 2019 07:36:42 -0800\, LeonT wrote:

On Thu\, Jan 17\, 2019 at 9:26 AM Fabian Groffen via RT \perlbug\-followup@perl\.org wrote:

I think that the GCC compiler is actually doing "too smart" things here when optimisations are enabled.

/usr/sfw/bin/gcc -O2 -> 4 gcc-6.4.0 -O2 -> 4 gcc-7.3.0 -O2 -> 4 gcc-8.2.0 -O2 -> 0 /usr/sfw/bin/gcc -g -> 4 gcc-6.4.0 -g -> 4 gcc-7.3.0 -g -> 4 gcc-8.2.0 -g -> 4

Behaviour seems to be pretty much limited to gcc-8.2 while optimising at the moment.

Yeah. I guess that means we need a better test that confuses the compiler a little bit more. One would expect volatile to take care of that but apparently not; The C standard is notoriously fuzzy on volatiles (C99 6.7.6).

What happens if you put the array outside the function?

The results are the same\, unfortunately.

Yes\, including the fact that at the time I compiled 5.26.2\, I used GCC-7.3\, which produced the correct result for the unaligned check.

This makes me wonder what the problem of OP is\, though. His env seems to suggest using GCC-4.9\, which I don't have anymore for verification of the results.

They did use -O3\, so I guess that always enabled that optimization.

It seems you're correct about that (code from original test):

/usr/sfw/bin/gcc -O3 -> 4 gcc-6.4.0 -O3 -> 0 gcc-7.3.0 -O3 -> 0 gcc-8.2.0 -O3 -> 0 /usr/sfw/bin/gcc -O2 -> 4 gcc-6.4.0 -O2 -> 4 gcc-7.3.0 -O2 -> 4 gcc-8.2.0 -O2 -> 0 /usr/sfw/bin/gcc -g -> 4 gcc-6.4.0 -g -> 4 gcc-7.3.0 -g -> 4 gcc-8.2.0 -g -> 4

Fabian

p5pRT commented 5 years ago

From grobian@gentoo.org

On Mon\, 21 Jan 2019 01:26:47 -0800\, grobian@gentoo.org wrote:

On Thu\, 17 Jan 2019 07:36:42 -0800\, LeonT wrote:

On Thu\, Jan 17\, 2019 at 9:26 AM Fabian Groffen via RT \perlbug\-followup@perl\.org wrote:

I think that the GCC compiler is actually doing "too smart" things here when optimisations are enabled.

/usr/sfw/bin/gcc -O2 -> 4 gcc-6.4.0 -O2 -> 4 gcc-7.3.0 -O2 -> 4 gcc-8.2.0 -O2 -> 0 /usr/sfw/bin/gcc -g -> 4 gcc-6.4.0 -g -> 4 gcc-7.3.0 -g -> 4 gcc-8.2.0 -g -> 4

Behaviour seems to be pretty much limited to gcc-8.2 while optimising at the moment.

Yeah. I guess that means we need a better test that confuses the compiler a little bit more. One would expect volatile to take care of that but apparently not; The C standard is notoriously fuzzy on volatiles (C99 6.7.6).

What happens if you put the array outside the function?

The results are the same\, unfortunately.

I've replaced\, however\, the stack allocated buf with a simple

buf = malloc(sizeof(U8) * 8);

and now gcc-8.2 returns 4 with -O2\, but -O3 still is too clever and returning 0. So I decided to complicate matters a bit more by getting the buffer offset no longer statically known:

data = malloc(sizeof(U8) * 32); bte = rand() % 24; buf = data + bte;

I chose rand because it lives in stdlib.h\, but I'm aware this might be a porting problem. Alternative would be time\, or parsing argv[1]\, just to ensure the compiler doesn't know what the offset is going to be. With this change gcc-8.2 -O3 returns 4\, as the test expects.

Fabian

Yes\, including the fact that at the time I compiled 5.26.2\, I used GCC-7.3\, which produced the correct result for the unaligned check.

This makes me wonder what the problem of OP is\, though. His env seems to suggest using GCC-4.9\, which I don't have anymore for verification of the results.

They did use -O3\, so I guess that always enabled that optimization.

It seems you're correct about that (code from original test):

/usr/sfw/bin/gcc -O3 -> 4 gcc-6.4.0 -O3 -> 0 gcc-7.3.0 -O3 -> 0 gcc-8.2.0 -O3 -> 0 /usr/sfw/bin/gcc -O2 -> 4 gcc-6.4.0 -O2 -> 4 gcc-7.3.0 -O2 -> 4 gcc-8.2.0 -O2 -> 0 /usr/sfw/bin/gcc -g -> 4 gcc-6.4.0 -g -> 4 gcc-7.3.0 -g -> 4 gcc-8.2.0 -g -> 4

Fabian

p5pRT commented 5 years ago

From mattst88@gmail.com

On Thu\, 17 Jan 2019 07:36:42 -0800\, LeonT wrote:

On Thu\, Jan 17\, 2019 at 9:26 AM Fabian Groffen via RT \perlbug\-followup@perl\.org wrote:

I think that the GCC compiler is actually doing "too smart" things here when optimisations are enabled.

/usr/sfw/bin/gcc -O2 -> 4 gcc-6.4.0 -O2 -> 4 gcc-7.3.0 -O2 -> 4 gcc-8.2.0 -O2 -> 0 /usr/sfw/bin/gcc -g -> 4 gcc-6.4.0 -g -> 4 gcc-7.3.0 -g -> 4 gcc-8.2.0 -g -> 4

Behaviour seems to be pretty much limited to gcc-8.2 while optimising at the moment.

Yeah. I guess that means we need a better test that confuses the compiler a little bit more. One would expect volatile to take care of that but apparently not; The C standard is notoriously fuzzy on volatiles (C99 6.7.6).

What happens if you put the array outside the function?

Yes\, including the fact that at the time I compiled 5.26.2\, I used GCC-7.3\, which produced the correct result for the unaligned check.

This makes me wonder what the problem of OP is\, though. His env seems to suggest using GCC-4.9\, which I don't have anymore for verification of the results.

They did use -O3\, so I guess that always enabled that optimization.

Leon

We filed a GCC bug upstream (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91197). They closed as INVALID because the C code in the test is provoking undefined behavior. Namely\, that it is accessing data through an unaligned pointer. So GCC is within its rights to do what it is doing\, frustratingly so.

See https://stackoverflow.com/questions/46790550/c-undefined-behavior-strict-aliasing-rule-or-incorrect-alignment for citations in the C11 spec for the claim that accessing data through unaligned pointers is undefined behavior.

Digging into the code more\, it's not just that the test in Configure provokes undefined behavior\, but actually that the code it enables (in the little-endian && unaligned-ok paths) is potentially provoking undefined behavior. For example\, in cpan/Digest-MD5/MD5.xs where buf is const U8* buf:

#ifndef U32_ALIGNMENT_REQUIRED const U32 *x = (U32*)buf; /* really just type casting */ #endif

Throughout the code base it looks like we go out of our way to optimize byte swaps and hit the little-endian && unaligned-ok path if possible\, but I don't think that's necessary at all. Comments like "Without a known fast bswap32 we're just as well off doing this" followed by an open-coded shift and OR implementation of bswap indicate to me that the authors didn't realize that compilers can easily recognize this pattern (or wrote the code when they couldn't). Regardless\, today compilers can easily see through this and generate a bswap/movbe instruction on x86. And in fact\, it's not clear to me that even using GCC __builtin_bswap* is better or worse than the open-coded implementations (see https://gitlab.freedesktop.org/xorg/xserver/merge_requests/176)

Similarly\, while what the C code is doing in accessing unaligned pointers is undefined behavior\, x86 instruction certainly can do that. But again\, the compiler is perfectly capable of making that decision on its own.

So\, I propose that we just cut all of that code out and use what is currently the alignment-required path today.

Please review the attached patches. I have never contributed to Perl before\, and I stepped on quite a few landmines during the development of these patches (md5sum of MD5.xs goes in the test case; make regen; BYTEORDER is 0x1234 vs 0x12345678)\, so some help getting the patches in would be extremely appreciated.

Passes the perl test suite when applied to 5.30.0 on Gentoo/Linux on amd64\, 32-bit userland sparc\, and 64-bit userland sparc.

p5pRT commented 5 years ago

From mattst88@gmail.com

p.patch

```diff From 0abdabaf4cbcac90473d48b17af60accca89c233 Mon Sep 17 00:00:00 2001 From: Matt Turner Date: Wed, 4 Sep 2019 21:04:47 -0700 Subject: [PATCH 1/3] Digest-MD5: Consolidate byte-swapping paths The code guarded by #ifndef U32_ALIGNMENT_REQUIRED attempts to optimize byte-swapping by doing unaligned loads, but accessing data through unaligned pointers is undefined behavior in C. Moreover, compilers are more than capable of recognizing these open-coded byte-swap patterns and emitting a bswap instruction, or an unaligned load instruction, or a combined load, etc. There's no need for multiple paths to attain the desired result. See https://rt.perl.org/Ticket/Display.html?id=133495 --- cpan/Digest-MD5/MD5.xs | 44 +------------- cpan/Digest-MD5/Makefile.PL | 114 ------------------------------------ cpan/Digest-MD5/t/files.t | 2 +- 3 files changed, 3 insertions(+), 157 deletions(-) diff --git a/cpan/Digest-MD5/MD5.xs b/cpan/Digest-MD5/MD5.xs index a48d951056..964d37fb0e 100644 --- a/cpan/Digest-MD5/MD5.xs +++ b/cpan/Digest-MD5/MD5.xs @@ -106,20 +106,6 @@ static MAGIC *THX_sv_magicext(pTHX_ SV *sv, SV *obj, int type, * values. The following macros (and functions) allow us to convert * between native integers and such values. */ -#undef BYTESWAP -#ifndef U32_ALIGNMENT_REQUIRED - #if BYTEORDER == 0x1234 /* 32-bit little endian */ - #define BYTESWAP(x) (x) /* no-op */ - - #elif BYTEORDER == 0x4321 /* 32-bit big endian */ - #define BYTESWAP(x) ((((x)&0xFF)<<24) \ - |(((x)>>24)&0xFF) \ - |(((x)&0x0000FF00)<<8) \ - |(((x)&0x00FF0000)>>8) ) - #endif -#endif - -#ifndef BYTESWAP static void u2s(U32 u, U8* s) { *s++ = (U8)(u & 0xFF); @@ -132,7 +118,6 @@ static void u2s(U32 u, U8* s) ((U32)(*(s+1)) << 8) | \ ((U32)(*(s+2)) << 16) | \ ((U32)(*(s+3)) << 24)) -#endif /* This structure keeps the current state of algorithm. */ @@ -279,29 +264,16 @@ MD5Transform(MD5_CTX* ctx, const U8* buf, STRLEN blocks) U32 C = ctx->C; U32 D = ctx->D; -#ifndef U32_ALIGNMENT_REQUIRED - const U32 *x = (U32*)buf; /* really just type casting */ -#endif - do { U32 a = A; U32 b = B; U32 c = C; U32 d = D; -#if BYTEORDER == 0x1234 && !defined(U32_ALIGNMENT_REQUIRED) - const U32 *X = x; - #define NEXTx (*x++) -#else - U32 X[16]; /* converted values, used in round 2-4 */ + U32 X[16]; /* little-endian values, used in round 2-4 */ U32 *uptr = X; U32 tmp; - #ifdef BYTESWAP - #define NEXTx (tmp=*x++, *uptr++ = BYTESWAP(tmp)) - #else #define NEXTx (s2u(buf,tmp), buf += 4, *uptr++ = tmp) - #endif -#endif #ifdef MD5_DEBUG if (buf == ctx->buffer) @@ -313,7 +285,7 @@ MD5Transform(MD5_CTX* ctx, const U8* buf, STRLEN blocks) int i; fprintf(stderr,"["); for (i = 0; i < 16; i++) { - fprintf(stderr,"%x,", x[i]); + fprintf(stderr,"%x,", x[i]); /* FIXME */ } fprintf(stderr,"]\n"); } @@ -468,30 +440,18 @@ MD5Final(U8* digest, MD5_CTX *ctx) bits_low = ctx->bytes_low << 3; bits_high = (ctx->bytes_high << 3) | (ctx->bytes_low >> 29); -#ifdef BYTESWAP - *(U32*)(ctx->buffer + fill) = BYTESWAP(bits_low); fill += 4; - *(U32*)(ctx->buffer + fill) = BYTESWAP(bits_high); fill += 4; -#else u2s(bits_low, ctx->buffer + fill); fill += 4; u2s(bits_high, ctx->buffer + fill); fill += 4; -#endif MD5Transform(ctx, ctx->buffer, fill >> 6); #ifdef MD5_DEBUG fprintf(stderr," Result: %s\n", ctx_dump(ctx)); #endif -#ifdef BYTESWAP - *(U32*)digest = BYTESWAP(ctx->A); digest += 4; - *(U32*)digest = BYTESWAP(ctx->B); digest += 4; - *(U32*)digest = BYTESWAP(ctx->C); digest += 4; - *(U32*)digest = BYTESWAP(ctx->D); -#else u2s(ctx->A, digest); u2s(ctx->B, digest+4); u2s(ctx->C, digest+8); u2s(ctx->D, digest+12); -#endif } #ifndef INT2PTR diff --git a/cpan/Digest-MD5/Makefile.PL b/cpan/Digest-MD5/Makefile.PL index 1015058bac..76906d1046 100644 --- a/cpan/Digest-MD5/Makefile.PL +++ b/cpan/Digest-MD5/Makefile.PL @@ -5,7 +5,6 @@ use Config qw(%Config); use ExtUtils::MakeMaker; my @extra; -push(@extra, DEFINE => "-DU32_ALIGNMENT_REQUIRED") unless free_u32_alignment(); push(@extra, INSTALLDIRS => 'perl') if $] >= 5.008 && $] < 5.012; if ($^O eq 'VMS') { @@ -39,119 +38,6 @@ WriteMakefile( -sub free_u32_alignment -{ - $|=1; - if (exists $Config{d_u32align}) { - print "Perl's config says that U32 access must "; - print "not " unless $Config{d_u32align}; - print "be aligned.\n"; - return !$Config{d_u32align}; - } - - if ($^O eq 'VMS' || $^O eq 'MSWin32') { - print "Assumes that $^O implies free alignment for U32 access.\n"; - return 1; - } - - if ($^O eq 'hpux' && $Config{osvers} < 11.0) { - print "Will not test for free alignment on older HP-UX.\n"; - return 0; - } - - print "Testing alignment requirements for U32... "; - open(ALIGN_TEST, ">u32align.c") or die "$!"; - print ALIGN_TEST <<'EOT'; close(ALIGN_TEST); -/*--------------------------------------------------------------*/ -/* This program allocates a buffer of U8 (char) and then tries */ -/* to access it through a U32 pointer at every offset. The */ -/* program is expected to die with a bus error/seg fault for */ -/* machines that do not support unaligned integer read/write */ -/*--------------------------------------------------------------*/ - -#include -#include "EXTERN.h" -#include "perl.h" - -#ifdef printf - #undef printf -#endif - -int main(int argc, char** argv, char** env) -{ -#if BYTEORDER == 0x1234 || BYTEORDER == 0x4321 - volatile U8 buf[] = "\0\0\0\1\0\0\0\0"; - volatile U32 *up; - int i; - - if (sizeof(U32) != 4) { - printf("sizeof(U32) is not 4, but %d\n", sizeof(U32)); - exit(1); - } - - fflush(stdout); - - for (i = 0; i < 4; i++) { - up = (U32*)(buf + i); - if (! ((*up == 1 << (8*i)) || /* big-endian */ - (*up == 1 << (8*(3-i))) /* little-endian */ - ) - ) - { - printf("read failed (%x)\n", *up); - exit(2); - } - } - - /* write test */ - for (i = 0; i < 4; i++) { - up = (U32*)(buf + i); - *up = 0xBeef; - if (*up != 0xBeef) { - printf("write failed (%x)\n", *up); - exit(3); - } - } - - printf("no restrictions\n"); - exit(0); -#else - printf("unusual byteorder, playing safe\n"); - exit(1); -#endif - return 0; -} -/*--------------------------------------------------------------*/ -EOT - - my $cc_cmd = "$Config{cc} $Config{ccflags} -I$Config{archlibexp}/CORE"; - my $exe = "u32align$Config{exe_ext}"; - $cc_cmd .= " -o $exe"; - my $rc; - $rc = system("$cc_cmd $Config{ldflags} u32align.c $Config{libs}"); - if ($rc) { - print "Can't compile test program. Will ensure alignment to play safe.\n\n"; - unlink("u32align.c", $exe, "u32align$Config{obj_ext}"); - return 0; - } - - $rc = system("./$exe"); - unlink("u32align.c", $exe, "u32align$Config{obj_ext}"); - - return 1 unless $rc; - - if ($rc > 0x80) { - (my $cp = $rc) >>= 8; - print "Test program exit status was $cp\n"; - } - if ($rc & 0x80) { - $rc &= ~0x80; - unlink("core") && print "Core dump deleted\n"; - } - print "signal $rc\n" if $rc && $rc < 0x80; - return 0; -} - BEGIN { # compatibility with older versions of MakeMaker my $developer = -d ".git"; diff --git a/cpan/Digest-MD5/t/files.t b/cpan/Digest-MD5/t/files.t index 63479c24a3..ef64088c8c 100644 --- a/cpan/Digest-MD5/t/files.t +++ b/cpan/Digest-MD5/t/files.t @@ -21,7 +21,7 @@ EOT # This is the output of: 'md5sum README MD5.xs rfc1321.txt' $EXPECT = < Date: Wed, 4 Sep 2019 21:48:56 -0700 Subject: [PATCH 2/3] Clean up U8TO*_LE macro implementations The code guarded by #ifndef U32_ALIGNMENT_REQUIRED attempts to optimize byte-swapping by doing unaligned loads, but accessing data through unaligned pointers is undefined behavior in C. Moreover, compilers are more than capable of recognizing these open-coded byte-swap patterns and emitting a bswap instruction, or an unaligned load instruction, or a combined load, etc. There's no need for multiple paths to attain the desired result. See https://rt.perl.org/Ticket/Display.html?id=133495 --- hv_macro.h | 31 ++++++++++++----------------- stadtx_hash.h | 52 ------------------------------------------------- zaphod32_hash.h | 35 --------------------------------- 3 files changed, 12 insertions(+), 106 deletions(-) diff --git a/hv_macro.h b/hv_macro.h index 77a4c84896..02c0baad08 100644 --- a/hv_macro.h +++ b/hv_macro.h @@ -6,7 +6,7 @@ #endif /*----------------------------------------------------------------------------- - * Endianess, misalignment capabilities and util macros + * Endianess and util macros * * The following 3 macros are defined in this section. The other macros defined * are only needed to help derive these 3. @@ -20,29 +20,22 @@ * ROTR64(x,r) Rotate x right by r bits */ -#ifndef U32_ALIGNMENT_REQUIRED +#ifndef U8TO16_LE #if (BYTEORDER == 0x1234 || BYTEORDER == 0x12345678) - #define U8TO16_LE(ptr) (*((const U16*)(ptr))) - #define U8TO32_LE(ptr) (*((const U32*)(ptr))) - #define U8TO64_LE(ptr) (*((const U64*)(ptr))) + #define U8TO16_LE(ptr) ((U32)(ptr)[1]|(U32)(ptr)[0]<<8) + #define U8TO32_LE(ptr) ((U32)(ptr)[3]|(U32)(ptr)[2]<<8|(U32)(ptr)[1]<<16|(U32)(ptr)[0]<<24) + #define U8TO64_LE(ptr) ((U64)(ptr)[7]|(U64)(ptr)[6]<<8|(U64)(ptr)[5]<<16|(U64)(ptr)[4]<<24|\ + (U64)(ptr)[3]<<32|(U64)(ptr)[4]<<40|\ + (U64)(ptr)[1]<<48|(U64)(ptr)[0]<<56) #elif (BYTEORDER == 0x4321 || BYTEORDER == 0x87654321) - #if defined(__GNUC__) && (__GNUC__>4 || (__GNUC__==4 && __GNUC_MINOR__>=3)) - #define U8TO16_LE(ptr) (__builtin_bswap16(*((U16*)(ptr)))) - #define U8TO32_LE(ptr) (__builtin_bswap32(*((U32*)(ptr)))) - #define U8TO64_LE(ptr) (__builtin_bswap64(*((U64*)(ptr)))) - #endif + #define U8TO16_LE(ptr) ((U32)(ptr)[0]|(U32)(ptr)[1]<<8) + #define U8TO32_LE(ptr) ((U32)(ptr)[0]|(U32)(ptr)[1]<<8|(U32)(ptr)[2]<<16|(U32)(ptr)[3]<<24) + #define U8TO64_LE(ptr) ((U64)(ptr)[0]|(U64)(ptr)[1]<<8|(U64)(ptr)[2]<<16|(U64)(ptr)[3]<<24|\ + (U64)(ptr)[4]<<32|(U64)(ptr)[5]<<40|\ + (U64)(ptr)[6]<<48|(U64)(ptr)[7]<<56) #endif #endif -#ifndef U8TO16_LE - /* Without a known fast bswap32 we're just as well off doing this */ - #define U8TO16_LE(ptr) ((U32)(ptr)[0]|(U32)(ptr)[1]<<8) - #define U8TO32_LE(ptr) ((U32)(ptr)[0]|(U32)(ptr)[1]<<8|(U32)(ptr)[2]<<16|(U32)(ptr)[3]<<24) - #define U8TO64_LE(ptr) ((U64)(ptr)[0]|(U64)(ptr)[1]<<8|(U64)(ptr)[2]<<16|(U64)(ptr)[3]<<24|\ - (U64)(ptr)[4]<<32|(U64)(ptr)[5]<<40|\ - (U64)(ptr)[6]<<48|(U64)(ptr)[7]<<56) -#endif - #ifdef CAN64BITHASH #ifndef U64TYPE /* This probably isn't going to work, but failing with a compiler error due to diff --git a/stadtx_hash.h b/stadtx_hash.h index bd09c2f938..5ee879485d 100644 --- a/stadtx_hash.h +++ b/stadtx_hash.h @@ -43,58 +43,6 @@ #define STMT_END while(0) #endif -#ifndef STADTX_ALLOW_UNALIGNED_AND_LITTLE_ENDIAN -/* STADTX_ALLOW_UNALIGNED_AND_LITTLE_ENDIAN only matters if nothing has defined U8TO64_LE etc, - * and when built with Perl these should be defined before this file is loaded. - */ -#ifdef U32_ALIGNMENT_REQUIRED -#define STADTX_ALLOW_UNALIGNED_AND_LITTLE_ENDIAN 0 -#else -#define STADTX_ALLOW_UNALIGNED_AND_LITTLE_ENDIAN 1 -#endif -#endif - -#ifndef U8TO64_LE -#if STADTX_ALLOW_UNALIGNED_AND_LITTLE_ENDIAN -#define U8TO64_LE(ptr) (*((const U64 *)(ptr))) -#else -#define U8TO64_LE(ptr) (\ - (U64)(ptr)[7] << 56 | \ - (U64)(ptr)[6] << 48 | \ - (U64)(ptr)[5] << 40 | \ - (U64)(ptr)[4] << 32 | \ - (U64)(ptr)[3] << 24 | \ - (U64)(ptr)[2] << 16 | \ - (U64)(ptr)[1] << 8 | \ - (U64)(ptr)[0] \ -) -#endif -#endif - -#ifndef U8TO32_LE -#if STADTX_ALLOW_UNALIGNED_AND_LITTLE_ENDIAN -#define U8TO32_LE(ptr) (*((const U32 *)(ptr))) -#else -#define U8TO32_LE(ptr) (\ - (U32)(ptr)[3] << 24 | \ - (U32)(ptr)[2] << 16 | \ - (U32)(ptr)[1] << 8 | \ - (U32)(ptr)[0] \ -) -#endif -#endif - -#ifndef U8TO16_LE -#if STADTX_ALLOW_UNALIGNED_AND_LITTLE_ENDIAN -#define U8TO16_LE(ptr) (*((const U16 *)(ptr))) -#else -#define U8TO16_LE(ptr) (\ - (U16)(ptr)[1] << 8 | \ - (U16)(ptr)[0] \ -) -#endif -#endif - /* Find best way to ROTL32/ROTL64 */ #if defined(_MSC_VER) #include /* Microsoft put _rotl declaration in here */ diff --git a/zaphod32_hash.h b/zaphod32_hash.h index c9b60ccb32..2fb391a233 100644 --- a/zaphod32_hash.h +++ b/zaphod32_hash.h @@ -74,41 +74,6 @@ #define STMT_END while(0) #endif -#ifndef ZAPHOD32_ALLOW_UNALIGNED_AND_LITTLE_ENDIAN -/* ZAPHOD32_ALLOW_UNALIGNED_AND_LITTLE_ENDIAN only matters if nothing has defined U8TO64_LE etc, - * and when built with Perl these should be defined before this file is loaded. - */ -#ifdef U32_ALIGNMENT_REQUIRED -#define ZAPHOD32_ALLOW_UNALIGNED_AND_LITTLE_ENDIAN 0 -#else -#define ZAPHOD32_ALLOW_UNALIGNED_AND_LITTLE_ENDIAN 1 -#endif -#endif - -#ifndef U8TO32_LE -#if ZAPHOD32_ALLOW_UNALIGNED_AND_LITTLE_ENDIAN -#define U8TO32_LE(ptr) (*((const U32 *)(ptr))) -#else -#define U8TO32_LE(ptr) (\ - (U32)(ptr)[3] << 24 | \ - (U32)(ptr)[2] << 16 | \ - (U32)(ptr)[1] << 8 | \ - (U32)(ptr)[0] \ -) -#endif -#endif - -#ifndef U8TO16_LE -#if ZAPHOD32_ALLOW_UNALIGNED_AND_LITTLE_ENDIAN -#define U8TO16_LE(ptr) (*((const U16 *)(ptr))) -#else -#define U8TO16_LE(ptr) (\ - (U16)(ptr)[1] << 8 | \ - (U16)(ptr)[0] \ -) -#endif -#endif - /* This is two marsaglia xor-shift permutes, with a prime-multiple * sandwiched inside. The end result of doing this twice with different * primes is a completely avalanched v. */ -- 2.21.0 From f74c38c62e455c7da828d5525fa4712688e2d27e Mon Sep 17 00:00:00 2001 From: Matt Turner Date: Tue, 3 Sep 2019 21:43:47 -0700 Subject: [PATCH 3/3] Configure: Remove d_u32align test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It is undefined behavior in C to access data through an unaligned pointer. C11 (draft n1570) Appendix J.2: 1 The behavior is undefined in the following circumstances: .... · Conversion between two pointer types produces a result that is incorrectly aligned (6.3.2.3). With 6.3.2.3p7 saying [...] If the resulting pointer is not correctly aligned [68] for the referenced type, the behavior is undefined. [...] As discussed in the ticket listed below, this test is provoking undefined behavior. As a result, GCC is within its rights to optimize it into garbage, which it does to degrees varying with the optimization level. The result of this test is used to select C code paths that also rely on unaligned accesses (i.e., more undefined behavior), which the previous patches have removed. See https://rt.perl.org/Ticket/Display.html?id=133495 --- Configure | 106 --------------------------------- Cross/config.sh-arm-linux | 1 - Cross/config.sh-arm-linux-n770 | 1 - NetWare/config.wc | 1 - NetWare/config_H.wc | 6 -- Porting/Glossary | 4 -- Porting/config.sh | 1 - Porting/config_H | 8 --- config_h.SH | 8 --- configure.com | 1 - hints/hpux.sh | 7 --- plan9/config.plan9 | 8 --- plan9/config_h.sample | 8 --- plan9/config_sh.sample | 1 - symbian/config.sh | 1 - uconfig.h | 8 --- uconfig.sh | 1 - uconfig64.sh | 1 - win32/config.gc | 1 - win32/config.vc | 1 - win32/config_H.gc | 8 --- win32/config_H.vc | 8 --- 22 files changed, 190 deletions(-) diff --git a/Configure b/Configure index 818deb8378..4b504b117f 100755 --- a/Configure +++ b/Configure @@ -914,7 +914,6 @@ d_truncl='' d_ttyname_r='' ttyname_r_proto='' d_tzname='' -d_u32align='' d_ualarm='' d_umask='' d_semctl_semid_ds='' @@ -19699,110 +19698,6 @@ EOM ;; esac -: Checking 32bit alignedness -$cat <try.c < -#$i_stdlib I_STDLIB -#ifdef I_STDLIB -#include -#endif -#define U32 $u32type -#define BYTEORDER 0x$byteorder -#define U8 $u8type -#include -#ifdef SIGBUS -$signal_t bletch(int s) { exit(4); } -#endif -int main() { -#if BYTEORDER == 0x1234 || BYTEORDER == 0x4321 - volatile U8 buf[8]; - volatile U32 *up; - int i; - - if (sizeof(U32) != 4) { - printf("sizeof(U32) is not 4, but %d\n", sizeof(U32)); - exit(1); - } - - fflush(stdout); - -#ifdef SIGBUS - signal(SIGBUS, bletch); -#endif - - buf[0] = 0; - buf[1] = 0; - buf[2] = 0; - buf[3] = 1; - buf[4] = 0; - buf[5] = 0; - buf[6] = 0; - buf[7] = 1; - - for (i = 0; i < 4; i++) { - up = (U32*)(buf + i); - if (! ((*up == 1 << (8*i)) || /* big-endian */ - (*up == 1 << (8*(3-i))) /* little-endian */ - ) - ) - { - printf("read failed (%x)\n", *up); - exit(2); - } - } - - /* write test */ - for (i = 0; i < 4; i++) { - up = (U32*)(buf + i); - *up = 0xBeef; - if (*up != 0xBeef) { - printf("write failed (%x)\n", *up); - exit(3); - } - } - - exit(0); -#else - printf("1\n"); - exit(1); -#endif - return 0; -} -EOCP -set try -if eval $compile_ok; then - echo "(Testing for character data alignment may crash the test. That's okay.)" >&4 - $run ./try 2>&1 >/dev/null - case "$?" in - 0) cat >&4 <&4 <$CONFIG_H -e 's!^#undef$.*/$\*!/\*#define\1 \*!' -e 's!^#un */ #$d_truncl HAS_TRUNCL /**/ -/* U32_ALIGNMENT_REQUIRED: - * This symbol, if defined, indicates that you must access - * character data through U32-aligned pointers. - */ -#ifndef U32_ALIGNMENT_REQUIRED -#$d_u32align U32_ALIGNMENT_REQUIRED /**/ -#endif - /* HAS_UALARM: * This symbol, if defined, indicates that the ualarm routine is * available to do alarms with microsecond granularity. diff --git a/configure.com b/configure.com index dc251dfe04..4f2f80496f 100644 --- a/configure.com +++ b/configure.com @@ -6424,7 +6424,6 @@ $ WC "d_truncate='" + d_truncate + "'" $ WC "d_trunc='" + d_trunc + "'" $ WC "d_truncl='" + d_truncl + "'" $ WC "d_tzname='" + d_tzname + "'" -$ WC "d_u32align='define'" $ WC "d_ualarm='" + d_ualarm + "'" $ WC "d_umask='define'" $ WC "d_uname='" + d_uname + "'" diff --git a/hints/hpux.sh b/hints/hpux.sh index 8f273a6ef9..689a39b2b3 100644 --- a/hints/hpux.sh +++ b/hints/hpux.sh @@ -34,13 +34,6 @@ else selecttype='int *' fi -# For some strange reason, the u32align test from Configure hangs in -# HP-UX 10.20 since the December 2001 patches. So hint it to avoid -# the test. -if [ "$xxOsRevMajor" -le 10 ]; then - d_u32align=$define - fi - echo "Archname is $archname" # Fix XSlib (CPAN) confusion when re-using a prefix but changing from ILP32 diff --git a/plan9/config.plan9 b/plan9/config.plan9 index efe6488dfd..2d12d15725 100644 --- a/plan9/config.plan9 +++ b/plan9/config.plan9 @@ -3677,14 +3677,6 @@ */ /*#define HAS_SYSCALL_PROTO / **/ -/* U32_ALIGNMENT_REQUIRED: - * This symbol, if defined, indicates that you must access - * character data through U32-aligned pointers. - */ -#ifndef U32_ALIGNMENT_REQUIRED -#define U32_ALIGNMENT_REQUIRED /**/ -#endif - /* HAS_USLEEP_PROTO: * This symbol, if defined, indicates that the system provides * a prototype for the usleep() function. Otherwise, it is up diff --git a/plan9/config_h.sample b/plan9/config_h.sample index f71d55bd53..d2096dbbf2 100644 --- a/plan9/config_h.sample +++ b/plan9/config_h.sample @@ -3613,14 +3613,6 @@ */ /*#define HAS_SYSCALL_PROTO / **/ -/* U32_ALIGNMENT_REQUIRED: - * This symbol, if defined, indicates that you must access - * character data through U32-aligned pointers. - */ -#ifndef U32_ALIGNMENT_REQUIRED -#define U32_ALIGNMENT_REQUIRED /**/ -#endif - /* HAS_USLEEP_PROTO: * This symbol, if defined, indicates that the system provides * a prototype for the usleep() function. Otherwise, it is up diff --git a/plan9/config_sh.sample b/plan9/config_sh.sample index 6b2d0d4f61..036afe1fb9 100644 --- a/plan9/config_sh.sample +++ b/plan9/config_sh.sample @@ -606,7 +606,6 @@ d_truncate='undef' d_truncl='undef' d_ttyname_r='undef' d_tzname='define' -d_u32align='define' d_ualarm='undef' d_umask='define' d_uname='define' diff --git a/symbian/config.sh b/symbian/config.sh index 8b3122b286..719f7dc90b 100644 --- a/symbian/config.sh +++ b/symbian/config.sh @@ -555,7 +555,6 @@ d_truncate='undef' d_truncl='undef' d_ttyname_r='undef' d_tzname='undef' -d_u32align='define' d_ualarm='undef' d_umask='undef' d_uname='undef' diff --git a/uconfig.h b/uconfig.h index 09fd622baa..757d2e4278 100644 --- a/uconfig.h +++ b/uconfig.h @@ -3491,14 +3491,6 @@ */ /*#define HAS_TRUNCL / **/ -/* U32_ALIGNMENT_REQUIRED: - * This symbol, if defined, indicates that you must access - * character data through U32-aligned pointers. - */ -#ifndef U32_ALIGNMENT_REQUIRED -#define U32_ALIGNMENT_REQUIRED /**/ -#endif - /* HAS_UALARM: * This symbol, if defined, indicates that the ualarm routine is * available to do alarms with microsecond granularity. diff --git a/uconfig.sh b/uconfig.sh index fca0e4dacd..8fe327dd0c 100644 --- a/uconfig.sh +++ b/uconfig.sh @@ -546,7 +546,6 @@ d_truncate='undef' d_truncl='undef' d_ttyname_r='undef' d_tzname='undef' -d_u32align='define' d_ualarm='undef' d_umask='undef' d_uname='undef' diff --git a/uconfig64.sh b/uconfig64.sh index a6b9901acd..923bc899ba 100644 --- a/uconfig64.sh +++ b/uconfig64.sh @@ -546,7 +546,6 @@ d_truncate='undef' d_truncl='undef' d_ttyname_r='undef' d_tzname='undef' -d_u32align='define' d_ualarm='undef' d_umask='undef' d_uname='undef' diff --git a/win32/config.gc b/win32/config.gc index ce9a6b9ad7..5cd641cc44 100644 --- a/win32/config.gc +++ b/win32/config.gc @@ -595,7 +595,6 @@ d_truncate='undef' d_truncl='define' d_ttyname_r='undef' d_tzname='define' -d_u32align='define' d_ualarm='undef' d_umask='define' d_uname='define' diff --git a/win32/config.vc b/win32/config.vc index 0855dc8c09..0f0859b1cd 100644 --- a/win32/config.vc +++ b/win32/config.vc @@ -595,7 +595,6 @@ d_truncate='undef' d_truncl='undef' d_ttyname_r='undef' d_tzname='define' -d_u32align='define' d_ualarm='undef' d_umask='define' d_uname='define' diff --git a/win32/config_H.gc b/win32/config_H.gc index 115c88cde1..e9023fe086 100644 --- a/win32/config_H.gc +++ b/win32/config_H.gc @@ -3422,14 +3422,6 @@ */ #define HAS_TRUNCL /**/ -/* U32_ALIGNMENT_REQUIRED: - * This symbol, if defined, indicates that you must access - * character data through U32-aligned pointers. - */ -#ifndef U32_ALIGNMENT_REQUIRED -#define U32_ALIGNMENT_REQUIRED /**/ -#endif - /* HAS_UALARM: * This symbol, if defined, indicates that the ualarm routine is * available to do alarms with microsecond granularity. diff --git a/win32/config_H.vc b/win32/config_H.vc index 05c8093615..ae4d1cbbe0 100644 --- a/win32/config_H.vc +++ b/win32/config_H.vc @@ -3413,14 +3413,6 @@ */ /*#define HAS_TRUNCL / **/ -/* U32_ALIGNMENT_REQUIRED: - * This symbol, if defined, indicates that you must access - * character data through U32-aligned pointers. - */ -#ifndef U32_ALIGNMENT_REQUIRED -#define U32_ALIGNMENT_REQUIRED /**/ -#endif - /* HAS_UALARM: * This symbol, if defined, indicates that the ualarm routine is * available to do alarms with microsecond granularity. -- 2.21.0 ```

p5pRT commented 5 years ago

From @khwilliamson

On Wed\, 04 Sep 2019 22:12:06 -0700\, mattst88@gmail.com wrote:

On Thu\, 17 Jan 2019 07:36:42 -0800\, LeonT wrote:

On Thu\, Jan 17\, 2019 at 9:26 AM Fabian Groffen via RT \perlbug\-followup@perl\.org wrote:

I think that the GCC compiler is actually doing "too smart" things here when optimisations are enabled.

/usr/sfw/bin/gcc -O2 -> 4 gcc-6.4.0 -O2 -> 4 gcc-7.3.0 -O2 -> 4 gcc-8.2.0 -O2 -> 0 /usr/sfw/bin/gcc -g -> 4 gcc-6.4.0 -g -> 4 gcc-7.3.0 -g -> 4 gcc-8.2.0 -g -> 4

Behaviour seems to be pretty much limited to gcc-8.2 while optimising at the moment.

Yeah. I guess that means we need a better test that confuses the compiler a little bit more. One would expect volatile to take care of that but apparently not; The C standard is notoriously fuzzy on volatiles (C99 6.7.6).

What happens if you put the array outside the function?

Yes\, including the fact that at the time I compiled 5.26.2\, I used GCC-7.3\, which produced the correct result for the unaligned check.

This makes me wonder what the problem of OP is\, though. His env seems to suggest using GCC-4.9\, which I don't have anymore for verification of the results.

They did use -O3\, so I guess that always enabled that optimization.

Leon

We filed a GCC bug upstream (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91197). They closed as INVALID because the C code in the test is provoking undefined behavior. Namely\, that it is accessing data through an unaligned pointer. So GCC is within its rights to do what it is doing\, frustratingly so.

See https://stackoverflow.com/questions/46790550/c-undefined-behavior- strict-aliasing-rule-or-incorrect-alignment for citations in the C11 spec for the claim that accessing data through unaligned pointers is undefined behavior.

Digging into the code more\, it's not just that the test in Configure provokes undefined behavior\, but actually that the code it enables (in the little-endian && unaligned-ok paths) is potentially provoking undefined behavior. For example\, in cpan/Digest-MD5/MD5.xs where buf is const U8* buf:

#ifndef U32_ALIGNMENT_REQUIRED const U32 *x = (U32*)buf; /* really just type casting */ #endif

Throughout the code base it looks like we go out of our way to optimize byte swaps and hit the little-endian && unaligned-ok path if possible\, but I don't think that's necessary at all. Comments like "Without a known fast bswap32 we're just as well off doing this" followed by an open-coded shift and OR implementation of bswap indicate to me that the authors didn't realize that compilers can easily recognize this pattern (or wrote the code when they couldn't). Regardless\, today compilers can easily see through this and generate a bswap/movbe instruction on x86. And in fact\, it's not clear to me that even using GCC __builtin_bswap* is better or worse than the open-coded implementations (see https://gitlab.freedesktop.org/xorg/xserver/merge_requests/176)

Similarly\, while what the C code is doing in accessing unaligned pointers is undefined behavior\, x86 instruction certainly can do that. But again\, the compiler is perfectly capable of making that decision on its own.

So\, I propose that we just cut all of that code out and use what is currently the alignment-required path today.

Please review the attached patches. I have never contributed to Perl before\, and I stepped on quite a few landmines during the development of these patches (md5sum of MD5.xs goes in the test case; make regen; BYTEORDER is 0x1234 vs 0x12345678)\, so some help getting the patches in would be extremely appreciated.

Passes the perl test suite when applied to 5.30.0 on Gentoo/Linux on amd64\, 32-bit userland sparc\, and 64-bit userland sparc.

This did not make it to perl5-porters yet. I'm replying to it to see if that thaws it. -- Karl Williamson

p5pRT commented 5 years ago

From mattst88@gmail.com

I've opened a pull request again Digest-MD5 here: https://github.com/gisle/digest-md5/pull/16

p5pRT commented 5 years ago

From mattst88@gmail.com

In the patches' commit messages I made the claim:

Moreover\, compilers are more than capable of recognizing these open-coded byte-swap patterns and emitting a bswap instruction\, or an unaligned load instruction

Here's proof that GCC can do those things:

Bswap: https://godbolt.org/z/w__OsV Unaligned store: https://godbolt.org/z/xSQm6m

Unaligned load left as an exercise to the reader :)

p5pRT commented 5 years ago

From hiroshi-hakoyama@nagano.ac.jp

Please tell me if you want to ask me to do some tests on Solaris 10 / sparc64.

Hiroshi

2019/09/05 14:12、mattst88@gmail.com via RT \perlbug\-followup@perl\.orgのメール:

On Thu\, 17 Jan 2019 07:36:42 -0800\, LeonT wrote:

On Thu\, Jan 17\, 2019 at 9:26 AM Fabian Groffen via RT \perlbug\-followup@perl\.org wrote:

I think that the GCC compiler is actually doing "too smart" things here when optimisations are enabled.

/usr/sfw/bin/gcc -O2 -> 4 gcc-6.4.0 -O2 -> 4 gcc-7.3.0 -O2 -> 4 gcc-8.2.0 -O2 -> 0 /usr/sfw/bin/gcc -g -> 4 gcc-6.4.0 -g -> 4 gcc-7.3.0 -g -> 4 gcc-8.2.0 -g -> 4

Behaviour seems to be pretty much limited to gcc-8.2 while optimising at the moment.

Yeah. I guess that means we need a better test that confuses the compiler a little bit more. One would expect volatile to take care of that but apparently not; The C standard is notoriously fuzzy on volatiles (C99 6.7.6).

What happens if you put the array outside the function?

Yes\, including the fact that at the time I compiled 5.26.2\, I used GCC-7.3\, which produced the correct result for the unaligned check.

This makes me wonder what the problem of OP is\, though. His env seems to suggest using GCC-4.9\, which I don't have anymore for verification of the results.

They did use -O3\, so I guess that always enabled that optimization.

Leon

We filed a GCC bug upstream (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91197). They closed as INVALID because the C code in the test is provoking undefined behavior. Namely\, that it is accessing data through an unaligned pointer. So GCC is within its rights to do what it is doing\, frustratingly so.

See https://stackoverflow.com/questions/46790550/c-undefined-behavior-strict-aliasing-rule-or-incorrect-alignment for citations in the C11 spec for the claim that accessing data through unaligned pointers is undefined behavior.

Digging into the code more\, it's not just that the test in Configure provokes undefined behavior\, but actually that the code it enables (in the little-endian && unaligned-ok paths) is potentially provoking undefined behavior. For example\, in cpan/Digest-MD5/MD5.xs where buf is const U8* buf:

#ifndef U32_ALIGNMENT_REQUIRED const U32 *x = (U32*)buf; /* really just type casting */ #endif

Throughout the code base it looks like we go out of our way to optimize byte swaps and hit the little-endian && unaligned-ok path if possible\, but I don't think that's necessary at all. Comments like "Without a known fast bswap32 we're just as well off doing this" followed by an open-coded shift and OR implementation of bswap indicate to me that the authors didn't realize that compilers can easily recognize this pattern (or wrote the code when they couldn't). Regardless\, today compilers can easily see through this and generate a bswap/movbe instruction on x86. And in fact\, it's not clear to me that even using GCC __builtin_bswap* is better or worse than the open-coded implementations (see https://gitlab.freedesktop.org/xorg/xserver/merge_requests/176)

Similarly\, while what the C code is doing in accessing unaligned pointers is undefined behavior\, x86 instruction certainly can do that. But again\, the compiler is perfectly capable of making that decision on its own.

So\, I propose that we just cut all of that code out and use what is currently the alignment-required path today.

Please review the attached patches. I have never contributed to Perl before\, and I stepped on quite a few landmines during the development of these patches (md5sum of MD5.xs goes in the test case; make regen; BYTEORDER is 0x1234 vs 0x12345678)\, so some help getting the patches in would be extremely appreciated.

Passes the perl test suite when applied to 5.30.0 on Gentoo/Linux on amd64\, 32-bit userland sparc\, and 64-bit userland sparc. From 0abdabaf4cbcac90473d48b17af60accca89c233 Mon Sep 17 00:00:00 2001 From: Matt Turner \mattst88@gmail\.com Date: Wed\, 4 Sep 2019 21:04:47 -0700 Subject: [PATCH 1/3] Digest-MD5: Consolidate byte-swapping paths

The code guarded by #ifndef U32_ALIGNMENT_REQUIRED attempts to optimize byte-swapping by doing unaligned loads\, but accessing data through unaligned pointers is undefined behavior in C. Moreover\, compilers are more than capable of recognizing these open-coded byte-swap patterns and emitting a bswap instruction\, or an unaligned load instruction\, or a combined load\, etc. There's no need for multiple paths to attain the desired result.

See https://rt-archive.perl.org/perl5/Ticket/Display.html?id=133495 --- cpan/Digest-MD5/MD5.xs | 44 +------------- cpan/Digest-MD5/Makefile.PL | 114 ------------------------------------ cpan/Digest-MD5/t/files.t | 2 +- 3 files changed\, 3 insertions(+)\, 157 deletions(-)

diff --git a/cpan/Digest-MD5/MD5.xs b/cpan/Digest-MD5/MD5.xs index a48d951056..964d37fb0e 100644 --- a/cpan/Digest-MD5/MD5.xs +++ b/cpan/Digest-MD5/MD5.xs @@ -106\,20 +106\,6 @@ static MAGIC *THX_sv_magicext(pTHX_ SV *sv\, SV *obj\, int type\, * values. The following macros (and functions) allow us to convert * between native integers and such values. */ -#undef BYTESWAP -#ifndef U32_ALIGNMENT_REQUIRED - #if BYTEORDER == 0x1234 /* 32-bit little endian */ - #define BYTESWAP(x) (x) /* no-op */ - - #elif BYTEORDER == 0x4321 /* 32-bit big endian */ - #define BYTESWAP(x) ((((x)&0xFF)\<\<24) \ - |(((x)>>24)&0xFF) \ - |(((x)&0x0000FF00)\<\<8) \ - |(((x)&0x00FF0000)>>8) ) - #endif -#endif - -#ifndef BYTESWAP static void u2s(U32 u\, U8* s) { *s++ = (U8)(u & 0xFF); @@ -132\,7 +118\,6 @@ static void u2s(U32 u\, U8* s) ((U32)(*(s+1)) \<\< 8) | \ ((U32)(*(s+2)) \<\< 16) | \ ((U32)(*(s+3)) \<\< 24)) -#endif

/* This structure keeps the current state of algorithm. */ @@ -279\,29 +264\,16 @@ MD5Transform(MD5_CTX* ctx\, const U8* buf\, STRLEN blocks) U32 C = ctx->C; U32 D = ctx->D;

-#ifndef U32_ALIGNMENT_REQUIRED - const U32 *x = (U32*)buf; /* really just type casting */ -#endif - do { U32 a = A; U32 b = B; U32 c = C; U32 d = D;

-#if BYTEORDER == 0x1234 && !defined(U32_ALIGNMENT_REQUIRED) - const U32 *X = x; - #define NEXTx (*x++) -#else - U32 X[16]; /* converted values\, used in round 2-4 */ + U32 X[16]; /* little-endian values\, used in round 2-4 */ U32 *uptr = X; U32 tmp; - #ifdef BYTESWAP - #define NEXTx (tmp=*x++\, *uptr++ = BYTESWAP(tmp)) - #else #define NEXTx (s2u(buf\,tmp)\, buf += 4\, *uptr++ = tmp) - #endif -#endif

#ifdef MD5_DEBUG if (buf == ctx->buffer) @@ -313\,7 +285\,7 @@ MD5Transform(MD5_CTX* ctx\, const U8* buf\, STRLEN blocks) int i; fprintf(stderr\,"["); for (i = 0; i \< 16; i++) { - fprintf(stderr\,"%x\,"\, x[i]); + fprintf(stderr\,"%x\,"\, x[i]); /* FIXME */ } fprintf(stderr\,"]\n"); } @@ -468\,30 +440\,18 @@ MD5Final(U8* digest\, MD5_CTX *ctx)
bits\_low = ctx\->bytes\_low \<\< 3;
bits\_high = $ctx\->bytes\_high \<\< 3$ | $ctx\->bytes\_low  >> 29$;
-#ifdef BYTESWAP - *(U32*)(ctx->buffer + fill) = BYTESWAP(bits_low); fill += 4; - *(U32*)(ctx->buffer + fill) = BYTESWAP(bits_high); fill += 4; -#else u2s(bits_low\, ctx->buffer + fill); fill += 4; u2s(bits_high\, ctx->buffer + fill); fill += 4; -#endif
MD5Transform$ctx\, ctx\->buffer\, fill >> 6$;
#ifdef MD5_DEBUG fprintf(stderr\," Result: %s\n"\, ctx_dump(ctx)); #endif

-#ifdef BYTESWAP - *(U32*)digest = BYTESWAP(ctx->A); digest += 4; - *(U32*)digest = BYTESWAP(ctx->B); digest += 4; - *(U32*)digest = BYTESWAP(ctx->C); digest += 4; - *(U32*)digest = BYTESWAP(ctx->D); -#else u2s(ctx->A\, digest); u2s(ctx->B\, digest+4); u2s(ctx->C\, digest+8); u2s(ctx->D\, digest+12); -#endif }

#ifndef INT2PTR diff --git a/cpan/Digest-MD5/Makefile.PL b/cpan/Digest-MD5/Makefile.PL index 1015058bac..76906d1046 100644 --- a/cpan/Digest-MD5/Makefile.PL +++ b/cpan/Digest-MD5/Makefile.PL @@ -5\,7 +5\,6 @@ use Config qw(%Config); use ExtUtils::MakeMaker;

my @extra; -push(@extra\, DEFINE => "-DU32_ALIGNMENT_REQUIRED") unless free_u32_alignment(); push(@extra\, INSTALLDIRS => 'perl') if $] >= 5.008 && $] \< 5.012;

if ($^O eq 'VMS') { @@ -39\,119 +38\,6 @@ WriteMakefile(

-sub free_u32_alignment -{ - $|=1; - if (exists $Config{d_u32align}) { - print "Perl's config says that U32 access must "; - print "not " unless $Config{d_u32align}; - print "be aligned.\n"; - return !$Config{d_u32align}; - } -
- if ($^O eq 'VMS' || $^O eq 'MSWin32') { - print "Assumes that $^O implies free alignment for U32 access.\n"; - return 1; - } -
- if ($^O eq 'hpux' && $Config{osvers} \< 11.0) { - print "Will not test for free alignment on older HP-UX.\n"; - return 0; - } -
- print "Testing alignment requirements for U32... "; - open(ALIGN_TEST\, ">u32align.c") or die "$!"; - print ALIGN_TEST \<\<'EOT'; close(ALIGN_TEST); -/*--------------------------------------------------------------*/ -/* This program allocates a buffer of U8 (char) and then tries */ -/* to access it through a U32 pointer at every offset. The */ -/* program is expected to die with a bus error/seg fault for */ -/* machines that do not support unaligned integer read/write */ -/*--------------------------------------------------------------*/ - -#include \<stdio.h> -#include "EXTERN.h" -#include "perl.h" - -#ifdef printf - #undef printf -#endif - -int main(int argc\, char** argv\, char** env) -{ -#if BYTEORDER == 0x1234 || BYTEORDER == 0x4321 - volatile U8 buf[] = "\0\0\0\1\0\0\0\0"; - volatile U32 *up; - int i; - - if (sizeof(U32) != 4) { - printf("sizeof(U32) is not 4\, but %d\n"\, sizeof(U32)); - exit(1); - } - - fflush(stdout); - - for (i = 0; i \< 4; i++) { - up = (U32*)(buf + i); - if (! ((*up == 1 \<\< (8*i)) || /* big-endian */ - (*up == 1 \<\< (8*(3-i))) /* little-endian */ - ) - ) - { - printf("read failed (%x)\n"\, *up); - exit(2); - } - } - - /* write test */ - for (i = 0; i \< 4; i++) { - up = (U32*)(buf + i); - *up = 0xBeef; - if (*up != 0xBeef) { - printf("write failed (%x)\n"\, *up); - exit(3); - } - } - - printf("no restrictions\n"); - exit(0); -#else - printf("unusual byteorder\, playing safe\n"); - exit(1); -#endif - return 0; -} -/*--------------------------------------------------------------*/ -EOT - - my $cc_cmd = "$Config{cc} $Config{ccflags} -I$Config{archlibexp}/CORE"; - my $exe = "u32align$Config{exe_ext}"; - $cc_cmd .= " -o $exe"; - my $rc; - $rc = system("$cc_cmd $Config{ldflags} u32align.c $Config{libs}"); - if ($rc) { - print "Can't compile test program. Will ensure alignment to play safe.\n\n"; - unlink("u32align.c"\, $exe\, "u32align$Config{obj_ext}"); - return 0; - } - - $rc = system("./$exe"); - unlink("u32align.c"\, $exe\, "u32align$Config{obj_ext}"); - - return 1 unless $rc; - - if ($rc > 0x80) { - (my $cp = $rc) >>= 8; - print "Test program exit status was $cp\n"; - } - if ($rc & 0x80) { - $rc &= ~0x80; - unlink("core") && print "Core dump deleted\n"; - } - print "signal $rc\n" if $rc && $rc \< 0x80; - return 0; -} - BEGIN { # compatibility with older versions of MakeMaker my $developer = -d ".git"; diff --git a/cpan/Digest-MD5/t/files.t b/cpan/Digest-MD5/t/files.t index 63479c24a3..ef64088c8c 100644 --- a/cpan/Digest-MD5/t/files.t +++ b/cpan/Digest-MD5/t/files.t @@ -21\,7 +21\,7 @@ EOT # This is the output of: 'md5sum README MD5.xs rfc1321.txt' $EXPECT = \<\<EOT; 2f93400875dbb56f36691d5f69f3eba5 README -9572832f3628e3bebcdd54f47c43dc5a MD5.xs +5b8b4f96bc27a425501307c5461970db MD5.xs 754b9db19f79dbc4992f7166eb0f37ce rfc1321.txt EOT } -- 2.21.0

From 17ba1f5403101c611f997cbf3d2afc2663bfccb9 Mon Sep 17 00:00:00 2001 From: Matt Turner \mattst88@gmail\.com Date: Wed\, 4 Sep 2019 21:48:56 -0700 Subject: [PATCH 2/3] Clean up U8TO*_LE macro implementations

The code guarded by #ifndef U32_ALIGNMENT_REQUIRED attempts to optimize byte-swapping by doing unaligned loads\, but accessing data through unaligned pointers is undefined behavior in C. Moreover\, compilers are more than capable of recognizing these open-coded byte-swap patterns and emitting a bswap instruction\, or an unaligned load instruction\, or a combined load\, etc. There's no need for multiple paths to attain the desired result.

See https://rt-archive.perl.org/perl5/Ticket/Display.html?id=133495 --- hv_macro.h | 31 ++++++++++++----------------- stadtx_hash.h | 52 ------------------------------------------------- zaphod32_hash.h | 35 --------------------------------- 3 files changed\, 12 insertions(+)\, 106 deletions(-)

diff --git a/hv_macro.h b/hv_macro.h index 77a4c84896..02c0baad08 100644 --- a/hv_macro.h +++ b/hv_macro.h @@ -6\,7 +6\,7 @@ #endif

/*----------------------------------------------------------------------------- - * Endianess\, misalignment capabilities and util macros + * Endianess and util macros * * The following 3 macros are defined in this section. The other macros defined * are only needed to help derive these 3. @@ -20\,29 +20\,22 @@ * ROTR64(x\,r) Rotate x right by r bits */

-#ifndef U32_ALIGNMENT_REQUIRED +#ifndef U8TO16_LE #if (BYTEORDER == 0x1234 || BYTEORDER == 0x12345678) - #define U8TO16_LE(ptr) (*((const U16*)(ptr))) - #define U8TO32_LE(ptr) (*((const U32*)(ptr))) - #define U8TO64_LE(ptr) (*((const U64*)(ptr))) + #define U8TO16_LE(ptr) ((U32)(ptr)[1]|(U32)(ptr)[0]\<\<8) + #define U8TO32_LE(ptr) ((U32)(ptr)[3]|(U32)(ptr)[2]\<\<8|(U32)(ptr)[1]\<\<16|(U32)(ptr)[0]\<\<24) + #define U8TO64_LE(ptr) ((U64)(ptr)[7]|(U64)(ptr)[6]\<\<8|(U64)(ptr)[5]\<\<16|(U64)(ptr)[4]\<\<24|\ + (U64)(ptr)[3]\<\<32|(U64)(ptr)[4]\<\<40|\ + (U64)(ptr)[1]\<\<48|(U64)(ptr)[0]\<\<56) #elif (BYTEORDER == 0x4321 || BYTEORDER == 0x87654321) - #if defined(__GNUC__) && (__GNUC__>4 || (__GNUC__==4 && __GNUC_MINOR__>=3)) - #define U8TO16_LE(ptr) (__builtin_bswap16(*((U16*)(ptr)))) - #define U8TO32_LE(ptr) (__builtin_bswap32(*((U32*)(ptr)))) - #define U8TO64_LE(ptr) (__builtin_bswap64(*((U64*)(ptr)))) - #endif + #define U8TO16_LE(ptr) ((U32)(ptr)[0]|(U32)(ptr)[1]\<\<8) + #define U8TO32_LE(ptr) ((U32)(ptr)[0]|(U32)(ptr)[1]\<\<8|(U32)(ptr)[2]\<\<16|(U32)(ptr)[3]\<\<24) + #define U8TO64_LE(ptr) ((U64)(ptr)[0]|(U64)(ptr)[1]\<\<8|(U64)(ptr)[2]\<\<16|(U64)(ptr)[3]\<\<24|\ + (U64)(ptr)[4]\<\<32|(U64)(ptr)[5]\<\<40|\ + (U64)(ptr)[6]\<\<48|(U64)(ptr)[7]\<\<56) #endif #endif

-#ifndef U8TO16_LE - /* Without a known fast bswap32 we're just as well off doing this */ - #define U8TO16_LE(ptr) ((U32)(ptr)[0]|(U32)(ptr)[1]\<\<8) - #define U8TO32_LE(ptr) ((U32)(ptr)[0]|(U32)(ptr)[1]\<\<8|(U32)(ptr)[2]\<\<16|(U32)(ptr)[3]\<\<24) - #define U8TO64_LE(ptr) ((U64)(ptr)[0]|(U64)(ptr)[1]\<\<8|(U64)(ptr)[2]\<\<16|(U64)(ptr)[3]\<\<24|\ - (U64)(ptr)[4]\<\<32|(U64)(ptr)[5]\<\<40|\ - (U64)(ptr)[6]\<\<48|(U64)(ptr)[7]\<\<56) -#endif - #ifdef CAN64BITHASH #ifndef U64TYPE /* This probably isn't going to work\, but failing with a compiler error due to diff --git a/stadtx_hash.h b/stadtx_hash.h index bd09c2f938..5ee879485d 100644 --- a/stadtx_hash.h +++ b/stadtx_hash.h @@ -43\,58 +43\,6 @@ #define STMT_END while(0) #endif

-#ifndef STADTX_ALLOW_UNALIGNED_AND_LITTLE_ENDIAN -/* STADTX_ALLOW_UNALIGNED_AND_LITTLE_ENDIAN only matters if nothing has defined U8TO64_LE etc\, - * and when built with Perl these should be defined before this file is loaded. - */ -#ifdef U32_ALIGNMENT_REQUIRED -#define STADTX_ALLOW_UNALIGNED_AND_LITTLE_ENDIAN 0 -#else -#define STADTX_ALLOW_UNALIGNED_AND_LITTLE_ENDIAN 1 -#endif -#endif - -#ifndef U8TO64_LE -#if STADTX_ALLOW_UNALIGNED_AND_LITTLE_ENDIAN -#define U8TO64_LE(ptr) (*((const U64 *)(ptr))) -#else -#define U8TO64_LE(ptr) (\ - (U64)(ptr)[7] \<\< 56 | \ - (U64)(ptr)[6] \<\< 48 | \ - (U64)(ptr)[5] \<\< 40 | \ - (U64)(ptr)[4] \<\< 32 | \ - (U64)(ptr)[3] \<\< 24 | \ - (U64)(ptr)[2] \<\< 16 | \ - (U64)(ptr)[1] \<\< 8 | \ - (U64)(ptr)[0] \ -) -#endif -#endif - -#ifndef U8TO32_LE -#if STADTX_ALLOW_UNALIGNED_AND_LITTLE_ENDIAN -#define U8TO32_LE(ptr) (*((const U32 *)(ptr))) -#else -#define U8TO32_LE(ptr) (\ - (U32)(ptr)[3] \<\< 24 | \ - (U32)(ptr)[2] \<\< 16 | \ - (U32)(ptr)[1] \<\< 8 | \ - (U32)(ptr)[0] \ -) -#endif -#endif - -#ifndef U8TO16_LE -#if STADTX_ALLOW_UNALIGNED_AND_LITTLE_ENDIAN -#define U8TO16_LE(ptr) (*((const U16 *)(ptr))) -#else -#define U8TO16_LE(ptr) (\ - (U16)(ptr)[1] \<\< 8 | \ - (U16)(ptr)[0] \ -) -#endif -#endif - /* Find best way to ROTL32/ROTL64 */ #if defined(_MSC_VER) #include \<stdlib.h> /* Microsoft put _rotl declaration in here */ diff --git a/zaphod32_hash.h b/zaphod32_hash.h index c9b60ccb32..2fb391a233 100644 --- a/zaphod32_hash.h +++ b/zaphod32_hash.h @@ -74\,41 +74\,6 @@ #define STMT_END while(0) #endif

-#ifndef ZAPHOD32_ALLOW_UNALIGNED_AND_LITTLE_ENDIAN -/* ZAPHOD32_ALLOW_UNALIGNED_AND_LITTLE_ENDIAN only matters if nothing has defined U8TO64_LE etc\, - * and when built with Perl these should be defined before this file is loaded. - */ -#ifdef U32_ALIGNMENT_REQUIRED -#define ZAPHOD32_ALLOW_UNALIGNED_AND_LITTLE_ENDIAN 0 -#else -#define ZAPHOD32_ALLOW_UNALIGNED_AND_LITTLE_ENDIAN 1 -#endif -#endif - -#ifndef U8TO32_LE -#if ZAPHOD32_ALLOW_UNALIGNED_AND_LITTLE_ENDIAN -#define U8TO32_LE(ptr) (*((const U32 *)(ptr))) -#else -#define U8TO32_LE(ptr) (\ - (U32)(ptr)[3] \<\< 24 | \ - (U32)(ptr)[2] \<\< 16 | \ - (U32)(ptr)[1] \<\< 8 | \ - (U32)(ptr)[0] \ -) -#endif -#endif - -#ifndef U8TO16_LE -#if ZAPHOD32_ALLOW_UNALIGNED_AND_LITTLE_ENDIAN -#define U8TO16_LE(ptr) (*((const U16 *)(ptr))) -#else -#define U8TO16_LE(ptr) (\ - (U16)(ptr)[1] \<\< 8 | \ - (U16)(ptr)[0] \ -) -#endif -#endif - /* This is two marsaglia xor-shift permutes\, with a prime-multiple * sandwiched inside. The end result of doing this twice with different * primes is a completely avalanched v. */ -- 2.21.0

From f74c38c62e455c7da828d5525fa4712688e2d27e Mon Sep 17 00:00:00 2001 From: Matt Turner \mattst88@gmail\.com Date: Tue\, 3 Sep 2019 21:43:47 -0700 Subject: [PATCH 3/3] Configure: Remove d_u32align test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit

It is undefined behavior in C to access data through an unaligned pointer.

C11 (draft n1570) Appendix J.2:
1 The behavior is undefined in the following circumstances&#8203;:

\.\.\.\.

· Conversion between two pointer types produces a result that is
  incorrectly aligned $6\.3\.2\.3$\.
With 6.3.2.3p7 saying
\[\.\.\.\] If the resulting pointer is not correctly aligned \[68\] for
the referenced type\, the behavior is undefined\. \[\.\.\.\]
As discussed in the ticket listed below\, this test is provoking undefined behavior. As a result\, GCC is within its rights to optimize it into garbage\, which it does to degrees varying with the optimization level. The result of this test is used to select C code paths that also rely on unaligned accesses (i.e.\, more undefined behavior)\, which the previous patches have removed.

See https://rt-archive.perl.org/perl5/Ticket/Display.html?id=133495 --- Configure | 106 --------------------------------- Cross/config.sh-arm-linux | 1 - Cross/config.sh-arm-linux-n770 | 1 - NetWare/config.wc | 1 - NetWare/config_H.wc | 6 -- Porting/Glossary | 4 -- Porting/config.sh | 1 - Porting/config_H | 8 --- config_h.SH | 8 --- configure.com | 1 - hints/hpux.sh | 7 --- plan9/config.plan9 | 8 --- plan9/config_h.sample | 8 --- plan9/config_sh.sample | 1 - symbian/config.sh | 1 - uconfig.h | 8 --- uconfig.sh | 1 - uconfig64.sh | 1 - win32/config.gc | 1 - win32/config.vc | 1 - win32/config_H.gc | 8 --- win32/config_H.vc | 8 --- 22 files changed\, 190 deletions(-)

diff --git a/Configure b/Configure index 818deb8378..4b504b117f 100755 --- a/Configure +++ b/Configure @@ -914\,7 +914\,6 @@ d_truncl='' d_ttyname_r='' ttyname_r_proto='' d_tzname='' -d_u32align='' d_ualarm='' d_umask='' d_semctl_semid_ds='' @@ -19699\,110 +19698\,6 @@ EOM ;; esac

-: Checking 32bit alignedness -$cat \<\<EOM - -Checking to see whether you can access character data unalignedly... -EOM -case "$d_u32align" in -'') $cat >try.c \<\<EOCP -#include \<stdio.h> -#$i_stdlib I_STDLIB -#ifdef I_STDLIB -#include \<stdlib.h> -#endif -#define U32 $u32type -#define BYTEORDER 0x$byteorder -#define U8 $u8type -#include \<signal.h> -#ifdef SIGBUS -$signal_t bletch(int s) { exit(4); } -#endif -int main() { -#if BYTEORDER == 0x1234 || BYTEORDER == 0x4321 - volatile U8 buf[8]; - volatile U32 *up; - int i; - - if (sizeof(U32) != 4) { - printf("sizeof(U32) is not 4\, but %d\n"\, sizeof(U32)); - exit(1); - } - - fflush(stdout); - -#ifdef SIGBUS - signal(SIGBUS\, bletch); -#endif - - buf[0] = 0; - buf[1] = 0; - buf[2] = 0; - buf[3] = 1; - buf[4] = 0; - buf[5] = 0; - buf[6] = 0; - buf[7] = 1; - - for (i = 0; i \< 4; i++) { - up = (U32*)(buf + i); - if (! ((*up == 1 \<\< (8*i)) || /* big-endian */ - (*up == 1 \<\< (8*(3-i))) /* little-endian */ - ) - ) - { - printf("read failed (%x)\n"\, *up); - exit(2); - } - } - - /* write test */ - for (i = 0; i \< 4; i++) { - up = (U32*)(buf + i); - *up = 0xBeef; - if (*up != 0xBeef) { - printf("write failed (%x)\n"\, *up); - exit(3); - } - } - - exit(0); -#else - printf("1\n"); - exit(1); -#endif - return 0; -} -EOCP -set try -if eval $compile_ok; then - echo "(Testing for character data alignment may crash the test. That's okay.)" >&4 - $run ./try 2>&1 >/dev/null - case "$?" in - 0) cat >&4 \<\<EOM -You can access character data pretty unalignedly. -EOM - d_u32align="$undef" - ;; - *) cat >&4 \<\<EOM -It seems that you must access character data in an aligned manner. -EOM - d_u32align="$define" - ;; - esac -else - rp='Can you access character data at unaligned addresses?' - dflt='n' - . ./myread - case "$ans" in - [yY]*) d_u32align="$undef" ;; - *) d_u32align="$define" ;; - esac -fi -$rm_try -;; -esac - : see if ualarm exists set ualarm d_ualarm eval $inlibc @@ -24535\,7 +24430\,6 @@ d_truncate='$d_truncate' d_truncl='$d_truncl' d_ttyname_r='$d_ttyname_r' d_tzname='$d_tzname' -d_u32align='$d_u32align' d_ualarm='$d_ualarm' d_umask='$d_umask' d_uname='$d_uname' diff --git a/Cross/config.sh-arm-linux b/Cross/config.sh-arm-linux index 9bc9903ed8..bc66187b4d 100644 --- a/Cross/config.sh-arm-linux +++ b/Cross/config.sh-arm-linux @@ -606\,7 +606\,6 @@ d_truncate='define' d_truncl='define' d_ttyname_r='undef' d_tzname='define' -d_u32align='undef' d_ualarm='define' d_umask='define' d_uname='define' diff --git a/Cross/config.sh-arm-linux-n770 b/Cross/config.sh-arm-linux-n770 index 5382163779..e4b6d047df 100644 --- a/Cross/config.sh-arm-linux-n770 +++ b/Cross/config.sh-arm-linux-n770 @@ -605\,7 +605\,6 @@ d_truncate='define' d_truncl='define' d_ttyname_r='undef' d_tzname='define' -d_u32align='undef' d_ualarm='define' d_umask='define' d_uname='define' diff --git a/NetWare/config.wc b/NetWare/config.wc index 9173c9de0f..7138e690f2 100644 --- a/NetWare/config.wc +++ b/NetWare/config.wc @@ -596\,7 +596\,6 @@ d_truncate='undef' d_truncl='undef' d_ttyname_r='undef' d_tzname='define' -d_u32align='undef' d_ualarm='undef' d_umask='define' d_uname='define' diff --git a/NetWare/config_H.wc b/NetWare/config_H.wc index 8348d439f5..e3eb3f1c6e 100644 --- a/NetWare/config_H.wc +++ b/NetWare/config_H.wc @@ -3236\,12 +3236\,6 @@ */ /*#define HAS_SYSCALL_PROTO /**/

-/* U32_ALIGNMENT_REQUIRED: - * This symbol\, if defined\, indicates that you must access - * character data through U32-aligned pointers. - */ -/*#define U32_ALIGNMENT_REQUIRED /**/ - /* HAS_USLEEP_PROTO: * This symbol\, if defined\, indicates that the system provides * a prototype for the usleep() function. Otherwise\, it is up diff --git a/Porting/Glossary b/Porting/Glossary index 16acb5e26f..dcfaf39898 100644 --- a/Porting/Glossary +++ b/Porting/Glossary @@ -2805\,10 +2805\,6 @@ d_tzname (d_tzname.U): This variable conditionally defines HAS_TZNAME if tzname[] is available to access timezone names.

-d_u32align (d_u32align.U): - This variable tells whether you must access character data - through U32-aligned pointers. - d_ualarm (d_ualarm.U): This variable conditionally defines the HAS_UALARM symbol\, which indicates to the C program that the ualarm() routine is available. diff --git a/Porting/config.sh b/Porting/config.sh index ebcbd06ea2..3fe1237549 100644 --- a/Porting/config.sh +++ b/Porting/config.sh @@ -622\,7 +622\,6 @@ d_truncate='define' d_truncl='define' d_ttyname_r='undef' d_tzname='define' -d_u32align='define' d_ualarm='define' d_umask='define' d_uname='define' diff --git a/Porting/config_H b/Porting/config_H index 758a1b2291..b1fc1baca1 100644 --- a/Porting/config_H +++ b/Porting/config_H @@ -3470\,14 +3470\,6 @@ */ #define HAS_TRUNCL /**/

-/* U32_ALIGNMENT_REQUIRED: - * This symbol\, if defined\, indicates that you must access - * character data through U32-aligned pointers. - */ -#ifndef U32_ALIGNMENT_REQUIRED -#define U32_ALIGNMENT_REQUIRED /**/ -#endif - /* HAS_UALARM: * This symbol\, if defined\, indicates that the ualarm routine is * available to do alarms with microsecond granularity. diff --git a/config_h.SH b/config_h.SH index e776983080..e88649d285 100755 --- a/config_h.SH +++ b/config_h.SH @@ -3526\,14 +3526\,6 @@ sed \<\<!GROK!THIS! >$CONFIG_H -e 's!^#undef$.*/$\*!/\*#define\1 \*!' -e 's!^#un */ #$d_truncl HAS_TRUNCL /**/

-/* U32_ALIGNMENT_REQUIRED: - * This symbol\, if defined\, indicates that you must access - * character data through U32-aligned pointers. - */ -#ifndef U32_ALIGNMENT_REQUIRED -#$d_u32align U32_ALIGNMENT_REQUIRED /**/ -#endif - /* HAS_UALARM: * This symbol\, if defined\, indicates that the ualarm routine is * available to do alarms with microsecond granularity. diff --git a/configure.com b/configure.com index dc251dfe04..4f2f80496f 100644 --- a/configure.com +++ b/configure.com @@ -6424\,7 +6424\,6 @@ $ WC "d_truncate='" + d_truncate + "'" $ WC "d_trunc='" + d_trunc + "'" $ WC "d_truncl='" + d_truncl + "'" $ WC "d_tzname='" + d_tzname + "'" -$ WC "d_u32align='define'" $ WC "d_ualarm='" + d_ualarm + "'" $ WC "d_umask='define'" $ WC "d_uname='" + d_uname + "'" diff --git a/hints/hpux.sh b/hints/hpux.sh index 8f273a6ef9..689a39b2b3 100644 --- a/hints/hpux.sh +++ b/hints/hpux.sh @@ -34\,13 +34\,6 @@ else selecttype='int *' fi

-# For some strange reason\, the u32align test from Configure hangs in -# HP-UX 10.20 since the December 2001 patches. So hint it to avoid -# the test. -if [ "$xxOsRevMajor" -le 10 ]; then - d_u32align=$define - fi - echo "Archname is $archname"

# Fix XSlib (CPAN) confusion when re-using a prefix but changing from ILP32 diff --git a/plan9/config.plan9 b/plan9/config.plan9 index efe6488dfd..2d12d15725 100644 --- a/plan9/config.plan9 +++ b/plan9/config.plan9 @@ -3677\,14 +3677\,6 @@ */ /*#define HAS_SYSCALL_PROTO / **/

-/* U32_ALIGNMENT_REQUIRED: - * This symbol\, if defined\, indicates that you must access - * character data through U32-aligned pointers. - */ -#ifndef U32_ALIGNMENT_REQUIRED -#define U32_ALIGNMENT_REQUIRED /**/ -#endif - /* HAS_USLEEP_PROTO: * This symbol\, if defined\, indicates that the system provides * a prototype for the usleep() function. Otherwise\, it is up diff --git a/plan9/config_h.sample b/plan9/config_h.sample index f71d55bd53..d2096dbbf2 100644 --- a/plan9/config_h.sample +++ b/plan9/config_h.sample @@ -3613\,14 +3613\,6 @@ */ /*#define HAS_SYSCALL_PROTO / **/

-/* U32_ALIGNMENT_REQUIRED: - * This symbol\, if defined\, indicates that you must access - * character data through U32-aligned pointers. - */ -#ifndef U32_ALIGNMENT_REQUIRED -#define U32_ALIGNMENT_REQUIRED /**/ -#endif - /* HAS_USLEEP_PROTO: * This symbol\, if defined\, indicates that the system provides * a prototype for the usleep() function. Otherwise\, it is up diff --git a/plan9/config_sh.sample b/plan9/config_sh.sample index 6b2d0d4f61..036afe1fb9 100644 --- a/plan9/config_sh.sample +++ b/plan9/config_sh.sample @@ -606\,7 +606\,6 @@ d_truncate='undef' d_truncl='undef' d_ttyname_r='undef' d_tzname='define' -d_u32align='define' d_ualarm='undef' d_umask='define' d_uname='define' diff --git a/symbian/config.sh b/symbian/config.sh index 8b3122b286..719f7dc90b 100644 --- a/symbian/config.sh +++ b/symbian/config.sh @@ -555\,7 +555\,6 @@ d_truncate='undef' d_truncl='undef' d_ttyname_r='undef' d_tzname='undef' -d_u32align='define' d_ualarm='undef' d_umask='undef' d_uname='undef' diff --git a/uconfig.h b/uconfig.h index 09fd622baa..757d2e4278 100644 --- a/uconfig.h +++ b/uconfig.h @@ -3491\,14 +3491\,6 @@ */ /*#define HAS_TRUNCL / **/

-/* U32_ALIGNMENT_REQUIRED: - * This symbol\, if defined\, indicates that you must access - * character data through U32-aligned pointers. - */ -#ifndef U32_ALIGNMENT_REQUIRED -#define U32_ALIGNMENT_REQUIRED /**/ -#endif - /* HAS_UALARM: * This symbol\, if defined\, indicates that the ualarm routine is * available to do alarms with microsecond granularity. diff --git a/uconfig.sh b/uconfig.sh index fca0e4dacd..8fe327dd0c 100644 --- a/uconfig.sh +++ b/uconfig.sh @@ -546\,7 +546\,6 @@ d_truncate='undef' d_truncl='undef' d_ttyname_r='undef' d_tzname='undef' -d_u32align='define' d_ualarm='undef' d_umask='undef' d_uname='undef' diff --git a/uconfig64.sh b/uconfig64.sh index a6b9901acd..923bc899ba 100644 --- a/uconfig64.sh +++ b/uconfig64.sh @@ -546\,7 +546\,6 @@ d_truncate='undef' d_truncl='undef' d_ttyname_r='undef' d_tzname='undef' -d_u32align='define' d_ualarm='undef' d_umask='undef' d_uname='undef' diff --git a/win32/config.gc b/win32/config.gc index ce9a6b9ad7..5cd641cc44 100644 --- a/win32/config.gc +++ b/win32/config.gc @@ -595\,7 +595\,6 @@ d_truncate='undef' d_truncl='define' d_ttyname_r='undef' d_tzname='define' -d_u32align='define' d_ualarm='undef' d_umask='define' d_uname='define' diff --git a/win32/config.vc b/win32/config.vc index 0855dc8c09..0f0859b1cd 100644 --- a/win32/config.vc +++ b/win32/config.vc @@ -595\,7 +595\,6 @@ d_truncate='undef' d_truncl='undef' d_ttyname_r='undef' d_tzname='define' -d_u32align='define' d_ualarm='undef' d_umask='define' d_uname='define' diff --git a/win32/config_H.gc b/win32/config_H.gc index 115c88cde1..e9023fe086 100644 --- a/win32/config_H.gc +++ b/win32/config_H.gc @@ -3422\,14 +3422\,6 @@ */ #define HAS_TRUNCL /**/

-/* U32_ALIGNMENT_REQUIRED: - * This symbol\, if defined\, indicates that you must access - * character data through U32-aligned pointers. - */ -#ifndef U32_ALIGNMENT_REQUIRED -#define U32_ALIGNMENT_REQUIRED /**/ -#endif - /* HAS_UALARM: * This symbol\, if defined\, indicates that the ualarm routine is * available to do alarms with microsecond granularity. diff --git a/win32/config_H.vc b/win32/config_H.vc index 05c8093615..ae4d1cbbe0 100644 --- a/win32/config_H.vc +++ b/win32/config_H.vc @@ -3413\,14 +3413\,6 @@ */ /*#define HAS_TRUNCL / **/

-/* U32_ALIGNMENT_REQUIRED: - * This symbol\, if defined\, indicates that you must access - * character data through U32-aligned pointers. - */ -#ifndef U32_ALIGNMENT_REQUIRED -#define U32_ALIGNMENT_REQUIRED /**/ -#endif - /* HAS_UALARM: * This symbol\, if defined\, indicates that the ualarm routine is * available to do alarms with microsecond granularity. -- 2.21.0

p5pRT commented 5 years ago

From mattst88@gmail.com

On Tue\, 10 Sep 2019 22:24:59 -0700\, hiroshi-hakoyama@nagano.ac.jp wrote:

Please tell me if you want to ask me to do some tests on Solaris 10 / sparc64.

That would be welcome\, but certainly not required :)

p5pRT commented 5 years ago

From mattst88@gmail.com

Now fixed with

https://perl5.git.perl.org/perl.git/commit/ee9ac1cd8eb988fea70841eae211b11355711416 https://perl5.git.perl.org/perl.git/commit/e8864dba80952684bf3afe83438d4eee0c3939a9

Should be in the 5.32 release.

p5pRT commented 5 years ago

From @tonycoz

On Tue\, 08 Oct 2019 13:21:39 -0700\, mattst88@gmail.com wrote:

Now fixed with

https://perl5.git.perl.org/perl.git/commit/ee9ac1cd8eb988fea70841eae211b11355711416 https://perl5.git.perl.org/perl.git/commit/e8864dba80952684bf3afe83438d4eee0c3939a9

Should be in the 5.32 release.

And closing.

Tony

p5pRT commented 5 years ago

@tonycoz - Status changed from 'open' to 'pending release'

demerphq commented 5 years ago

This ticket needs to be reopened. The patch applied is broken and causes perl to read little endian data as big-endian, and big-endian data as little endian. I do not understand why a bug report from solaris would lead to gross changes to win32 logic. All that was required was to mark Solaris as a system where unaligned reads are not allowed.

I am going to revert e8864dba8095.

mattst88 commented 5 years ago

I didn't see that this was reopened until just now, but I submitted a patch to reverse the macros correctly. I would prefer that you didn't revert the patch. It's unfortunate and embarrassing that the patch was broken but it was quite the ordeal to get anything committed at all -- at least an order of magnitude more effort in administrative work than debugging the problem.

I very much appreciate your review.

See https://github.com/Perl/perl5/issues/17244 for the fix.

demerphq commented 5 years ago

FWIW, the administrative burden is part of the reason we migrated to github, and hopefully should be less painful. Certainly while I am following this issue closely it should not be so painful.

mattst88 commented 4 years ago

We've sorted out all the issues, so this can be closed again.

Perl / perl5