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

sv_vcatpvfn_flags and format string friends crash on numbered args #13085

Open
p5pRT opened this issue Jul 5, 2013 · 4 comments
Open

sv_vcatpvfn_flags and format string friends crash on numbered args #13085

p5pRT opened this issue Jul 5, 2013 · 4 comments

Comments

@p5pRT
Copy link

p5pRT commented Jul 5, 2013

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

Searchable as RT118775$

@p5pRT
Copy link
Author

p5pRT commented Jul 5, 2013

From @bulk88

Created by @bulk88

Per

perl5/sv.c

Line 10452 in 6de85bb

\d+\$ explicit format parameter index

perl's internal printf supports numbered index format string args. When I try to use them, for example

SV *
MakeFString()
CODE:
RETVAL = newSVpvf("%3$d %2$d %1$s", "Just another perl hacker", 2, 3);
OUTPUT:
RETVAL
____________________________________________________
perl -MLocal::XS -e"print(Local::XS::MakeFString())"
____________________________________________________
I get a crash in strlen() "Unhandled exception at 0x7c362849
(msvcr71.dll) in perl.exe: 0xC0000005: Access violation reading location
0x00000003.".
____________________________________________________
> msvcr71.dll!strlen() Line 66 Asm
perl519.dll!Perl_sv_vcatpvfn_flags(interpreter *
my_perl=0x003642a4, sv * const sv=0x00368e44, const char * const
pat=0x00386600, const unsigned int patlen=0, char * * const
args=0x0012fac8, sv * * const svargs=0x00000000, const long svmax=0,
char * const maybe_tainted=0x00000000, const unsigned long flags=0)
Line 10803 + 0x6 C
perl519.dll!Perl_sv_vsetpvfn(interpreter * my_perl=0x003642a4, sv *
const sv=0x00368e44, const char * const pat=0x00386600, const unsigned
int patlen=14, char * * const args=0x0012fac8, sv * * const
svargs=0x00000000, const long svmax=0, char * const
maybe_tainted=0x00000000) Line 10182 + 0x1f C
perl519.dll!Perl_vnewSVpvf(interpreter * my_perl=0x003642a4, const
char * const pat=0x00386600, char * * const args=0x0012fac8) Line 8859
+ 0x29 C
perl519.dll!Perl_newSVpvf_nocontext(const char * const
pat=0x00386600, ...) Line 8819 + 0xd C
XS.dll!XS_Local__XS_MakeFString(interpreter * my_perl=0x003642a4,
cv * cv=0x009fdcec) Line 3520 C
perl519.dll!Perl_pp_entersub(interpreter * my_perl=0x00000003)
Line 2878 C
perl519.dll!Perl_runops_standard(interpreter * my_perl=0x003642a4)
Line 42 + 0x4 C
perl519.dll!S_run_body(interpreter * my_perl=0x00000003, long
oldscope=1) Line 2495 + 0xa C
perl519.dll!perl_run(interpreter * my_perl=0x003642a4) Line 2411 +
0x8 C
perl519.dll!RunPerl(int argc=3, char * * argv=0x01362cf0, char * *
env=0x00363308) Line 270 + 0x6 C
perl.exe!mainCRTStartup() Line 398 + 0xe C
kernel32.dll!_BaseProcessStart@4() + 0x23
___________________________________________________

line 10803 is
___________________________________________________
case 's':
if (vectorize)
goto unknown;
if (args) {
eptr = va_arg(*args, char*);
if (eptr)
elen = strlen(eptr);<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
else {
eptr = (char *)nullstr;
elen = sizeof nullstr - 1;
}
________________________________________________

I *guess* (correct me if I am wrong) numbered args are only implemented
for "svargs", not "args" based on
https://github.com/Perl/perl5/blob/211dfcf14199529e353c08dea10d7050e6a4a22a/sv.c
and
https://github.com/Perl/perl5/compare/e9c6cca7affc8f1a686236b954e87e5fa39d5127..eb3fce905f8436bbc374998ec8c7c34ce2b73e4e#diff-d237a07e2749aaf0dd44085eee9bd94b

I would like the numbered args to work for C stack varargs too, so my
XSUB above works instead of crashes.

I'm not sure if this is a bug report, feature request, doc request, or
I'm writing the format specifier wrong (a help request).

Perl Info

Flags:
    category=core
    severity=low

Site configuration information for perl 5.19.2:

Configured by Owner at Fri Jul  5 11:41:39 2013.

Summary of my perl5 (revision 5 version 19 subversion 2) configuration:
  Derived from: e06f856f7209ef9f86a20d46b2c039a3c7852762
  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
    useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
    use64bitint=undef, use64bitall=undef, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='cl', ccflags ='-nologo -GF -W3 -O1 -MD -Zi -DNDEBUG -GL -DWIN32 
-D_CONSOLE -DNO_STRICT  -DPERL_TEXTMODE_SCRIPTS -DPERL_IMPLICIT_CONTEXT 
-DPERL_IMPLICIT_SYS -DUSE_PERLIO -D_USE_32BIT_TIME_T',
    optimize='-O1 -MD -Zi -DNDEBUG -GL',
    cppflags='-DWIN32'
    ccversion='13.10.6030', gccversion='', gccosandvers=''
    intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
    d_longlong=undef, longlongsize=8, d_longdbl=define, longdblsize=8
    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 
-ltcg  -libpath:"c:\perl517\lib\CORE"  -machine:x86'
    libpth="C:\Program Files\Microsoft Visual Studio .NET 2003\VC7\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=perl519.lib
    gnulibc_version=''
  Dynamic Linking:
    dlsrc=dl_win32.xs, dlext=dll, d_dlsymun=undef, ccdlflags=' '
    cccdlflags=' ', lddlflags='-dll -nologo -nodefaultlib -debug 
-opt:ref,icf -ltcg  -libpath:"c:\perl517\lib\CORE"  -machine:x86'

Locally applied patches:
    uncommitted-changes
    253975a33d050576764c52847be36f0c20359d8d


@INC for perl 5.19.2:
    C:/perl517/site/lib
    C:/perl517/lib
    .


Environment for perl 5.19.2:
    HOME (unset)
    LANG (unset)
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=C:\perl517\bin;C:\Program Files\Microsoft Visual Studio .NET 
2003\Common7\IDE;C:\Program Files\Microsoft Visual Studio .NET 
2003\VC7\BIN;C:\Program Files\Microsoft Visual Studio .NET 
2003\Common7\Tools;C:\Program Files\Microsoft Visual Studio .NET 
2003\Common7\Tools\bin\prerelease;C:\WINDOWS\system32;C:\WINDOWS;C:\WINDOWS\system32\wbem;
    PERL_BADLANG (unset)
    SHELL (unset)


@p5pRT
Copy link
Author

p5pRT commented Jul 6, 2013

From @hvds

bulk88 (via RT) <perlbug-followup@​perl.org> wrote​:
:Per
:http​://perl5.git.perl.org/perl.git/blob/6de85bb45a5ea25528026a26cac854ee4dcdcd45​:/sv.c#l10452
:perl's internal printf supports numbered index format string args. When
:I try to use them, for example
:____________________________________________________
:SV *
:MakeFString()
:CODE​:
: RETVAL = newSVpvf("%3$d %2$d %1$s", "Just another perl hacker", 2, 3);
:OUTPUT​:
: RETVAL
:____________________________________________________
:perl -MLocal​::XS -e"print(Local​::XS​::MakeFString())"
:____________________________________________________
:I get a crash in strlen() "Unhandled exception at 0x7c362849
:(msvcr71.dll) in perl.exe​: 0xC0000005​: Access violation reading location
:0x00000003.".

[...]

:I *guess* (correct me if I am wrong) numbered args are only implemented
:for "svargs", not "args" based on
:http​://perl5.git.perl.org/perl.git/commit/211dfcf14199529e353c08dea10d7050e6a4a22a?f=sv.c
:and
:http​://perl5.git.perl.org/perl.git/blobdiff/e9c6cca7affc8f1a686236b954e87e5fa39d5127..eb3fce9​:/sv.c
:.
:
:I would like the numbered args to work for C stack varargs too, so my
:XSUB above works instead of crashes.
:
:I'm not sure if this is a bug report, feature request, doc request, or
:I'm writing the format specifier wrong (a help request).

I think it's a feature request (though the docs may also be lacking).

As I understand it, the way varargs are accessed you cannot find the n'th
arg without knowing the type (and thus the size) of the preceding args.
So it might be possible for format strings that refer to (and thus imply
the type of) all the args as in your example, but probably would not be
possible for "%3$d %1$s".

I think it would require a rather more complex process than the existing
code though - I think you'd need to make two passes over the format string,
the first one to determine an inferred array of types for all the args,
some intermediate logic [1] to build up an array of pointers to the arg
values, then a second pass over the format string to actually render the
output.

It would be worth looking at other implementations of this; here's an
extract from the sprintf man page on this (Ubuntu) system​:

  The C99 standard does not include the style using '$',
  which comes from the Single Unix Specification. If the style using '$'
  is used, it must be used throughout for all conversions taking an argu-
  ment and all width and precision arguments, but it may be mixed with
  "%%" formats which do not consume an argument. There may be no gaps in
  the numbers of arguments specified using '$'; for example, if arguments
  1 and 3 are specified, argument 2 must also be specified somewhere in
  the format string.

.. which sounds like it's implementing something similar to what I describe
(though I'm not sure why it has the additional "must be used throughout"
restriction).

There's a risk that introducing a bunch of extra complexity to this code
could destabilize existing stuff, for both perl and C/XS users. You'd also
be asking for even more complexity if you want existing cases not to be
slowed down by this support - you'd want to avoid the new logic until you
first discover you need it, rather than always doing the 2.5-pass version.

So I'd be tempted to go for the fallback position first - make it recognise
you're trying to do this, and raise a "not yet supported" error - so that
if we try to implement something but fail to get it stable in time for a
release, we'll at least crash less rudely.

Hugo

[1] Somehow, not sure how you do it with varargs; I suspect also that
doing anything too funky with varargs is likely to be highly unportable.

@p5pRT
Copy link
Author

p5pRT commented Jul 6, 2013

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

@p5pRT
Copy link
Author

p5pRT commented Feb 9, 2017

From @hvds

On Sat, 06 Jul 2013 01​:46​:07 -0700, hv wrote​:

bulk88 (via RT) <perlbug-followup@​perl.org> wrote​:
:I would like the numbered args to work for C stack varargs too, so my
:XSUB above works instead of crashes.
:
:I'm not sure if this is a bug report, feature request, doc request, or
:I'm writing the format specifier wrong (a help request).

I think it's a feature request (though the docs may also be lacking).

As I understand it, the way varargs are accessed you cannot find the n'th
arg without knowing the type (and thus the size) of the preceding args.
So it might be possible for format strings that refer to (and thus imply
the type of) all the args as in your example, but probably would not be
possible for "%3$d %1$s".
[snip possible approaches]

We don't have this. I think we're unlikely to get it.

So I'd be tempted to go for the fallback position first - make it recognise
you're trying to do this, and raise a "not yet supported" error - so that
if we try to implement something but fail to get it stable in time for a
release, we'll at least crash less rudely.

We do have this, since 46e58bd​:
  commit 46e58bd
  Author​: Aaron Crane <arc@​cpan.org>
  Date​: Tue Jul 7 18​:16​:36 2015 +0100

  Document and ensure that sv_catpvf() does no argument ordering

I'm not sure whether to leave this ticket as is, or mark it as one of stalled or rejected. I will mark it as wishlist though.

Hugo

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

3 participants