Re: com php-src: Temporary fixed the invalid read warning: Zend/zend_string.h

On Mon, Jul 14, 2014 at 8:01 AM, Xinchen Hui <laruence@php.net> wrote: > Commit: b7ff8f28b279b19d48af5f8c991d0ab93c5476b3 > Author: Xinchen Hui <laruence@php.net> Mon, 14 Jul 2014 > 12:31:55 +0800 > Parents: 6c8db864e047271fed082562a183e141b40c130e > Branches: phpng > > Link: > > http://git.php.net/?p=php-src.git;a=commitdiff;h=b7ff8f28b279b19d48af5f8c991d0ab93c5476b3 > > Log: > Temporary fixed the invalid read warning > > Changed paths: > M Zend/zend_string.h > > > Diff: > diff --git a/Zend/zend_string.h b/Zend/zend_string.h > index 5a9b0e7..e1780d7 100644 > --- a/Zend/zend_string.h > +++ b/Zend/zend_string.h > @@ -57,7 +57,7 @@ END_EXTERN_C() > #define _STR_HEADER_SIZE XtOffsetOf(zend_string, val) > > #define STR_ALLOCA_ALLOC(str, _len, use_heap) do { \ > - (str) = (zend_string *)do_alloca(_STR_HEADER_SIZE + (_len) + 1, > (use_heap)); \ > + (str) = (zend_string > *)do_alloca(ZEND_MM_ALIGNED_SIZE(_STR_HEADER_SIZE + (_len) + 1), > (use_heap)); \ > GC_REFCOUNT(str) = 1; \ > (str)->h = 0; \ > (str)->len = (_len); \ > @@ -109,7 +109,7 @@ static zend_always_inline zend_uint > zend_str_delref(zend_string *s) > > static zend_always_inline zend_string *zend_str_alloc(int len, int > persistent) > { > - zend_string *ret = (zend_string *)pemalloc(_STR_HEADER_SIZE + len > + 1, persistent); > + zend_string *ret = (zend_string > *)pemalloc(ZEND_MM_ALIGNED_SIZE(_STR_HEADER_SIZE + len + 1), persistent); > > GC_REFCOUNT(ret) = 1; > #if 1 > @@ -127,7 +127,7 @@ static zend_always_inline zend_string > *zend_str_alloc(int len, int persistent) > > static zend_always_inline zend_string *zend_str_safe_alloc(size_t n, > size_t m, size_t l, int persistent) > { > - zend_string *ret = (zend_string *)safe_pemalloc(n, m, > _STR_HEADER_SIZE + l + 1, persistent); > + zend_string *ret = (zend_string *)safe_pemalloc(n, m, > ZEND_MM_ALIGNED_SIZE(_STR_HEADER_SIZE + l + 1), persistent); > > GC_REFCOUNT(ret) = 1; > #if 1 > @@ -177,7 +177,7 @@ static zend_always_inline zend_string > *zend_str_realloc(zend_string *s, int len, > ret = STR_ALLOC(len, persistent); > memcpy(ret->val, s->val, (len > s->len ? s->len : len) + > 1); > } else if (EXPECTED(STR_REFCOUNT(s) == 1)) { > - ret = (zend_string *)perealloc(s, _STR_HEADER_SIZE + len + > 1, persistent); > + ret = (zend_string *)perealloc(s, > ZEND_MM_ALIGNED_SIZE(_STR_HEADER_SIZE + len + 1), persistent); > ret->len = len; > STR_FORGET_HASH_VAL(ret); > } else { > @@ -196,7 +196,7 @@ static zend_always_inline zend_string > *zend_str_safe_realloc(zend_string *s, siz > ret = STR_SAFE_ALLOC(n, m, l, persistent); > memcpy(ret->val, s->val, ((n * m) + l > s->len ? s->len : > ((n * m) + l)) + 1); > } else if (STR_REFCOUNT(s) == 1) { > - ret = (zend_string *)safe_perealloc(s, n, m, > _STR_HEADER_SIZE + l + 1, persistent); > + ret = (zend_string *)safe_perealloc(s, n, m, > ZEND_MM_ALIGNED_SIZE(_STR_HEADER_SIZE + l + 1), persistent); > ret->len = (n * m) + l; > STR_FORGET_HASH_VAL(ret); > } else { > Hey Xinchen! I didn't catch you on IRC after our last conversation, so now saying it here: I wasn't able to reproduce the issue on Ubuntu x86-64 with GCC 4.6. You mentioned that you're using GCC 4.1, so it's likely that this optimization was removed in newer versions, as it is unsafe for the struct hack usage. Anyway, I think solving it this way is okay, especially as it won't make a difference if the Zend MM is used. Thanks, Nikita