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

Uninitialized value $ExtUtils::ParseXS::num in subtraction #12085

Closed
p5pRT opened this issue May 5, 2012 · 17 comments
Closed

Uninitialized value $ExtUtils::ParseXS::num in subtraction #12085

p5pRT opened this issue May 5, 2012 · 17 comments

Comments

@p5pRT
Copy link

@p5pRT p5pRT commented May 5, 2012

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

Searchable as RT112776$

@p5pRT
Copy link
Author

@p5pRT p5pRT commented May 5, 2012

From perl@cjmweb.net

This is a bug report for perl from perl@​cjmweb.net,
generated with the help of perlbug 1.39 running under perl 5.14.2.


I'm working on a Perl module using XS to bind to a C library. When I
build it, I'm getting a warning message saying

  Use of uninitialized value $num in subtraction (-) at
  /usr/lib/perl5/vendor_perl/5.14.2/ExtUtils/ParseXS.pm line 1769,
  <GEN8> line 90.

This is triggered by the code generated by ExtUtils​::Constant.
Commenting out the INCLUDE​: const-xs.inc line in Foo.xs removes the
warning. I don't know whether the bug is in ExtUtils​::ParseXS or
in ExtUtils​::Constant, but even if EU​::C is generating bogus code,
EU​::PXS shouldn't be emitting warnings like that.

I'm using ExtUtils​::Constant 0.23, ExtUtils​::ParseXS 3.15,
Module​::Build 0.38, and Perl 5.14.2.

I've managed to pare it down to a reasonably small test case that
doesn't require any external C library. I've placed it in a GitHub
repo​: https://github.com/madsen/ParseXS-ExtUtils-Constant-bug
and attached a copy.

To reproduce the bug, clone the repo, type "perl Build.PL" and then
"./Build". You should see​:

  $ perl Build.PL
  Regenerating constants...
  Created MYMETA.yml and MYMETA.json
  Creating new 'Build' script for 'Foo' version '0.01'
  $ ./Build
  Building Foo
  Use of uninitialized value $num in subtraction ...
  gcc ... -o lib/Foo.o lib/Foo.c
  ExtUtils​::Mkbootstrap​::Mkbootstrap('blib/arch/auto/Foo/Foo.bs')
  gcc ... -o blib/arch/auto/Foo/Foo.so lib/Foo.o

You'll only see "Regenerating constants..." if you have
ExtUtils​::Constant installed. You shouldn't need it to reproduce the
bug, because I've added the generated files to the repo.

Whatever the problem is, it doesn't seem to stop the code from
working, because the included test does pass.

I've asked about this on http​://stackoverflow.com/q/10378986/8355
but nobody's come up with an answer.



Flags​:
  category=library
  severity=low
  module=ExtUtils​::ParseXS


Site configuration information for perl 5.14.2​:

Configured by Gentoo at Sat Oct 22 18​:25​:36 CDT 2011.

Summary of my perl5 (revision 5 version 14 subversion 2) configuration​:

  Platform​:
  osname=linux, osvers=2.6.37-gentoo-r4, archname=i686-linux
  uname='linux byte 2.6.37-gentoo-r4 #2 smp preempt tue may 10
18​:56​:04 cdt 2011 i686 intel(r) core(tm)2 duo cpu e6750 @​ 2.66ghz
genuineintel gnulinux '
  config_args='-des -Duseshrplib -Darchname=i686-linux
-Dcc=i686-pc-linux-gnu-gcc -Doptimize=-march=core2 -O2 -pipe -ggdb
-Dldflags=-Wl,-O1 -Wl,--as-needed -Dprefix=/usr -Dsiteprefix=/usr
-Dvendorprefix=/usr -Dscriptdir=/usr/bin -Dprivlib=/usr/lib/perl5/5.14.2
-Darchlib=/usr/lib/perl5/5.14.2/i686-linux
-Dsitelib=/usr/local/lib/perl5/5.14.2
-Dsitearch=/usr/local/lib/perl5/5.14.2/i686-linux
-Dvendorlib=/usr/lib/perl5/vendor_perl/5.14.2
-Dvendorarch=/usr/lib/perl5/vendor_perl/5.14.2/i686-linux
-Dman1dir=/usr/share/man/man1 -Dman3dir=/usr/share/man/man3
-Dsiteman1dir=/usr/share/man/man1 -Dsiteman3dir=/usr/share/man/man3
-Dvendorman1dir=/usr/share/man/man1 -Dvendorman3dir=/usr/share/man/man3
-Dman1ext=1 -Dman3ext=3pm -Dlibperl=libperl.so.5.14.2 -Dlocincpth=
-Duselargefiles -Dd_semctl_semun -Dcf_by=Gentoo -Dmyhostname=localhost
-Dperladmin=root@​localhost -Dinstallusrbinperl=n -Ud_csh -Uusenm
-Di_ndbm -Di_gdbm -Di_db -DDEBUGGING=-g
-Dinc_version_list=5.14.0/i686-linux 5.14.0 5.14.1/i686-linux 5.14.1 '
  hint=recommended, useposix=true, d_sigaction=define
  useithreads=undef, usemultiplicity=undef
  useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
  use64bitint=undef, use64bitall=undef, uselongdouble=undef
  usemymalloc=n, bincompat5005=undef
  Compiler​:
  cc='i686-pc-linux-gnu-gcc', ccflags ='-fno-strict-aliasing -pipe
-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
  optimize='-march=core2 -O2 -pipe -ggdb',
  cppflags='-fno-strict-aliasing -pipe'
  ccversion='', gccversion='4.5.3', gccosandvers=''
  intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
  d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12
  ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t',
lseeksize=8
  alignbytes=4, prototype=define
  Linker and Libraries​:
  ld='i686-pc-linux-gnu-gcc', ldflags ='-Wl,-O1 -Wl,--as-needed
-L/usr/local/lib'
  libpth=/usr/local/lib /lib /usr/lib
  libs=-lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc -lgdbm_compat
  perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc
  libc=/lib/libc-2.11.3.so, so=so, useshrplib=true,
libperl=libperl.so.5.14.2
  gnulibc_version='2.11.3'
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
  cccdlflags='-fPIC', lddlflags='-shared -march=core2 -O2 -pipe -ggdb
-L/usr/local/lib -Wl,-O1 -Wl,--as-needed'

Locally applied patches​:
  0001-gentoo_MakeMaker-RUNPATH.diff
  0002-gentoo_MakeMaker-delete_packlist.diff
  0003-gentoo_config_over.diff
  0004-gentoo_cpan_definstalldirs.diff
  0005-gentoo_cpanplus_definstalldirs.diff
  0006-gentoo_create-libperl-soname.diff
  0007-gentoo_drop-fstack-protector.diff
  0008-gentoo_enc2xs.diff
  0009-gentoo_mod-paths.diff


@​INC for perl 5.14.2​:
  /etc/perl
  /usr/local/lib/perl5/5.14.2/i686-linux
  /usr/local/lib/perl5/5.14.2
  /usr/lib/perl5/vendor_perl/5.14.2/i686-linux
  /usr/lib/perl5/vendor_perl/5.14.2
  /usr/local/lib/perl5
  /usr/lib/perl5/vendor_perl
  /usr/lib/perl5/5.14.2/i686-linux
  /usr/lib/perl5/5.14.2
  .


Environment for perl 5.14.2​:
  HOME=/home/cjm
  LANG=en_US.utf8
  LANGUAGE (unset)
  LD_LIBRARY_PATH (unset)
  LOGDIR (unset)

PATH=/home/cjm/bin​:/usr/local/bin​:/usr/bin​:/bin​:/opt/bin​:/usr/i686-pc-linux-gnu/gcc-bin/4.5.3​:/usr/games/bin​:/var/qmail/bin
  PERL_BADLANG (unset)
  PERL_UNICODE=SAL
  SHELL=/bin/bash

@p5pRT
Copy link
Author

@p5pRT p5pRT commented May 5, 2012

@p5pRT
Copy link
Author

@p5pRT p5pRT commented May 5, 2012

From @jkeenan

On Sat May 05 12​:37​:25 2012, cjm wrote​:

This is a bug report for perl from perl@​cjmweb.net,
generated with the help of perlbug 1.39 running under perl 5.14.2.

-----------------------------------------------------------------

I'm working on a Perl module using XS to bind to a C library. When I
build it, I'm getting a warning message saying

Use of uninitialized value $num in subtraction (-) at
/usr/lib/perl5/vendor_perl/5.14.2/ExtUtils/ParseXS.pm line 1769,
<GEN8> line 90.

This is triggered by the code generated by ExtUtils​::Constant.
Commenting out the INCLUDE​: const-xs.inc line in Foo.xs removes the
warning. I don't know whether the bug is in ExtUtils​::ParseXS or
in ExtUtils​::Constant, but even if EU​::C is generating bogus code,
EU​::PXS shouldn't be emitting warnings like that.

I'm using ExtUtils​::Constant 0.23, ExtUtils​::ParseXS 3.15,
Module​::Build 0.38, and Perl 5.14.2.

Do you get this warning if you use the (older) version of
ExtUtils​::ParseXS that came with Perl 5.14.2 -- v2.2210?

jimk

@p5pRT
Copy link
Author

@p5pRT p5pRT commented May 5, 2012

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

@p5pRT
Copy link
Author

@p5pRT p5pRT commented May 5, 2012

From @jkeenan

On Sat May 05 15​:15​:09 2012, jkeenan wrote​:

Do you get this warning if you use the (older) version of
ExtUtils​::ParseXS that came with Perl 5.14.2 -- v2.2210?

I answered my own question​: Yes.

#####
$ ./Build
Building Foo
Use of uninitialized value $ExtUtils​::ParseXS​::num in subtraction (-) at
/usr/local/lib/perl5/5.14.2/ExtUtils/ParseXS.pm line 1780, <GEN8> line 90.
#####

In the version you are using, 3.15, modify line 1117 of
ExtUtils/ParseXS.pm from this​:

  1117 $self->{var_num} = $self->{args_match}->{$var_name};

to​:

  $self->{var_num} = ($self->{args_match}->{$var_name}) || 0;

That should eliminate the uninitialized value warning. However, I
suspect we should proceed farther back than that point to see why we
assumed that the RHS of line 1117 would always be initialized.

Thank you very much.
Jim Keenan

@p5pRT
Copy link
Author

@p5pRT p5pRT commented May 5, 2012

From perl@cjmweb.net

On Sat May 05 15​:38​:10 2012, jkeenan wrote​:

In the version you are using, 3.15, modify line 1117 of
ExtUtils/ParseXS.pm

That won't do much good for me, since the people installing my module
won't have the patch, and they're the ones who are most likely to be
concerned about it. I can just ignore the warning.

I guess I'll just have to include a note about it in the README.

@p5pRT
Copy link
Author

@p5pRT p5pRT commented May 6, 2012

From @jkeenan

On Sat May 05 16​:05​:47 2012, cjm wrote​:

On Sat May 05 15​:38​:10 2012, jkeenan wrote​:

In the version you are using, 3.15, modify line 1117 of
ExtUtils/ParseXS.pm

That won't do much good for me, since the people installing my module
won't have the patch, and they're the ones who are most likely to be
concerned about it. I can just ignore the warning.

I guess I'll just have to include a note about it in the README.

I was only asking you to do that for our diagnostic purposes. Your
(presumably) legitimate use-case has uncovered a bug in ExtUtils​::ParseXS.

If I import Data​::Dumper into ParseXS.pm and modify ExtUtils​::ParseXS
3.15 at the point described above like this​:

#####
print STDERR Dumper $self->{args_match};
print STDERR "\$var_name​: $var_name\n";
  $self->{var_num} = $self->{args_match}->{$var_name} || 0;
#####

... then try to build your module, I get this output​:

#####
Regenerating constants...
Created MYMETA.yml and MYMETA.json
Creating new 'Build' script for 'Foo' version '0.01'
Building Foo
$VAR1 = {
  'sv' => 1
};
$var_name​: sv
$VAR1 = {
  'sv' => 1
};
$var_name​: s
cc -I/usr/local/lib/perl5/5.14.2/darwin-2level/CORE -DXS_VERSION="0.01"
-DVERSION="0.01" -c -fno-common -DPERL_DARWIN -fno-strict-aliasing -pipe
-I/usr/local/include -I/opt/local/include -O3 -o lib/Foo.o lib/Foo.c
ExtUtils​::Mkbootstrap​::Mkbootstrap('blib/arch/auto/Foo/Foo.bs')
env MACOSX_DEPLOYMENT_TARGET=10.3 cc -bundle -undefined dynamic_lookup
-L/usr/local/lib -L/opt/local/lib -o blib/arch/auto/Foo/Foo.bundle lib/Foo.o
#####

So the second time through a loop, there is a difference between the key
of the sole element in $self->{args_match} and the content of $var_name.
This is what can leave $self->{args_match}->{$var_name} undefined.

However, I don't know/remember enough of ParseXS to say what that means.

Thank you very much.
Jim Keenan

@p5pRT
Copy link
Author

@p5pRT p5pRT commented May 6, 2012

From perl@cjmweb.net

On Sat May 05 17​:56​:48 2012, jkeenan wrote​:

I was only asking you to do that for our diagnostic purposes. Your
(presumably) legitimate use-case has uncovered a bug in ExtUtils​::ParseXS.

Sorry, I misunderstood. Yes, adding "|| 0" to that line removes the
warning.

@p5pRT
Copy link
Author

@p5pRT p5pRT commented May 6, 2012

From @jkeenan

Please review the patch attached.

Thank you very much.
Jim Keenan

@p5pRT
Copy link
Author

@p5pRT p5pRT commented May 6, 2012

From @jkeenan

0001-Diagnosis-of-uninitialized-value-warning-reported-by.patch
From f8c3bbd4e2315ec1599f7dc346dd1c00836479bf Mon Sep 17 00:00:00 2001
From: jkeenan <jkeenan@cpan.org>
Date: Sat, 5 May 2012 23:31:52 -0400
Subject: [PATCH] Diagnosis of uninitialized value warning reported by Christopher J. Madsen++.

The set of keys found in the hash $self->{args_found} fundamentally derives
from the parsing of $func_header inside process_file().  The members of that
set are fixed by the time INPUT_handler() is called for the first time within
that subroutine.  They therefore do not necessarily include every instance of
$var_name; that variable only exists within INPUT_handler().
$self->{args_match}->{$var_name} does not necessarily exist.  So, in order to
be able to use $self->{var_num} in arithmetic expressions, we have to provide
it with a defined default value of 0.

For RT #112776.
---
 dist/ExtUtils-ParseXS/lib/ExtUtils/ParseXS.pm |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/dist/ExtUtils-ParseXS/lib/ExtUtils/ParseXS.pm b/dist/ExtUtils-ParseXS/lib/ExtUtils/ParseXS.pm
index 883d905..19c46c0 100644
--- a/dist/ExtUtils-ParseXS/lib/ExtUtils/ParseXS.pm
+++ b/dist/ExtUtils-ParseXS/lib/ExtUtils/ParseXS.pm
@@ -1114,7 +1114,7 @@ sub INPUT_handler {
       print "\t" . map_type($self, $var_type, undef);
       $printed_name = 0;
     }
-    $self->{var_num} = $self->{args_match}->{$var_name};
+    $self->{var_num} = $self->{args_match}->{$var_name} || 0;
 
     if ($self->{var_num}) {
       my $typemap = $self->{typemap}->get_typemap(ctype => $var_type);
-- 
1.6.3.2

@p5pRT
Copy link
Author

@p5pRT p5pRT commented May 6, 2012

From perl@cjmweb.net

On Sat May 05 20​:53​:41 2012, jkeenan wrote​:

Please review the patch attached.

Well, I can confirm that it solves my problem, but I don't know enough
about ExtUtils​::ParseXS to be sure it's the right solution, or if it's
just masking the real problem.

@p5pRT
Copy link
Author

@p5pRT p5pRT commented May 8, 2012

From @tsee

On 05/06/2012 05​:58 AM, Christopher J. Madsen via RT wrote​:

On Sat May 05 20​:53​:41 2012, jkeenan wrote​:

Please review the patch attached.

Well, I can confirm that it solves my problem, but I don't know enough
about ExtUtils​::ParseXS to be sure it's the right solution, or if it's
just masking the real problem.

That is precisely why I hadn't included a simple change like the
proposed patch yet -- I am also not entirely certain about whether it's
a fix or just plaster. Jim's finding of key (var name) mismatches does
make it sound like there's a deeper issue.

I'm on the fence on whether it's better to apply the patch to avoid
confusing users or whether it's better not to in the hope that one of
the affected users cares as much as Christopher and figures it out.
Alas, I don't currently have the time to dig in myself.

--Steffen

@p5pRT
Copy link
Author

@p5pRT p5pRT commented May 8, 2012

From @nwc10

On Tue, May 08, 2012 at 07​:47​:48AM +0200, Steffen Mueller wrote​:

On 05/06/2012 05​:58 AM, Christopher J. Madsen via RT wrote​:

On Sat May 05 20​:53​:41 2012, jkeenan wrote​:

Please review the patch attached.

Well, I can confirm that it solves my problem, but I don't know enough
about ExtUtils​::ParseXS to be sure it's the right solution, or if it's
just masking the real problem.

That is precisely why I hadn't included a simple change like the
proposed patch yet -- I am also not entirely certain about whether it's
a fix or just plaster. Jim's finding of key (var name) mismatches does
make it sound like there's a deeper issue.

I'm on the fence on whether it's better to apply the patch to avoid
confusing users or whether it's better not to in the hope that one of
the affected users cares as much as Christopher and figures it out.
Alas, I don't currently have the time to dig in myself.

I fear that it's plaster, and hence applying it does nothing to solve the
actual problem.

Nicholas Clark

@p5pRT
Copy link
Author

@p5pRT p5pRT commented May 8, 2012

From @tonycoz

On Tue, May 08, 2012 at 10​:13​:58AM +0100, Nicholas Clark wrote​:

On Tue, May 08, 2012 at 07​:47​:48AM +0200, Steffen Mueller wrote​:

On 05/06/2012 05​:58 AM, Christopher J. Madsen via RT wrote​:

On Sat May 05 20​:53​:41 2012, jkeenan wrote​:

Please review the patch attached.

Well, I can confirm that it solves my problem, but I don't know enough
about ExtUtils​::ParseXS to be sure it's the right solution, or if it's
just masking the real problem.

That is precisely why I hadn't included a simple change like the
proposed patch yet -- I am also not entirely certain about whether it's
a fix or just plaster. Jim's finding of key (var name) mismatches does
make it sound like there's a deeper issue.

I'm on the fence on whether it's better to apply the patch to avoid
confusing users or whether it's better not to in the hope that one of
the affected users cares as much as Christopher and figures it out.
Alas, I don't currently have the time to dig in myself.

I fear that it's plaster, and hence applying it does nothing to solve the
actual problem.

There's one or both of two problems here​:

a) the generated Build forces $^W on, which isn't the normal state
when the code is run within xsubpp

b) the undefined value itself is caused because the​:

  const char * s = SvPV(sv, len);

in const-xs.inc is for a name that isn't a parameter.

I can produce the same warning with code like​:

#include "EXTERN.h"
#include "perl.h"
#include "XSUB.h"

#include "ppport.h"

MODULE = Foo PACKAGE = Foo PREFIX = LIBMTP_

PROTOTYPES​: DISABLE

void
foo()
  const char *s = "world";
  CODE​:
  printf("hello %s\n", s);

I suspect the better change would be to change​:

  my $arg = "ST(" . ($num - 1) . ")";

to something along the lines of​:

  my $arg = $num ? "ST(" . ($num - 1) . ")"
  : qq(die "$var isn't an argument\\n");

Tony

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Jun 20, 2012

From @cpansprout

On Tue May 08 06​:34​:12 2012, tonyc wrote​:

On Tue, May 08, 2012 at 10​:13​:58AM +0100, Nicholas Clark wrote​:

On Tue, May 08, 2012 at 07​:47​:48AM +0200, Steffen Mueller wrote​:

On 05/06/2012 05​:58 AM, Christopher J. Madsen via RT wrote​:

On Sat May 05 20​:53​:41 2012, jkeenan wrote​:

Please review the patch attached.

Well, I can confirm that it solves my problem, but I don't know
enough
about ExtUtils​::ParseXS to be sure it's the right solution, or
if it's
just masking the real problem.

That is precisely why I hadn't included a simple change like the
proposed patch yet -- I am also not entirely certain about whether
it's
a fix or just plaster. Jim's finding of key (var name) mismatches
does
make it sound like there's a deeper issue.

I'm on the fence on whether it's better to apply the patch to avoid
confusing users or whether it's better not to in the hope that one of
the affected users cares as much as Christopher and figures it out.
Alas, I don't currently have the time to dig in myself.

I fear that it's plaster, and hence applying it does nothing to
solve the
actual problem.

There's one or both of two problems here​:

a) the generated Build forces $^W on, which isn't the normal state
when the code is run within xsubpp

b) the undefined value itself is caused because the​:

    const char \*    s = SvPV\(sv\, len\);

in const-xs.inc is for a name that isn't a parameter.

I can produce the same warning with code like​:

#include "EXTERN.h"
#include "perl.h"
#include "XSUB.h"

#include "ppport.h"

MODULE = Foo PACKAGE = Foo PREFIX = LIBMTP_

PROTOTYPES​: DISABLE

void
foo()
const char *s = "world";
CODE​:
printf("hello %s\n", s);

I suspect the better change would be to change​:

my $arg = "ST(" . ($num - 1) . ")";

to something along the lines of​:

my $arg = $num ? "ST\(" \. \($num \- 1\) \. "\)"
   : qq\(die "$var isn't an argument\\\\n"\);

Just a reminder. :-)

--

Father Chrysostomos

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Aug 31, 2012

From @tonycoz

On Tue Jun 19 21​:58​:01 2012, sprout wrote​:

On Tue May 08 06​:34​:12 2012, tonyc wrote​:

I suspect the better change would be to change​:

my $arg = "ST(" . ($num - 1) . ")";

to something along the lines of​:

my $arg = $num ? "ST\(" \. \($num \- 1\) \. "\)"
   : qq\(die "$var isn't an argument\\\\n"\);

Just a reminder. :-)

Fixed in 3f305fa and
115e60b

Tony

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Aug 31, 2012

@tonycoz - Status changed from 'open' to 'resolved'

@p5pRT p5pRT closed this Aug 31, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant