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

[dmorissette]

The following zend_hash_find call...

        pval **phandle;
        if (zend_hash_find(Z_OBJPROP_P(pThis), "_handle_", 
                           sizeof("_handle_"), 
                           (void **)&phandle) == SUCCESS)

... results in the following warning:

php_mapscript.c:11874: warning: dereferencing type-punned pointer will break
strict-aliasing rules

I have confirmed that the warning is caused by the (void **)&phandle cast when
using -O2 optimization with GCC 3.3.

It seems that using (void *)&phandle avoids the warning, I'm just not very clear
why.

The following thread from the PHP developers list discusses the issue and
suggests (void*) as an alternative as well:
http://www.zend.com/lists/php-dev/200308/msg00535.html

[dmorissette]

Here is a copy of the last message in this thread, in case the archive ever goes
away:

----------------------------

Subject: Re: [PHP-DEV] Re: gcc 3.3 warnings about strict-aliasing rules  	  

To: internals at lists dot php dot net
From: "Ard Biesheuvel" <abies at php dot net>
Date: Tue, 26 Aug 2003 22:52:07 +0200
list-help: <mailto:internals-help@lists.php.net>
list-post: <mailto:internals@lists.php.net>
list-unsubscribe: <mailto:internals-unsubscribe@lists.php.net>

> These unions would only be in the calling code, where they can be type
> specific, but I don't like this solution, too.
>
> But what's the best way then? Changing the casts from void** to void*?
> Compile with -fno-strict-aliasing?

The warning is emitted because, when compiling zend_hash_find(), the
compiler didn't expect your void** pData argument to actually be a
zval**, so any zval**s inside the definition of zend_hash_find() are
assumed not to point to the same location as pData.

If you look at the semantics of the function, you will find that this
assumption remains valid, as the void** argument is really the return
value of the hash lookup. No aliasing problems here.

As I said, passing a void* instead of a void** takes care of the warning
but not of the problem. Therefore, you can just pass -Wstrict-aliasing to
suppress the error. The 'correct' way to reimplement this would be to
return the pointer as a result instead of storing it in a location pointed
to
by an argument. The quick fix would be my prior suggestion to change
the prototype and all invocations to be a void*.

Personally, I wouldn't worry about it too much though.

Ard

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

[dmorissette]

Another interesting message earlier in this same thread:

----------------

Subject: Re: [PHP-DEV] Re: gcc 3.3 warnings about strict-aliasing rules  	  

To: internals at lists dot php dot net
From: Ard Biesheuvel <abies at php dot net>
Date: Tue, 26 Aug 2003 11:57:36 +0200
list-help: <mailto:internals-help@lists.php.net>
list-post: <mailto:internals@lists.php.net>
list-unsubscribe: <mailto:internals-unsubscribe@lists.php.net>
User-Agent: KNode/0.6.1

 

Stefan Roehrich wrote:

> On 2003-08-25 14:54:48, Ard Biesheuvel wrote:
>> Casting to void* instead of void** cures the problem. As the cast
> 
> Yes, but why?

Because a pointer to a pointer isn't untyped (because you know where it 
points to: to an untyped pointer)

> I don't know if with some optimizations void* or zval pointers need
> special alignment.

It's not about alignment (all pointers are always the same size, regardless 
of where they point to) It's about the strict-aliasing code being able to
rely on the fact that your void** pointer doesn't contain the same address
as a zval** or any other pointer (which is unlikely in the case where the
cast value is an actual function argument, like the case we're discussing)

> Yes, or we would be on the safe side if we compile with
> -fno-strict-aliasing, then gcc doesn't do such expression based

... and doesn't whine about it.

> optimizations. Or (as I have seen on some patches for other projects
> after a search for the gcc warning message) we could change the zval**
> data to something like this:
> 
> union {
>   zval **zval;
>   void *ptr;
> } data;

The problem here is that zend_hash_find() is meant to be generic. If you 
look through the code, you will see that it's not only being used for zval
but for other (extension-specific) types as well. Maintaining a union with
all these types would be ludicrous.

> call zend_hash_find(..., &data.ptr) and after this use
> data.zval. Access via unions is allowed because then the data must be
> correctly aligned for both types.

Alignment isn't the problem with pointers. The reason for a union is that 
the compiler can tell for sure that the pointers are aliased. In our case, 
the pointers are _not_ aliased, which could leave room for optimization by 
-fstrict-aliasing.

> The gcc generated code for this variant only uses some other
> registers, otherwise it seems similar.

I expect it to be identical to code generated for different variables that 
are known to contain the same address.

Ard

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

[dmorissette]

I'll change all occurences of this in php_mapscript.c and php_mapscript_util.c
to use a (void*) cast.

[dmorissette]

Done in 4.4: use (void*) instead of (void**) to avoid the warnings.