(perl #131746) avoid undefined behaviour in Copy() etc
authorTony Cook <tony@develop-help.com>
Mon, 14 Aug 2017 01:52:39 +0000 (11:52 +1000)
committerTony Cook <tony@develop-help.com>
Mon, 4 Sep 2017 04:47:29 +0000 (14:47 +1000)
These functions depend on C library functions which have undefined
behaviour when passed NULL pointers, even when passed a zero 'n' value.

Some compilers use this information, ie. assume the pointers are
non-NULL when optimizing any following code, so we do need to
prevent such unguarded calls.

My initial thought was to add conditionals to each macro to skip the
call to the library function when n is zero, but this adds a cost to
every use of these macros, even when the n value is always true.

So instead I added asserts() which will give us a much more visible
indicator of such broken code and revealed the pp_caller and Glob.xs
issues also patched here.

ext/File-Glob/Glob.pm
ext/File-Glob/Glob.xs
handy.h
pp_ctl.c
pp_hot.c

index 4f74023..6614e82 100644 (file)
@@ -37,7 +37,7 @@ pop @{$EXPORT_TAGS{bsd_glob}}; # no "glob"
 
 @EXPORT_OK   = (@{$EXPORT_TAGS{'glob'}}, 'csh_glob');
 
-$VERSION = '1.29';
+$VERSION = '1.30';
 
 sub import {
     require Exporter;
index e0a3681..9779d54 100644 (file)
@@ -121,7 +121,7 @@ iterate(pTHX_ bool(*globber)(pTHX_ AV *entries, const char *pat, STRLEN len, boo
 
     /* chuck it all out, quick or slow */
     if (gimme == G_ARRAY) {
-       if (!on_stack) {
+       if (!on_stack && AvFILLp(entries) + 1) {
            EXTEND(SP, AvFILLp(entries)+1);
            Copy(AvARRAY(entries), SP+1, AvFILLp(entries)+1, SV *);
            SP += AvFILLp(entries)+1;
diff --git a/handy.h b/handy.h
index c3848bf..7ef7e25 100644 (file)
--- a/handy.h
+++ b/handy.h
@@ -2409,17 +2409,17 @@ void Perl_mem_log_del_sv(const SV *sv, const char *filename, const int linenumbe
 #define Safefree(d)    safefree(MEM_LOG_FREE((Malloc_t)(d)))
 #endif
 
-#define Move(s,d,n,t)  (MEM_WRAP_CHECK_(n,t) (void)memmove((char*)(d),(const char*)(s), (n) * sizeof(t)))
-#define Copy(s,d,n,t)  (MEM_WRAP_CHECK_(n,t) (void)memcpy((char*)(d),(const char*)(s), (n) * sizeof(t)))
-#define Zero(d,n,t)    (MEM_WRAP_CHECK_(n,t) (void)memzero((char*)(d), (n) * sizeof(t)))
+#define Move(s,d,n,t)  (MEM_WRAP_CHECK_(n,t) assert(d), assert(s), (void)memmove((char*)(d),(const char*)(s), (n) * sizeof(t)))
+#define Copy(s,d,n,t)  (MEM_WRAP_CHECK_(n,t) assert(d), assert(s), (void)memcpy((char*)(d),(const char*)(s), (n) * sizeof(t)))
+#define Zero(d,n,t)    (MEM_WRAP_CHECK_(n,t) assert(d), (void)memzero((char*)(d), (n) * sizeof(t)))
 
-#define MoveD(s,d,n,t) (MEM_WRAP_CHECK_(n,t) memmove((char*)(d),(const char*)(s), (n) * sizeof(t)))
-#define CopyD(s,d,n,t) (MEM_WRAP_CHECK_(n,t) memcpy((char*)(d),(const char*)(s), (n) * sizeof(t)))
+#define MoveD(s,d,n,t) (MEM_WRAP_CHECK_(n,t) assert(d), assert(s), memmove((char*)(d),(const char*)(s), (n) * sizeof(t)))
+#define CopyD(s,d,n,t) (MEM_WRAP_CHECK_(n,t) assert(d), assert(s), memcpy((char*)(d),(const char*)(s), (n) * sizeof(t)))
 #ifdef HAS_MEMSET
-#define ZeroD(d,n,t)   (MEM_WRAP_CHECK_(n,t) memzero((char*)(d), (n) * sizeof(t)))
+#define ZeroD(d,n,t)   (MEM_WRAP_CHECK_(n,t) assert(d), memzero((char*)(d), (n) * sizeof(t)))
 #else
 /* Using bzero(), which returns void.  */
-#define ZeroD(d,n,t)   (MEM_WRAP_CHECK_(n,t) memzero((char*)(d), (n) * sizeof(t)),d)
+#define ZeroD(d,n,t)   (MEM_WRAP_CHECK_(n,t) assert(d), memzero((char*)(d), (n) * sizeof(t)),d)
 #endif
 
 #define PoisonWith(d,n,t,b)    (MEM_WRAP_CHECK_(n,t) (void)memset((char*)(d), (U8)(b), (n) * sizeof(t)))
index b16e12d..5f3cfdf 100644 (file)
--- a/pp_ctl.c
+++ b/pp_ctl.c
@@ -1991,7 +1991,8 @@ PP(pp_caller)
 
        if (AvMAX(PL_dbargs) < AvFILLp(ary) + off)
            av_extend(PL_dbargs, AvFILLp(ary) + off);
-       Copy(AvALLOC(ary), AvARRAY(PL_dbargs), AvFILLp(ary) + 1 + off, SV*);
+        if (AvFILLp(ary) + 1 + off)
+            Copy(AvALLOC(ary), AvARRAY(PL_dbargs), AvFILLp(ary) + 1 + off, SV*);
        AvFILLp(PL_dbargs) = AvFILLp(ary) + off;
     }
     mPUSHi(CopHINTS_get(cx->blk_oldcop));
index b891d79..40b8507 100644 (file)
--- a/pp_hot.c
+++ b/pp_hot.c
@@ -4330,7 +4330,8 @@ PP(pp_entersub)
                 AvARRAY(av) = ary;
             }
 
-           Copy(MARK+1,AvARRAY(av),items,SV*);
+            if (items)
+                Copy(MARK+1,AvARRAY(av),items,SV*);
            AvFILLp(av) = items - 1;
        }
        if (UNLIKELY((cx->blk_u16 & OPpENTERSUB_LVAL_MASK) == OPpLVAL_INTRO &&