Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perl-5.28.0 fails to build on Solaris 10 #16680

Closed
p5pRT opened this issue Sep 3, 2018 · 26 comments
Closed

perl-5.28.0 fails to build on Solaris 10 #16680

p5pRT opened this issue Sep 3, 2018 · 26 comments

Comments

@p5pRT
Copy link

p5pRT commented Sep 3, 2018

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

Searchable as RT133495$

@p5pRT
Copy link
Author

p5pRT commented Sep 3, 2018

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
Copy link
Author

p5pRT commented Sep 3, 2018

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
Copy link
Author

p5pRT commented Sep 3, 2018

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

@p5pRT
Copy link
Author

p5pRT commented Sep 3, 2018

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
Copy link
Author

p5pRT commented Sep 11, 2018

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
Copy link
Author

p5pRT commented Jan 16, 2019

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=<optimized out>)
  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=<optimized out>)
  at zaphod32_hash.h​:280
#1 Perl_newGP (gv=<optimized out>) 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=<optimized out>) 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=<optimized out>) at perly.c​:340
#8 0x00118d14 in S_doeval_compile (gimme=<optimized out>,
  outside=<optimized out>, seq=<optimized out>, hh=0x0) at pp_ctl.c​:3501
#9 0x0011ec90 in S_require_file (sv=<optimized out>) 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=<optimized out>)
  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=<optimized out>,
  fullname=<optimized out>, cv=<optimized out>, gv=<optimized out>)
#15 0x000317e0 in Perl_newATTRSUB_x (floor=47, o=<optimized out>,
  proto=<optimized out>, attrs=<optimized out>, block=<optimized out>,
  o_is_gv=<optimized out>) at op.c​:10269
#16 0x00035e40 in Perl_utilize (aver=<optimized out>, floor=47,
  version=<optimized out>, idop=<optimized out>, arg=<optimized out>)
  at op.c​:7484
#17 0x00077988 in Perl_yyparse (gramtype=<optimized out>) at perly.y​:329
#18 0x00042930 in S_parse_body (xsinit=<optimized out>, env=<optimized out>)
  at perl.c​:2509
#19 perl_parse (my_perl=<optimized out>, xsinit=<optimized out>,
  argc=<optimized out>, argv=<optimized out>, env=<optimized out>)
  at perl.c​:1803
#20 0x00012e38 in main (argc=<optimized out>, argv=<optimized out>,
  env=<optimized out>) 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
Copy link
Author

p5pRT commented Jan 16, 2019

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
Copy link
Author

p5pRT commented Jan 17, 2019

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
Copy link
Author

p5pRT commented Jan 17, 2019

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 -&gt; $?" ; 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
Copy link
Author

p5pRT commented Jan 17, 2019

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
Copy link
Author

p5pRT commented Jan 21, 2019

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
Copy link
Author

p5pRT commented Jan 29, 2019

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
Copy link
Author

p5pRT commented Sep 5, 2019

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
Copy link
Author

p5pRT commented Sep 5, 2019

From mattst88@gmail.com

p.patch
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.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 <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.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 <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:

	....

	· 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 <<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
Copy link
Author

p5pRT commented Sep 7, 2019

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
Copy link
Author

p5pRT commented Sep 7, 2019

From mattst88@gmail.com

I've opened a pull request again Digest-MD5 here​: gisle/digest-md5#16

@p5pRT
Copy link
Author

p5pRT commented Sep 7, 2019

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
Copy link
Author

p5pRT commented Sep 11, 2019

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 $] &gt;= 5.008 &amp;&amp; $] < 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
Copy link
Author

p5pRT commented Sep 11, 2019

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
Copy link
Author

p5pRT commented Oct 8, 2019

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
Copy link
Author

p5pRT commented Oct 8, 2019

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

@demerphq
Copy link
Collaborator

demerphq commented Nov 5, 2019

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 e8864db.

@demerphq demerphq reopened this Nov 5, 2019
@mattst88
Copy link
Contributor

mattst88 commented Nov 5, 2019

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 #17244 for the fix.

@demerphq
Copy link
Collaborator

demerphq commented Nov 5, 2019

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
Copy link
Contributor

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

gentoo-bot pushed a commit to gentoo/gentoo that referenced this issue Aug 11, 2020
As its been indicated that this detection has been fixed upstream in
perl since 5.31.5, and the workaround should no longer be needed

Bug: https://bugs.gentoo.org/676062
Bug: https://bugs.gentoo.org/688432
Bug: Perl/perl5#16828
Bug: Perl/perl5#16680
Commit: Perl/perl5@ee9ac1c
Commit: Perl/perl5@e8864db
Package-Manager: Portage-2.3.103, Repoman-2.3.22
Signed-off-by: Kent Fredric <kentnl@gentoo.org>
@toddr toddr closed this as completed Oct 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants