(Fwd) segfault with pcre_study alone

(Fwd) segfault with pcre_study alone

[Bart Schaefer]

I really don't know anything about the pcre module, so I'm forwarding
this to zsh-workers.


--- Forwarded mail from Matthias Kopfermann <matthias@kopfermann.org>

Date: Tue, 16 Mar 2004 13:45:44 +0100
From: Matthias Kopfermann <matthias@kopfermann.org>
To: Bart Schaefer <schaefer@brasslantern.com>
Subject: segfault with pcre_study alone

hi bart,
greetings from hamburg/germany.
i today when playing with pcre-module found out that pcre_study alone
leads to a segfault when invoked without using pcre_compile.
seems to be at all versions i tried. for some strange reason debian does
no longer come with pcre-module so i compiled zsh myself and used
zsh-4.2-pre4 for it.

        Matthias
-- 
"Sure, you can play a lot of notes, kid, but can your
 mom recognize you on the radio?"
	Les Paul


---End of forwarded mail from Matthias Kopfermann <matthias@kopfermann.org>

Re: (Fwd) segfault with pcre_study alone

[Clint Adams]

> greetings from hamburg/germany.
> i today when playing with pcre-module found out that pcre_study alone
> leads to a segfault when invoked without using pcre_compile.

The shell should return an error instead of segfaulting if pcre_study is
called before pcre_compile.

> seems to be at all versions i tried. for some strange reason debian does
> no longer come with pcre-module so i compiled zsh myself and used
> zsh-4.2-pre4 for it.

That was a mistake.  I'll attempt to fix both problems shortly.

Re: (Fwd) segfault with pcre_study alone

[Clint Adams]

> The shell should return an error instead of segfaulting if pcre_study is
> called before pcre_compile.

Index: Doc/Zsh/mod_pcre.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/mod_pcre.yo,v
retrieving revision 1.3
diff -u -r1.3 mod_pcre.yo
--- Doc/Zsh/mod_pcre.yo	4 Jul 2001 15:13:33 -0000	1.3
+++ Doc/Zsh/mod_pcre.yo	16 Mar 2004 15:10:20 -0000
@@ -8,6 +8,13 @@
 findex(pcre_compile)
 item(tt(pcre_compile) [ tt(-aimx) ] var(PCRE))(
 Compiles a perl-compatible regular expression.
+
+Option tt(-a) will force the pattern to be anchored.
+Option tt(-i) will compile a case-insensitive pattern.
+Option tt(-m) will compile a multi-line pattern; that is,
+tt(^) and tt($) will match newlines within the pattern.
+Option tt(-x) will compile an extended pattern, wherein
+whitespace and tt(#) comments are ignored.
 )
 findex(pcre_study)
 item(tt(pcre_study))(
Index: Src/Modules/pcre.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Modules/pcre.c,v
retrieving revision 1.5
diff -u -r1.5 pcre.c
--- Src/Modules/pcre.c	27 Aug 2002 21:10:34 -0000	1.5
+++ Src/Modules/pcre.c	16 Mar 2004 15:10:20 -0000
@@ -3,7 +3,7 @@
  *
  * This file is part of zsh, the Z shell.
  *
- * Copyright (c) 2001 Clint Adams
+ * Copyright (c) 2001, 2002, 2003, 2004 Clint Adams
  * All rights reserved.
  *
  * Permission is hereby granted, without written agreement and without
@@ -71,6 +71,13 @@
 bin_pcre_study(char *nam, char **args, Options ops, int func)
 {
     const char *pcre_error;
+
+    if (pcre_pattern == NULL)
+    {
+	zwarnnam(nam, "no pattern has been compiled for study: %s",
+		 pcre_error, 0);
+	return 1;
+    }
     
     pcre_hints = pcre_study(pcre_pattern, 0, &pcre_error);
     if (pcre_error != NULL)

Re: (Fwd) segfault with pcre_study alone

[Oliver Kiddle]

Clint Adams wrote:

> +
> +    if (pcre_pattern == NULL)
> +    {
> +	zwarnnam(nam, "no pattern has been compiled for study: %s",
> +		 pcre_error, 0);
> +	return 1;
> +    }

At this point, pcre_error has been declared a char * but not assigned
to by anything. You need:  
   zwarnnam(nam, "no pattern has been compiled for study", NULL, 0);

Also, shouldn't you initialise pcre_pattern to NULL where it is
declared since we are now relying on the compiler to initialise it to
null by doing this.

It might also be nice to put the same if statement in pcre_match:
"error in fullinfo" is not the most helpful error message.

Are you aware that zsh modules can define condition codes that can be
used inside [[ ... ]]? The completion module defines a -prefix
condition for example. This could be much nicer than the current
interface which is just a mapping onto the C calls. We'd be able to do:
  if [[ value -pcre-anchored pattern ]]; then

Oliver

Re: (Fwd) segfault with pcre_study alone

[Wayne Davison]

On Tue, Mar 16, 2004 at 07:05:28PM +0100, Oliver Kiddle wrote:
> Also, shouldn't you initialise pcre_pattern to NULL where it is
> declared since we are now relying on the compiler to initialise it to
> null by doing this.

It's a static variable, so the C standard guarantees that it will be
initialized to NULL by default.  Some folks like to explicitly
initialize static variables anyway, just for clarity, but it's not
required (and wouldn't change the code produced by the compiler).

..wayne..

Re: (Fwd) segfault with pcre_study alone

[Clint Adams]

> Are you aware that zsh modules can define condition codes that can be
> used inside [[ ... ]]? The completion module defines a -prefix
> condition for example. This could be much nicer than the current
> interface which is just a mapping onto the C calls. We'd be able to do:
>   if [[ value -pcre-anchored pattern ]]; then

That would be nice.

Re: (Fwd) segfault with pcre_study alone

[Clint Adams]

> > +    if (pcre_pattern == NULL)
> > +    {
> > +	zwarnnam(nam, "no pattern has been compiled for study: %s",
> > +		 pcre_error, 0);
> > +	return 1;
> > +    }
> 
> 
> when invoking pcre_study alone I now get:
> 
> pcre_study: no pattern has been compiled for study: @õ^R
> 
> 
> the @o^R looks strenge to me, purpose?

No, that was a mistake.  The ": %s" and pcre_error should be removed.

Re: (Fwd) segfault with pcre_study alone

[Clint Adams]

> At this point, pcre_error has been declared a char * but not assigned
> to by anything. You need:  
>    zwarnnam(nam, "no pattern has been compiled for study", NULL, 0);
> 
> It might also be nice to put the same if statement in pcre_match:
> "error in fullinfo" is not the most helpful error message.

I think this improves those two.

Index: Src/Modules/pcre.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Modules/pcre.c,v
retrieving revision 1.6
diff -u -r1.6 pcre.c
--- Src/Modules/pcre.c	16 Mar 2004 15:14:39 -0000	1.6
+++ Src/Modules/pcre.c	16 Mar 2004 19:39:07 -0000
@@ -74,8 +74,8 @@
 
     if (pcre_pattern == NULL)
     {
-	zwarnnam(nam, "no pattern has been compiled for study: %s",
-		 pcre_error, 0);
+	zwarnnam(nam, "no pattern has been compiled for study",
+		 NULL, 0);
 	return 1;
     }
     
@@ -112,9 +112,9 @@
 	}
     }
     
-    if (pcre_fullinfo(pcre_pattern, pcre_hints, PCRE_INFO_CAPTURECOUNT, &capcount))
+    if (ret = pcre_fullinfo(pcre_pattern, pcre_hints, PCRE_INFO_CAPTURECOUNT, &capcount))
     {
-	zwarnnam(nam, "error in fullinfo", NULL, 0);
+	zwarnnam(nam, "error %d in fullinfo", NULL, ret);
 	return 1;
     }
     

Re: (Fwd) segfault with pcre_study alone

[Clint Adams]

> Are you aware that zsh modules can define condition codes that can be
> used inside [[ ... ]]? The completion module defines a -prefix
> condition for example. This could be much nicer than the current
> interface which is just a mapping onto the C calls. We'd be able to do:
>   if [[ value -pcre-anchored pattern ]]; then

Here goes.

Index: Src/Modules/pcre.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Modules/pcre.c,v
retrieving revision 1.7
diff -u -r1.7 pcre.c
--- Src/Modules/pcre.c	16 Mar 2004 19:41:02 -0000	1.7
+++ Src/Modules/pcre.c	16 Mar 2004 21:39:32 -0000
@@ -31,6 +31,8 @@
 #include "pcre.mdh"
 #include "pcre.pro"
 
+#define CPCRE_PLAIN 0
+
 /**/
 #if defined(HAVE_PCRE_COMPILE) && defined(HAVE_PCRE_EXEC)
 #include <pcre.h>
@@ -99,10 +101,30 @@
 
 /**/
 static int
+zpcre_get_substrings(char *arg, int *ovec, int ret, char *receptacle)
+{
+    char **captures, **matches;
+
+	if(!pcre_get_substring_list(arg, ovec, ret, (const char ***)&captures)) {
+	    
+	    matches = zarrdup(&captures[1]); /* first one would be entire string */
+	    if (receptacle == NULL)
+		setaparam("match", matches);
+	    else
+		setaparam(receptacle, matches);
+	    
+	    pcre_free_substring_list((const char **)captures);
+	}
+
+	return 0;
+}
+
+/**/
+static int
 bin_pcre_match(char *nam, char **args, Options ops, int func)
 {
     int ret, capcount, *ovec, ovecsize;
-    char **captures, **matches, *receptacle = NULL;
+    char *receptacle = NULL;
     
     if(OPT_ISSET(ops,'a')) {
 	receptacle = *args++;
@@ -112,7 +134,7 @@
 	}
     }
     
-    if (ret = pcre_fullinfo(pcre_pattern, pcre_hints, PCRE_INFO_CAPTURECOUNT, &capcount))
+    if ((ret = pcre_fullinfo(pcre_pattern, pcre_hints, PCRE_INFO_CAPTURECOUNT, &capcount)))
     {
 	zwarnnam(nam, "error %d in fullinfo", NULL, ret);
 	return 1;
@@ -126,17 +148,8 @@
     if (ret==0) return 0;
     else if (ret==PCRE_ERROR_NOMATCH) return 1; /* no match */
     else if (ret>0) {
-	if(!pcre_get_substring_list(*args, ovec, ret, (const char ***)&captures)) {
-	    
-	    matches = zarrdup(&captures[1]); /* first one would be entire string */
-	    if (receptacle == NULL)
-		setaparam("match", matches);
-	    else
-		setaparam(receptacle, matches);
-	    
-	    pcre_free_substring_list((const char **)captures);
-	    return 0;
-      	}
+	zpcre_get_substrings(*args, ovec, ret, receptacle);
+	return 0;
     }
     else {
 	zwarnnam(nam, "error in pcre_exec", NULL, 0);
@@ -147,11 +160,43 @@
 }
 
 /**/
+static int
+cond_pcre_match(char **a, int id)
+{
+    pcre *pcre_pat;
+    const char *pcre_err;
+    char *lhstr, *rhre;
+    int r = 0, pcre_opts = 0, pcre_errptr, capcnt, *ov, ovsize;
+
+    lhstr = cond_str(a,0,0);
+    rhre = cond_str(a,1,0);
+
+    switch(id) {
+	 case CPCRE_PLAIN:
+		 pcre_pat = pcre_compile(rhre, pcre_opts, &pcre_err, &pcre_errptr, NULL);
+                 pcre_fullinfo(pcre_pat, NULL, PCRE_INFO_CAPTURECOUNT, &capcnt);
+    		 ovsize = (capcnt+1)*3;
+		 ov = zalloc(ovsize*sizeof(int));
+    		 r = pcre_exec(pcre_pat, NULL, lhstr, strlen(lhstr), 0, 0, ov, ovsize);
+    		if (r==0) return 1;
+	        else if (r==PCRE_ERROR_NOMATCH) return 0; /* no match */
+                else if (r>0) {
+		    zpcre_get_substrings(lhstr, ov, r, NULL);
+		    return 1;
+		}
+		break;
+    }
+
+    return 0;
+}
+
+/**/
 #else /* !(HAVE_PCRE_COMPILE && HAVE_PCRE_EXEC) */
 
 # define bin_pcre_compile bin_notavail
 # define bin_pcre_study bin_notavail
 # define bin_pcre_match bin_notavail
+# define cond_pcre_match cond_match
 
 /**/
 #endif /* !(HAVE_PCRE_COMPILE && HAVE_PCRE_EXEC) */
@@ -162,6 +207,11 @@
     BUILTIN("pcre_match",   0, bin_pcre_match,   1, 2, 0, "a",    NULL)
 };
 
+static struct conddef cotab[] = {
+    CONDDEF("pcre-match", CONDF_INFIX, cond_pcre_match, 0, 0, CPCRE_PLAIN)
+};
+
+
 /**/
 int
 setup_(Module m)
@@ -173,7 +223,8 @@
 int
 boot_(Module m)
 {
-    return !addbuiltins(m->nam, bintab, sizeof(bintab)/sizeof(*bintab));
+    return !addbuiltins(m->nam, bintab, sizeof(bintab)/sizeof(*bintab)) ||
+	   !addconddefs(m->nam, cotab, sizeof(cotab)/sizeof(*cotab));
 }
 
 /**/
@@ -181,6 +232,7 @@
 cleanup_(Module m)
 {
     deletebuiltins(m->nam, bintab, sizeof(bintab)/sizeof(*bintab));
+    deleteconddefs(m->nam, cotab, sizeof(cotab)/sizeof(*cotab));
     return 0;
 }