Skip Menu |
Report information
Id: 122701
Status: resolved
Priority: 0/
Queue: perl5

Owner: Nobody
Requestors:
Cc:
AdminCc:

Operating System: (no value)
PatchStatus: (no value)
Severity: low
Type:
Perl Version: (no value)
Fixed In: (no value)

Attachments


Date: Thu, 04 Sep 2014 09:57:05 -0500
To: perlbug [...] perl.org
From: rurban [...] cpanel.net
Subject: [PATCH] attributes: fix memEQs shared
Download (untitled) / with headers
text/plain 6.4k
This is a bug report for perl from rurban@cpanel.net, generated with the help of perlbug 1.40 running under perl 5.21.4. ----------------------------------------------------------------- attributes: fix memEQs shared Do not access memory buffers shorter than 6, and also check for longer buffers starting with "shared". This bug caused an ASAN error with my sv_grow performance improvement on short strings (commit 880c169, reverted with 3c239be), and went undetected because the shortest strings are 10 byte long. This bug blocks sv_grow and future string comparison improvements. ----------------------------------------------------------------- --- Flags: category=library severity=medium Type=Patch PatchStatus=HasPatch module=attributes --- Site configuration information for perl 5.21.4: Configured by rurban at Mon Sep 1 04:59:49 CDT 2014. Summary of my perl5 (revision 5 version 21 subversion 4) configuration: Local Commit: fe69a2bea4a56f064d3515845f0a08ee7b7b3fe3 Ancestor: f410e802720b8edaf0128f0af008ac504e9d98bf Platform: osname=linux, osvers=3.14-2-amd64, archname=x86_64-linux-debug-asan@32818149 uname='linux reini 3.14-2-amd64 #1 smp debian 3.14.13-2 (2014-07-24) x86_64 gnulinux ' config_args='-de -Dusedevel -Uversiononly -Dinstallman1dir=none -Dinstallman3dir=none -Dinstallsiteman1dir=none -Dinstallsiteman3dir=none -DEBUGGING -Uuseithreads -D'cc=clang' -D'optimize='-fno-omit-frame-pointer -gline-tables-only'' -A'ccflags=-fsanitize=address' -A'ldflags=-fsanitize=address' -A'lddlflags=-shared\ -fsanitize=address' -Duseshrplib -Dcf_email=''rurban@cpanel.net'' -Dperladmin=''rurban@cpanel.net'' -Accflags=-Wno-unused-value' hint=recommended, useposix=true, d_sigaction=define useithreads=undef, usemultiplicity=undef use64bitint=define, use64bitall=define, uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='clang', ccflags ='-fsanitize=address -Wno-unused-value -DDEBUGGING -fno-strict-aliasing -pipe -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64', optimize='-fno-omit-frame-pointer -gline-tables-only', cppflags='-fsanitize=address -Wno-unused-value -DDEBUGGING -fno-strict-aliasing -pipe -I/usr/local/include' ccversion='', gccversion='4.2.1 Compatible Debian Clang 3.4.2 (tags/RELEASE_34/dot2-final)', gccosandvers='' intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678 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='clang', ldflags =' -fsanitize=address -L/usr/local/lib' libpth=/usr/local/lib /usr/include/x86_64-linux-gnu /usr/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib /lib64 /usr/lib64 libs=-lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc -lgdbm_compat perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc libc=libc-2.19.so, so=so, useshrplib=true, libperl=libperl.so gnulibc_version='2.19' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E -Wl,-rpath,/usr/local/lib/perl5/5.21.4/x86_64-linux-debug-asan@32818149/CORE' cccdlflags='-fPIC', lddlflags=' -shared -fsanitize=address -L/usr/local/lib ' Locally applied patches: 32818149be28edd2a58ea345eea388bf0d3dc329 fe69a2bea4a56f064d3515845f0a08ee7b7b3fe3 --- @INC for perl 5.21.4: /usr/local/lib/perl5/site_perl/5.21.4/x86_64-linux-debug-asan@32818149 /usr/local/lib/perl5/site_perl/5.21.4 /usr/local/lib/perl5/5.21.4/x86_64-linux-debug-asan@32818149 /usr/local/lib/perl5/5.21.4 /usr/local/lib/perl5/site_perl/5.21.3 /usr/local/lib/perl5/site_perl/5.21.2 /usr/local/lib/perl5/site_perl/5.21.1 /usr/local/lib/perl5/site_perl/5.21.0 /usr/local/lib/perl5/site_perl/5.20.1 /usr/local/lib/perl5/site_perl/5.20.0 /usr/local/lib/perl5/site_perl/5.19.11 /usr/local/lib/perl5/site_perl/5.19.10 /usr/local/lib/perl5/site_perl/5.19.9 /usr/local/lib/perl5/site_perl/5.19.8 /usr/local/lib/perl5/site_perl/5.19.7 /usr/local/lib/perl5/site_perl/5.19.6 /usr/local/lib/perl5/site_perl/5.19.5 /usr/local/lib/perl5/site_perl/5.19.4 /usr/local/lib/perl5/site_perl/5.19.3 /usr/local/lib/perl5/site_perl/5.19.2 /usr/local/lib/perl5/site_perl/5.19.1 /usr/local/lib/perl5/site_perl/5.19.0 /usr/local/lib/perl5/site_perl/5.18.2 /usr/local/lib/perl5/site_perl/5.18.1 /usr/local/lib/perl5/site_perl/5.18.0 /usr/local/lib/perl5/site_perl/5.17.11 /usr/local/lib/perl5/site_perl/5.17.10 /usr/local/lib/perl5/site_perl/5.17.8 /usr/local/lib/perl5/site_perl/5.17.7 /usr/local/lib/perl5/site_perl/5.17.6 /usr/local/lib/perl5/site_perl/5.17.5 /usr/local/lib/perl5/site_perl/5.17.4 /usr/local/lib/perl5/site_perl/5.17.3 /usr/local/lib/perl5/site_perl/5.17.2 /usr/local/lib/perl5/site_perl/5.17.1 /usr/local/lib/perl5/site_perl/5.17.0 /usr/local/lib/perl5/site_perl/5.17 /usr/local/lib/perl5/site_perl/5.16.3 /usr/local/lib/perl5/site_perl/5.16.2 /usr/local/lib/perl5/site_perl/5.16.1 /usr/local/lib/perl5/site_perl/5.16.0 /usr/local/lib/perl5/site_perl/5.15.9 /usr/local/lib/perl5/site_perl/5.15.8 /usr/local/lib/perl5/site_perl/5.15.7 /usr/local/lib/perl5/site_perl/5.15.6 /usr/local/lib/perl5/site_perl/5.15.5 /usr/local/lib/perl5/site_perl/5.15.4 /usr/local/lib/perl5/site_perl/5.15.3 /usr/local/lib/perl5/site_perl/5.15.2 /usr/local/lib/perl5/site_perl/5.14.4 /usr/local/lib/perl5/site_perl/5.14.3 /usr/local/lib/perl5/site_perl/5.14.2 /usr/local/lib/perl5/site_perl/5.14.1 /usr/local/lib/perl5/site_perl/5.12.5 /usr/local/lib/perl5/site_perl/5.12.4 /usr/local/lib/perl5/site_perl/5.10.1 /usr/local/lib/perl5/site_perl/5.8.9 /usr/local/lib/perl5/site_perl/5.8.8 /usr/local/lib/perl5/site_perl/5.8.7 /usr/local/lib/perl5/site_perl/5.8.6 /usr/local/lib/perl5/site_perl/5.8.5 /usr/local/lib/perl5/site_perl/5.8.4 /usr/local/lib/perl5/site_perl/5.8.3 /usr/local/lib/perl5/site_perl/5.8.2 /usr/local/lib/perl5/site_perl/5.8.1 /usr/local/lib/perl5/site_perl/5.6.2 /usr/local/lib/perl5/site_perl . --- Environment for perl 5.21.4: HOME=/home/rurban LANG=en_US.utf8 LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/home/rurban/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games PERL_BADLANG (unset) SHELL=/bin/bash

Message body is not shown because sender requested not to inline it.

From: Andy Dougherty <doughera [...] lafayette.edu>
Date: Thu, 4 Sep 2014 12:47:04 -0400
To: perl5-porters [...] perl.org
Subject: Re: [perl #122701] [PATCH] attributes: fix memEQs shared
Download (untitled) / with headers
text/plain 1.3k
On Thu, Sep 04, 2014 at 07:57:21AM -0700, rurban @ cpanel . net wrote: Show quoted text
> # New Ticket Created by rurban@cpanel.net > # Please include the string: [perl #122701] > # in the subject line of all future correspondence about this issue. > # <URL: https://rt.perl.org/Ticket/Display.html?id=122701 > > > > This is a bug report for perl from rurban@cpanel.net, > generated with the help of perlbug 1.40 running under perl 5.21.4. > > > ----------------------------------------------------------------- > attributes: fix memEQs shared > > Do not access memory buffers shorter than 6, and also check for > longer buffers starting with "shared". This bug caused an ASAN error > with my sv_grow performance improvement on short strings (commit 880c169, > reverted with 3c239be), > and went undetected because the shortest strings are 10 byte long. > > This bug blocks sv_grow and future string comparison improvements. > > -----------------------------------------------------------------
Thanks for the quick and accurate diagnosis and fix. I've applied a simplified version of your patch in gdd36996. (The memEQs macro does the length calculations for you, if you call it with 'len' instead of the hardcoded '6'.) Also, t/porting/cmp_versions.t wasn't happy until I bumped the version number in attributes.pm. Thanks, -- Andy Dougherty doughera@lafayette.edu
Date: Thu, 04 Sep 2014 12:06:01 -0500
To: perlbug-followup [...] perl.org
From: Reini Urban <reini [...] cpanel.net>
Subject: Re: [perl #122701] [PATCH] attributes: fix memEQs shared
Download (untitled) / with headers
text/plain 1.8k
On 09/04/2014 11:47 AM, Andy Dougherty via RT wrote: Show quoted text
> On Thu, Sep 04, 2014 at 07:57:21AM -0700, rurban @ cpanel . net wrote:
>> # New Ticket Created by rurban@cpanel.net >> # Please include the string: [perl #122701] >> # in the subject line of all future correspondence about this issue. >> # <URL: https://rt.perl.org/Ticket/Display.html?id=122701 > >> >> >> This is a bug report for perl from rurban@cpanel.net, >> generated with the help of perlbug 1.40 running under perl 5.21.4. >> >> >> ----------------------------------------------------------------- >> attributes: fix memEQs shared >> >> Do not access memory buffers shorter than 6, and also check for >> longer buffers starting with "shared". This bug caused an ASAN error >> with my sv_grow performance improvement on short strings (commit 880c169, >> reverted with 3c239be), >> and went undetected because the shortest strings are 10 byte long. >> >> This bug blocks sv_grow and future string comparison improvements. >> >> -----------------------------------------------------------------
> > Thanks for the quick and accurate diagnosis and fix. I've applied > a simplified version of your patch in gdd36996. (The memEQs macro > does the length calculations for you, if you call it with 'len' instead > of the hardcoded '6'.) Also, t/porting/cmp_versions.t wasn't happy > until I bumped the version number in attributes.pm.
I see. I wondered about the t/porting/cmp_versions.t fail also, but didn't check. Thanks! In a subsequent patch I want to change min. SvLEN to wordsize (4|8) with calloc of the first word only, and then in memEQ check the first word, and then doing the memcmp of the rest if that fails. Should be much faster I hope. It was much faster in my tests because we don't need to setup the loop if the string is short. Numbers at http://blogs.perl.org/users/rurban/2014/08/perfect-hashes-and-faster-than-memcmp.html
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 250b
On Thu Sep 04 09:47:40 2014, doughera wrote: Show quoted text
> Thanks for the quick and accurate diagnosis and fix. I've applied > a simplified version of your patch in gdd36996. (The memEQs macro
gdd36996 isnt a known commit -- bulk88 ~ bulk88 at hotmail.com
CC: perlbug-followup [...] perl.org
Subject: Re: [perl #122701] [PATCH] attributes: fix memEQs shared
From: "H.Merijn Brand" <h.m.brand [...] xs4all.nl>
Date: Mon, 8 Sep 2014 15:48:04 +0200
To: perl5-porters [...] perl.org
On Mon, 8 Sep 2014 06:33:34 -0700, "bulk88 via RT" <perlbug-followup@perl.org> wrote: Show quoted text
> On Thu Sep 04 09:47:40 2014, doughera wrote:
> > Thanks for the quick and accurate diagnosis and fix. I've applied > > a simplified version of your patch in gdd36996. (The memEQs macro
> > gdd36996 isnt a known commit
Drop the leading g $ git show dd36996 commit dd369969a58d736f281ffebe36fc24ab45b29fa6 Author: Andy Dougherty <doughera@lafayette.edu> Date: Thu Sep 4 12:24:42 2014 -0400 Correct usage of memEQs in attributes.xs [perl #122701] Reported and diagnosed by Reini Urban <rurban@cpanel.net>. The call to memEQs(name, 6, "shared") could fail if name were shorter than 6 bytes, or if name were longer than 6, but started with "shared". -- H.Merijn Brand http://tux.nl Perl Monger http://amsterdam.pm.org/ using perl5.00307 .. 5.19 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/
Download (untitled)
application/pgp-signature 490b

Message body not shown because it is not plain text.

From: Reini Urban <reini [...] cpanel.net>
To: perlbug-followup [...] perl.org
Date: Mon, 08 Sep 2014 10:40:38 -0500
Subject: Re: [perl #122701] [PATCH] attributes: fix memEQs shared
Download (untitled) / with headers
text/plain 488b
On 09/08/2014 08:33 AM, bulk88 via RT wrote: Show quoted text
> On Thu Sep 04 09:47:40 2014, doughera wrote:
>> Thanks for the quick and accurate diagnosis and fix. I've applied >> a simplified version of your patch in gdd36996. (The memEQs macro
> > > gdd36996 isnt a known commit
It is known. The first char specifies the version control software (g for git, s for a potential svn, p for a potential perforce, ...), the rest specifies the commit in question. $ git describe v5.21.3-448-gdd36996


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org