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.

Sign up for GitHub

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

Jump to bottom

builtin::trim return value numifies incorrectly in a loop #22784

Open
Grinnz opened this issue Nov 26, 2024 · 1 comment · May be fixed by #22788
Open

builtin::trim return value numifies incorrectly in a loop #22784

Grinnz opened this issue Nov 26, 2024 · 1 comment · May be fixed by #22788
Labels
Bug

Comments

@Grinnz
Copy link
Contributor

Grinnz commented Nov 26, 2024

Description
When used in a foreach loop, builtin::trim returns the same PV in subsequent iterations which then numifies to the same IV as in the first loop even though the PV has changed.

Reported by damien on IRC

Steps to Reproduce

perl -Mv5.40 -MDevel::Peek -E'for my $i ("4" .. "5") { Dump builtin::trim($i); Dump 0+builtin::trim($i) }'

SV = PV(0xd61dbc0) at 0xd65ba58
  REFCNT = 1
  FLAGS = (PADTMP,POK,pPOK)
  PV = 0xd5cce70 "4"\0
  CUR = 1
  LEN = 16
SV = IV(0xd65b970) at 0xd65b980
  REFCNT = 1
  FLAGS = (PADTMP,IOK,pIOK)
  IV = 4
SV = PV(0xd61dbc0) at 0xd65ba58
  REFCNT = 1
  FLAGS = (PADTMP,POK,pPOK)
  PV = 0xd5cce70 "5"\0
  CUR = 1
  LEN = 16
SV = IV(0xd65b970) at 0xd65b980
  REFCNT = 1
  FLAGS = (PADTMP,IOK,pIOK)
  IV = 4

Expected behavior
outputting a PV of '4', IV of 4, PV of '5', and IV of 5

Perl configuration

Summary of my perl5 (revision 5 version 40 subversion 0) configuration:
   
  Platform:
    osname=linux
    osvers=6.3.8-200.fc38.x86_64
    archname=x86_64-linux
    uname='linux home.grinnz.com 6.3.8-200.fc38.x86_64 #1 smp preempt_dynamic thu jun 15 02:15:40 utc 2023 x86_64 gnulinux '
    config_args='-Dprefix=/home/grinnz/.plenv/versions/5.40.0 -de -Dversiononly -A'eval:scriptdir=/home/grinnz/.plenv/versions/5.40.0/bin''
    hint=recommended
    useposix=true
    d_sigaction=define
    useithreads=undef
    usemultiplicity=undef
    use64bitint=define
    use64bitall=define
    uselongdouble=undef
    usemymalloc=n
    default_inc_excludes_dot=define
  Compiler:
    cc='cc'
    ccflags ='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2'
    optimize='-O2'
    cppflags='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
    ccversion=''
    gccversion='13.2.1 20231011 (Red Hat 13.2.1-4)'
    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='cc'
    ldflags =' -fstack-protector-strong -L/usr/local/lib'
    libpth=/usr/local/lib /usr/lib /usr/lib64 /usr/local/lib64
    libs=-lpthread -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc -lgdbm_compat
    perllibs=-lpthread -ldl -lm -lcrypt -lutil -lc
    libc=/lib/../lib64/libc.so.6
    so=so
    useshrplib=false
    libperl=libperl.a
    gnulibc_version='2.37'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs
    dlext=so
    d_dlsymun=undef
    ccdlflags='-Wl,-E'
    cccdlflags='-fPIC'
    lddlflags='-shared -O2 -L/usr/local/lib -fstack-protector-strong'
 
 
Characteristics of this binary (from libperl):
  Compile-time options:
    HAS_LONG_DOUBLE
    HAS_STRTOLD
    HAS_TIMES
    PERLIO_LAYERS
    PERL_COPY_ON_WRITE
    PERL_DONT_CREATE_GVSV
    PERL_HASH_FUNC_SIPHASH13
    PERL_HASH_USE_SBOX32
    PERL_MALLOC_WRAP
    PERL_OP_PARENT
    PERL_PRESERVE_IVUV
    PERL_USE_SAFE_PUTENV
    USE_64_BIT_ALL
    USE_64_BIT_INT
    USE_LARGE_FILES
    USE_LOCALE
    USE_LOCALE_COLLATE
    USE_LOCALE_CTYPE
    USE_LOCALE_NUMERIC
    USE_LOCALE_TIME
    USE_PERLIO
    USE_PERL_ATOF
  Built under linux
  Compiled at Jun 10 2024 21:21:12
  @INC:
    /home/grinnz/.plenv/versions/5.40.0/lib/perl5/site_perl/5.40.0/x86_64-linux
    /home/grinnz/.plenv/versions/5.40.0/lib/perl5/site_perl/5.40.0
    /home/grinnz/.plenv/versions/5.40.0/lib/perl5/5.40.0/x86_64-linux
    /home/grinnz/.plenv/versions/5.40.0/lib/perl5/5.40.0
@tonycoz
Copy link
Contributor

tonycoz commented Nov 26, 2024

It's not just for():

 ./perl -Ilib  -Mbuiltin=trim -le 'sub f { print 0+trim($_[0]); } f(4); f(123)'
4
4

I expect it's TARG not being properly reset, looking into it.

tonycoz added a commit to tonycoz/perl5 that referenced this issue Nov 26, 2024
@tonycoz
builtin::trim: ensure TARG is properly reset
9c2a10f 
In the reported test case, trim() sets TARG (used for the return
value), and that is then evaluated in numeric context, setting the IV
slot and pIOK and possible IOK.

These aren't reset on the second call to trim from the same entersub
OP, and since TARG is re-used it still has the IV slot and pIOK/IOK
set, and so uses the cached numeric value when evaluating TARG in
numeric context again.

Clear all but the string related flags.

Fixes Perl#22784
@jkeenan jkeenan added Bug and removed Needs Triage labels Nov 26, 2024
@tonycoz tonycoz linked a pull request Nov 26, 2024 that will close this issue
Open
tonycoz added a commit to tonycoz/perl5 that referenced this issue Nov 27, 2024
@tonycoz
builtin::trim: ensure TARG is properly reset
bdec274 
In the reported test case, trim() sets TARG (used for the return
value), and that is then evaluated in numeric context, setting the IV
slot and pIOK and possible IOK.

These aren't reset on the second call to trim from the same entersub
OP, and since TARG is re-used it still has the IV slot and pIOK/IOK
set, and so uses the cached numeric value when evaluating TARG in
numeric context again.

Clear all but the string related flags.

Fixes Perl#22784
tonycoz added a commit to tonycoz/perl5 that referenced this issue Dec 1, 2024
@tonycoz
builtin::trim: ensure TARG is properly reset
35e27db 
refactor a lot of custom "set SV to a string code" away to
sv_setpvn(), this:

 - fixed the original problem reported for Perl#22784, where TARG wasn't
   being reset properly and contained a cached numeric version of the
   result from the previous call.

 - removed some never executed code, since builtin::trim is only XS
   and is not an OP with the TARGLEX optimization

 - fixes a possible problem if the result of the first call to trim()
   is COWed.

This does slightly change the taint behaviour, rather than making TARG
tainted iff source is tainted, it changes to the behaviour of the rest
of perl, making TARG tainted if any tainted input is seen in the
current expression.

See thr PR Perl#22788 for some discussion on how we got here.

Fixes Perl#22784
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix trim() handling of TARG (and minor refactor) tonycoz/perl5
3 participants
@jkeenan @tonycoz @Grinnz