Bugs in zend.c and zend_compile.c in ZTS mode View as Markdown

We always prefer patches over long stories. :)
And preferrably first to somehow reproduce the problem easily..

--Jani

Hello all,

I have recently switched to PHP 5.1, and have been encountering crashes
during shutdown. I have tried delving into the PHP code to locate the
source of the problems, and have discovered two issues, both of which only
happen when compiled with ZTS enabled.

The first is very probably a bug.

In zend_compile.c, in the zend_initialize_class_data function, inside the
#ifdef ZTS block, the following line can be found:

ce->static_members = (HashTable*)n;

This casts an int variable, which denotes an index of the static_members
array, to a pointer. I assume what was meant is:

ce->static_members = (HashTable*) (CG(static_members)[n]);

In addition, support for runtime declaration (handled by the if block) is
broken, as it will not work when CG(static_members) is NULL and we're
trying to add the first member, so the if block should be followed by an
else block similar to this one.

else if (!CG(static_members)) {
// n is probably 0 at this point
CG(last_static_member) = n+1;
CG(static_members) = (HashTable**)calloc(n+1, sizeof(HashTable*));
}

If needed, I can provide a patch for this issue. The second issue may be
a bug, but I'm not familiar enough with the Zend Engine internals to be
certain.

In zend.c, the zend_post_startup function unlinks the global (r/o) copies
of several tables, and creates fresh r/w copies for the startup threads.

From looking at the code, it would seem that the unlinking of the
persistent_list is NOT done cleanly: The persistent list of the running
thread is destroyed and reinitialized (by executor_globals_ctor), and this
is what the global_persistent_list variable points to. In other words, the
global persistent list is shared with the startup thread.

Again, in this case it feels wrong, but this may also be what was intended.

I hope I've managed to make sense, and I'd appreciate any feedback on this.

Thanks,

David Oren

Hi David,

Would be happy to take a look at these bugs.
Any chance you can send a short reproducing script?

Thanks,

Andi

At 12:15 AM 12/8/2005, David Oren wrote:

Hello all,

I have recently switched to PHP 5.1, and have been encountering
crashes during shutdown. I have tried delving into the PHP code to
locate the source of the problems, and have discovered two issues,
both of which only happen when compiled with ZTS enabled.

The first is very probably a bug.

In zend_compile.c, in the zend_initialize_class_data function,
inside the #ifdef ZTS block, the following line can be found:

ce->static_members = (HashTable*)n;

This casts an int variable, which denotes an index of the
static_members array, to a pointer. I assume what was meant is:

ce->static_members = (HashTable*) (CG(static_members)[n]);

In addition, support for runtime declaration (handled by the if
block) is broken, as it will not work when CG(static_members) is
NULL and we're trying to add the first member, so the if block
should be followed by an else block similar to this one.

else if (!CG(static_members)) {

    // n is probably 0 at this point

    CG(last_static_member) = n+1;

    CG(static_members) = (HashTable**)calloc(n+1, sizeof(HashTable*));

}

If needed, I can provide a patch for this issue.

The second issue may be a bug, but I'm not familiar enough with
the Zend Engine internals to be certain.

In zend.c, the zend_post_startup function unlinks the global (r/o)
copies of several tables, and creates fresh r/w copies for the startup threads.

From looking at the code, it would seem that the unlinking of the
persistent_list is NOT done cleanly: The persistent list of the
running thread is destroyed and reinitialized (by
executor_globals_ctor), and this is what the global_persistent_list
variable points to. In other words, the global persistent list is
shared with the startup thread.

Again, in this case it feels wrong, but this may also be what was intended.

I hope I've managed to make sense, and I'd appreciate any feedback on this.

Thanks,

David Oren

---

Join Excite! - http://www.excite.com
The most personalized portal on the Web!