Everything

php

gcc 3.3 warnings about strict-aliasing rules

php.internals

Stefan Roehrich

23 years ago

Hello!

If PHP5 cvs is compiled with gcc 3.3 and -Wall, there are warnings
about: "dereferencing type-punned pointer will break strict-aliasing
rules". E.g. they occur if the last parameter of zend_hash_find is a
zval ** and used as zend_hash_find(..., (void **) &data).

This warning has to do with the strict aliasing rules (which are
turned on by the default -O2), "in particular, an object of one type
is assumed never to reside at the same address as an object of a
different type, unless the types are almost the same." (from the gcc
man page). It can be circumvented by using unions, but this would
require changes for nearly every invocation of zend_hash_find() (or
should there be a variant of zend_hash_find which takes a zval **
instead of a void **?).

Does anybody who knows gcc or the C standard better than I know
another solution?

Stefan

--
Stefan Röhrich               stefan@roehri.ch, sr@linux.de
http://www.roehri.ch/~sr/

Ard Biesheuvel

23 years ago

> If PHP5 cvs is compiled with gcc 3.3 and -Wall, there are
> warnings about: "dereferencing type-punned pointer will break
> strict-aliasing rules". E.g. they occur if the last parameter of
> zend_hash_find is a zval ** and used as
> zend_hash_find(..., (void **) &data).

Casting to void* instead of void** cures the problem. As the cast
is used to 'remove' the type from a variable, and not to change it,
I would consider it a justified change. I don't think there will be
consequences for the validity of the optimization.

> Does anybody who knows gcc or the C standard better than I
> know another solution?

Code will have to be changed regardless, as it's not the function
invocation but the cast itself that triggers the warning.

Ard

Ard Biesheuvel

23 years ago

> If PHP5 cvs is compiled with gcc 3.3 and -Wall, there are
> warnings about: "dereferencing type-punned pointer will break
> strict-aliasing rules". E.g. they occur if the last parameter of
> zend_hash_find is a zval ** and used as
> zend_hash_find(..., (void **) &data).

Casting to void* instead of void** cures the problem. As the cast
is used to 'remove' the type from a variable, and not to change it,
I would consider it a justified change. I don't think there will be
consequences for the validity of the optimization.

> Does anybody who knows gcc or the C standard better than I
> know another solution?

Code will have to be changed regardless, as it's not the function
invocation but the cast itself that triggers the warning.

Ard

Stefan Roehrich

23 years ago

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?

> I would consider it a justified change. I don't think there will be
> consequences for the validity of the optimization.

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

> Code will have to be changed regardless, as it's not the function
> invocation but the cast itself that triggers the warning.

Yes, or we would be on the safe side if we compile with
-fno-strict-aliasing, then gcc doesn't do such expression based
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;

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.

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

Stefan

--
Stefan Röhrich               stefan@roehri.ch, sr@linux.de
http://www.roehri.ch/~sr/

Ard Biesheuvel

23 years ago

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

Stefan Roehrich

23 years ago

On 2003-08-26 11:57:36, Ard Biesheuvel wrote:

> but for other (extension-specific) types as well. Maintaining a union with
> all these types would be ludicrous.

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?

Stefan

--
Stefan Röhrich               stefan@roehri.ch, sr@linux.de
http://www.roehri.ch/~sr/

Ard Biesheuvel

23 years ago

> 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