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

[PATCH] remove all usage of dl_undef_symbols on platforms where its a nop #14623

Closed
p5pRT opened this issue Mar 27, 2015 · 7 comments
Closed

Comments

@p5pRT
Copy link

p5pRT commented Mar 27, 2015

Migrated from rt.perl.org#124189 (status was 'open')

Searchable as RT124189$

@p5pRT
Copy link
Author

p5pRT commented Mar 27, 2015

From @bulk88

Created by @bulk88

See attached patch.

Perl Info

Flags:
                   category=core
                   severity=low

Site configuration information for perl 5.21.4:

Configured by Owner at Thu Sep 18 12:08:58 2014.

Summary of my perl5 (revision 5 version 21 subversion 4) configuration:
                 Derived from: 7d2b2edb94ab56333b9049a3e26d15ea18445512
                 Ancestor: 19be3be6968e2337bcdfe480693fff795ecd1304
                 Platform:
                   osname=MSWin32, osvers=5.1,
archname=MSWin32-x86-multi-thread
                   uname=''
                   config_args='undef'
                   hint=recommended, useposix=true, d_sigaction=undef
                   useithreads=define, usemultiplicity=define
                   use64bitint=undef, use64bitall=undef, uselongdouble=undef
                   usemymalloc=n, bincompat5005=undef
                 Compiler:
                   cc='cl', ccflags ='-nologo -GF -W3 -O1 -MD -Zi -DNDEBUG
-DWIN32
-D_CONSOLE -DNO_STRICT  -DPERL_TEXTMODE_SCRIPTS
-DPERL_HASH_FUNC_ONE_AT_A_TIME -DPERL_IMPLICIT_CONTEXT
-DPERL_IMPLICIT_SYS -DUSE_PERLIO -D_USE_32BIT_TIME_T',
                   optimize='-O1 -MD -Zi -DNDEBUG',
                   cppflags='-DWIN32'
                   ccversion='12.00.8168', gccversion='', gccosandvers=''
                   intsize=4, longsize=4, ptrsize=4, doublesize=8,
byteorder=1234
                   d_longlong=undef, longlongsize=8, d_longdbl=define,
longdblsize=8,
longdblkind=0
                   ivtype='long', ivsize=4, nvtype='double', nvsize=8,
Off_t='__int64',
lseeksize=8
                   alignbytes=8, prototype=define
                 Linker and Libraries:
                   ld='link', ldflags ='-nologo -nodefaultlib -debug
-opt:ref,icf
-libpath:"c:\perl521\lib\CORE"  -machine:x86'
                   libpth=C:\PROGRA~1\MIAF9D~1\VC98\lib
                   libs=oldnames.lib kernel32.lib user32.lib gdi32.lib
winspool.lib
comdlg32.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib
netapi32.lib uuid.lib ws2_32.lib mpr.lib winmm.lib  version.lib
odbc32.lib odbccp32.lib comctl32.lib msvcrt.lib
                   perllibs=oldnames.lib kernel32.lib user32.lib gdi32.lib
winspool.lib  comdlg32.lib advapi32.lib shell32.lib ole32.lib
oleaut32.lib  netapi32.lib uuid.lib ws2_32.lib mpr.lib winmm.lib
version.lib odbc32.lib odbccp32.lib comctl32.lib msvcrt.lib
                   libc=msvcrt.lib, so=dll, useshrplib=true,
libperl=perl521.lib
                   gnulibc_version=''
                 Dynamic Linking:
                   dlsrc=dl_win32.xs, dlext=dll, d_dlsymun=undef,
ccdlflags=' '
                   cccdlflags=' ', lddlflags='-dll -nologo -nodefaultlib
-debug
-opt:ref,icf  -libpath:"c:\perl521\lib\CORE"  -machine:x86'

Locally applied patches:
                   uncommitted-changes
                   a0fe7a7e75de29e59f1da0d6822dc06e5be658fe
                   a261faffee83d0145642ab5d1d046c9f813bc497
                   6506ab86ad1602a9ca720fcd30446dce1461d23d
                   7d2b2edb94ab56333b9049a3e26d15ea18445512


@INC for perl 5.21.4:
                   lib
                   C:/perl521/srcnew/lib
                   .


Environment for perl 5.21.4:
                   HOME (unset)
                   LANG (unset)
                   LANGUAGE (unset)
                   LD_LIBRARY_PATH (unset)
                   LOGDIR (unset)
                   PATH=
                   PERL_BADLANG (unset)
                   PERL_JSON_BACKEND=Cpanel::JSON::XS
                   PERL_YAML_BACKEND=YAML
                   SHELL (unset)


























@p5pRT
Copy link
Author

p5pRT commented Mar 27, 2015

From @bulk88

0001-remove-all-usage-of-dl_undef_symbols-on-platforms-wh.patch
From 1c6f087bacfb12997d191cb805453be063114598 Mon Sep 17 00:00:00 2001
From: Daniel Dragan <bulk88@hotmail.com>
Date: Fri, 27 Mar 2015 03:52:41 -0400
Subject: [PATCH] remove all usage of dl_undef_symbols on platforms where its
 a nop

Only freemint uses it. Change dl_undef_symbols to be a vararg on platforms
that dont implement it, since a vararg skips the computing the number of
incoming items, and the conditional branch testing the item count, and
the call to Perl_croak_xs_usage. This makes the machine code smaller for
dl_undef_symbols on platforms that dont implement it. Retain returning
empty list for back compat. CODE: is more efficient than "SP-= items;"
that PPCODE blocks do, since CODE: will just copy mark to sp, not caring
how many items ahead, sp was, compared to mark.
---
 dist/XSLoader/XSLoader_pm.PL    |    8 ++++++++
 ext/DynaLoader/DynaLoader_pm.PL |    4 +++-
 ext/DynaLoader/dl_aix.xs        |    4 ++--
 ext/DynaLoader/dl_dllload.xs    |    2 +-
 ext/DynaLoader/dl_dlopen.xs     |    2 +-
 ext/DynaLoader/dl_dyld.xs       |    4 ++--
 ext/DynaLoader/dl_hpux.xs       |    2 +-
 ext/DynaLoader/dl_symbian.xs    |    2 +-
 ext/DynaLoader/dl_vms.xs        |    4 ++--
 ext/DynaLoader/dl_win32.xs      |    2 +-
 pod/perldelta.pod               |    4 ++++
 11 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/dist/XSLoader/XSLoader_pm.PL b/dist/XSLoader/XSLoader_pm.PL
index 11404db..40a9db6 100644
--- a/dist/XSLoader/XSLoader_pm.PL
+++ b/dist/XSLoader/XSLoader_pm.PL
@@ -150,12 +150,20 @@ print OUT <<'EOT';
     };
     push(@DynaLoader::dl_librefs,$libref);  # record loaded object
 
+EOT
+
+    if ($^O eq 'freemint') {
+print OUT <<'EOT';
     my @unresolved = dl_undef_symbols();
     if (@unresolved) {
         require Carp;
         Carp::carp("Undefined symbols present after loading $file: @unresolved\n");
     }
 
+EOT
+    }
+
+print OUT <<'EOT';
     $boot_symbol_ref = dl_find_symbol($libref, $bootname) or do {
         require Carp;
         Carp::croak("Can't find '$bootname' symbol in $file\n");
diff --git a/ext/DynaLoader/DynaLoader_pm.PL b/ext/DynaLoader/DynaLoader_pm.PL
index 45dd6d3..c75ac65 100644
--- a/ext/DynaLoader/DynaLoader_pm.PL
+++ b/ext/DynaLoader/DynaLoader_pm.PL
@@ -85,7 +85,7 @@ package DynaLoader;
 # Tim.Bunce@ig.co.uk, August 1994
 
 BEGIN {
-    $VERSION = '1.32';
+    $VERSION = '1.33';
 }
 
 use Config;
@@ -394,12 +394,14 @@ sub bootstrap {
 
     push(@dl_librefs,$libref);  # record loaded object
 
+    <<$^O-eq-freemint>>
     my @unresolved = dl_undef_symbols();
     if (@unresolved) {
 	require Carp;
 	Carp::carp("Undefined symbols present after loading $file: @unresolved\n");
     }
 
+    <</$^O-eq-freemint>>
     $boot_symbol_ref = dl_find_symbol($libref, $bootname) or
          croak("Can't find '$bootname' symbol in $file\n");
 
diff --git a/ext/DynaLoader/dl_aix.xs b/ext/DynaLoader/dl_aix.xs
index 137af19..483e93e 100644
--- a/ext/DynaLoader/dl_aix.xs
+++ b/ext/DynaLoader/dl_aix.xs
@@ -739,8 +739,8 @@ dl_find_symbol(libhandle, symbolname)
 
 
 void
-dl_undef_symbols()
-	CODE:
+dl_undef_symbols(...)
+    CODE:
 
 
 
diff --git a/ext/DynaLoader/dl_dllload.xs b/ext/DynaLoader/dl_dllload.xs
index aac2f7b..1e989fc 100644
--- a/ext/DynaLoader/dl_dllload.xs
+++ b/ext/DynaLoader/dl_dllload.xs
@@ -163,7 +163,7 @@ dl_find_symbol(libhandle, symbolname)
 
 
 void
-dl_undef_symbols()
+dl_undef_symbols(...)
     CODE:
 
 
diff --git a/ext/DynaLoader/dl_dlopen.xs b/ext/DynaLoader/dl_dlopen.xs
index 3dce1ef..6a131b5 100644
--- a/ext/DynaLoader/dl_dlopen.xs
+++ b/ext/DynaLoader/dl_dlopen.xs
@@ -239,7 +239,7 @@ dl_find_symbol(libhandle, symbolname)
 
 
 void
-dl_undef_symbols()
+dl_undef_symbols(...)
     CODE:
 
 
diff --git a/ext/DynaLoader/dl_dyld.xs b/ext/DynaLoader/dl_dyld.xs
index 7822878..bd3e1b2 100644
--- a/ext/DynaLoader/dl_dyld.xs
+++ b/ext/DynaLoader/dl_dyld.xs
@@ -193,8 +193,8 @@ dl_find_symbol(libhandle, symbolname)
 
 
 void
-dl_undef_symbols()
-    PPCODE:
+dl_undef_symbols(...)
+    CODE:
 
 
 
diff --git a/ext/DynaLoader/dl_hpux.xs b/ext/DynaLoader/dl_hpux.xs
index 0ed0173..8b4b826 100644
--- a/ext/DynaLoader/dl_hpux.xs
+++ b/ext/DynaLoader/dl_hpux.xs
@@ -152,7 +152,7 @@ dl_find_symbol(libhandle, symbolname)
 
 
 void
-dl_undef_symbols()
+dl_undef_symbols(...)
     CODE:
 
 
diff --git a/ext/DynaLoader/dl_symbian.xs b/ext/DynaLoader/dl_symbian.xs
index 562b034..ea4a2eb 100644
--- a/ext/DynaLoader/dl_symbian.xs
+++ b/ext/DynaLoader/dl_symbian.xs
@@ -195,7 +195,7 @@ dl_find_symbol(libhandle, symbolname)
 
 
 void
-dl_undef_symbols()
+dl_undef_symbols(...)
     CODE:
 
 
diff --git a/ext/DynaLoader/dl_vms.xs b/ext/DynaLoader/dl_vms.xs
index ca8d54f..eebd8654 100644
--- a/ext/DynaLoader/dl_vms.xs
+++ b/ext/DynaLoader/dl_vms.xs
@@ -328,8 +328,8 @@ dl_find_symbol(librefptr,symname)
 
 
 void
-dl_undef_symbols()
-    PPCODE:
+dl_undef_symbols(...)
+    CODE:
 
 
 # These functions should not need changing on any platform:
diff --git a/ext/DynaLoader/dl_win32.xs b/ext/DynaLoader/dl_win32.xs
index 605b63f..eb4fa63 100644
--- a/ext/DynaLoader/dl_win32.xs
+++ b/ext/DynaLoader/dl_win32.xs
@@ -176,7 +176,7 @@ dl_find_symbol(libhandle, symbolname)
 
 
 void
-dl_undef_symbols()
+dl_undef_symbols(...)
     CODE:
 
 
diff --git a/pod/perldelta.pod b/pod/perldelta.pod
index 0cf5603..c814764 100644
--- a/pod/perldelta.pod
+++ b/pod/perldelta.pod
@@ -163,6 +163,10 @@ C<IPC::Open3> threw to the callers of C<open3> would have an irrelavent
 message derived from C<$!> which was in an undefined state, instead of the
 C<$@> message which triggers the failure path inside C<open3>.
 
+=item *
+
+L<DynaLoader> has been upgraded from version 1.32 to 1.33.
+
 =back
 
 =head2 Removed Modules and Pragmata
-- 
1.7.9.msysgit.0

@p5pRT
Copy link
Author

p5pRT commented Mar 29, 2015

From @tonycoz

On Fri Mar 27 01​:00​:09 2015, bulk88 wrote​:

Only freemint uses it. Change dl_undef_symbols to be a vararg on platforms
that dont implement it, since a vararg skips the computing the number of
incoming items, and the conditional branch testing the item count, and
the call to Perl_croak_xs_usage. This makes the machine code smaller for
dl_undef_symbols on platforms that dont implement it.

It also means that code that mistakenly calls dl_undef_symbols() with parameters
will only break on the platform where it's a no-op.

Tony

@p5pRT
Copy link
Author

p5pRT commented Mar 29, 2015

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

@p5pRT
Copy link
Author

p5pRT commented Mar 30, 2015

From @bulk88

On Sun Mar 29 15​:28​:26 2015, tonyc wrote​:

It also means that code that mistakenly calls dl_undef_symbols() with
parameters
will only break on the platform where it's a no-op.

Tony

The intention of the patch is to make dl_undef_symbols as close to "deprecated" as possible, or as close to "not public API" as possible without officially saying that. Old code that calls dl_undef_symbols will continue to work uneventfully. There is little usage of this sub on CPAN grep http​://grep.cpan.me/?q=+dl_undef_symbols . Anyone who calls this very specialized rarely written sub, will read DynaLoader.pm and know it takes no args. If they want to pass args that will be ignored and burn CPU we can't stop them. Most PP code, including p5p code never checks @​_ count on sub entry.

The patch turns "sub { die "wrong items" if @​_ != 0; return ();}" into "sub {return ();}". Maybe freemint should also be made "..." prototype, if you want it that way, so all platforms behavior is identical. The prototype of dl_undef_symbols in the docs shouldn't change.

--
bulk88 ~ bulk88 at hotmail.com

@p5pRT
Copy link
Author

p5pRT commented Aug 11, 2015

From @tonycoz

On Sun Mar 29 17​:50​:56 2015, bulk88 wrote​:

On Sun Mar 29 15​:28​:26 2015, tonyc wrote​:

It also means that code that mistakenly calls dl_undef_symbols() with
parameters
will only break on the platform where it's a no-op.

Tony

The intention of the patch is to make dl_undef_symbols as close to
"deprecated" as possible, or as close to "not public API" as possible
without officially saying that. Old code that calls dl_undef_symbols
will continue to work uneventfully. There is little usage of this sub
on CPAN grep http​://grep.cpan.me/?q=+dl_undef_symbols . Anyone who
calls this very specialized rarely written sub, will read
DynaLoader.pm and know it takes no args. If they want to pass args
that will be ignored and burn CPU we can't stop them. Most PP code,
including p5p code never checks @​_ count on sub entry.

The patch turns "sub { die "wrong items" if @​_ != 0; return ();}" into
"sub {return ();}". Maybe freemint should also be made "..."
prototype, if you want it that way, so all platforms behavior is
identical. The prototype of dl_undef_symbols in the docs shouldn't
change.

The patch is good except for making dl_undef_symbols() varargs, leave the various implementations as () and I'll apply it.

Tony

@toddr
Copy link
Member

toddr commented Feb 13, 2020

@bulk88 with a minor rebase and taking @tonycoz 's suggestions, we could merge this into blead. Could you please submit a pull request so it can also be smoked?

@toddr toddr closed this as completed Feb 13, 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

2 participants