New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

array_unique crashes on refcounted strings #8011

Closed
ivyhjk opened this Issue Sep 28, 2017 · 60 comments

Comments

Projects
None yet
5 participants
@ivyhjk

ivyhjk commented Sep 28, 2017

HHVM Version

$ hhvm --version
HipHop VM 3.22.0 (rel)
Compiler: tags/HHVM-3.22.0-0-g4965441b640cb003a7185e78b18df72118763b59
Repo schema: abc6330f02a0c02cba7114040d5e1bf6b3f37fb9

Standalone code, or other way to reproduce the problem

<?php

// Error!
class Test
{
    public function foo()
    {
        $file = '/tmp/bar';

        touch($file);

        $files = [
            realpath($file)
        ];

        return array_unique($files);
    }
}

var_dump((new Test())->foo());

Expected result

$ hhvm test.php 
array(1) {
  [0]=>
  string(8) "/tmp/bar"
}

Actual result

$ hhvm test.php 
array(1) {
  [0]=>
  string(8) "�	�"
}
Core dumped: Segmentation fault
    #0 var_dump(), called at [/tmp/test.php:20]
Stack trace in /tmp/stacktrace.32234.log
    #0 var_dump(), called at [/tmp/test.php:20]
Segmentation fault

Additional comments

If you remove realpath or array_unique, all works correctly.

@ivyhjk

This comment has been minimized.

ivyhjk commented Sep 28, 2017

$ cat /tmp/stacktrace.32234.log
Host: dev01
ProcessID: 32234
ThreadID: 140004203959296
ThreadPID: 32234
Name: /usr/bin/hhvm
Type: Segmentation fault
Runtime: hhvm
Version: tags/HHVM-3.22.0-0-g4965441b640cb003a7185e78b18df72118763b59
DebuggerCount: 0

Arguments: test.php
ThreadType: CLI

# 0  0000000000e45da6
# 1  000000000122e153
# 2  00007f5542bc7890
# 3  00000000012b0180
# 4  000000000367a3a0
# 5  00000000050003ef
# 6  000000000500056b
# 7  000000000500031e
# 8  0000000003696104
# 9  0000000003695e9a
# 10 0000000003696626
# 11 0000000003694f4f
# 12 00000000037b19f1
# 13 00000000037b1821
# 14 00000000037b1505
# 15 00000000013bb5c4
# 16 00000000013bbe85
# 17 00000000013d1297
# 18 00000000013d3145
# 19 0000000000a203c6
# 20 00007f553cdafb45
# 21 0000000000a1bd29

PHP Stacktrace:

#0  var_dump() called at [/tmp/test.php:20]
@fredemmott

This comment has been minimized.

Contributor

fredemmott commented Oct 2, 2017

This also only happens when the result is returned - if it's just dumped in the scope, it's fine: https://3v4l.org/T0vjW

@fredemmott

This comment has been minimized.

Contributor

fredemmott commented Oct 2, 2017

@ivyhjk : whats your host operating system? I see this is reproducible on 3v4l, but I can't reproduce it with mac, or with our docker containers (which are 16.04)

@ivyhjk

This comment has been minimized.

ivyhjk commented Oct 2, 2017

@fredemmott

$ uname -a
Linux dev01 3.16.0-4-amd64 #1 SMP Debian 3.16.43-2+deb8u5 (2017-09-19) x86_64 GNU/Linux

Debian Jessie, 8.9

@fredemmott

This comment has been minimized.

Contributor

fredemmott commented Oct 2, 2017

Thanks, can repro:

fredemmott-fb:~ fredemmott$ docker run -it debian:jessie /bin/bash -l
root@6acf0468f962:/# apt-key adv --recv-keys --keyserver hkp://keyserver.ubuntu.com:80 0x5a16e7281be7a449
root@6acf0468f962:/# echo deb http://dl.hhvm.com/debian jessie main > /etc/apt/sources.list.d/hhvm.list
root@6acf0468f962:/# apt-get update; apt-get install hhvm
root@6acf0468f962:/# cat > test.php
<?php

// Error!
class Test
{
    public function foo()
    {
        $file = '/tmp/bar';

        touch($file);

        $files = [
            realpath($file)
        ];

        return array_unique($files);
    }
}

var_dump((new Test())->foo());
root@6acf0468f962:/# hhvm -d hhvm.jit=0 test.php
set_mempolicy: Operation not permitted
array(1) {
  [0]=>
  string(8) "	"
}
Core dumped: Segmentation fault
Stack trace in /tmp/stacktrace.5843.log
Segmentation fault
root@6acf0468f962:/# hhvm -d hhvm.jit=1 test.php
set_mempolicy: Operation not permitted
array(1) {
  [0]=>
  string(8) "	"
}
Core dumped: Segmentation fault
Stack trace in /tmp/stacktrace.5844.log
Segmentation fault
@fredemmott

This comment has been minimized.

Contributor

fredemmott commented Oct 2, 2017

... added bonus: the issue isn't reproducible with the hhvm-dbg package, so not getting a usable backtrace out of it.

@fredemmott

This comment has been minimized.

Contributor

fredemmott commented Oct 2, 2017

And yeah, seems likely to be something weird with array_unique:

  • can't reproduce by using array_values instead
  • can't reproduce by using array_map(function($x) { return $x; }, instead
  • can reproduce with either of the above if wrapped in array_unique
@fredemmott

This comment has been minimized.

Contributor

fredemmott commented Oct 2, 2017

Looks like recent changes to ArrayUtil::StringUnique(const Array& input) - doesn't crash if SORT_REGULAR flag is passed - and SORT_STRING is the default

@fredemmott

This comment has been minimized.

Contributor

fredemmott commented Oct 2, 2017

Simplified:

<?php

class Test
{
    public function foo()
    {
        $x = ['foo', 'bar'];
        // The actual operation isn't important, just need to do something to turn the literal
        // strings into refcounted strings
        $y = array_map(function($it) { return $it.$it; }, $x);
        return array_unique($y, SORT_STRING);
    }
}

var_dump((new Test())->foo());

@fredemmott fredemmott changed the title from realpath and array_unique problem to array_unique crashes on refcounted strings Oct 2, 2017

@fredemmott

This comment has been minimized.

Contributor

fredemmott commented Oct 4, 2017

I'm not able to spend more time on this for 2-3 weeks; I'll see if I can get someone else to take a look, but unsure, sorry.

@fredemmott

This comment has been minimized.

Contributor

fredemmott commented Oct 5, 2017

https://3v4l.org/UvYaH is particular weird: the zval dumps disagree with var_dump on the contents

@fredemmott

This comment has been minimized.

Contributor

fredemmott commented Oct 5, 2017

https://3v4l.org/kKqJK shows that we have an incorrect-looking refcounting change in 3.22 - but I'm not able to reproduce that locally except on Debian 8

@fredemmott

This comment has been minimized.

Contributor

fredemmott commented Oct 6, 2017

@RyanGordon has some backtraces from RelWithDebInfo and DebugOpt builds: https://gist.github.com/RyanGordon/82993fd73a8355944204029a70982781 https://gist.github.com/RyanGordon/659006ac4725f5c59854d935496d63e4 - the first seems extremely likely to be related, the second might be, but is at least also showing a refcounting issue.

@fredemmott

This comment has been minimized.

Contributor

fredemmott commented Oct 6, 2017

Ryan's able to reproduce it on 7c4c95a , but not the parent commit

@aloiret

This comment has been minimized.

aloiret commented Oct 9, 2017

Hello,

First of all, thank you for brilliant work the your are doing by developing HHVM and providing it to the community.

The commit you mention has also caused a regression on our code opening a file uploaded in $_FILES using Imagick. We have $_FILES[0]['tmp_name'] holding something like '/tmp/Ofs0C9'. Then the following code causes a segmentation fault:

$imagick = new \Imagick($_FILES[0]['tmp_name']);

The following code works fine however:

$imagick = new \Imagick();
$imagick->readImage($_FILES[0]['tmp_name']);

This code works fine as well:

move_uploaded_file($_FILES[0]['tmp_name'], '/tmp/a');
$imagick = new \Imagick('/tmp/a');

The regression in the constructor seems to be caused by 7c4c95a#diff-1399b17bf4e35323c7279094cc4f4f5cL614

while the readImage function still uses:

  imagickReadOp(wand->getWand(), filename, MagickReadImage);

without a call to tvCastToString.

Given the need to have $_FILES populated it is not trivial to provide a minimal test case. But I could try to provide one if you give me advises on how to do it.

@fredemmott

This comment has been minimized.

Contributor

fredemmott commented Oct 9, 2017

The regression in the constructor seems to be caused by 7c4c95a#diff-1399b17bf4e35323c7279094cc4f4f5cL614

What makes you say that? Did you try rebuilding without that change?

@fredemmott

This comment has been minimized.

Contributor

fredemmott commented Oct 9, 2017

Also, what distribution version are you using, and what gcc version does that include?

@fredemmott

This comment has been minimized.

Contributor

fredemmott commented Oct 9, 2017

This code works fine as well:

move_uploaded_file($_FILES[0]['tmp_name'], '/tmp/a');
$imagick = new \Imagick('/tmp/a');

How about replacing the second line with:

$name = '/tmp/a';
$imagick = new \Imagick(substr($name.$name, 0, strlen($name)));

?

fredemmott added a commit to hhvm/packaging that referenced this issue Oct 24, 2017

Start of scripts to use standard debian build processes
- Prep work for stretch support (though that needs webscalesql update for openssl1.1)
- Will also make getting a development environment on jessie easier

refs #155, #128, #125, facebook/hhvm#8011
@fredemmott

This comment has been minimized.

Contributor

fredemmott commented Oct 25, 2017

master on jessie relwithdebinfo gets me this:

(gdb) bt
#0  0x0000000000fcef10 in HPHP::StringData::unProxy() ()
#1  0x0000000000fcef71 in HPHP::StringData::releaseProxy() ()
#2  0x0000000002321d40 in HPHP::PackedArray::Release(HPHP::ArrayData*) ()
#3  0x00000000058003ef in ?? ()
#4  0x00007f92e8a7ff80 in ?? ()
#5  0x0000000000000020 in ?? ()
#6  0x00000000058003d5 in ?? ()
#7  0x0000000009400c79 in ?? ()
#8  0x000000000580004f in ?? ()
#9  0x00000000023609ab in HPHP::VarEnv::enterFP(HPHP::ActRec*, HPHP::ActRec*) ()

Full repro:

  1. follow https://github.com/hhvm/packaging/tree/master/ng#local-usage to get an hhvm-nightly package on debian jessie, in a container
  2. dpkg -i /var/out/hhvm-nightly{,-dbg}_*.deb (ignore errors)
  3. apt-get install -f -y (fixes the dpkg errors)
  4. ulimit -c unlimited
  5. run the test case from #8011 (comment)
  6. gdb $(which hhvm) core

The final output of (1) will point you to a directory containing a build HHVM tree, for debugging changes.

@Megaf

This comment has been minimized.

Megaf commented Nov 12, 2017

Up!
Guys, we need this to be fixed. I upgraded my HHVM and now I get the same error with my wordpress installation. That's a new introduced bug.
Edit:
#8011 (comment)

root@Test:~# hhvm test.php 
array(1) {
  [0]=>
  string(8) "   "
}
Core dumped: Segmentation fault
Stack trace in /tmp/stacktrace.1241.log
Segmentation fault

Edit 2: It works on HHVM Nightly package.

root@Test:~# hhvm test.php 
array(1) {
  [0]=>
  string(8) "/tmp/bar"
}
@fredemmott

This comment has been minimized.

Contributor

fredemmott commented Nov 12, 2017

We have been investigating on this, I'll follow-up. Still reproducible on hhvm-nightly packages for jessie.

@fredemmott

This comment has been minimized.

Contributor

fredemmott commented Nov 23, 2017

Basically a call to ->incRefCount() isn't increasing the refcount in some cases; we have working fixes, but before rolling them in, we want to understand the actual cause - most likely we're hitting a compiler bug, though subtle undefined behavior is also a possibility - and if it's the latter, we'll likely want a more involved fix.

@fredemmott

This comment has been minimized.

Contributor

fredemmott commented Nov 29, 2017

For those of you doing custom builds, you can replace:

auto const str = String::attach(tvCastToString(iter.secondVal()));

with several supposedly-equivalent statements, to 'fix' this:

auto const entry = iter.secondVal();
auto const str = tvAsCVarRef(&entry).toString();

We're not doing this in a release yet as we want to understand the cause, and if there are likely to be other cases first - but if it's causing major issues for you, you might want to test with it anyway.

cc @RyanGordon , @Megaf

@fredemmott

This comment has been minimized.

Contributor

fredemmott commented Dec 19, 2017

We're confident this is a compiler bug, and are figuring out the minimal way to stop GCC from messing it up. Sorry for the slow progress - I've been repeatedly messing up with CMake :(

Anyway:

  • -O3 (default) makes it crash
  • -O0 gives me a non-crashing release build
  • I'm going to try -O1, -O2, then once I've found the level, bisect the individual optimizations to figure out which one's broken, if none stand out.
@jim-saxman

This comment has been minimized.

Contributor

jim-saxman commented Dec 20, 2017

@fredemmott If you bisect individual optimizations, try -fno-optimize-sibling-calls . I was running into an issue on Arm64 which looked very similar to this one and my 1ecac73 fixed it. I recall using the -fno-optimize-sibling-calls during my debugging.

@aloiret

This comment has been minimized.

aloiret commented Dec 21, 2017

I just ran two builds:

  • -O0 + all -O1 flags dumped using g++ -Q --help=optimizers -O1
  • -O1 + all same -O1 flags

The former indeed runs fine while the latter crashes.

Interestingly, dumping the assembly from the produced object files, one can notice many optimizations implied by -O1 which don't seem controlled by a -f flag.

I am digging further in GCC's sources to see what can be triggered by -O1.

@aloiret

This comment has been minimized.

aloiret commented Dec 21, 2017

From https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html

Not all optimizations are controlled directly by a flag. Only optimizations that have a flag are listed in this section.

@aloiret

This comment has been minimized.

aloiret commented Dec 22, 2017

Compiling all with -O1 except hphp/runtime/base/tv-conversions.cpp with -O0, it doesn't crash.

Next step: compile hphp/runtime/base/tv-conversions.cpp with -O1 again and locally use GCC attributes to disable optimizations function-by-function to see which one gets mis-optimized.

@aloiret

This comment has been minimized.

aloiret commented Dec 23, 2017

Adding NO_OPT (which is __attribute__((__optimize__("O0")))) to tvCastToStringData, it doesn't crash.

@aloiret

This comment has been minimized.

aloiret commented Dec 23, 2017

Compiling hphp/runtime/base/tv-conversions.cpp with -O1 + all controlled-by-a-flag-optimizations disabled, there isn't any interesting difference in the GIMPLE middle-end representation for tvCastToStringData with / without NO_OPT:

@@ -4,12 +4,10 @@
   DataType D.$2;
   struct RefData * D.$3;
   struct Cell * D.$4;
-  DataType D.$a;
   int D.$5;
   struct StringData * D.$6;
   struct StringData * iftmp.144;
   long int D.$7;
-  long int D.$b;
   double D.$8;
   struct MaybeCountable * D.$9;
   struct String D.$10;
@@ -36,8 +34,8 @@
   <D.$38>:
   <D.$39>:
   {
-    D.$a = tv.m_type;
-    D.$5 = (int) D.$a;
+    D.$2 = tv.m_type;
+    D.$5 = (int) D.$2;
     switch (D.$5) <default: <D.$15>, case 0: <D.$16>, case 1: <D.$17>, case 17: <D.$18>, case 25: <D.$19>, case 33: <D.$20>, case 35: <D.$21>, case 41: <D.$22>, case 49: <D.$23>, case 53: <D.$24>, case 57: <D.$25>, case 64: <D.$26>, case 80: <D.$27>, case 88: <D.$28>, case 98: <D.$29>, case 104: <D.$30>, case 112: <D.$31>, case 116: <D.$32>, case 120: <D.$33>>
     { 
       <D.$16>:
@@ -56,8 +54,8 @@
       D.$6 = iftmp.144;
       return D.$6;
       <D.$18>:
-      D.$b = tv.m_data.num;
-      D.$6 = HPHP::buildStringData (D.$b);
+      D.$7 = tv.m_data.num;
+      D.$6 = HPHP::buildStringData (D.$7);
       return D.$6;
       <D.$23>:
       D.$8 = tv.m_data.dbl;

Which means the bug may be in the i386 back-end.

This patch workarounds the crash at -O1:

diff --git a/hphp/runtime/base/tv-conversions.cpp b/hphp/runtime/base/tv-conversions.cpp
index 811da9d74b..86c49f5cc4 100644
--- a/hphp/runtime/base/tv-conversions.cpp
+++ b/hphp/runtime/base/tv-conversions.cpp
@@ -389,6 +389,15 @@ void tvCastToStringInPlace(TypedValue* tv) {
   not_reached();
 }
 
+// Workaround a GCC bug leading to an incorrect refcounting issue by
+// stripping this part out of tvCastToStringData.
+NO_OPT NEVER_INLINE StringData* tvCastToStringData_KindOfString(TypedValue tv) {
+  assert(tv.m_type == KindOfString);
+  auto s = tv.m_data.pstr;
+  s->incRefCount();
+  return s;
+}
+
 StringData* tvCastToStringData(TypedValue tv) {
   assert(tvIsPlausible(tv));
   if (tv.m_type == KindOfRef) {
@@ -412,11 +421,8 @@ StringData* tvCastToStringData(TypedValue tv) {
     case KindOfPersistentString:
       return tv.m_data.pstr;
 
-    case KindOfString: {
-      auto s = tv.m_data.pstr;
-      s->incRefCount();
-      return s;
-    }
+    case KindOfString:
+      return tvCastToStringData_KindOfString(tv);
 
     case KindOfPersistentVec:
     case KindOfVec:

Next steps:

  • test the patch at -O3
  • investigate differences in generated assembly for tvCastToStringData_KindOfString with / without NO_OPT
  • test newer GCC versions and eventually bisect it if it doesn't crash
@fredemmott

This comment has been minimized.

Contributor

fredemmott commented Dec 23, 2017

  • we want to avoid a source change if at all possible; this specific change is also likely to have a performance cost that we can't take
  • we've only seen this on GCC 4.9:
    • jessie is 4.9.2
    • our wheezy builds use 4.9.4 and still have this issue

One potential fix would be to build using GCC 5, though there's an ABI change here - some of our third-party build systems have issues with the flags to force GCC5 to generate GCC4 ABI-compatible code.

@aloiret

This comment has been minimized.

aloiret commented Dec 23, 2017

@fredemmott Thanks for your comments.

This patch was indeed made on the purpose of investigating further, it is understandable its overhead is too heavy.

By the way, the NO_OPT on the splitted-out tvCastToStringData_KindOfString is finally not necessary as long as the function doesn't get inlined. Actually, just moving out the call to s->incRefCount() in a never-inlined function workarounds the crash.

--- a/hphp/runtime/base/tv-conversions.cpp
+++ b/hphp/runtime/base/tv-conversions.cpp
@@ -389,6 +389,13 @@ void tvCastToStringInPlace(TypedValue* tv) {
   not_reached();
 }
 
+// Workaround a GCC bug leading to an incorrect refcounting issue by
+// stripping this part out of tvCastToStringData.
+NEVER_INLINE
+static void tvCastToStringData_KindOfString(MaybeCountable *mc) {
+  mc->incRefCount();
+}
+
 StringData* tvCastToStringData(TypedValue tv) {
   assert(tvIsPlausible(tv));
   if (tv.m_type == KindOfRef) {
@@ -414,7 +421,7 @@ StringData* tvCastToStringData(TypedValue tv) {
 
     case KindOfString: {
       auto s = tv.m_data.pstr;
-      s->incRefCount();
+      tvCastToStringData_KindOfString(s);
       return s;
     }
@aloiret

This comment has been minimized.

aloiret commented Dec 24, 2017

No crash with GCC 5.5.0 20171010 (Debian 5.5.0-6).

Built as a Jessie backport of https://packages.qa.debian.org/g/gcc-5.html with a patch for debian rules in the aim of:

  • disabling version check for dpkg-dev, binutils and binutils-multiarch
  • providing library packages such as libstdc++6
  • disabling non-C,C++ languages to speedup build
  • disabling the testsuite to speedup build
--- debian/rules.conf
+++ debian/rules.conf
@@ -1447,12 +1447,12 @@
 # don't encode versioned build dependencies in the control file, but check
 # these here instead.
 check-versions:
-	v=$$(dpkg-query -l dpkg-dev | awk '/^.i/ {print $$3}'); \
+#	v=$$(dpkg-query -l dpkg-dev | awk '/^.i/ {print $$3}'); \
 	if dpkg --compare-versions "$$v" lt "$(DPKGV)"; then \
 	  echo "dpkg-dev (>= $(DPKGV)) required, found $$v"; \
 	  exit 1; \
 	fi
-	v=$$(dpkg-query -l binutils binutils-multiarch | awk '/^.i/ {print $$3;exit}'); \
+#	v=$$(dpkg-query -l binutils binutils-multiarch | awk '/^.i/ {print $$3;exit}'); \
 	if dpkg --compare-versions "$$v" lt "$(BINUTILSBDV)"; then \
 	  echo "binutils (>= $(BINUTILSBDV)) required, found $$v"; \
 	  exit 1; \
--- debian/rules.defs
+++ debian/rules.defs
@@ -410,9 +410,9 @@
 # common things ---------------
 # build common packages, where package names don't differ in different
 # gcc versions (fixincludes, libgcj-common) ...
-#with_common_pkgs := yes
+with_common_pkgs := yes
 # ... and some libraries, which do not change (libgcc1, libssp0).
-#with_common_libs := yes
+with_common_libs := yes
 # XXX: should with_common_libs be "yes" only if this is the default compiler
 # version on the targeted arch?
 
@@ -659,7 +659,7 @@
   with_ada :=
 endif
 with_ada := $(call envfilt, ada, , , $(with_ada))
-
+with_ada := disabled
 
 ifeq ($(DEB_STAGE)-$(filter libgnat, $(with_rtlibs)),rtlibs-)
   with_ada := disabled for rtlibs stage
@@ -785,6 +785,7 @@
   with_java := disabled for cross compiler package
 endif
 with_java := $(call envfilt, java, , c++, $(with_java))
+with_java := disabled
 
 ifeq ($(DEB_STAGE)-$(filter libgcj, $(with_rtlibs)),rtlibs-)
   with_java := disabled for rtlibs stage
@@ -914,6 +915,7 @@
   with_go := disabled for rtlibs stage
 endif
 with_go := $(call envfilt, go, , , $(with_go))
+with_go := disabled
 
 # Build all packages needed for Go development
 ifneq (,$(findstring gcc, $(PKGSOURCE)))
@@ -960,6 +962,7 @@
   with_d := disabled for rtlibs stage
 endif
 with_d := $(call envfilt, d, , , $(with_d))
+with_d := disabled
 
 ifeq ($(with_base_only),yes)
   with_d := no
@@ -1015,6 +1018,7 @@
 endif
 
 with_fortran := $(call envfilt, fortran, , , $(with_fortran))
+with_fortran := disabled
 
 # Build all packages needed for Fortran development
 ifeq ($(with_fortran),yes)
@@ -1062,6 +1066,7 @@
   with_objc := disabled for rtlibs stage
 endif
 with_objc := $(call envfilt, objc, obj-c++, , $(with_objc))
+with_objc := disabled
 
 ifeq ($(with_objc),yes)
   # the ObjC runtime with garbage collection enabled needs the Boehm GC
@@ -1104,6 +1109,7 @@
   with_objcxx := disabled for cross compiler package
 endif
 with_objcxx := $(call envfilt, obj-c++, , c++ objc, $(with_objcxx))
+with_objcxx := disabled
 
 ifeq ($(with_objcxx),yes)
   enabled_languages += obj-c++
@@ -1455,7 +1461,7 @@
 # run testsuite ---------------
 with_check := yes
 # if you don't want to run the gcc testsuite, uncomment the next line
-#with_check := disabled by hand
+with_check := disabled by hand
 ifeq ($(with_base_only),yes)
   with_check := no
 endif

Configured with:

../src/configure -v --with-pkgversion='Debian 5.5.0-6' --with-bugurl=file:///usr/share/doc/gcc-5/README.Bugs --enable-languages=c,c++ --prefix=/usr --program-suffix=-5 --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --libdir=/usr/lib --enable-nls --with-sysroot=/ --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --with-default-libstdcxx-abi=gcc4-compatible --disable-libstdcxx-dual-abi --enable-gnu-unique-object --disable-vtable-verify --disable-libquadmath --enable-libmpx --enable-plugin --with-system-zlib --enable-multiarch --with-arch-32=i586 --with-abi=m64 --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu

which is the gcc4-compatible ABI only.

I can leave the .deb binary packages available somewhere if anyone is interested.

@fredemmott

This comment has been minimized.

Contributor

fredemmott commented Dec 24, 2017

Thanks; if you're able to send a pull request against https://github.com/hhvm/packaging/blob/master/build-deps/build-gcc (can test with the wheezy build), that'd be awesome - and I'll then easily be able to use it for Jessie too.

aloiret pushed a commit to aloiret/packaging that referenced this issue Dec 24, 2017

Arthur Loiret
switch to GCC 5.5.0 with gcc4-compatible ABI.
GCC 4.9 seems to be having a bug leading to a regression into HHVM. See:

facebook/hhvm#8011

Signed-off-by: Arthur Loiret <arthur.loiret@adeline.mobi>
@aloiret

This comment has been minimized.

aloiret commented Dec 24, 2017

@fredemmott hhvm/packaging#160

The GCC build went fine using build-deps/build-gcc:

$ /opt/hhvm-build-deps/bin/gcc -v
Using built-in specs.
COLLECT_GCC=/opt/hhvm-build-deps/bin/gcc
COLLECT_LTO_WRAPPER=/opt/hhvm-build-deps/libexec/gcc/x86_64-unknown-linux-gnu/5.5.0/lto-wrapper
Target: x86_64-unknown-linux-gnu
Configured with: /tmp/objdir/../gcc-5.5.0/configure --prefix=/opt/hhvm-build-deps --enable-languages=c,c++ --disable-multilib --with-default-libstdcxx-abi=gcc4-compatible --disable-libstdcxx-dual-abi
Thread model: posix
gcc version 5.5.0 (GCC) 

Waiting for your feedback on HHVM building / testing. :)

@fredemmott

This comment has been minimized.

Contributor

fredemmott commented Dec 25, 2017

Thanks, I’ll take a look Wednesday or Thursday

fredemmott added a commit to hhvm/packaging that referenced this issue Dec 27, 2017

switch to GCC 5.5.0 with gcc4-compatible ABI.
GCC 4.9 seems to be having a bug leading to a regression into HHVM. See:

facebook/hhvm#8011

Signed-off-by: Arthur Loiret <arthur.loiret@adeline.mobi>

fredemmott added a commit to hhvm/packaging that referenced this issue Dec 27, 2017

fredemmott added a commit to hhvm/packaging that referenced this issue Dec 27, 2017

fredemmott added a commit to hhvm/packaging that referenced this issue Dec 27, 2017

Add wget to jessie build-deps
needed for gcc5 build script

refs #160
refs facebook/hhvm#8011

fredemmott added a commit to hhvm/packaging that referenced this issue Dec 27, 2017

fredemmott added a commit to hhvm/packaging that referenced this issue Dec 27, 2017

@fredemmott

This comment has been minimized.

Contributor

fredemmott commented Dec 27, 2017

Looks good in my local testing. Will close tomorrow assuming the nightlies are fine.

Fix is to require GCC 5, and build it for distributions that still have earlier versions of GCC. Alternatively, you can build with GCC 4.9 and -O0, however we're likely to stop testing this very soon, and performance is likely to be an issue.

@aloiret

This comment has been minimized.

aloiret commented Dec 27, 2017

@fredemmott great news, thanks.

Do you want a pull request that adds the testcase and generates a cmake warning if the compiler is older than GCC 5?

@fredemmott

This comment has been minimized.

Contributor

fredemmott commented Dec 27, 2017

That'd be great, thanks :)

fredemmott added a commit to hhvm/user-documentation that referenced this issue Dec 27, 2017

Rework 'building from source'
 - don't duplicate info
 - specify gcc 5 requirement

refs facebook/hhvm#8011

aloiret pushed a commit to aloiret/hhvm that referenced this issue Dec 28, 2017

Arthur Loiret
Warn about a GCC 4.9 bug
Summary: HHVM triggers an optimization bug in GCC 4.9 which leads to an
incorrect refcounting issue when compiled with -O1 or greater.

Add a testcase from @fredemmott for the bug and add a warning in CMake
about the compiler version.

See: facebook#8011

Signed-off-by: Arthur Loiret <arthur.loiret@adeline.mobi>
@aloiret

This comment has been minimized.

aloiret commented Dec 28, 2017

@fredemmott

This comment has been minimized.

Contributor

fredemmott commented Dec 28, 2017

Nightly builds are fine.

@fredemmott fredemmott closed this Dec 28, 2017

@fredemmott

This comment has been minimized.

Contributor

fredemmott commented Jan 3, 2018

This should be fixed in the new releases: https://hhvm.com/blog/2018/01/02/hhvm-3.23.3.html

hhvm-bot added a commit that referenced this issue Jan 3, 2018

Warn about a GCC 4.9 bug
Summary:
HHVM triggers an optimization bug in GCC 4.9 which leads to an
incorrect refcounting issue when compiled with -O1 or greater.

Add a testcase from fredemmott for the bug and add a warning in CMake
about the compiler version.

See: #8011

Signed-off-by: Arthur Loiret <arthur.loiret@adeline.mobi>
Closes #8086

Differential Revision: D6642219

Pulled By: fredemmott

fbshipit-source-id: 34e7ed588aa308c9cc10b445c4056250506bd9b0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment