[xdebug-general] Re: Details on issue #1229?

On Wed, 16 Dec 2015, Rainer Jung wrote:

> Am 14.12.2015 um 23:18 schrieb Rainer Jung:
> > Am 14.12.2015 um 22:00 schrieb Derick Rethans:
> > > Hi Rainer!
> > >
> > > On Mon, 14 Dec 2015, Rainer Jung wrote:
> > >
> > > > I'm testing XDebug 2.4.0RC3 in combination with PHP 7.0.0 on Solaris
> > > > 10 Sparc.
> > > >
> > > > I'm seeing crashes with slightly unclear reproducibility. I wonder
> > > > whether they are related to your issue #1229 (crashes because of -O2
> > > > for GCC 4.8). I'm using GCC 4.9.3 in combination with -O2.
> > > >
> > > > The issue says same code gets removed under -O2. What part of the code
> > > > is it? I'd like to check, whether that's also happening for gcc 4.9.3.
> > > > Could you please provide any details?
> > >
> > > It was line 1004 in xdebug_var.c - the one just above this one:
> > >
> > > > #2 0xfe9d555c in xdebug_var_export (struc=0xffbfe528,
> > > > struc[@]entry=0xffbfe5ec,
> > > > str=str[@]entry=0xffbfe59c, level=level[@]entry=1,
> > > > debug_zval=debug_zval[@]entry=0,
> > > > options=options[@]entry=0x2398c0) at
> > > > /tmp/pear/temp/xdebug/xdebug_var.c:1005
> > > > tmpz = 0x0
> > > > myht = <optimized out>
> > > > tmp_str = <optimized out>
> > > > is_temp = 0
> > > > num = <optimized out>
> > > > key = <optimized out>
> > > > val = <optimized out>
> > > >
> > > > The tmpz 0x0 looks fishy to me, also I don't see how it could crash
> > > > in this
> > > > line (but it could in the next effective line). See below.
> > >
> > > So it looks GCC 4.9 is affected too, for some reason, the
> > >
> > > zval *tmpz = &((*struc)->value.ref->val);
> > >
> > > Got removed while compiling... Removing -O2 fixed it, but I am wondering
> > > whether I am not doing something really wrong and that the compiler is
> > > right? If you could provide some insight, that'd be awesome.
> >
> > Wild guess: Maybe because tmpz is only declared locally in that block,
> > so goes out of scope immediately after line 1005. I don't know what the
> > C specs say, but wouldn't using &tmpz after that block result in
> > undefined behavior?
> >
> > I tried moving the declaration of tmpz higher up:
> >
> > 980 void xdebug_var_export(zval **struc, xdebug_str *str, int
> > level, int debug_zval, xdebug_var_export_options *options TSRMLS_DC)
> > 981 {
> > 982 HashTable *myht;
> > 983 char* tmp_str;
> > 984 int is_temp;
> > 985 #if PHP_VERSION_ID >= 70000
> > 986 zend_ulong num;
> > 987 zend_string *key;
> > 988 zval *val;
> > 989 zval *tmpz;
> > 990 #endif
> >
> > (line 989 added) and then the crash does not happen, not even with -O2.
> > The disassembled xdebug_var_export() differs indeed, but I didn't try t
> > understand the difference in detail.
> >
> > Not sure whether that's convincing, but maybe you can try on other
> > platforms or with your reproduction scenario as well.
>
> The following patch seems to fix the "-O2" problem for me:
>
> Index: xdebug_var.c xdebug_var.c
> --- xdebug_var.c 2015-12-14 18:17:05.878432000 +0100
> +++ xdebug_var.c 2015-12-14 23:26:42.581184000 +0100
> @@ -986,6 +986,7 @@
> zend_ulong num;
> zend_string *key;
> zval *val;
> + zval *tmpz;
> #endif
>
> if (!struc || !(*struc)) {
> @@ -1001,7 +1002,7 @@
> }
> }
> if (Z_TYPE_P(*struc) == IS_REFERENCE) {
> - zval *tmpz = &((*struc)->value.ref->val);
> + tmpz = &((*struc)->value.ref->val);
> struc = &tmpz;
> }
> #else

Yeah, that seems like it suitable fix. I'll make a PR and commit it.

cheers,
Derick

-- 
Like Xdebug? Consider a donation: http://xdebug.org/donate.php
Xdebug | http://xdebug.org | xdebug-general@lists.xdebug.org
twitter: @derickr and @xdebug