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

Please define _GNU_SOURCE unconditionally on Linux to make memmem() available #16807

Closed
p5pRT opened this issue Jan 10, 2019 · 24 comments

Comments

@p5pRT
Copy link
Collaborator

@p5pRT p5pRT commented Jan 10, 2019

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

Searchable as RT133760$

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 10, 2019

From aranea@aixah.de

Created by aranea@aixah.de

Since 5.25, Perl uses the memmem() function to implement ninstr().
memmem() is only available from the string.h header if _GNU_SOURCE is
defined. However, the perl build system currently only defines
_GNU_SOURCE for threaded builds.

On systems with the glibc libc, things appear to work regardless (I
don't know why), but with the musl libc, this breaks ninstr(). Perl
builds fine, but at runtime, functions using ninstr() fail​:

$ perl -e '($foo) = "abc" =~ /^(.*)$/; print index("abc", $foo)'
-94330366722048

Please define _GNU_SOURCE unconditionally to fix this issue.

Perl Info

Flags:
    category=core
    severity=medium

Site configuration information for perl 5.26.2:

Configured by Gentoo at Thu Jan 10 14:01:07 CET 2019.

Summary of my perl5 (revision 5 version 26 subversion 2) configuration:
   
  Platform:
    osname=linux
    osvers=4.20.0
    archname=x86_64-linux
    uname='linux deneb.skynet.aixah.de 4.20.0 #2 smp wed jan 9 22:36:55 cet 2019 x86_64 intel(r) core(tm) i7-9700k cpu @ 3.60ghz genuineintel gnulinux '
    config_args='-des -Dinstallprefix=/usr -Dinstallusrbinperl=n -Ui_ndbm -Di_gdbm -Ui_db -DDEBUGGING=-g -Dnoextensions=ODBM_File -Duseshrplib -Darchname=x86_64-linux -Dcc=x86_64-gentoo-linux-musl-gcc -Doptimize=-march=native -O2 -pipe -ggdb -Dldflags=-Wl,-O1 -Wl,--as-needed -Dprefix=/usr -Dsiteprefix=/usr/local -Dvendorprefix=/usr -Dscriptdir=/usr/bin -Dprivlib=/usr/lib/perl5/5.26.2 -Darchlib=/usr/lib/perl5/5.26.2/x86_64-linux -Dsitelib=/usr/local/lib/perl5/5.26.2 -Dsitearch=/usr/local/lib/perl5/5.26.2/x86_64-linux -Dvendorlib=/usr/lib/perl5/vendor_perl/5.26.2 -Dvendorarch=/usr/lib/perl5/vendor_perl/5.26.2/x86_64-linux -Dman1dir=/usr/share/man/man1 -Dman3dir=/usr/share/man/man3 -Dsiteman1dir=/usr/local/man/man1 -Dsiteman3dir=/usr/local/man/man3 -Dvendorman1dir=/usr/share/man/man1 -Dvendorman3dir=/usr/share/man/man3 -Dman1ext=1 -Dman3ext=3pm -Dlibperl=libperl.so.5.26.2
-Dlocincpth=/usr/include  -Dglibpth=/lib /usr/lib  -Duselargefiles -Dd_semctl_semun -Dcf_by=Gentoo -Dmyhostname=localhost -Dperladmin=root@localhost -Ud_csh -Dsh=/bin/sh -Dtargetsh=/bin/sh -Uusenm -Ui_ndbm -Di_gdbm -Ui_db -DDEBUGGING=-g -Dnoextensions=ODBM_File'
    hint=recommended
    useposix=true
    d_sigaction=define
    useithreads=undef
    usemultiplicity=undef
    use64bitint=define
    use64bitall=define
    uselongdouble=undef
    usemymalloc=n
    default_inc_excludes_dot=define
    bincompat5005=undef
  Compiler:
    cc='x86_64-gentoo-linux-musl-gcc'
    ccflags ='-fwrapv -fno-strict-aliasing -pipe -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64'
    optimize='-march=native -O2 -pipe -ggdb'
    cppflags='-fwrapv -fno-strict-aliasing -pipe'
    ccversion=''
    gccversion='7.3.0'
    gccosandvers=''
    intsize=4
    longsize=8
    ptrsize=8
    doublesize=8
    byteorder=12345678
    doublekind=3
    d_longlong=define
    longlongsize=8
    d_longdbl=define
    longdblsize=16
    longdblkind=3
    ivtype='long'
    ivsize=8
    nvtype='double'
    nvsize=8
    Off_t='off_t'
    lseeksize=8
    alignbytes=8
    prototype=define
  Linker and Libraries:
    ld='x86_64-gentoo-linux-musl-gcc'
    ldflags ='-Wl,-O1 -Wl,--as-needed'
    libpth=/usr/lib /usr/lib/../lib64 /lib
    libs=-lpthread -lgdbm -ldl -lm -lcrypt -lutil -lc -lgdbm_compat
    perllibs=-lpthread -ldl -lm -lcrypt -lutil -lc
    libc=/usr/lib/libc.so
    so=so
    useshrplib=true
    libperl=libperl.so.5.26.2
    gnulibc_version=''
  Dynamic Linking:
    dlsrc=dl_dlopen.xs
    dlext=so
    d_dlsymun=undef
    ccdlflags='-Wl,-E'
    cccdlflags='-fPIC'
    lddlflags='-shared -march=native -O2 -pipe -ggdb -Wl,-O1 -Wl,--as-needed'

Locally applied patches:
    gentoo/hints_hpux - Fix hpux hints
    gentoo/aix_soname - aix gcc detection and shared library soname support
    gentoo/EUMM-RUNPATH - https://bugs.gentoo.org/105054 cpan/ExtUtils-MakeMaker: drop $PORTAGE_TMPDIR from LD_RUN_PATH
    gentoo/config_over - Remove -rpath and append LDFLAGS to lddlflags
    gentoo/opensolaris_headers - Add headers for opensolaris
    gentoo/patchlevel - List packaged patches for perl-5.26.2(#1) in patchlevel.h
    gentoo/cleanup-paths - Cleanup PATH and shrpenv
    gentoo/enc2xs - Tweak enc2xs to follow symlinks and ignore missing @INC directories.
    gentoo/darwin-cc-ld - https://bugs.gentoo.org/297751 darwin: Use $CC to link
    gentoo/cpan_definstalldirs - Provide a sensible INSTALLDIRS default for modules installed from CPAN.
    gentoo/interix - Fix interix hints
    gentoo/create_libperl_soname - https://bugs.gentoo.org/286840 Set libperl soname
    gentoo/mod_paths - Add /etc/perl to @INC
    gentoo/EUMM_perllocalpod - cpan/ExtUtils-MakeMaker: remove targets that generate perllocal.pod
    gentoo/drop_fstack_protector - https://bugs.gentoo.org/348557 Don't force -fstack-protector on everyone
    gentoo/usr_local - Configure: Don't include sources in /usr/local/ for compiling perl
    gentoo/D-SHA-CFLAGS - https://bugs.gentoo.org/506818 Do not set custom CFLAGS in cpan/Digest-SHA
    gentoo/io_socket_ip_tests - cpan/IO-Socket-IP: Disable network tests
    gentoo/tests - Fix EUMM podlocal tests
    gentoo/no-nsl.patch -
    debian/cpan-missing-site-dirs - Fix CPAN::FirstTime defaults with nonexisting site dirs if a parent is writable
    debian/makemaker-pasthru - Pass LD settings through to subdirectories
    fixes/memoize_storable_nstore - [rt.cpan.org #77790] Memoize::Storable: respect 'nstore' option not respected
    fixes/podman-pipe - Better errors for man pages from standard input
    fixes/respect_umask - Respect umask during installation
    fixes/net_smtp_docs - [rt.cpan.org #36038] Document the Net::SMTP 'Port' option
    fixes/document_makemaker_ccflags - [rt.cpan.org #68613] Document that CCFLAGS should include $Config{ccflags}
    fixes/parallel-manisort.patch - Fix parallel building


@INC for perl 5.26.2:
    /etc/perl
    /usr/local/lib/perl5/5.26.2/x86_64-linux
    /usr/local/lib/perl5/5.26.2
    /usr/lib/perl5/vendor_perl/5.26.2/x86_64-linux
    /usr/lib/perl5/vendor_perl/5.26.2
    /usr/lib/perl5/5.26.2/x86_64-linux
    /usr/lib/perl5/5.26.2


Environment for perl 5.26.2:
    HOME=/home/lre
    LANG=en_US.utf8
    LANGUAGE (unset)
    LC_CTYPE=en_US.UTF-8
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/bin:/sbin:/usr/bin:/usr/sbin:/usr/local/bin:/usr/local/sbin:/home/lre/bin
    PERL_BADLANG (unset)
    SHELL=/bin/fish

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 11, 2019

From @jkeenan

On Thu, 10 Jan 2019 16​:24​:06 GMT, aranea@​aixah.de wrote​:

This is a bug report for perl from aranea@​aixah.de,
generated with the help of perlbug 1.40 running under perl 5.26.2.

-----------------------------------------------------------------
[Please describe your issue here]

Since 5.25, Perl uses the memmem() function to implement ninstr().
memmem() is only available from the string.h header if _GNU_SOURCE is
defined. However, the perl build system currently only defines
_GNU_SOURCE for threaded builds.

On systems with the glibc libc, things appear to work regardless (I
don't know why), but with the musl libc, this breaks ninstr(). Perl
builds fine, but at runtime, functions using ninstr() fail​:

$ perl -e '($foo) = "abc" =~ /^(.*)$/; print index("abc", $foo)'
-94330366722048

How can I safely test this behavior?

On Debian/Ubuntu, I see that the following packages are available via 'apt'​:

#####
musl - standard C library
musl-dev - standard C library development files
musl-tools - standard C library tools
#####

If I were to install them, would that interfere with 'libc' at all?

What arguments would I pass to perl's ./Configure to say "use musl instead of libc"?

Please define _GNU_SOURCE unconditionally to fix this issue.

My initial reaction​: That's overkill. We should detect 'musl' first and only then extend use of _GNU_SOURCE.

But this is not an area of expertise for me, so others should comment.

Thank you very much.

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

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 11, 2019

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

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 12, 2019

From aranea@aixah.de

On Thu, 10 Jan 2019 18​:14​:38 -0800
"James E Keenan via RT" <perlbug-followup@​perl.org> wrote​:

How can I safely test this behavior?

You can't just install musl on a system that uses a different libc;
it'd pave over essential components of that other libc. Your best bet
is to download an image of a distro that uses musl (for example
https://alpinelinux.org/, that's only a few 100 MB large), and run that
in a VM or from a USB drive.

Please define _GNU_SOURCE unconditionally to fix this issue.

My initial reaction​: That's overkill. We should detect 'musl' first
and only then extend use of _GNU_SOURCE.

But this is not an area of expertise for me, so others should comment.

I disagree. glibc too only defines memmem() conditionally (#ifdef
__USE_GNU, which itself is enabled under various circumstances, for
example if _GNU_SOURCE is defined). That Perl currently works on glibc
systems is pure luck (__USE_GNU is probably already defined for other
reasons), and this might change at any time.

The glibc manpage for memmem() even explicitly states that you should
define _GNU_SOURCE if you're using it. Considering that this is a very
ugly bug, which doesn't show itself during build time, but
silently corrupts data at runtime, I'd strongly recommend to enable
_GNU_SOURCE for both musl and glibc.

Thanks a lot!

Luis Ressel

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 13, 2019

From @tonycoz

On Sat, 12 Jan 2019 13​:20​:23 -0800, aranea@​aixah.de wrote​:

I disagree. glibc too only defines memmem() conditionally (#ifdef
__USE_GNU, which itself is enabled under various circumstances, for
example if _GNU_SOURCE is defined). That Perl currently works on glibc
systems is pure luck (__USE_GNU is probably already defined for other
reasons), and this might change at any time.

It's not pure luck.

The generated config.h with glibc has​:

#define HAS_GNULIBC /**/
#if defined(HAS_GNULIBC) && !defined(_GNU_SOURCE)
# define _GNU_SOURCE
#endif

Does musl define symbols similar to __GLIBC__ that can be used to detect musl at build-time?

Tony

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 15, 2019

From aranea@aixah.de

On Sun, 13 Jan 2019 15​:22​:20 -0800
"Tony Cook via RT" <perlbug-followup@​perl.org> wrote​:

The generated config.h with glibc has​:

#define HAS_GNULIBC /**/
#if defined(HAS_GNULIBC) && !defined(_GNU_SOURCE)
# define _GNU_SOURCE
#endif

Does musl define symbols similar to __GLIBC__ that can be used to
detect musl at build-time?

Ahh, thanks for digging that up. No, musl intentionally doesn't have
__MUSL__ or a similar macro; the author feels this would invite
non-futureproof code (cf.
https://wiki.musl-libc.org/faq.html#Q:-Why-is-there-no-%3Ccode%3E__MUSL__%3C/code%3E-macro? ).

Defining a feature test macro such as _GNU_SOURCE globally and
unconditionally is safe though; even on the BSDs, it wouldn't cause any
problems. And memmem() isn't the only function that's affected by this​:
As felix@​adjust.com pointed out, strlcpy() and strlcat() are only
defined by the musl (and glibc) headers if _GNU_SOURCE or _BSD_SOURCE
is defined, and Perl may currently be defining them implicitly on those
platforms.

Regards,
Luis Ressel

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 19, 2019

From @craigberry

On Mon, Jan 14, 2019 at 6​:12 PM Luis Ressel <aranea@​aixah.de> wrote​:

On Sun, 13 Jan 2019 15​:22​:20 -0800
"Tony Cook via RT" <perlbug-followup@​perl.org> wrote​:

The generated config.h with glibc has​:

#define HAS_GNULIBC /**/
#if defined(HAS_GNULIBC) && !defined(_GNU_SOURCE)
# define _GNU_SOURCE
#endif

Does musl define symbols similar to __GLIBC__ that can be used to
detect musl at build-time?

Ahh, thanks for digging that up. No, musl intentionally doesn't have
__MUSL__ or a similar macro; the author feels this would invite
non-futureproof code (cf.
https://wiki.musl-libc.org/faq.html#Q:-Why-is-there-no-%3Ccode%3E__MUSL__%3C/code%3E-macro? ).

The author's original statement is here​:

<https://www.openwall.com/lists/musl/2013/03/29/13>

and says, "it's a bug to assume a certain implementation has
particular properties rather than testing." But Perl's Configure
script does have a standard "inlibc" test for memmem, so if that test
fails with musl even though musl actually does have memem, why is it
Perl that has to change? It's inconsistent for musl to say that one
should not use feature macros to determine the presence of something
and then hide a bunch of functions behind feature macros.

Defining a feature test macro such as _GNU_SOURCE globally and
unconditionally is safe though; even on the BSDs, it wouldn't cause any
problems. And memmem() isn't the only function that's affected by this​:
As felix@​adjust.com pointed out, strlcpy() and strlcat() are only
defined by the musl (and glibc) headers if _GNU_SOURCE or _BSD_SOURCE
is defined, and Perl may currently be defining them implicitly on those
platforms.

That does not sound like a good idea to me. It means assuming that
musl guarantees support of all the non-standard functions and the
non-POSIX behaviors of standard functions in GNU libc. The FAQ
specifically says this is not the case under "Is musl compatible with
glibc?"​:

<http​://www.musl-libc.org/faq.html>

It also means assuming that any hypothetical libc replacement on Linux
other than musl would support all the non-standard behaviors of glibc.

I think the only way around this is to define _GNU_SOURCE when using
musl as long as the musl users are ok with that even though it's
asserting something that is partially but not completely true. Which
means detecting musl whether it wants to be detected or not.

LLVM does this to determine if musl is present​:

#if defined(__linux__) and !defined(__GLIBC__) and !defined(__ANDROID__)

which assumes there are (and never will be) any other libc
implementations on Linux except glibc and musl and bionic. Doesn't
seem like a good idea.

This node module​:

<https://github.com/lovell/detect-libc/blob/master/lib/detect-libc.js>

parses the output of ldd --version to detect musl. OpenJDK does the same​:

<http​://cr.openjdk.java.net/~mikael/webrevs/portola/musl-config.guess/webrev.00/webrev/common/autoconf/build-aux/config.guess.udiff.html>

So adding something like that to hints/linux.sh may be the best way forward.

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 19, 2019

From @doughera88

On Sat, Jan 19, 2019 at 11​:29​:17AM -0600, Craig A. Berry wrote​:

On Mon, Jan 14, 2019 at 6​:12 PM Luis Ressel <aranea@​aixah.de> wrote​:

On Sun, 13 Jan 2019 15​:22​:20 -0800
"Tony Cook via RT" <perlbug-followup@​perl.org> wrote​:

The generated config.h with glibc has​:

#define HAS_GNULIBC /**/
#if defined(HAS_GNULIBC) && !defined(_GNU_SOURCE)
# define _GNU_SOURCE
#endif

Does musl define symbols similar to __GLIBC__ that can be used to
detect musl at build-time?

Ahh, thanks for digging that up. No, musl intentionally doesn't have
__MUSL__ or a similar macro; the author feels this would invite
non-futureproof code (cf.
https://wiki.musl-libc.org/faq.html#Q:-Why-is-there-no-%3Ccode%3E__MUSL__%3C/code%3E-macro? ).

The author's original statement is here​:

<https://www.openwall.com/lists/musl/2013/03/29/13>

and says, "it's a bug to assume a certain implementation has
particular properties rather than testing." But Perl's Configure
script does have a standard "inlibc" test for memmem, so if that test
fails with musl even though musl actually does have memem, why is it
Perl that has to change?

Perl's Configure test should be better. Currently, it tests that
the memmem symbol exists in the library, but without the __GNU_SOURCE
#define, perl does not call it correctly. If we change the the Configure
test to compile & run a test program, it should fail on musl due to
the incorrect prototype. I'm working on a patch. Whether musl also
defines __GNU_SOURCE is a good question, but perl should ultiately work
either way.

--
  Andy Dougherty doughera@​lafayette.edu

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 22, 2019

From @doughera88

On Sat, Jan 19, 2019 at 01​:56​:55PM -0500, Andy Dougherty wrote​:

Perl's Configure test should be better. Currently, it tests that
the memmem symbol exists in the library, but without the __GNU_SOURCE
#define, perl does not call it correctly. If we change the the Configure
test to compile & run a test program, it should fail on musl due to
the incorrect prototype. I'm working on a patch.

I have pushed an improved test​:

commit ca152fd
Author​: Andy Dougherty <doughera@​lafayette.edu>
Date​: Tue Jan 22 14​:17​:05 2019 -0500

  Improve Configure detection of memmem() [perl #133760].
 
  Linux systems have memmem, but the header prototype is only visible if
  the C library, but didn't check if the correct prototype is available.
  This patch compiles & runs a test program that will fail if the prototype
  is needed but not available.
 
  This does not completely close [perl #133760]. The tests for strlcat()
  and strlcpy() may also need to be similarly changed. Also, this patch
  does not change whether _GNU_SOURCE is defined or not. Presumably that
  would be done separately in the linux hints file.

I haven't finished up the corresponding metaconfig patch yet, but hope to soon.

--
  Andy Dougherty doughera@​lafayette.edu

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 22, 2019

From @doughera88

On Tue, Jan 22, 2019 at 02​:33​:22PM -0500, Andy Dougherty wrote​:

On Sat, Jan 19, 2019 at 01​:56​:55PM -0500, Andy Dougherty wrote​:

Perl's Configure test should be better. Currently, it tests that
the memmem symbol exists in the library, but without the __GNU_SOURCE
#define, perl does not call it correctly. If we change the the Configure
test to compile & run a test program, it should fail on musl due to
the incorrect prototype. I'm working on a patch.

I have pushed an improved test​:

commit ca152fd
Author​: Andy Dougherty <doughera@​lafayette.edu>
Date​: Tue Jan 22 14​:17​:05 2019 -0500

Improve Configure detection of memmem\(\) \[perl \#133760\]\.

I am attaching the accompanying metaconfig patch. I don't have a github account,
so can't push to the respository there.

--
  Andy Dougherty doughera@​lafayette.edu

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 22, 2019

From @doughera88

Inline Patch
diff --git a/U/perl/d_memmem.U b/U/perl/d_memmem.U
index 404b098..1c01239 100644
--- a/U/perl/d_memmem.U
+++ b/U/perl/d_memmem.U
@@ -1,7 +1,7 @@
 ?RCS: You may distribute under the terms of either the GNU General Public
 ?RCS: License or the Artistic License, as specified in the README file.
 ?RCS:
-?MAKE:d_memmem: Inlibc
+?MAKE:d_memmem: d_gnulibc Compile cat rm_try run Setvar
 ?MAKE:	-pick add $@ %<
 ?S:d_memmem:
 ?S:	This variable conditionally defines the HAS_MEMMEM symbol, which
@@ -13,11 +13,55 @@
 ?C:	This symbol, if defined, indicates that the memmem routine is
 ?C:	available to return a pointer to the start of the first occurance
 ?C:	of a substring in a memory area (or NULL if not found).
+?C:	In glibc, memmem is a GNU extension.  The function is visible in
+?C:	libc, but the prototype is only visible if _GNU_SOURCE is #defined.
+?C:	Thus we compile & run a test to be sure we have a valid prototype.
 ?C:.
 ?H:#$d_memmem HAS_MEMMEM		/**/
 ?H:.
 ?LINT:set d_memmem
+?T:rc
+?F:!try
 : see if memmem exists
-set memmem d_memmem
-eval $inlibc
+echo " "
+echo "Checking if you have a working memmem()" >&4
+$cat >try.c <<EOCP
+#$d_gnulibc HAS_GNULIBC         /**/
+#if defined(HAS_GNULIBC) && !defined(_GNU_SOURCE)
+#   define _GNU_SOURCE
+#endif
+#include <stdio.h>
+#include <stdlib.h>
+#include <stddef.h>
+#include <string.h>
+int main(int argc, char **argv)
+{
+    char *big = "abcdefghiabcdefghi";
+    char *little = "def";
+    char *rtn;
+    ptrdiff_t diff;
+    rtn = (char *) memmem(big, strlen(big), little, strlen(little));
+    diff = rtn - big;
+    exit(diff == 3 ?  EXIT_SUCCESS : EXIT_FAILURE);
+}
+EOCP
+set try
+if eval $compile; then
+    `$run ./try`
+    rc=$?
+    case "$rc" in
+        0)  echo "Yes, you do." >&4
+            val="$define"
+            ;;
+        *)  echo "Well, you have memmem, but it isn't working." >&4
+            val="$undef"
+            ;;
+    esac
+else
+    echo "No, you do not." >&4
+    val="$undef"
+fi
+set d_memmem
+eval $setvar
+$rm_try
 
@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 22, 2019

From @tonycoz

On Tue, Jan 22, 2019 at 02​:33​:22PM -0500, Andy Dougherty wrote​:

On Sat, Jan 19, 2019 at 01​:56​:55PM -0500, Andy Dougherty wrote​:

Perl's Configure test should be better. Currently, it tests that
the memmem symbol exists in the library, but without the __GNU_SOURCE
#define, perl does not call it correctly. If we change the the Configure
test to compile & run a test program, it should fail on musl due to
the incorrect prototype. I'm working on a patch.

I have pushed an improved test​:

commit ca152fd
Author​: Andy Dougherty <doughera@​lafayette.edu>
Date​: Tue Jan 22 14​:17​:05 2019 -0500

Improve Configure detection of memmem\(\) \[perl \#133760\]\.

Linux systems have memmem\, but the header prototype is only visible if
the C library\, but didn't check if the correct prototype is available\.
This patch compiles & runs a test program that will fail if the prototype
is needed but not available\.

This does not completely close \[perl \#133760\]\.  The tests for strlcat\(\)
and strlcpy\(\) may also need to be similarly changed\.  Also\, this patch
does not change whether \_GNU\_SOURCE is defined or not\.  Presumably that
would be done separately in the linux hints file\.

I haven't finished up the corresponding metaconfig patch yet, but hope to soon.

I'm not sure this is sufficient.

Currently the code in perl that uses memmem() compiles successfuly,
and if the executable loads below the 2GB mark on a 64-bit machine the
new code in Configure will indicate success even if a prototype wasn't
found.

Configure has hasproto, would that be a better check to see if there's
a memmem() prototype available?

Tony

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 23, 2019

From @doughera88

On Wed, Jan 23, 2019 at 09​:27​:55AM +1100, Tony Cook wrote​:

On Tue, Jan 22, 2019 at 02​:33​:22PM -0500, Andy Dougherty wrote​:

On Sat, Jan 19, 2019 at 01​:56​:55PM -0500, Andy Dougherty wrote​:

commit ca152fd
Author​: Andy Dougherty <doughera@​lafayette.edu>
Date​: Tue Jan 22 14​:17​:05 2019 -0500

Improve Configure detection of memmem\(\) \[perl \#133760\]\.

Linux systems have memmem\, but the header prototype is only visible if
the C library\, but didn't check if the correct prototype is available\.
This patch compiles & runs a test program that will fail if the prototype
is needed but not available\.

I'm not sure this is sufficient.

Currently the code in perl that uses memmem() compiles successfuly,
and if the executable loads below the 2GB mark on a 64-bit machine the
new code in Configure will indicate success even if a prototype wasn't
found.

Good point. I hadn't thought of that, and didn't encounter it in my testing.

Configure has hasproto, would that be a better check to see if there's
a memmem() prototype available?

Good idea. I'll investigate. Thanks for the thoughtful feedback!

--
  Andy Dougherty doughera@​lafayette.edu

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 23, 2019

From @Leont

On Mon, Jan 14, 2019 at 12​:23 AM Tony Cook via RT
<perlbug-followup@​perl.org> wrote​:

On Sat, 12 Jan 2019 13​:20​:23 -0800, aranea@​aixah.de wrote​:

I disagree. glibc too only defines memmem() conditionally (#ifdef
__USE_GNU, which itself is enabled under various circumstances, for
example if _GNU_SOURCE is defined). That Perl currently works on glibc
systems is pure luck (__USE_GNU is probably already defined for other
reasons), and this might change at any time.

It's not pure luck.

The generated config.h with glibc has​:

#define HAS_GNULIBC /**/
#if defined(HAS_GNULIBC) && !defined(_GNU_SOURCE)
# define _GNU_SOURCE
#endif

Does musl define symbols similar to __GLIBC__ that can be used to detect musl at build-time?

Tony

Is there any circumstance in which we wouldn't want to define
_GNU_SOURCE on Linux?

Leon

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 23, 2019

From @tonycoz

On Wed, Jan 23, 2019 at 02​:08​:35AM +0100, Leon Timmermans wrote​:

On Mon, Jan 14, 2019 at 12​:23 AM Tony Cook via RT
<perlbug-followup@​perl.org> wrote​:

On Sat, 12 Jan 2019 13​:20​:23 -0800, aranea@​aixah.de wrote​:

I disagree. glibc too only defines memmem() conditionally (#ifdef
__USE_GNU, which itself is enabled under various circumstances, for
example if _GNU_SOURCE is defined). That Perl currently works on glibc
systems is pure luck (__USE_GNU is probably already defined for other
reasons), and this might change at any time.

It's not pure luck.

The generated config.h with glibc has​:

#define HAS_GNULIBC /**/
#if defined(HAS_GNULIBC) && !defined(_GNU_SOURCE)
# define _GNU_SOURCE
#endif

Does musl define symbols similar to __GLIBC__ that can be used to detect musl at build-time?

Tony

Is there any circumstance in which we wouldn't want to define
_GNU_SOURCE on Linux?

I don't know.

I don't think it's likely that anyone would produce a Linux libc that
reacted badly to defining _GNU_SOURCE, but it's a name reserved for
the implementation (underscore followed by capital letter), so I'd
rather not turn it on for every random libc.

Tony

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 24, 2019

From @doughera88

On Tue, Jan 22, 2019 at 07​:54​:35PM -0500, Andy Dougherty wrote​:

On Wed, Jan 23, 2019 at 09​:27​:55AM +1100, Tony Cook wrote​:

On Tue, Jan 22, 2019 at 02​:33​:22PM -0500, Andy Dougherty wrote​:

On Sat, Jan 19, 2019 at 01​:56​:55PM -0500, Andy Dougherty wrote​:

commit ca152fd
Author​: Andy Dougherty <doughera@​lafayette.edu>
Date​: Tue Jan 22 14​:17​:05 2019 -0500

Improve Configure detection of memmem\(\) \[perl \#133760\]\.

Configure has hasproto, would that be a better check to see if there's
a memmem() prototype available?

Good idea. I'll investigate. Thanks for the thoughtful feedback!

Now implemented in the following 2 commits. (The first enhances the hasproto
check; the second uses that enhancement.)

commit 63c1fa6
Author​: Andy Dougherty <doughera@​lafayette.edu>
Date​: Wed Jan 23 21​:12​:29 2019 -0500

  Add ability to include literal text in the prototype check.
 
  This is the same technique as in the metaconfig unit Protochk.U.
  See that unit for more usage information. It is a bit clunky,
  but does work.

commit f8d82a1
Author​: Andy Dougherty <doughera@​lafayette.edu>
Date​: Wed Jan 23 21​:39​:39 2019 -0500

  Another attempt to improve Configure detection of memmem() [perl #133760].
 
  This updates commit ca152fd.
  Linux systems have memmem, but the prototype in <string.h> is only
  visible if __GNU_SOURCE is defined. This version tests for both the
  prototype in <string.h> and the symbol in libc. (Thanks to Tony C. for
  the suggestion.) (For BSD systems, no extra define is needed.)

--
  Andy Dougherty doughera@​lafayette.edu

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 28, 2019

From @Tux

On Wed, 23 Jan 2019 21​:58​:04 -0500, Andy Dougherty
<doughera@​lafayette.edu> wrote​:

On Tue, Jan 22, 2019 at 07​:54​:35PM -0500, Andy Dougherty wrote​:

On Wed, Jan 23, 2019 at 09​:27​:55AM +1100, Tony Cook wrote​:

On Tue, Jan 22, 2019 at 02​:33​:22PM -0500, Andy Dougherty wrote​:
[...]

Configure has hasproto, would that be a better check to see if
there's a memmem() prototype available?

Good idea. I'll investigate. Thanks for the thoughtful feedback!

Now implemented in the following 2 commits. (The first enhances the
hasproto check; the second uses that enhancement.)

commit 63c1fa6
Author​: Andy Dougherty <doughera@​lafayette.edu>
Date​: Wed Jan 23 21​:12​:29 2019 -0500

Add ability to include literal text in the prototype check\.

This is the same technique as in the metaconfig unit Protochk\.U\.
See that unit for more usage information\.  It is a bit clunky\,
but does work\.

commit f8d82a1
Author​: Andy Dougherty <doughera@​lafayette.edu>
Date​: Wed Jan 23 21​:39​:39 2019 -0500

Another attempt to improve Configure detection of memmem\(\) \[perl

#133760].
This updates commit ca152fd.
Linux systems have memmem, but the prototype in <string.h> is only
visible if __GNU_SOURCE is defined. This version tests for both
the prototype in <string.h> and the symbol in libc. (Thanks to Tony
C. for the suggestion.) (For BSD systems, no extra define is needed.)

Sorry to chime in late. I was on a ski-trip

Do you have the final meta-patch? I'll apply
If you want I can give you commit on the current meta

--
H.Merijn Brand http​://tux.nl Perl Monger http​://amsterdam.pm.org/
using perl5.00307 .. 5.29 porting perl5 on HP-UX, AIX, and openSUSE
http​://mirrors.develooper.com/hpux/ http​://www.test-smoke.org/
http​://qa.perl.org http​://www.goldmark.org/jeff/stupid-disclaimers/

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 31, 2019

From @doughera88

On Wed, Jan 23, 2019 at 09​:58​:04PM -0500, Andy Dougherty wrote​:

Now implemented in the following 2 commits. (The first enhances the hasproto
check; the second uses that enhancement.)

The following 2 commits handle the other problematic functions in musl
(that I am aware of), namely memrchr, strlcat, and strlcpy. The last
one also updates the hints file to always define _GNU_SOURCE for Linux
distributions using the musl C library. I hope this is sufficient to
close this ticket!

I will send metaconfig updates separately.

commit 04db542
Author​: Andy Dougherty <doughera@​lafayette.edu>
Date​: Thu Jan 31 14​:05​:41 2019 -0500

  Define _GNU_SOURCE if using the musl libc on linux.

  Together with prior commits ba73a4c, f8d82a1, and 63c1fa6,
  this should close [perl #133760].

commit ba73a4c
Author​: Andy Dougherty <doughera@​lafayette.edu>
Date​: Thu Jan 31 13​:04​:32 2019 -0500

  Improve detection of memrchr, strlcat, and strlcpy.

  This is continuation of commit f8d82a1 addressing [perl #133760].
  Linux systems using the musl C library have memmem, memrchr, strlcat, and
  strlcpy, but the prototypes are only visible if _GNU_SOURCE is defined.
  This patch makes Configure test both whether the prototype is visible
  and whether the C symbol is visible.

  Still to be done is automatically adding _GNU_SOURCE if the musl library
  is being used -- probably in hints/linux.sh.

--
  Andy Dougherty doughera@​lafayette.edu

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 31, 2019

From @doughera88

On Thu, Jan 31, 2019 at 03​:33​:31PM -0500, Andy Dougherty wrote​:

On Wed, Jan 23, 2019 at 09​:58​:04PM -0500, Andy Dougherty wrote​:

Now implemented in the following 2 commits. (The first enhances the hasproto
check; the second uses that enhancement.)

The following 2 commits handle the other problematic functions in musl
(that I am aware of), namely memrchr, strlcat, and strlcpy. The last
one also updates the hints file to always define _GNU_SOURCE for Linux
distributions using the musl C library. I hope this is sufficient to
close this ticket!

I will send metaconfig updates separately.

I have attached the relevant metaconfig updates.

--
  Andy Dougherty doughera@​lafayette.edu

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 31, 2019

From @doughera88

0001-Improve-detection-of-memrchr-strlcat-and-strlcpy.patch
From d69c1265abdfdeb41bf35021e36f1b417462e7f3 Mon Sep 17 00:00:00 2001
From: Andy Dougherty <doughera@lafayete.edu>
Date: Thu, 31 Jan 2019 15:39:27 -0500
Subject: [PATCH] Improve detection of memrchr, strlcat, and strlcpy.

This is continuation of commit b6f9c611a4 addressing [perl #133760].
Linux systems using the musl C library have memmem, memrchr, strlcat, and
strlcpy, but the prototypes are only visible if _GNU_SOURCE is defined.
This patch makes Configure test both whether the prototype is visible
and whether the C symbol is visible.
---
 U/perl/d_memrchr.U | 24 +++++++++++++++++++++---
 U/perl/d_strlcat.U | 24 +++++++++++++++++++++---
 U/perl/d_strlcpy.U | 24 +++++++++++++++++++++---
 3 files changed, 63 insertions(+), 9 deletions(-)

diff --git a/U/perl/d_memrchr.U b/U/perl/d_memrchr.U
index 4fea9a6..8b0cf88 100644
--- a/U/perl/d_memrchr.U
+++ b/U/perl/d_memrchr.U
@@ -1,7 +1,7 @@
 ?RCS: You may distribute under the terms of either the GNU General Public
 ?RCS: License or the Artistic License, as specified in the README file.
 ?RCS:
-?MAKE:d_memrchr: Inlibc
+?MAKE:d_memrchr: d_gnulibc Hasproto Inlibc Setvar
 ?MAKE:	-pick add $@ %<
 ?S:d_memrchr:
 ?S:	This variable conditionally defines the HAS_MEMRCHR symbol, which
@@ -17,7 +17,25 @@
 ?H:#$d_memrchr HAS_MEMRCHR		/**/
 ?H:.
 ?LINT:set d_memrchr
+?T:xx1 xx2 xx3 xx4 d_memrchr_proto
 : see if memrchr exists
-set memrchr d_memrchr
-eval $inlibc
+: We need both a prototype in string.h and the symbol in libc.
+echo " "
+d_memrchr_proto=''
+xx1="#$d_gnulibc HAS_GNULIBC"
+xx2='#if defined(HAS_GNULIBC) && !defined(_GNU_SOURCE)'
+xx3='#   define _GNU_SOURCE'
+xx4='#endif'
+set d_memrchr_proto memrchr literal "$xx1" literal "$xx2" literal "$xx3" literal "$xx4" define string.h
+eval $hasproto
+case "$d_memrchr_proto" in
+    define) # see if memrchr exists
+	set memrchr d_memrchr
+	eval $inlibc
+	;;
+    *)  val=$undef
+	set d_memrchr
+	eval $setvar
+	;;
+esac
 
diff --git a/U/perl/d_strlcat.U b/U/perl/d_strlcat.U
index c082be9..0fb1d82 100644
--- a/U/perl/d_strlcat.U
+++ b/U/perl/d_strlcat.U
@@ -5,7 +5,7 @@
 ?RCS: You may distribute under the terms of either the GNU General Public
 ?RCS: License or the Artistic License, as specified in the README file.
 ?RCS:
-?MAKE:d_strlcat: Inlibc
+?MAKE:d_strlcat: d_gnulibc Hasproto Inlibc Setvar
 ?MAKE:	-pick add $@ %<
 ?S:d_strlcat:
 ?S:	This variable conditionally defines the HAS_STRLCAT symbol, which
@@ -18,7 +18,25 @@
 ?H:#$d_strlcat HAS_STRLCAT		/**/
 ?H:.
 ?LINT:set d_strlcat
+?T:xx1 xx2 xx3 xx4 d_strlcat_proto
 : see if strlcat exists
-set strlcat d_strlcat
-eval $inlibc
+: We need both a prototype in string.h and the symbol in libc.
+echo " "
+d_strlcat_proto=''
+xx1="#$d_gnulibc HAS_GNULIBC"
+xx2='#if defined(HAS_GNULIBC) && !defined(_GNU_SOURCE)'
+xx3='#   define _GNU_SOURCE'
+xx4='#endif'
+set d_strlcat_proto strlcat literal "$xx1" literal "$xx2" literal "$xx3" literal "$xx4" define string.h
+eval $hasproto
+case "$d_strlcat_proto" in
+    define) # see if strlcat exists
+	set strlcat d_strlcat
+	eval $inlibc
+	;;
+    *)  val=$undef
+	set d_strlcat
+	eval $setvar
+	;;
+esac
 
diff --git a/U/perl/d_strlcpy.U b/U/perl/d_strlcpy.U
index 02193c3..8f0a572 100644
--- a/U/perl/d_strlcpy.U
+++ b/U/perl/d_strlcpy.U
@@ -5,7 +5,7 @@
 ?RCS: You may distribute under the terms of either the GNU General Public
 ?RCS: License or the Artistic License, as specified in the README file.
 ?RCS:
-?MAKE:d_strlcpy: Inlibc
+?MAKE:d_strlcpy: d_gnulibc Hasproto Inlibc Setvar
 ?MAKE:	-pick add $@ %<
 ?S:d_strlcpy:
 ?S:	This variable conditionally defines the HAS_STRLCPY symbol, which
@@ -18,7 +18,25 @@
 ?H:#$d_strlcpy HAS_STRLCPY		/**/
 ?H:.
 ?LINT:set d_strlcpy
+?T:xx1 xx2 xx3 xx4 d_strlcpy_proto
 : see if strlcpy exists
-set strlcpy d_strlcpy
-eval $inlibc
+: We need both a prototype in string.h and the symbol in libc.
+echo " "
+d_strlcpy_proto=''
+xx1="#$d_gnulibc HAS_GNULIBC"
+xx2='#if defined(HAS_GNULIBC) && !defined(_GNU_SOURCE)'
+xx3='#   define _GNU_SOURCE'
+xx4='#endif'
+set d_strlcpy_proto strlcpy literal "$xx1" literal "$xx2" literal "$xx3" literal "$xx4" define string.h
+eval $hasproto
+case "$d_strlcpy_proto" in
+    define) # see if strlcpy exists
+	set strlcpy d_strlcpy
+	eval $inlibc
+	;;
+    *)  val=$undef
+	set d_strlcpy
+	eval $setvar
+	;;
+esac
 
-- 
2.11.0

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Feb 4, 2019

From @tonycoz

On Thu, 31 Jan 2019 12​:33​:45 -0800, doughera wrote​:

On Wed, Jan 23, 2019 at 09​:58​:04PM -0500, Andy Dougherty wrote​:

Now implemented in the following 2 commits. (The first enhances the
hasproto
check; the second uses that enhancement.)

The following 2 commits handle the other problematic functions in musl
(that I am aware of), namely memrchr, strlcat, and strlcpy. The last
one also updates the hints file to always define _GNU_SOURCE for Linux
distributions using the musl C library. I hope this is sufficient to
close this ticket!

Thanks, closing.

I've proposed the fixes for backporting to 5.28.

Tony

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Feb 4, 2019

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

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented May 22, 2019

From @khwilliamson

Thank you for filing this report. You have helped make Perl better.

With the release today of Perl 5.30.0, this and 160 other issues have been
resolved.

Perl 5.30.0 may be downloaded via​:
https://metacpan.org/release/XSAWYERX/perl-5.30.0

If you find that the problem persists, feel free to reopen this ticket.

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented May 22, 2019

@khwilliamson - Status changed from 'pending release' to 'resolved'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.