Strict-aliasing warnings

Strict-aliasing warnings

[Wayne Davison]

[-- Attachment #1: Type: text/plain, Size: 290 bytes --]

I'm using a version of gcc 3.3 to compile zsh, and it is spitting out a
bunch of warnings like this:

warning: dereferencing type-punned pointer will break strict-aliasing rules

The attached patch silences these warnings, but I'm not sure if this is
the right way to fix these.

..wayne..

[-- Attachment #2: zsh-aliasing.patch --]
[-- Type: text/plain, Size: 5806 bytes --]

--- Src/exec.c	13 Nov 2003 14:34:38 -0000	1.57
+++ Src/exec.c	19 Feb 2004 09:46:55 -0000
@@ -1844,7 +1844,7 @@ execcmd(Estate state, int input, int out
 			if (nextnode(firstnode(args)))
 			    next = (char *) getdata(nextnode(firstnode(args)));
 		    } else {
-			hn = (HashNode)&commandbn;
+			hn = (void *) &commandbn;
 			is_builtin = 1;
 			break;
 		    }
--- Src/utils.c	17 Dec 2003 20:47:39 -0000	1.57
+++ Src/utils.c	19 Feb 2004 09:46:56 -0000
@@ -509,7 +509,7 @@ finddir(char *s)
     strcpy(finddir_full, s);
     finddir_best=0;
     finddir_last=NULL;
-    finddir_scan((HashNode)&homenode, 0);
+    finddir_scan((void *) &homenode, 0);
     scanhashtable(nameddirtab, 0, 0, 0, finddir_scan, 0);
     return finddir_last;
 }
--- Src/zsh.h	15 Dec 2003 22:45:29 -0000	1.52
+++ Src/zsh.h	19 Feb 2004 09:46:57 -0000
@@ -371,7 +371,7 @@ struct linklist {
         (N).first = &__n0; \
         (N).last = &__n0; \
         __n0.next = NULL; \
-        __n0.last = (LinkNode) &(N); \
+        __n0.last = (void *) &(N); \
         __n0.dat = (void *) (V0); \
     } while (0)
 
--- Src/Modules/mapfile.c	11 Apr 2001 00:09:06 -0000	1.5
+++ Src/Modules/mapfile.c	19 Feb 2004 09:46:57 -0000
@@ -320,7 +320,7 @@ scanpmmapfile(HashTable ht, ScanFunc fun
 	 */
 	pm.nam = dupstring(pm.nam);
 	pm.u.str = "";
-	func((HashNode) &pm, flags);
+	func((void *) &pm, flags);
     }
     closedir(dir);
 }
--- Src/Modules/parameter.c	13 Nov 2003 14:34:38 -0000	1.28
+++ Src/Modules/parameter.c	19 Feb 2004 09:46:57 -0000
@@ -188,7 +188,7 @@ scanpmparameters(HashTable ht, ScanFunc 
 		((flags & (SCANPM_WANTVALS|SCANPM_MATCHVAL)) ||
 		 !(flags & SCANPM_WANTKEYS)))
 		pm.u.str = paramtypestr((Param) hn);
-	    func((HashNode) &pm, flags);
+	    func((void *) &pm, flags);
 	}
 }
 
@@ -327,7 +327,7 @@ scanpmcommands(HashTable ht, ScanFunc fu
 		    strcat(pm.u.str, cmd->nam);
 		}
 	    }
-	    func((HashNode) &pm, flags);
+	    func((void *) &pm, flags);
 	}
 }
 
@@ -545,7 +545,7 @@ scanfunctions(HashTable ht, ScanFunc fun
 			zsfree(t);
 		    }
 		}
-		func((HashNode) &pm, flags);
+		func((void *) &pm, flags);
 	    }
 	}
 }
@@ -663,7 +663,7 @@ scanbuiltins(HashTable ht, ScanFunc func
 
 		    pm.u.str = dupstring(t);
 		}
-		func((HashNode) &pm, flags);
+		func((void *) &pm, flags);
 	    }
 	}
 }
@@ -835,7 +835,7 @@ scanpmoptions(HashTable ht, ScanFunc fun
 	    pm.nam = hn->nam;
 	    ison = optno < 0 ? !opts[-optno] : opts[optno];
 	    pm.u.str = dupstring(ison ? "on" : "off");
-	    func((HashNode) &pm, flags);
+	    func((void *) &pm, flags);
 	}
 }
 
@@ -965,7 +965,7 @@ scanpmmodules(HashTable ht, ScanFunc fun
 	    pm.u.str = ((m->flags & MOD_ALIAS) ?
 			dyncat("alias:", m->u.alias) : loaded);
 	    addlinknode(done, pm.nam);
-	    func((HashNode) &pm, flags);
+	    func((void *) &pm, flags);
 	}
     }
     pm.u.str = dupstring("autoloaded");
@@ -975,14 +975,14 @@ scanpmmodules(HashTable ht, ScanFunc fun
 		!findmodnode(done, ((Builtin) hn)->optstr)) {
 		pm.nam = ((Builtin) hn)->optstr;
 		addlinknode(done, pm.nam);
-		func((HashNode) &pm, flags);
+		func((void *) &pm, flags);
 	    }
 	}
     for (p = condtab; p; p = p->next)
 	if (p->module && !findmodnode(done, p->module)) {
 	    pm.nam = p->module;
 	    addlinknode(done, pm.nam);
-	    func((HashNode) &pm, flags);
+	    func((void *) &pm, flags);
 	}
     for (i = 0; i < realparamtab->hsize; i++)
 	for (hn = realparamtab->nodes[i]; hn; hn = hn->next) {
@@ -990,7 +990,7 @@ scanpmmodules(HashTable ht, ScanFunc fun
 		!findmodnode(done, ((Param) hn)->u.str)) {
 		pm.nam = ((Param) hn)->u.str;
 		addlinknode(done, pm.nam);
-		func((HashNode) &pm, flags);
+		func((void *) &pm, flags);
 	    }
 	}
 }
@@ -1096,7 +1096,7 @@ scanpmhistory(HashTable ht, ScanFunc fun
 		!(flags & SCANPM_WANTKEYS))
 		pm.u.str = dupstring(he->text);
 	}
-	func((HashNode) &pm, flags);
+	func((void *) &pm, flags);
 
 	he = up_histent(he);
     }
@@ -1220,7 +1220,7 @@ scanpmjobtexts(HashTable ht, ScanFunc fu
 		    !(flags & SCANPM_WANTKEYS))
 		    pm.u.str = pmjobtext(job);
 	    }
-	    func((HashNode) &pm, flags);
+	    func((void *) &pm, flags);
 	}
     }
 }
@@ -1330,7 +1330,7 @@ scanpmjobstates(HashTable ht, ScanFunc f
 		    !(flags & SCANPM_WANTKEYS))
 		    pm.u.str = pmjobstate(job);
 	    }
-	    func((HashNode) &pm, flags);
+	    func((void *) &pm, flags);
 	}
     }
 }
@@ -1405,7 +1405,7 @@ scanpmjobdirs(HashTable ht, ScanFunc fun
 		   !(flags & SCANPM_WANTKEYS))
 		   pm.u.str = pmjobdir(job);
 	   }
-           func((HashNode) &pm, flags);
+           func((void *) &pm, flags);
        }
     }
 }
@@ -1540,7 +1540,7 @@ scanpmnameddirs(HashTable ht, ScanFunc f
 		    ((flags & (SCANPM_WANTVALS|SCANPM_MATCHVAL)) ||
 		     !(flags & SCANPM_WANTKEYS)))
 		    pm.u.str = dupstring(nd->dir);
-		func((HashNode) &pm, flags);
+		func((void *) &pm, flags);
 	    }
 	}
 }
@@ -1606,7 +1606,7 @@ scanpmuserdirs(HashTable ht, ScanFunc fu
 		    ((flags & (SCANPM_WANTVALS|SCANPM_MATCHVAL)) ||
 		     !(flags & SCANPM_WANTKEYS)))
 		    pm.u.str = dupstring(nd->dir);
-		func((HashNode) &pm, flags);
+		func((void *) &pm, flags);
 	    }
 	}
 }
@@ -1894,7 +1894,7 @@ scanaliases(HashTable alht, HashTable ht
 		    ((pmflags & (SCANPM_WANTVALS|SCANPM_MATCHVAL)) ||
 		     !(pmflags & SCANPM_WANTKEYS)))
 		    pm.u.str = dupstring(al->text);
-		func((HashNode) &pm, pmflags);
+		func((void *) &pm, pmflags);
 	    }
 	}
 }
--- Src/Zle/zleparameter.c	31 May 2001 09:44:00 -0000	1.2
+++ Src/Zle/zleparameter.c	19 Feb 2004 09:46:57 -0000
@@ -147,7 +147,7 @@ scanpmwidgets(HashTable ht, ScanFunc fun
 		((flags & (SCANPM_WANTVALS|SCANPM_MATCHVAL)) ||
 		 !(flags & SCANPM_WANTKEYS)))
 		pm.u.str = widgetstr(((Thingy) hn)->widget);
-	    func((HashNode) &pm, flags);
+	    func((void *) &pm, flags);
 	}
 }
 

Re: Strict-aliasing warnings

[Peter Stephenson]

Wayne Davison wrote:
> 
> --zYM0uCDKw75PZbzx
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: inline
> 
> I'm using a version of gcc 3.3 to compile zsh, and it is spitting out a
> bunch of warnings like this:
> 
> warning: dereferencing type-punned pointer will break strict-aliasing rules
> 
> The attached patch silences these warnings, but I'm not sure if this is
> the right way to fix these.

Seems unlikely that changing a (HashNode) cast to (void *) when the
formal argument really is a HashNode would improve things.

The warning is highly obscure but my guess is it's worried because it
doesn't know that the type we're looking at really does look like a HashNode.

Or maybe it's more sinister than that and it's saying it may actually
break HashNode's which are really pointers to other nodes in the same
format.

We could of course rewrite the code to embed real HashNode's at the front
of anything which goes into the hasher.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 692070



**********************************************************************
This email and any files transmitted with it are confidential and
intended solely for the use of the individual or entity to whom they
are addressed. If you have received this email in error please notify
the system manager.

This footnote also confirms that this email message has been swept by
MIMEsweeper for the presence of computer viruses.

www.mimesweeper.com
**********************************************************************

Re: Strict-aliasing warnings

[Wayne Davison]

On Thu, Feb 19, 2004 at 10:33:32AM +0000, Peter Stephenson wrote:
> The warning is highly obscure but my guess is it's worried because it
> doesn't know that the type we're looking at really does look like a
> HashNode.

Kind of, but only in the context of modern aliasing restrictions.  If
two aliases for a single memory location differ in type in a radical
enough manner, the compiler does not have to consider that the two
different references might affect one another.  E.g. you could change a
variable from one pointer and not have that change get reflected in the
reading from another pointer (or the original variable) because the
compiler might have optimized away the memory-fetch.  However, certain
casts force the compiler to be less aggressive in its optimizations
(such as (char *) and (void *)).

That is the intent of using the (void *) cast -- it _should_ let the
optimizer know that that variable needs to be refetched.  However, I'm
not 100% sure that this is enough to make sure that the optimizations
are valid or if it's only enough to just silence the warnings.

..wayne..

Re: Strict-aliasing warnings

[Peter Stephenson]

Wayne Davison wrote:
> That is the intent of using the (void *) cast -- it _should_ let the
> optimizer know that that variable needs to be refetched.  However, I'm
> not 100% sure that this is enough to make sure that the optimizations
> are valid or if it's only enough to just silence the warnings.

Well, one would hope they were tied together...

One would also hope no-one else was messing around with the pointer at the
same time, or there is a bug.  However, with zsh's, er, organic growth it's
hard to be sure.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 692070


**********************************************************************
This email and any files transmitted with it are confidential and
intended solely for the use of the individual or entity to whom they
are addressed. If you have received this email in error please notify
the system manager.

This footnote also confirms that this email message has been swept by
MIMEsweeper for the presence of computer viruses.

www.mimesweeper.com
**********************************************************************

Re: Strict-aliasing warnings

[Wayne Davison]

On Thu, Feb 19, 2004 at 11:04:28AM +0000, Peter Stephenson wrote:
> Well, one would hope they were tied together...

Yes, this one certainly hopes that as well.

So, do we want the void* patch committed?

..wayne..