develooper Front page | perl.perl5.porters | Postings from October 2013

[perl #120314] t/re/fold_grind.t spews tons of "Attempt to free temp prematurely" warnings on DEBUGGING but ultimately passes

Thread Previous | Thread Next
From:
Father Chrysostomos via RT
Date:
October 23, 2013 19:45
Subject:
[perl #120314] t/re/fold_grind.t spews tons of "Attempt to free temp prematurely" warnings on DEBUGGING but ultimately passes
Message ID:
rt-3.6.HEAD-26210-1382557513-1400.120314-15-0@perl.org
On Wed Oct 23 09:25:17 2013, sprout wrote:
> On Wed Oct 23 07:08:38 2013, bulk88 wrote:
> > On Wed Oct 23 03:35:04 2013, davem wrote:
> > > >
> > > > The answer is plain as day in
> > > >
> > > > http://www.nntp.perl.org/group/perl.daily-
> > > build.reports/2013/10/msg152778.html
> > > >
> > > > I won't provide a patch to protest discrimination against C89 :p
> > >
> > > It may be plain to you, but not to me. Care to elaborate?
> > >
> >
> > A lack of knowledge of C89, since AFAIK, most P5Pers use GCC in C++
> > mode
> > here, and if my C++ knowledge is correct, they will never see the
> bug.
> >
> > Commit
> >
>
http://perl5.git.perl.org/perl.git/commit/db17973a7879fcb6ada1024d1c72e99a944746d1
> > is responsible. I'll leave it to khw to realize his mistake in that
> > commit and fix all the warnings in regcomp.c in
> > http://www.nntp.perl.org/group/perl.daily-
> > build.reports/2013/10/msg152778.html
> > .

I think it is a little unfair to say that that people are unaware of the
issue.  Within the last two months I made a similar mistake, caught the
mistake in my code before committing it, but then accidentally reverted
the correction before pushing to perl.git.  It’s a really easy mistake
to make, even for those aware of it, especially if the build  hides it.

> >
> > I am upset because the only reason I saw this is because I looked at
> > my
> > screen because my TV show went to commercials. The smoke report
> still
> > "passes" even though its outputting severe warnings. The timing of
> > commercials on the TV is not a reliable way to check for bugs in a
> > make
> > test.
> >
> > IMO if it wasn't for a TV commercial this would have silently gone
> > into
> > 5.20. Anyone agree or disagree?

That’s a hard question. :-)

> > Maybe those SV leak warnings should be fatal on DEBUGGING builds,
> not
> > warnings?

If so, we don’t want to suppress multiple warnings.  Maybe croak at the
end if any of those warnings have occurred?

> 
> I use GCC without C++ mode, and it seems to imply !! when casting to a
> bool.  That used not to be the case.  What I have now is version
> 4.2.1.
>  I don’t remember what I used before, or whether the GCC version has
> anything to do with it.  Could stdbool.h have something to do with it?
> 
> (My complaint is that I used to catch this type of bug regularly, and
> now I don’t, and can’t easily.)

It is stdbool.h doing it.  On Darwin it is hard to avoid, because perl.c
includes mach-o/dyld.h:

#ifdef USE_NSGETEXECUTABLEPATH
#  include <mach-o/dyld.h>
#endif

and dyld.h include stdbool.h.

If I disable I_STDBOOL in config.h and apply this (there are two
stdbools.h on Mac OS X, one for GCC and another for clang, each using a
different #define):

diff --git a/perl.c b/perl.c
index e9cf22a..b9ae8d0 100644
--- a/perl.c
+++ b/perl.c
@@ -43,6 +43,8 @@
 #endif
 
 #ifdef USE_NSGETEXECUTABLEPATH
+#  define _STDBOOL_H_
+#  define _STDBOOL_H
 #  include <mach-o/dyld.h>
 #endif
 

then I get this:

$  ./miniperl -Ilib -e 'my $c = "\x{003A}"; my $p =
qr/(?l:[^\x{003A}]?)/i; $c =~ $p;'
Attempt to free temp prematurely: SV 0x7fa4540316a0 at -e line 1.
Attempt to free unreferenced scalar: SV 0x7fa4540316a0.

(Defining _STDBOOL_H like that is inherently risky, as the system’s
header files may depend on that bool definition.)

I think defining bool as char for debugging builds is a good idea.  But
the order of the headers will have to change a bit (for Darwin), since
perl.c #includes perl.h (which includes config.h and handy.h; config.h
defines USE_NSGETEXECUTABLEPATH) and then uses USE_NSGETEXECUTABLEPATH
to determine whether to #include mach-o/dyld.h.

I think the only way around it is something like this:

diff --git a/handy.h b/handy.h
index f80ba2c..3ae9302 100644
--- a/handy.h
+++ b/handy.h
@@ -69,7 +69,7 @@ Null SV pointer. (No longer available when
C<PERL_CORE> is defined.)
 #define MUTABLE_IO(p)	((IO *)MUTABLE_PTR(p))
 #define MUTABLE_SV(p)	((SV *)MUTABLE_PTR(p))
 
-#ifdef I_STDBOOL
+#if defined(I_STDBOOL) && !defined(DEBUGGING)
 #  include <stdbool.h>
 #  ifndef HAS_BOOL
 #    define HAS_BOOL 1
@@ -85,9 +85,11 @@ Null SV pointer. (No longer available when
C<PERL_CORE> is defined.)
    Andy Dougherty	February 2000
 */
 #ifdef __GNUG__		/* GNU g++ has bool built-in */
+# ifndef DEBUGGING
 #  ifndef HAS_BOOL
 #    define HAS_BOOL 1
 #  endif
+# endif
 #endif
 
 /* The NeXT dynamic loader headers will not build with the bool macro
@@ -104,6 +106,9 @@ Null SV pointer. (No longer available when
C<PERL_CORE> is defined.)
 #endif /* NeXT || __NeXT__ */
 
 #ifndef HAS_BOOL
+# ifdef bool
+#  undef bool
+# endif
 # if defined(VMS)
 #  define bool int
 # else
diff --git a/perl.c b/perl.c
index e9cf22a..5ddad89 100644
--- a/perl.c
+++ b/perl.c
@@ -42,10 +42,6 @@
 #  include <sys/sysctl.h>
 #endif
 
-#ifdef USE_NSGETEXECUTABLEPATH
-#  include <mach-o/dyld.h>
-#endif
-
 #ifdef DEBUG_LEAKING_SCALARS_FORK_DUMP
 #  ifdef I_SYSUIO
 #    include <sys/uio.h>
diff --git a/perl.h b/perl.h
index b40f141..1405019 100644
--- a/perl.h
+++ b/perl.h
@@ -2300,6 +2300,12 @@ typedef SV PADNAME;
 # define PERL_SAWAMPERSAND
 #endif
 
+/* Include mach-o/dyld.h here for perl.c’s sake, since it may #define bool,
+   and handy.h needs to be able to re#define it. */
+#if defined(USE_NSGETEXECUTABLEPATH) && defined(PERL_IN_PERL_C)
+# include <mach-o/dyld.h>
+#endif
+
 #include "handy.h"
 
 #if defined(USE_LARGE_FILES) && !defined(NO_64_BIT_RAWIO)

Is this patch a good idea?  (I think it is.  It also brings broken-bool
goodness to C++, but I have not tested that.)

-- 

Father Chrysostomos


---
via perlbug:  queue: perl5 status: open
https://rt.perl.org:443/rt3/Ticket/Display.html?id=120314

Thread Previous | Thread Next


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About