zsh-workers
 help / color / mirror / code / Atom feed
* Warnings in compctl
       [not found] <CGME20180321101631eucas1p281904e89d2bc29ef8fa0c3c0aa6b06d6@eucas1p2.samsung.com>
@ 2018-03-21 10:16 ` Peter Stephenson
  2018-03-23  3:38   ` Jun T
  2018-05-12 12:57   ` Eitan Adler
  0 siblings, 2 replies; 5+ messages in thread
From: Peter Stephenson @ 2018-03-21 10:16 UTC (permalink / raw)
  To: Zsh hackers' list

This removes some warnings from gcc 7.3.0.  Technically I think a
pointer off the front is in fact invalid (one off the end is allowed so
long as it isn't dereferenced).

pws


diff --git a/Src/Zle/compctl.c b/Src/Zle/compctl.c
index 5414b8f..e9d1657 100644
--- a/Src/Zle/compctl.c
+++ b/Src/Zle/compctl.c
@@ -383,7 +383,7 @@ get_compctl(char *name, char ***av, Compctl cc, int first, int isdef, int cl)
      * cct is a temporary just to hold flags; it never needs freeing.
      */
     struct compctl cct;
-    char **argv = *av;
+    char **argv = *av, argv_end[2] = "x";
     int ready = 0, hx = 0;
 
     /* Handle `compctl + foo ...' specially:  turn it into
@@ -516,14 +516,14 @@ get_compctl(char *name, char ***av, Compctl cc, int first, int isdef, int cl)
 		    }
 		    if ((*argv)[1]) {
 			p = (*argv) + 1;
-			*argv = "" - 1;
+			*argv = argv_end;
 		    } else if (!argv[1]) {
 			zwarnnam(name, "retry specification expected after -%c",
 				 **argv);
 			return 1;
 		    } else {
 			p = *++argv;
-			*argv = "" - 1;
+			*argv = argv_end;
 		    }
 		    switch (*p) {
 		    case '+':
@@ -553,25 +553,25 @@ get_compctl(char *name, char ***av, Compctl cc, int first, int isdef, int cl)
 	    case 'k':
 		if ((*argv)[1]) {
 		    cct.keyvar = (*argv) + 1;
-		    *argv = "" - 1;
+		    *argv = argv_end;
 		} else if (!argv[1]) {
 		    zwarnnam(name, "variable name expected after -%c", **argv);
 		    return 1;
 		} else {
 		    cct.keyvar = *++argv;
-		    *argv = "" - 1;
+		    *argv = argv_end;
 		}
 		break;
 	    case 'K':
 		if ((*argv)[1]) {
 		    cct.func = (*argv) + 1;
-		    *argv = "" - 1;
+		    *argv = argv_end;
 		} else if (!argv[1]) {
 		    zwarnnam(name, "function name expected after -%c", **argv);
 		    return 1;
 		} else {
 		    cct.func = *++argv;
-		    *argv = "" - 1;
+		    *argv = argv_end;
 		}
 		break;
 	    case 'Y':
@@ -582,74 +582,74 @@ get_compctl(char *name, char ***av, Compctl cc, int first, int isdef, int cl)
 	    expl:
 		if ((*argv)[1]) {
 		    cct.explain = (*argv) + 1;
-		    *argv = "" - 1;
+		    *argv = argv_end;
 		} else if (!argv[1]) {
 		    zwarnnam(name, "string expected after -%c", **argv);
 		    return 1;
 		} else {
 		    cct.explain = *++argv;
-		    *argv = "" - 1;
+		    *argv = argv_end;
 		}
 		break;
 	    case 'y':
 		if ((*argv)[1]) {
 		    cct.ylist = (*argv) + 1;
-		    *argv = "" - 1;
+		    *argv = argv_end;
 		} else if (!argv[1]) {
 		    zwarnnam(name, "function/variable expected after -%c",
 			     **argv);
 		} else {
 		    cct.ylist = *++argv;
-		    *argv = "" - 1;
+		    *argv = argv_end;
 		}
 		break;
 	    case 'P':
 		if ((*argv)[1]) {
 		    cct.prefix = (*argv) + 1;
-		    *argv = "" - 1;
+		    *argv = argv_end;
 		} else if (!argv[1]) {
 		    zwarnnam(name, "string expected after -%c", **argv);
 		    return 1;
 		} else {
 		    cct.prefix = *++argv;
-		    *argv = "" - 1;
+		    *argv = argv_end;
 		}
 		break;
 	    case 'S':
 		if ((*argv)[1]) {
 		    cct.suffix = (*argv) + 1;
-		    *argv = "" - 1;
+		    *argv = argv_end;
 		} else if (!argv[1]) {
 		    zwarnnam(name, "string expected after -%c", **argv);
 		    return 1;
 		} else {
 		    cct.suffix = *++argv;
-		    *argv = "" - 1;
+		    *argv = argv_end;
 		}
 		break;
 	    case 'g':
 		if ((*argv)[1]) {
 		    cct.glob = (*argv) + 1;
-		    *argv = "" - 1;
+		    *argv = argv_end;
 		} else if (!argv[1]) {
 		    zwarnnam(name, "glob pattern expected after -%c", **argv);
 		    return 1;
 		} else {
 		    cct.glob = *++argv;
-		    *argv = "" - 1;
+		    *argv = argv_end;
 		}
 		break;
 	    case 's':
 		if ((*argv)[1]) {
 		    cct.str = (*argv) + 1;
-		    *argv = "" - 1;
+		    *argv = argv_end;
 		} else if (!argv[1]) {
 		    zwarnnam(name, "command string expected after -%c",
 			     **argv);
 		    return 1;
 		} else {
 		    cct.str = *++argv;
-		    *argv = "" - 1;
+		    *argv = argv_end;
 		}
 		break;
 	    case 'l':
@@ -658,13 +658,13 @@ get_compctl(char *name, char ***av, Compctl cc, int first, int isdef, int cl)
 		    return 1;
 		} else if ((*argv)[1]) {
 		    cct.subcmd = (*argv) + 1;
-		    *argv = "" - 1;
+		    *argv = argv_end;
 		} else if (!argv[1]) {
 		    zwarnnam(name, "command name expected after -%c", **argv);
 		    return 1;
 		} else {
 		    cct.subcmd = *++argv;
-		    *argv = "" - 1;
+		    *argv = argv_end;
 		}
 		break;
 	    case 'h':
@@ -673,49 +673,49 @@ get_compctl(char *name, char ***av, Compctl cc, int first, int isdef, int cl)
 		    return 1;
 		} else if ((*argv)[1]) {
 		    cct.substr = (*argv) + 1;
-		    *argv = "" - 1;
+		    *argv = argv_end;
 		} else if (!argv[1]) {
 		    zwarnnam(name, "command name expected after -%c", **argv);
 		    return 1;
 		} else {
 		    cct.substr = *++argv;
-		    *argv = "" - 1;
+		    *argv = argv_end;
 		}
 		break;
 	    case 'W':
 		if ((*argv)[1]) {
 		    cct.withd = (*argv) + 1;
-		    *argv = "" - 1;
+		    *argv = argv_end;
 		} else if (!argv[1]) {
 		    zwarnnam(name, "path expected after -%c", **argv);
 		    return 1;
 		} else {
 		    cct.withd = *++argv;
-		    *argv = "" - 1;
+		    *argv = argv_end;
 		}
 		break;
 	    case 'J':
 		if ((*argv)[1]) {
 		    cct.gname = (*argv) + 1;
-		    *argv = "" - 1;
+		    *argv = argv_end;
 		} else if (!argv[1]) {
 		    zwarnnam(name, "group name expected after -%c", **argv);
 		    return 1;
 		} else {
 		    cct.gname = *++argv;
-		    *argv = "" - 1;
+		    *argv = argv_end;
 		}
 		break;
 	    case 'V':
 		if ((*argv)[1]) {
 		    cct.gname = (*argv) + 1;
-		    *argv = "" - 1;
+		    *argv = argv_end;
 		} else if (!argv[1]) {
 		    zwarnnam(name, "group name expected after -%c", **argv);
 		    return 1;
 		} else {
 		    cct.gname = *++argv;
-		    *argv = "" - 1;
+		    *argv = argv_end;
 		}
 		cct.mask2 |= CC_NOSORT;
 		break;
@@ -738,7 +738,7 @@ get_compctl(char *name, char ***av, Compctl cc, int first, int isdef, int cl)
 			cct.mstr = NULL;
 			return 1;
 		    }
-		    *argv = "" - 1;
+		    *argv = argv_end;
 		} else if (!argv[1]) {
 		    zwarnnam(name, "matching specification expected after -%c",
 			     **argv);
@@ -751,7 +751,7 @@ get_compctl(char *name, char ***av, Compctl cc, int first, int isdef, int cl)
 			cct.mstr = NULL;
 			return 1;
 		    }
-		    *argv = "" - 1;
+		    *argv = argv_end;
 		}
 		break;
 	    case 'H':
@@ -772,7 +772,7 @@ get_compctl(char *name, char ***av, Compctl cc, int first, int isdef, int cl)
 		    cct.hnum = 0;
 		if (*cct.hpat == '*' && !cct.hpat[1])
 		    cct.hpat = "";
-		*argv = "" - 1;
+		*argv = argv_end;
 		break;
 	    case 'C':
 		if (cl) {


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Warnings in compctl
  2018-03-21 10:16 ` Warnings in compctl Peter Stephenson
@ 2018-03-23  3:38   ` Jun T
  2018-03-23  9:32     ` Peter Stephenson
  2018-05-12 10:29     ` Oliver Kiddle
  2018-05-12 12:57   ` Eitan Adler
  1 sibling, 2 replies; 5+ messages in thread
From: Jun T @ 2018-03-23  3:38 UTC (permalink / raw)
  To: zsh-workers


> 2018/03/21 19:16, Peter Stephenson <p.stephenson@samsung.com> wrote:
> 
> This removes some warnings from gcc 7.3.0.  Technically I think a
> pointer off the front is in fact invalid 

Which option did you used with gcc?

Three more files (complete.c, computil.c, zle_thingy.c) have the same
"off the front" pointer assignments; these are detected by
"clang -Warray-bounds-pointer-arithmetic".


diff --git a/Src/Zle/complete.c b/Src/Zle/complete.c
index 16f48c958..313dcb92f 100644
--- a/Src/Zle/complete.c
+++ b/Src/Zle/complete.c
@@ -715,11 +715,10 @@ bin_compadd(char *name, char **argv, UNUSED(Options ops), UNUSED(int func))
 	    case 'E':
                 if (p[1]) {
                     dat.dummies = atoi(p + 1);
-                    p = "" - 1;
+		    p += strlen(p+1);
                 } else if (argv[1]) {
                     argv++;
                     dat.dummies = atoi(*argv);
-                    p = "" - 1;
                 } else {
                     zwarnnam(name, "number expected after -%c", *p);
 		    zsfree(mstr);
@@ -744,13 +743,12 @@ bin_compadd(char *name, char **argv, UNUSED(Options ops), UNUSED(int func))
 		    /* Pasted argument: -Xfoo. */
 		    if (!*sp)
 			*sp = p + 1;
-		    p = "" - 1;
+		    p += strlen(p+1);
 		} else if (argv[1]) {
 		    /* Argument in a separate word: -X foo. */
 		    argv++;
 		    if (!*sp)
 			*sp = *argv;
-		    p = "" - 1;
 		} else {
 		    /* Missing argument: argv[N] == "-X", argv[N+1] == NULL. */
 		    zwarnnam(name, e, *p);
diff --git a/Src/Zle/computil.c b/Src/Zle/computil.c
index 0b1ba58dc..4ce8eeee5 100644
--- a/Src/Zle/computil.c
+++ b/Src/Zle/computil.c
@@ -1222,7 +1222,7 @@ parse_cadef(char *nam, char **args)
 	    else if (*p == 'A') {
 		if (p[1]) {
 		    nonarg = p + 1;
-		    p = "" - 1;
+		    p += strlen(p+1);
 		} else if (args[1])
 		    nonarg = *++args;
 		else
@@ -1230,7 +1230,7 @@ parse_cadef(char *nam, char **args)
 	    } else if (*p == 'M') {
 		if (p[1]) {
 		    match = p + 1;
-		    p = "" - 1;
+		    p += strlen(p+1);
 		} else if (args[1])
 		    match = *++args;
 		else
diff --git a/Src/Zle/zle_thingy.c b/Src/Zle/zle_thingy.c
index f7e9829c2..5601c1178 100644
--- a/Src/Zle/zle_thingy.c
+++ b/Src/Zle/zle_thingy.c
@@ -731,6 +731,7 @@ bin_zle_call(char *name, char **args, UNUSED(Options ops), UNUSED(char func))
 	    break;
 	}
 	while (*++(*args)) {
+	    char skip_this_arg[2] = "x";
 	    switch (**args) {
 	    case 'n':
 		num = args[0][1] ? args[0]+1 : args[1];
@@ -741,7 +742,7 @@ bin_zle_call(char *name, char **args, UNUSED(Options ops), UNUSED(char func))
 		    return 1;
 		}
 		if (!args[0][1])
-		    *++args = "" - 1;
+		    *++args = skip_this_arg;
 		saveflag = 1;
 		zmod.mult = atoi(num);
 		zmod.flags |= MOD_MULT;
@@ -760,7 +761,7 @@ bin_zle_call(char *name, char **args, UNUSED(Options ops), UNUSED(char func))
 		    return 1;
 		}
 		if (!args[0][1])
-		    *++args = "" - 1;
+		    *++args = skip_this_arg;
 		keymap_restore = dupstring(curkeymapname);
 		if (selectkeymap(keymap_tmp, 0)) {
 		    if (remetafy)




^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Warnings in compctl
  2018-03-23  3:38   ` Jun T
@ 2018-03-23  9:32     ` Peter Stephenson
  2018-05-12 10:29     ` Oliver Kiddle
  1 sibling, 0 replies; 5+ messages in thread
From: Peter Stephenson @ 2018-03-23  9:32 UTC (permalink / raw)
  To: zsh-workers

On Fri, 23 Mar 2018 12:38:43 +0900
Jun T <takimoto-j@kba.biglobe.ne.jp> wrote:
> > 2018/03/21 19:16, Peter Stephenson <p.stephenson@samsung.com> wrote:
> > 
> > This removes some warnings from gcc 7.3.0.  Technically I think a
> > pointer off the front is in fact invalid   
> 
> Which option did you used with gcc?

Nothing special: it's standard zsh debugging flags.

-O2 -Wall -Wno-implicit -Wmissing-prototypes -ggdb

his is Ubuntu 16.04 but I compiled the new gcc myself.

pws


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Warnings in compctl
  2018-03-23  3:38   ` Jun T
  2018-03-23  9:32     ` Peter Stephenson
@ 2018-05-12 10:29     ` Oliver Kiddle
  1 sibling, 0 replies; 5+ messages in thread
From: Oliver Kiddle @ 2018-05-12 10:29 UTC (permalink / raw)
  To: zsh-workers

On 23 Mar, Jun T wrote:
> Three more files (complete.c, computil.c, zle_thingy.c) have the same
> "off the front" pointer assignments; these are detected by
> "clang -Warray-bounds-pointer-arithmetic".

One of these changes now triggers stack-use-after-scope in the address
sanitizer because skip_this_arg is declared inside the while loop but is
accessed in the while loop's condition.

Oliver

diff --git a/Src/Zle/zle_thingy.c b/Src/Zle/zle_thingy.c
index 5601c1178..6b892b822 100644
--- a/Src/Zle/zle_thingy.c
+++ b/Src/Zle/zle_thingy.c
@@ -725,13 +725,13 @@ bin_zle_call(char *name, char **args, UNUSED(Options ops), UNUSED(char func))
 	remetafy = 0;
 
     while (*args && **args == '-') {
+	char skip_this_arg[2] = "x";
 	char *num;
 	if (!args[0][1] || args[0][1] == '-') {
 	    args++;
 	    break;
 	}
 	while (*++(*args)) {
-	    char skip_this_arg[2] = "x";
 	    switch (**args) {
 	    case 'n':
 		num = args[0][1] ? args[0]+1 : args[1];


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Warnings in compctl
  2018-03-21 10:16 ` Warnings in compctl Peter Stephenson
  2018-03-23  3:38   ` Jun T
@ 2018-05-12 12:57   ` Eitan Adler
  1 sibling, 0 replies; 5+ messages in thread
From: Eitan Adler @ 2018-05-12 12:57 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: Zsh hackers' list

On 21 March 2018 at 03:16, Peter Stephenson <p.stephenson@samsung.com> wrote:
> This removes some warnings from gcc 7.3.0.  Technically I think a
> pointer off the front is in fact invalid (one off the end is allowed so
> long as it isn't dereferenced).

This is correct.

The change LGTM


-- 
Eitan Adler


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-05-12 12:58 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20180321101631eucas1p281904e89d2bc29ef8fa0c3c0aa6b06d6@eucas1p2.samsung.com>
2018-03-21 10:16 ` Warnings in compctl Peter Stephenson
2018-03-23  3:38   ` Jun T
2018-03-23  9:32     ` Peter Stephenson
2018-05-12 10:29     ` Oliver Kiddle
2018-05-12 12:57   ` Eitan Adler

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).