ID:               48668
 User updated by:  dmda at yandex dot ru
 Reported By:      dmda at yandex dot ru
 Status:           Critical
 Bug Type:         Reproducible crash
 Operating System: Solaris
 PHP Version:      5.3.0RC4
 Assigned To:      dsp
 New Comment:

Dear sriram ,

Where did I say that the problem is in allocation size?
Once again, the problem is in the data alignment.
And fixing the code I mentioned, it fixes the Bus error.

diff -U5 ./php-5.3.0/Zend/zend_execute.h
./php-5.3.0D/Zend/zend_execute.h
--- ./php-5.3.0/Zend/zend_execute.h     2009-06-09 05:26:02.000000000
-0400
+++ ./php-5.3.0D/Zend/zend_execute.h    2009-08-18 12:27:18.080008000
-0400
@@ -154,11 +154,11 @@
                        zend_vm_stack_extend((count) TSRMLS_CC);                
                \
                }                                                               
                                                        \
        } while (0)
 
 static inline zend_vm_stack zend_vm_stack_new_page(int count) {
-       zend_vm_stack page =
(zend_vm_stack)emalloc(sizeof(*page)+sizeof(page->elements[0])*(count-1));
+       zend_vm_stack page =
(zend_vm_stack)emalloc(ZEND_MM_ALIGNED_SIZE(sizeof(*page))+ZEND_MM_ALIGNED_SIZE(sizeof(page->elements[0]))*(count-1));
 
        page->top = page->elements;
        page->end = page->elements + count;
        page->prev = NULL;
        return page;
@@ -217,11 +217,11 @@
 
 static inline void *zend_vm_stack_alloc(size_t size TSRMLS_DC)
 {
        void *ret;
 
-       size = (size + (sizeof(void*) - 1)) / sizeof(void*);
+       size = (size + (ZEND_MM_ALIGNED_SIZE(sizeof(void*)) - 1)) /
ZEND_MM_ALIGNED_SIZE(sizeof(void*));
 
        ZEND_VM_STACK_GROW_IF_NEEDED((int)size);
        ret = (void*)EG(argument_stack)->top;
        EG(argument_stack)->top += size;
        return ret;
diff -U5 ./php-5.3.0/Zend/zend_opcode.c
./php-5.3.0D/Zend/zend_opcode.c
--- ./php-5.3.0/Zend/zend_opcode.c      2009-06-05 19:20:59.000000000 -0400
+++ ./php-5.3.0D/Zend/zend_opcode.c     2009-08-18 12:29:21.630007000
-0400
@@ -43,11 +43,11 @@
        }
 }
 
 static void op_array_alloc_ops(zend_op_array *op_array)
 {
-       op_array->opcodes = erealloc(op_array->opcodes,
(op_array->size)*sizeof(zend_op));
+       op_array->opcodes = erealloc(op_array->opcodes,
(op_array->size)*ZEND_MM_ALIGNED_SIZE(sizeof(zend_op)));
 }
 
 void init_op_array(zend_op_array *op_array, zend_uchar type, int
initial_ops_size TSRMLS_DC)
 {
        op_array->type = type;
@@ -287,11 +287,11 @@
        }
 }
 
 void init_op(zend_op *op TSRMLS_DC)
 {
-       memset(op, 0, sizeof(zend_op));
+       memset(op, 0, ZEND_MM_ALIGNED_SIZE(sizeof(zend_op)));
        op->lineno = CG(zend_lineno);
        SET_UNUSED(op->result);
 }
 
 zend_op *get_next_op(zend_op_array *op_array TSRMLS_DC)
@@ -323,11 +323,11 @@
 }
 
 zend_brk_cont_element *get_next_brk_cont_element(zend_op_array
*op_array)
 {
        op_array->last_brk_cont++;
-       op_array->brk_cont_array = erealloc(op_array->brk_cont_array,
sizeof(zend_brk_cont_element)*op_array->last_brk_cont);
+       op_array->brk_cont_array = erealloc(op_array->brk_cont_array,
ZEND_MM_ALIGNED_SIZE(sizeof(zend_brk_cont_element))*op_array->last_brk_cont);
        return &op_array->brk_cont_array[op_array->last_brk_cont-1];
 }
 
 static void zend_update_extended_info(zend_op_array *op_array
TSRMLS_DC)
 {
@@ -372,11 +372,11 @@
        if (CG(compiler_options) & ZEND_COMPILE_HANDLE_OP_ARRAY) {
                zend_llist_apply_with_argument(&zend_extensions,
(llist_apply_with_arg_func_t) zend_extension_op_array_handler, op_array
TSRMLS_CC);
        }
 
        if (!(op_array->fn_flags & ZEND_ACC_INTERACTIVE) && op_array->size !=
op_array->last) {
-               op_array->opcodes = (zend_op *) erealloc(op_array->opcodes,
sizeof(zend_op)*op_array->last);
+               op_array->opcodes = (zend_op *) erealloc(op_array->opcodes,
ZEND_MM_ALIGNED_SIZE(sizeof(zend_op))*op_array->last);
                op_array->size = op_array->last;
        }
 
        opline = op_array->opcodes;
        end = opline + op_array->last;
diff -U5 ./php-5.3.0/Zend/zend_vm_def.h
./php-5.3.0D/Zend/zend_vm_def.h
--- ./php-5.3.0/Zend/zend_vm_def.h      2009-06-07 11:46:51.000000000 -0400
+++ ./php-5.3.0D/Zend/zend_vm_def.h     2009-08-18 12:24:29.649999000
-0400
@@ -4253,11 +4253,11 @@
        zend_uint catch_op_num;
        int catched = 0;
        zval restored_error_reporting;
  
        void **stack_frame = (void**)EX(Ts) +
-               (sizeof(temp_variable) * EX(op_array)->T) / sizeof(void*);
+               (ZEND_MM_ALIGNED_SIZE(sizeof(temp_variable)) * EX(op_array)->T) 
/
ZEND_MM_ALIGNED_SIZE(sizeof(void*));
 
        while (zend_vm_stack_top(TSRMLS_C) != stack_frame) {
                zval *stack_zval_p = zend_vm_stack_pop(TSRMLS_C);
                zval_ptr_dtor(&stack_zval_p);
        }
diff -U5 ./php-5.3.0/Zend/zend_vm_execute.h
./php-5.3.0D/Zend/zend_vm_execute.h
--- ./php-5.3.0/Zend/zend_vm_execute.h  2009-06-07 11:46:51.000000000
-0400
+++ ./php-5.3.0D/Zend/zend_vm_execute.h 2009-08-18 12:35:01.390003000
-0400
@@ -50,16 +50,16 @@
        EG(in_execution) = 1;
 
 zend_vm_enter:
        /* Initialize execute_data */
        execute_data = (zend_execute_data *)zend_vm_stack_alloc(
-               sizeof(zend_execute_data) +
-               sizeof(zval**) * op_array->last_var * (EG(active_symbol_table) 
? 1 :
2) +
-               sizeof(temp_variable) * op_array->T TSRMLS_CC);
+               ZEND_MM_ALIGNED_SIZE(sizeof(zend_execute_data)) +
+               ZEND_MM_ALIGNED_SIZE(sizeof(zval**)) * op_array->last_var *
(EG(active_symbol_table) ? 1 : 2) +
+               ZEND_MM_ALIGNED_SIZE(sizeof(temp_variable)) * op_array->T
TSRMLS_CC);
 
-       EX(CVs) = (zval***)((char*)execute_data +
sizeof(zend_execute_data));
-       memset(EX(CVs), 0, sizeof(zval**) * op_array->last_var);
+       EX(CVs) = (zval***)((char*)execute_data +
ZEND_MM_ALIGNED_SIZE(sizeof(zend_execute_data)));
+       memset(EX(CVs), 0, ZEND_MM_ALIGNED_SIZE(sizeof(zval**)) *
op_array->last_var);
        EX(Ts) = (temp_variable *)(EX(CVs) + op_array->last_var *
(EG(active_symbol_table) ? 1 : 2));
        EX(fbc) = NULL;
        EX(called_scope) = NULL;
        EX(object) = NULL;
        EX(old_error_reporting) = NULL;
@@ -601,11 +601,11 @@
        zend_uint catch_op_num;
        int catched = 0;
        zval restored_error_reporting;
 
        void **stack_frame = (void**)EX(Ts) +
-               (sizeof(temp_variable) * EX(op_array)->T) / sizeof(void*);
+               (ZEND_MM_ALIGNED_SIZE(sizeof(temp_variable)) * EX(op_array)->T) 
/
ZEND_MM_ALIGNED_SIZE(sizeof(void*));
 
        while (zend_vm_stack_top(TSRMLS_C) != stack_frame) {
                zval *stack_zval_p = zend_vm_stack_pop(TSRMLS_C);
                zval_ptr_dtor(&stack_zval_p);
        }
diff -U5 ./php-5.3.0/Zend/zend_vm_execute.skl
./php-5.3.0D/Zend/zend_vm_execute.skl
--- ./php-5.3.0/Zend/zend_vm_execute.skl        2008-06-11 09:18:41.000000000
-0400
+++ ./php-5.3.0D/Zend/zend_vm_execute.skl       2009-08-18 12:33:57.330001000
-0400
@@ -16,16 +16,16 @@
        EG(in_execution) = 1;
 
 zend_vm_enter:
        /* Initialize execute_data */
        execute_data = (zend_execute_data *)zend_vm_stack_alloc(
-               sizeof(zend_execute_data) +
-               sizeof(zval**) * op_array->last_var * (EG(active_symbol_table) 
? 1 :
2) +
-               sizeof(temp_variable) * op_array->T TSRMLS_CC);
+               ZEND_MM_ALIGNED_SIZE(sizeof(zend_execute_data)) +
+               ZEND_MM_ALIGNED_SIZE(sizeof(zval**)) * op_array->last_var *
(EG(active_symbol_table) ? 1 : 2) +
+               ZEND_MM_ALIGNED_SIZE(sizeof(temp_variable)) * op_array->T
TSRMLS_CC);
 
-       EX(CVs) = (zval***)((char*)execute_data +
sizeof(zend_execute_data));
-       memset(EX(CVs), 0, sizeof(zval**) * op_array->last_var);
+       EX(CVs) = (zval***)((char*)execute_data +
ZEND_MM_ALIGNED_SIZE(sizeof(zend_execute_data)));
+       memset(EX(CVs), 0, ZEND_MM_ALIGNED_SIZE(sizeof(zval**)) *
op_array->last_var);
        EX(Ts) = (temp_variable *)(EX(CVs) + op_array->last_var *
(EG(active_symbol_table) ? 1 : 2));
        EX(fbc) = NULL;
        EX(called_scope) = NULL;
        EX(object) = NULL;
        EX(old_error_reporting) = NULL;


Previous Comments:
------------------------------------------------------------------------

[2009-08-18 17:17:46] sriram dot natarajan at gmail dot com

no, I don't think it is just an issue with the allocation size. 

during php script execution, execute API within Zend/zend_vm_execute.h
will allocate space from 
zend_vm_stack (aka EG(argument_stack) for execution of a class or
function. however, the EG(argument_stack)->top is not always 8 byte
aligned because of the different number of arguments pushed etc.

the reason being that the offset (EG(argument_stack)->top) constantly
refers to the top of the stack and this is not always 8 byte aligned. 

for example,  I was able to move much further by aligning the address
returned from EX(CVs) and EX(Ts). 

unfortunately,my day job is going to keep me away from this interesting
bug until next week (till thursday). hopefully, i will be able to come
back to this before 5.3.1

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

[2009-08-18 12:12:44] dmda at yandex dot ru

another example is code below
        execute_data = (zend_execute_data *)zend_vm_stack_alloc(
                sizeof(zend_execute_data) +
                sizeof(zval**) * op_array->last_var * (EG(active_symbol_table) 
? 1 :
2) +
                sizeof(temp_variable) * op_array->T TSRMLS_CC);


If I got it right, it tries to allocate multiple entities in the same
chunk of memory under zend_vm_stack's control.
If that's the case, all the entities should be aligned separately, like
below

        execute_data = (zend_execute_data *)zend_vm_stack_alloc(
                ZEND_MM_ALIGNED_SIZE(sizeof(zend_execute_data)) +
                ZEND_MM_ALIGNED_SIZE(sizeof(zval**)) * op_array->last_var *
(EG(active_symbol_table) ? 1 : 2) +
                ZEND_MM_ALIGNED_SIZE(sizeof(temp_variable)) * op_array->T 
TSRMLS_CC);

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

[2009-08-18 10:42:27] dmda at yandex dot ru

everything is correct, except 8byte requirement.
The alignment requirement for all CPUs (including x86, of course) is
per following:
to access data of n bytes, its address should start with n-byte
boundary, in particular
-to access 8byte data (such as 64bit long long) the address should be
aligned to  8byte boundary (for example 0x12340 and 0x12348 are correct
addresses, while 0x12344 and 0x12342 are not)
-to access 4byte data (such as 32bit integer) the address must be
aligned to 4byte (0x12340, 0x12344, and 0x12348 are correct addresses,
while 0x12342 is not)
-to access 2byte data (such as 16bit short) the address must be aligned
to 2byte (0x12340, 0x12342, 0x12344, and 0x12348 are correct)

in case of x86 CPU, it has 2 modes: it can raise interrupt or issue an
extra bus cycle to fetch the remaining data. Under most OSes x86 is
configured to issue bus cycle. As of the other CPUs they raise Bus
error.

How to fix:
the code below taken from zend_execute.h is just an example of bad
practice in address calculation:

        size = (size + (sizeof(void*) - 1)) / sizeof(void*);
        ZEND_VM_STACK_GROW_IF_NEEDED((int)size);
        ret = (void*)EG(argument_stack)->top;
        EG(argument_stack)->top += size;


it takes into account size of the addres (which is 32bit in my case),
but target data size may be 64bit.


Interestingly enough that an APPROPRIATE code can also be found there,
see Zend.m4 with ZEND_MM_ALIGNMENT, so it would be enough to replace
sizeof(void*) with ZEND_MM_ALIGNMENT :)

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

[2009-08-18 04:02:34] sriram dot natarajan at gmail dot com

i have been looking into this issue over the week end and here is what
i found. 

a) this is memory alignment issue and nothing to do with compilers or
optimizations. hence, bug - 47230
(http://bugs.php.net/bug.php?id=47230&edit=1) should be closed as
duplicate of this bug. 

now, why this is happening ?

here is a rough and crude explanation : with php 5.3, addresses
returned from zend_vm_stack_top ,  _get_zval_ptr_tmp etc are not 8 byte
aligned. hence, accessing the 8 byte data results in crashes. 

i haven't come up with a solution yet. i will look more into this after
next week.

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

[2009-08-10 21:44:07] dmda at yandex dot ru

> by any chance, the submitter built this php5.3 on one machine 
> and ran it on another machine ?

no.

> currently, php 5.3 build process includes -xtarget=native 
> within the configure flags when used with sun studio compiler. 

I used only gcc which is available from sunfreeware. It did never
create any problems with various applications I compiled with it,
including php versions from 4.0.6 up to 5.2.10.

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

The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at
    http://bugs.php.net/48668

-- 
Edit this bug report at http://bugs.php.net/?id=48668&edit=1

Reply via email to