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

Multiconcat breaks AIX builds #16239

Closed
p5pRT opened this issue Nov 11, 2017 · 11 comments
Closed

Multiconcat breaks AIX builds #16239

p5pRT opened this issue Nov 11, 2017 · 11 comments

Comments

@p5pRT
Copy link

p5pRT commented Nov 11, 2017

Migrated from rt.perl.org#132430 (status was 'resolved')

Searchable as RT132430$

@p5pRT
Copy link
Author

p5pRT commented Nov 11, 2017

From @khwilliamson

This is a bug report for perl from khw@​cpan.org,
generated with the help of perlbug 1.40 running under perl 5.27.6.


AIX builds are giving assertion failures since multiconcat, like

  "../../miniperl" "-I../../lib" Functions_pm.PL
../../pod/perlfunc.pod
The assert subroutine failed​: ((void*)(p)) != 0, file op.c, line 3139
/bin/sh​: 798724 IOT/Abort trap(coredump)
make​: 1254-004 The error code from the last command is 134.

This is happening even with ilmari's patch applied
https://perl5.git.perl.org/perl.git/shortlog/refs/heads/smoke-me/ilmari/multiconcat-fix

A bisect did not give perfect results, but I believe it to be
e839e6e
Author​: David Mitchell <davem@​iabyn.com>
  Date​: Tue Aug 8 18​:42​:14 2017 +0100

  Add OP_MULTICONCAT op



Flags​:
  category=core
  severity=high


Site configuration information for perl 5.27.6​:

Configured by khw at Sat Nov 11 06​:08​:38 MET 2017.

Summary of my perl5 (revision 5 version 27 subversion 6) configuration​:
  Commit id​: d290513
  Platform​:
  osname=aix
  osvers=5.3.0.0
  archname=aix-thread-multi-64int
  uname='aix i3 3 5 0004898ad300 '
  config_args='-des -Uversiononly -Dprefix=/perl/usr/khw/devel
-Dusedevel -A'optimize=-g3' -A'optimize=-O0'
-Accflags='-DPERL_EXTERNAL_GLOB' -Accflags='-DNO_MATHOMS' -Dman1dir=none
-Dman3dir=none -Dcc=xlc -DDEBUGGING -Dusemorebits -Dusecbacktrace
-Dusethreads'
  hint=recommended
  useposix=true
  d_sigaction=define
  useithreads=define
  usemultiplicity=define
  use64bitint=define
  use64bitall=undef
  uselongdouble=define
  usemymalloc=n
  default_inc_excludes_dot=define
  bincompat5005=undef
  Compiler​:
  cc='xlc -q32'
  ccflags ='-D_ALL_SOURCE -D_ANSI_C_SOURCE -D_POSIX_SOURCE
-qmaxmem=-1 -qnoansialias -qlanglvl=extc99 -DUSE_NATIVE_DLOPEN
-DPERL_EXTERNAL_GLOB -DNO_MATHOMS -DNEED_PTHREAD_INIT -DDEBUGGING
-I/usr/local/include -q32 -D_LARGE_FILES'
  optimize=' -g3 -O0'
  cppflags='-D_ALL_SOURCE -D_ANSI_C_SOURCE -D_POSIX_SOURCE
-DUSE_NATIVE_DLOPEN -DPERL_EXTERNAL_GLOB -DNO_MATHOMS
-DNEED_PTHREAD_INIT -DDEBUGGING -I/usr/local/include'
  ccversion='12.1.0.12'
  gccversion=''
  gccosandvers=''
  intsize=4
  longsize=4
  ptrsize=4
  doublesize=8
  byteorder=87654321
  doublekind=4
  d_longlong=define
  longlongsize=8
  d_longdbl=define
  longdblsize=8
  longdblkind=0
  ivtype='long long'
  ivsize=8
  nvtype='long double'
  nvsize=8
  Off_t='off_t'
  lseeksize=8
  alignbytes=8
  prototype=define
  Linker and Libraries​:
  ld='ld'
  ldflags =' -brtl -bdynamic -L/usr/local/lib -b32 -bmaxdata​:0x80000000'
  libpth=/usr/local/lib /lib /usr/lib /usr/ccs/lib /usr/lib64
  libs=-lpthread -lbind -lnsl -ldbm -ldl -lld -lm -lcrypt -lpthreads -lc
  perllibs=-lpthread -lbind -lnsl -ldl -lld -lm -lcrypt -lpthreads -lc
  libc=/lib/libc.a
  so=a
  useshrplib=false
  libperl=libperl.a
  gnulibc_version=''
  Dynamic Linking​:
  dlsrc=dl_aix.xs
  dlext=so
  d_dlsymun=undef
  ccdlflags='
-bE​:/perl/usr/khw/devel/lib/5.27.6/aix-thread-multi-64int/CORE/perl.exp'
  cccdlflags=' '
  lddlflags='-bhalt​:4 -G -bI​:$(PERL_INC)/perl.exp -bE​:$(BASEEXT).exp
-bnoentry -lpthreads -lc -lm -L/usr/local/lib'


@​INC for perl 5.27.6​:
  /perl/usr/khw/perl/working/lib
  /perl/usr/khw/perl/working/t
  /perl/usr/khw/devel/lib/site_perl/5.27.6/aix-thread-multi-64int
  /perl/usr/khw/devel/lib/site_perl/5.27.6
  /perl/usr/khw/devel/lib/5.27.6/aix-thread-multi-64int
  /perl/usr/khw/devel/lib/5.27.6


Environment for perl 5.27.6​:
  HOME=/perl/usr/khw
  LANG=en_US
  LANGUAGE (unset)
  LC__FASTMSG=true
  LD_LIBRARY_PATH (unset)
  LIBPATH (unset)
  LOGDIR (unset)

PATH=/perl/usr/khw/bin​:/bin​:/usr/local/ppc64/bin​:/usr/bin​:/etc​:/usr/sbin​:/pro/local/bin​:/pro/bin​:/usr/local/bin​:/usr/vac/bin​:/usr/local/sbin​:/usr/ucb​:/usr/bin/X11​:/sbin​:/usr/java14/jre/bin​:/usr/java14/bin
  PERL5OPT=-w
  PERL_BADLANG (unset)
  PERL_DIFF_TOOL=wgdiff
  PERL_POD_PEDANTIC=1
  SHELL=/bin/ksh

@p5pRT
Copy link
Author

p5pRT commented Nov 13, 2017

From @khwilliamson

I verified that the recent fix
ca84e88
for vms did not affect this bug report.

That is, this problem persists after this commit is applied.
--
Karl Williamson

@p5pRT
Copy link
Author

p5pRT commented Nov 13, 2017

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

@p5pRT
Copy link
Author

p5pRT commented Nov 13, 2017

From @iabyn

On Fri, Nov 10, 2017 at 09​:51​:25PM -0800, karl williamson wrote​:

AIX builds are giving assertion failures since multiconcat, like

     "\.\./\.\./miniperl" "\-I\.\./\.\./lib" Functions\_pm\.PL 

../../pod/perlfunc.pod
The assert subroutine failed​: ((void*)(p)) != 0, file op.c, line 3139
/bin/sh​: 798724 IOT/Abort trap(coredump)
make​: 1254-004 The error code from the last command is 134.

Do you have a link to the smoke report, and/or the commit which this
assert failure corresponds to? I'm having difficulty matching line 3139
up with an assert, and that file has had lots of changes recently.

--
This is a great day for France!
  -- Nixon at Charles De Gaulle's funeral

@p5pRT
Copy link
Author

p5pRT commented Nov 13, 2017

From @khwilliamson

On 11/13/2017 09​:24 AM, Dave Mitchell wrote​:

On Fri, Nov 10, 2017 at 09​:51​:25PM -0800, karl williamson wrote​:

AIX builds are giving assertion failures since multiconcat, like

      "\.\./\.\./miniperl" "\-I\.\./\.\./lib" Functions\_pm\.PL

../../pod/perlfunc.pod
The assert subroutine failed​: ((void*)(p)) != 0, file op.c, line 3139
/bin/sh​: 798724 IOT/Abort trap(coredump)
make​: 1254-004 The error code from the last command is 134.

Do you have a link to the smoke report, and/or the commit which this
assert failure corresponds to? I'm having difficulty matching line 3139
up with an assert, and that file has had lots of changes recently.

It was today's blead as of b6f3718
Line 3139 is a Copy(), whose definition is

  #define Copy(s,d,n,t) (MEM_WRAP_CHECK_(n,t) perl_assert_ptr(d),
perl_assert_ptr(s), (void)memcpy((char*)(d),(const char*)(s), (n) *
sizeof(t)))

And looking at that makes me wonder what the point of perl_assert_ptr()
is. It is

#define perl_assert_ptr(p) assert( ((void*)(p)) != 0 )

Why is the cast necessary? and if it is then why doesn't proto.h use
this formulation? Instead it uses

assert(p)

@p5pRT
Copy link
Author

p5pRT commented Nov 13, 2017

From @demerphq

On 13 Nov 2017 17​:46, "Karl Williamson" <public@​khwilliamson.com> wrote​:

On 11/13/2017 09​:24 AM, Dave Mitchell wrote​:

On Fri, Nov 10, 2017 at 09​:51​:25PM -0800, karl williamson wrote​:

AIX builds are giving assertion failures since multiconcat, like

      "\.\./\.\./miniperl" "\-I\.\./\.\./lib" Functions\_pm\.PL

../../pod/perlfunc.pod
The assert subroutine failed​: ((void*)(p)) != 0, file op.c, line 3139
/bin/sh​: 798724 IOT/Abort trap(coredump)
make​: 1254-004 The error code from the last command is 134.

Do you have a link to the smoke report, and/or the commit which this
assert failure corresponds to? I'm having difficulty matching line 3139
up with an assert, and that file has had lots of changes recently.

It was today's blead as of b6f3718
Line 3139 is a Copy(), whose definition is

#define Copy(s,d,n,t) (MEM_WRAP_CHECK_(n,t) perl_assert_ptr(d),
perl_assert_ptr(s), (void)memcpy((char*)(d),(const char*)(s), (n) *
sizeof(t)))

And looking at that makes me wonder what the point of perl_assert_ptr()
is. It is

#define perl_assert_ptr(p) assert( ((void*)(p)) != 0 )

Why is the cast necessary?

It's a portability fix. I forget the details but this was a Zefram
Approved(TM) solution to making the assert warning free on all platforms. I
will try to find more context for you.

And if it is then why doesn't proto.h use this formulation? Instead it uses

assert(p)

Probably because the other macro is new and we haven't encountered the core
issue with it yet. Or we didn't care or something.

Yves

@p5pRT
Copy link
Author

p5pRT commented Nov 13, 2017

From @demerphq

On 13 Nov 2017 17​:52, "demerphq" <demerphq@​gmail.com> wrote​:

On 13 Nov 2017 17​:46, "Karl Williamson" <public@​khwilliamson.com> wrote​:

On 11/13/2017 09​:24 AM, Dave Mitchell wrote​:

On Fri, Nov 10, 2017 at 09​:51​:25PM -0800, karl williamson wrote​:

AIX builds are giving assertion failures since multiconcat, like

      "\.\./\.\./miniperl" "\-I\.\./\.\./lib" Functions\_pm\.PL

../../pod/perlfunc.pod
The assert subroutine failed​: ((void*)(p)) != 0, file op.c, line 3139
/bin/sh​: 798724 IOT/Abort trap(coredump)
make​: 1254-004 The error code from the last command is 134.

Do you have a link to the smoke report, and/or the commit which this
assert failure corresponds to? I'm having difficulty matching line 3139
up with an assert, and that file has had lots of changes recently.

It was today's blead as of b6f3718
Line 3139 is a Copy(), whose definition is

#define Copy(s,d,n,t) (MEM_WRAP_CHECK_(n,t) perl_assert_ptr(d),
perl_assert_ptr(s), (void)memcpy((char*)(d),(const char*)(s), (n) *
sizeof(t)))

And looking at that makes me wonder what the point of perl_assert_ptr()
is. It is

#define perl_assert_ptr(p) assert( ((void*)(p)) != 0 )

Why is the cast necessary?

It's a portability fix. I forget the details but this was a Zefram
Approved(TM) solution to making the assert warning free on all platforms. I
will try to find more context for you.

See the thread "New assert()s in handy.h cause annoying warnings"

The problem is that the simple assert form throws warnings about always
being true when the argument is something like a pointer to a literal on
some compilers. The cast and comparison apparently gives the compiler
enough info to understand that it shouldn't care.

Yves

@p5pRT
Copy link
Author

p5pRT commented Nov 14, 2017

From @iabyn

On Mon, Nov 13, 2017 at 09​:45​:29AM -0700, Karl Williamson wrote​:

On 11/13/2017 09​:24 AM, Dave Mitchell wrote​:

On Fri, Nov 10, 2017 at 09​:51​:25PM -0800, karl williamson wrote​:

AIX builds are giving assertion failures since multiconcat, like

      "\.\./\.\./miniperl" "\-I\.\./\.\./lib" Functions\_pm\.PL

../../pod/perlfunc.pod
The assert subroutine failed​: ((void*)(p)) != 0, file op.c, line 3139
/bin/sh​: 798724 IOT/Abort trap(coredump)
make​: 1254-004 The error code from the last command is 134.

Now fixed with

  commit 6623aa6
  Author​: David Mitchell <davem@​iabyn.com>
  AuthorDate​: Tue Nov 14 13​:27​:59 2017 +0000
  Commit​: David Mitchell <davem@​iabyn.com>
  CommitDate​: Tue Nov 14 14​:11​:57 2017 +0000

  OP_MULTICONCAT​: fix AIX
 
  The OP_MULTICONCAT work broke AIX builds because it turns out that
  PerlMemShared_malloc() isn't safe (in the sense of safemalloc());
  i.e. on AIX, PerlMemShared_malloc(0) returns NULL.

  M op.c

--
"Procrastination grows to fill the available time"
  -- Mitchell's corollary to Parkinson's Law

@p5pRT
Copy link
Author

p5pRT commented Nov 15, 2017

From @khwilliamson

I confirmed that AIX now builds. so am resolving this ticket
--
Karl Williamson

@p5pRT
Copy link
Author

p5pRT commented Nov 15, 2017

@khwilliamson - Status changed from 'open' to 'resolved'

@p5pRT p5pRT closed this as completed Nov 15, 2017
@p5pRT
Copy link
Author

p5pRT commented Nov 15, 2017

From @xsawyerx

On 11/13/2017 05​:52 PM, demerphq wrote​:

On 13 Nov 2017 17​:46, "Karl Williamson" <public@​khwilliamson.com
<mailto​:public@​khwilliamson.com>> wrote​:

On 11/13/2017 09&#8203;:24 AM\, Dave Mitchell wrote&#8203;:

    On Fri\, Nov 10\, 2017 at 09&#8203;:51&#8203;:25PM \-0800\, karl williamson wrote&#8203;:

        AIX builds are giving assertion failures since
        multiconcat\, like

                  "\.\./\.\./miniperl" "\-I\.\./\.\./lib" Functions\_pm\.PL
        \.\./\.\./pod/perlfunc\.pod
        The assert subroutine failed&#8203;: \(\(void\*\)\(p\)\) \!= 0\, file
        op\.c\, line 3139
        /bin/sh&#8203;: 798724 IOT/Abort trap\(coredump\)
        make&#8203;: 1254\-004 The error code from the last command is 134\.



    Do you have a link to the smoke report\, and/or the commit
    which this
    assert failure corresponds to? I'm having difficulty matching
    line 3139
    up with an assert\, and that file has had lots of changes recently\.


It was today's blead as of b6f3718f1b3218256c962a8f46e747048228d196
Line 3139 is a Copy\(\)\, whose definition is

 \#define Copy\(s\,d\,n\,t\)   \(MEM\_WRAP\_CHECK\_\(n\,t\) perl\_assert\_ptr\(d\)\,
perl\_assert\_ptr\(s\)\, \(void\)memcpy\(\(char\*\)\(d\)\,\(const char\*\)\(s\)\, \(n\)
\* sizeof\(t\)\)\)


And looking at that makes me wonder what the point of
perl\_assert\_ptr\(\) is\.  It is

\#define perl\_assert\_ptr\(p\) assert\( \(\(void\*\)\(p\)\) \!= 0 \)

Why is the cast necessary? 

It's a portability fix. I forget the details but this was a Zefram
Approved(TM) solution to making the assert warning free on all
platforms. I will try to find more context for you.

Hey Yves,

Any chance we could add documentation in the code explaining the reason
for this macro? It would be easier for anyone to understanding
immediately instead of reviewing the commit history.I think we should do
that every time we add a new non-trivial macro.

Also, shouldn't this macro be uppercase?

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant