[v8.x] node_crypto: Use <cmath> definitions of isnan and isinf by jer-gentoo · Pull Request #19196 · nodejs/node

jer-gentoo · 2018-03-07T10:04:56Z

Unless you specify C++11, std::isnan and std::isinf are not guaranteed to be available. Instead, explicitly use the cmath functions.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

Unless you specify C++11, std::isnan and std::isinf are not guaranteed to be available. Instead, just use the math.h functions for now.

addaleax · 2018-03-07T10:07:11Z

Unless you specify C++11

I’m not sure I understand … building Node.js does require C++11 or above, so that’s something we shouldn’t have to worry about, right?

jer-gentoo · 2018-03-07T10:14:05Z

With GCC 5.5.0 I see

../src/node_crypto.cc: In function ‘void node::crypto::PBKDF2(const v8::FunctionCallbackInfo<v8::Value>&)’:
../src/node_crypto.cc:5626:27: error: ‘__builtin_isnan’ is not a member of ‘std’
if (raw_keylen < 0.0 || std::isnan(raw_keylen) || std::isinf(raw_keylen) ||
^
../src/node_crypto.cc:5626:27: note: suggested alternative:
<built-in>: note: ‘__builtin_isnan’
../src/node_crypto.cc:5626:53: error: ‘__builtin_isinf_sign’ is not a member of ‘std’
if (raw_keylen < 0.0 || std::isnan(raw_keylen) || std::isinf(raw_keylen) ||
^
../src/node_crypto.cc:5626:53: note: suggested alternative:
<built-in>: note: ‘__builtin_isinf_sign’

I checked whether nodejs requires C++11 in its configuration, and I must say I have a hard time finding where in the build system this is enforced for GCC. That's why I chose the other solution to begin with.

richardlau · 2018-03-07T10:32:26Z

I checked whether nodejs requires C++11 in its configuration, and I must say I have a hard time finding where in the build system this is enforced for GCC.

On master -std=gnu++1y is specified:

node/common.gypi

Line 296 in cb5f9a6

'cflags_cc': [ '-fno-rtti', '-fno-exceptions', '-std=gnu++1y' ],

but this change (#17489) was deemed semver-major and not backported, so e.g. v9.x still uses -std=gnu++0x:

node/common.gypi

Line 296 in 4e9ac10

'cflags_cc': [ '-fno-rtti', '-fno-exceptions', '-std=gnu++0x' ],

jer-gentoo · 2018-03-07T10:39:32Z

I'll leave it up to you whether to choose the compiler flag change or this one.

richardlau · 2018-03-07T12:12:51Z

The lines in question for src/node_crypto.cc come from #18327 (comment) in response to #16701 (comment).

cc @gibfahn @yhwang

addaleax · 2018-03-07T12:15:48Z

So, I assume that means this PR could be re-targeted against v9.x and we could land it on v8.x + v6.x, and be done with it?

That would sound perfectly fine to me.

richardlau · 2018-03-07T12:19:03Z

I would certainly favour not changing the compiler flag for LTS.

addaleax · 2018-03-07T21:43:22Z

Ah, sorry, I checked and this doesn't actually appear in the v9.x source code. So this is fine & should totally land on v8.x!

@jer-gentoo If you want, could you update the commit message to start with src: use [...] (lower-case)? If not, that can also be done while landing the PR.

yhwang · 2018-03-07T23:26:38Z

I backported this to v8 in this PR: #18327
and the root cause is the algorithm header file. It includes cmath and re-declare these two function under std namespace.

gibfahn · 2018-03-20T20:32:13Z

I backported this to v8 in this PR: #18327

Given that the fix has already been backported to v8.x-staging, I think this can be closed. Thanks anyway @jer-gentoo !

richardlau · 2018-03-21T08:20:22Z

I backported this to v8 in this PR: #18327
Given that the fix has already been backported to v8.x-staging, I think this can be closed. Thanks anyway @jer-gentoo !

My understanding of the history was that #18327 caused the issue in the first place since we don't require C++11 on 8.0.

jer-gentoo · 2018-03-21T14:47:58Z

@gibfahn : You misinterpreted @yhwang's comment, which didn't intend to mean the issue was already fixed. As @richardlau explained quite well, #18327 unintentionally caused the issue. This PR fixes the issue.

Please reopen.

yhwang · 2018-03-21T21:12:45Z

@jer-gentoo one question: since my PR is landed to v8.x-staging already. In your PR, you also have the same change. I guess the change here in your PR won't go to v8.x-staging. What you plan to do next?

[Edit] Sorry that I just realize that your change revert #18327. However, the root cause is actually the using of algorithm, because it indirectly includes cmath. I think reverting #18327 can't fix the issue.

yhwang · 2018-03-22T18:52:56Z

more information about the std:isnan. If cmath is included, then you have to use std::isnan. Because it re-export functions under math.h to std namespace.

Check this code in deps/v8:
https://github.com/nodejs/node/blob/v8.x-staging/deps/v8/src/base/platform/platform-posix.cc#L381
The reason of using std::isnan in the code above is the cmath header. Unfortunately, I found the code above also failed to compile in GCC 5.5.0

../deps/v8/src/base/platform/platform-posix.cc: In member function 'virtual double v8::base::PosixTimezoneCache::DaylightSavingsOffset(double)':
../deps/v8/src/base/platform/platform-posix.cc:381:7: error: '__builtin_isnan' is not a member of 'std'
if (std::isnan(time)) return std::numeric_limits<double>::quiet_NaN();
^
../deps/v8/src/base/platform/platform-posix.cc:381:7: note: suggested alternative:
<built-in>: note: '__builtin_isnan'
LD_LIBRARY_PATH=/home/vagrant/WS_git/node/out/Release/lib.host:/home/vagrant/WS_git/node/out/Release/lib.target:$LD_LIBRARY_PATH; export LD_LIBRARY_PATH; cd ../deps/v8/src; mkdir -p /home/vagrant/WS_git/node/o
ut/Release/obj/gen; python ../tools/js2c.py "/home/vagrant/WS_git/node/out/Release/obj/gen/extras-libraries.cc" EXTRAS
deps/v8/src/v8_libbase.target.mk:118: recipe for target '/home/vagrant/WS_git/node/out/Release/obj.target/v8_libbase/deps/v8/src/base/platform/platform-posix.o' failed
make[1]: *** [/home/vagrant/WS_git/node/out/Release/obj.target/v8_libbase/deps/v8/src/base/platform/platform-posix.o] Error 1
make[1]: *** Waiting for unfinished jobs....
../deps/v8/src/base/platform/platform-posix-time.cc: In member function 'virtual const char* v8::base::PosixDefaultTimezoneCache::LocalTimezone(double)':
../deps/v8/src/base/platform/platform-posix-time.cc:13:7: error: '__builtin_isnan' is not a member of 'std'
if (std::isnan(time)) return "";
^
../deps/v8/src/base/platform/platform-posix-time.cc:13:7: note: suggested alternative:
<built-in>: note: '__builtin_isnan'
deps/v8/src/v8_libbase.target.mk:118: recipe for target '/home/vagrant/WS_git/node/out/Release/obj.target/v8_libbase/deps/v8/src/base/platform/platform-posix-time.o' failed
make[1]: *** [/home/vagrant/WS_git/node/out/Release/obj.target/v8_libbase/deps/v8/src/base/platform/platform-posix-time.o] Error 1
../deps/v8/src/libplatform/tracing/trace-writer.cc: In member function 'void v8::platform::tracing::JSONTraceWriter::AppendArgValue(uint8_t, v8::platform::tracing::TraceObject::ArgValue)':
../deps/v8/src/libplatform/tracing/trace-writer.cc:73:11: error: '__builtin_isfinite' is not a member of 'std'

Currently, for v8.x-staging, it doesn't support GCC 5.5. After switching to GCC 7.2, I can compile the code without any error. Also GCC 5.4 is good as well. If we don't want to backport the cflags, then maybe we need to document that we don't support GCC 5.5 in v8.x.

Martii · 2018-03-25T20:12:09Z

A lot of this is greek to me since most of my C/C++ is older Windows based... however we have been encountering this with node@8.10.0 using the current build instructions here :

$ g++ --versiong++ (Linaro GCC 5.4-2017.05) 5.4.1 20170404 (ROSA)

and

$ g++ --versiong++ (Debian 4.9.2-10+deb8u1) 4.9.2

In summary, which is sort of what I'm looking for... Will the next node update release resolve this? (seems that way from what I'm reading but since I didn't even know that there were different C++ versions in GNU I'm still the noob here :)

Thanks for your patience.

P.S. node@8.9.4 compiles okay... but not node@8.10.0

gibfahn · 2018-03-25T22:00:40Z

In summary, which is sort of what I'm looking for... Will the next node update release resolve this?

It's still unclear. Initially it seemed that this patch would fix it, but AIUI @yhwang is saying that V8 itself has this issue, so we won't be able to fix it (the answer would be to use a different version of gcc).

However @yhwang said that gcc 5.4 was okay, but you're having an issue with 5.4.1 and 4.9.2? Could you post the build failure output?

Martii · 2018-03-26T17:19:43Z

@gibfahn

but you're having an issue with 5.4.1 and 4.9.2?

Yes. 5.4.1 is my dev station here. 4.9.2 is our production environment. I'll rerun it on production but that will take a few hours as our VPS isn't nearly as quick as my dev station especially since we're online. :)

Could you post the build failure output?

It's exactly the same as @jer-gentoo comment above at #19196 (comment) with trailing lines of this:

make[1]: *** [node.target.mk:202: ~/repo/git/node.js/nodejs/node2/out/Release/obj.target/node/src/node_crypto.o] Error 1rm 29706770d08d6f8d67360fd4f1377c53f58eb206.intermediatemake: *** [Makefile:88: node] Error 2

... this is how I found this issue was searching on node_crypto Error 1 and a little bit of "light reading" ;)

jer-gentoo · 2018-03-26T18:18:05Z

I honestly no longer see how this went so wrong.

This PR fixes an actual issue in the LTS branch when using toolchains that do not implement C++11. An important security release is due this week, so it's probably a good thing to get this in now so as to not proliferate the build problem.

Some people appear to get very confused over later comments to this PR, causing uncertainty to others which then led more people to believe the issue had already been fixed elsewhere, which may or may not be the case for the master branch but certainly is not for this PR, as it targets the v8.x branch.

I was requested early on to (somehow, you know, git-fu) change the commit's metadata and while I haven't respectfully declined to do so, I want to do so now just the clear the air in that regard.

Martii · 2018-03-26T18:28:21Z

@jer-gentoo

Some people appear to get very confused over later comments to this PR

You hit that right on for me... this is why I am here... and I appreciate your clear comments.

I've sent a message/ticket out to our VPS to see if the system can be updated from "Jesse" to "Stretch" but it may (if it's possible) cause more issues than retrying nvm and hoping for the best (we had nvm installed last month but I still prefer to compile in case there is a cross-distro issue). My distro can be twiddled with but our production has to be handled with extreme care since we're live.

Adding "Jessie"'s unstable repo doesn't seem like a good thing to see if g++ is newer.

Martii · 2018-03-26T22:37:47Z

@gibfahn

but you're having an issue with ... 4.9.2?

Not anymore with 4.9.2 although I do get this message in $ ./configure --prefix=/usr of:

WARNING: C++ compiler too old, need g++ 4.9.4 or clang++ 3.4.2 (CXX=g++)

... but it does compile and is running so far. Still going to look into migration to Debian 9 ("Stretch") if possible even though it appears to be relatively "new" in the chronological timeline.

Being more inexperienced with the *nix side of C++ it's going to be a major set back until I can figure out what repercussions straying from my distro's g++ is going to be... e.g. dependency h*ll is my first thought, many possible broken packages is another, and snap-shotting dev station during the process... perhaps I'll just do a VM of another distro *shrugs*. This path forward seems rather hacky though and it will slow things down between dev testing and production... it would be nice to have this resolved in node here with this PR.

gibfahn · 2018-03-27T11:09:34Z

This PR fixes an actual issue in the LTS branch when using toolchains that do not implement C++11.
Some people appear to get very confused over later comments to this PR, causing uncertainty to others which then led more people to believe the issue had already been fixed elsewhere,

@jer-gentoo can you give me a way to reproduce this (preferably via vagrant). I've tried:

vagrant init ubuntu/xenial64
# I had to increase the RAM and CPUs of the machine by adding this to the Vagrantfile:# config.vm.provider "virtualbox" do |vb|# vb.memory = "4096"# vb.cpus = "2"# end
vagrant up
vagrant ssh
# Inside the vagrant VM:
sudo apt-get update
sudo apt-get install -y gcc-5 g++-5 build-essential python
git clone https://github.com/nodejs/node --branch v8.x
cd node
./configure && make -j4

This is using gcc-5.4, and it passes:

vagrant@ubuntu-xenial:/vagrant/node$ gcc -vUsing built-in specs.COLLECT_GCC=gccCOLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/5/lto-wrapperTarget: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu 5.4.0-6ubuntu1~16.04.9' --with-bugurl=file:///usr/share/doc/gcc-5/README.Bugs --enable-languages=c,ada,c++,java,go,d,fortran,objc,obj-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=new --enable-gnu-unique-object --disable-vtable-verify --enable-libmpx --enable-plugin --with-system-zlib --disable-browser-plugin --enable-java-awt=gtk --enable-gtk-cairo --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-5-amd64/jre --enable-java-home --with-jvm-root-dir=/usr/lib/jvm/java-1.5.0-gcj-5-amd64 --with-jvm-jar-dir=/usr/lib/jvm-exports/java-1.5.0-gcj-5-amd64 --with-arch-directory=amd64 --with-ecj-jar=/usr/share/java/eclipse-ecj.jar --enable-objc-gc --enable-multiarch --disable-werror --with-arch-32=i686 --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
Thread model: posixgcc version 5.4.0 20160609 (Ubuntu 5.4.0-6ubuntu1~16.04.9) vagrant@ubuntu-xenial:/vagrant/node$ g++ -vUsing built-in specs.COLLECT_GCC=g++COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/5/lto-wrapperTarget: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu 5.4.0-6ubuntu1~16.04.9' --with-bugurl=file:///usr/share/doc/gcc-5/README.Bugs --enable-languages=c,ada,c++,java,go,d,fortran,objc,obj-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=new --enable-gnu-unique-object --disable-vtable-verify --enable-libmpx --enable-plugin --with-system-zlib --disable-browser-plugin --enable-java-awt=gtk --enable-gtk-cairo --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-5-amd64/jre --enable-java-home --with-jvm-root-dir=/usr/lib/jvm/java-1.5.0-gcj-5-amd64 --with-jvm-jar-dir=/usr/lib/jvm-exports/java-1.5.0-gcj-5-amd64 --with-arch-directory=amd64 --with-ecj-jar=/usr/share/java/eclipse-ecj.jar --enable-objc-gc --enable-multiarch --disable-werror --with-arch-32=i686 --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
Thread model: posixgcc version 5.4.0 20160609 (Ubuntu 5.4.0-6ubuntu1~16.04.9)

Given that @yhwang is saying that this patch doesn't actually fix the compile issues with gcc-5.5, I'd like to be able to reproduce the issue on v8.x and confirm that this patch fixes things.

modify patch. nodejs/node#19196 made not to use libressl headers fix to include path not to use "host/include" Signed-off-by: Hirokazu MORIKAWA <morikw2@gmail.com>

MylesBorins · 2018-05-22T15:52:38Z

One more CI https://ci.nodejs.org/job/node-test-pull-request/15023/

MylesBorins · 2018-06-14T20:21:05Z

Looks like this is all red. Can someone who knows about the PR take a look

addaleax · 2018-06-14T20:35:26Z

New CI to see what the failures actually are: https://ci.nodejs.org/job/node-test-pull-request/15468/

Martii · 2018-06-14T22:56:06Z

Is it failing at line 1023 of https://ci.nodejs.org/job/node-test-commit-aix/nodes=aix61-ppc64/15786/ ?? e.g. it's really difficult to tell where since I'm not used to this prettified HTML output but it looks like it's trying to test that before 1024.

As far as Linux goes here in the CI linkage of https://ci.nodejs.org/job/node-test-commit-linux/nodes=debian8-64/18953/ this returns a 404 for me. e.g. my guess is it's mis-linked, or somehow other broken.

Plus some jobs aren't completed yet over there so console output is a bit vague.

Martii · 2018-06-14T22:58:52Z

Here we go... seems that it timedout See https://ci.nodejs.org/job/node-test-commit-aix/nodes=aix61-ppc64/15786/testReport/junit/%28root%29/test/parallel_test_benchmark_path/ . Probably just needs to be rerun.

yhwang · 2018-06-18T19:52:19Z

new CI: https://ci.nodejs.org/job/node-test-commit/19314/

bnoordhuis · 2018-06-22T19:50:45Z

Back-porting 7124b46 to v8.x would fix this because it simply gets rid of the isnan and isinf calls.

MylesBorins · 2018-07-09T20:10:13Z

@bnoordhuis that commit is semver major is it not?

MylesBorins · 2018-07-31T18:14:45Z

ping @bnoordhuis

yhwang · 2018-08-02T15:25:19Z

trying to kick off another CI to verify the change: https://ci.nodejs.org/job/node-test-commit/20154/

yhwang · 2018-08-02T17:43:58Z

seems CI failed on two test cases:

parallel/test-timers-throw-reschedule
parallel/test-error-reporting # TODO : Fix flaky test

for the first one, we have an issue to track it now: #21188

overall, I think the CI is good for this PR. Because this just changes one line of code to include the cmath. It's easier then back-port another PR(unless you plan to back-port the PR). I vote for merging the current change directly.

Martii · 2018-08-02T23:26:06Z

If it makes any difference node@10.x isn't immune from the math.h vs cmath inclusion either. The same solution for a different error was patched at 2db74f2

I've also been distro upgraded on dev station, since this issue began, to:

$ g++ --versiong++ (Linaro GCC 5.5-2017.10) 5.5.0 20171010 (ROSA)

Unless you specify C++11, std::isnan and std::isinf are not guaranteed to be available. Instead, just use the math.h functions for now. PR-URL: #19196 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Yihong Wang <yh.wang@ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com>

MylesBorins · 2018-08-07T20:48:43Z

landed in d8cf13a

Unless you specify C++11, std::isnan and std::isinf are not guaranteed to be available. Instead, just use the math.h functions for now. PR-URL: #19196 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Yihong Wang <yh.wang@ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com>

modify patch. nodejs/node#19196 made not to use libressl headers fix to include path not to use "host/include" Signed-off-by: Hirokazu MORIKAWA <morikw2@gmail.com> (cherry picked from commit 818770d)

modify patch. nodejs/node#19196 made not to use libressl headers fix to include path not to use "host/include" Signed-off-by: Hirokazu MORIKAWA <morikw2@gmail.com>

node_crypto: Use math.h definitions of isnan and isinf
65172e1
Unless you specify C++11, std::isnan and std::isinf are not guaranteed to be available. Instead, just use the math.h functions for now.

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem. v8.x labels Mar 7, 2018

addaleax approved these changes Mar 7, 2018
View reviewed changes

nxhack mentioned this pull request Mar 14, 2018
failed to compile nxhack/openwrt-node-packages#252
Closed

gibfahn closed this Mar 20, 2018

richardlau reopened this Mar 21, 2018

nxhack mentioned this pull request Mar 26, 2018
node: bump to 8.10.0 openwrt/packages#5734
Merged

nxhack mentioned this pull request May 7, 2018
node: fix host build fail openwrt/packages#6015
Merged

jer-gentoo changed the title ~~[v8.x] node_crypto: Use math.h definitions of isnan and isinf~~[v8.x] node_crypto: Use <cmath> definitions of isnan and isinfMay 14, 2018

nxhack added a commit to nxhack/packages that referenced this pull request May 16, 2018
node: fix host build fail
7214260
modify patch. nodejs/node#19196 made not to use libressl headers fix to include path not to use "host/include" Signed-off-by: Hirokazu MORIKAWA <morikw2@gmail.com>

yousong pushed a commit to openwrt/packages that referenced this pull request May 16, 2018
node: fix host build fail
818770d
modify patch. nodejs/node#19196 made not to use libressl headers fix to include path not to use "host/include" Signed-off-by: Hirokazu MORIKAWA <morikw2@gmail.com>

jasnell approved these changes Jun 21, 2018
View reviewed changes

MylesBorins closed this Aug 7, 2018

MylesBorins mentioned this pull request Aug 16, 2018
v8.12.0 proposal #21593
Merged

jer-gentoo deleted the patch-1 branch September 11, 2018 11:56

Martii mentioned this pull request Dec 17, 2018
Site sluggishness OpenUserJS/OpenUserJS.org#1548
Open

lynxis pushed a commit to lynxis/packages that referenced this pull request Jan 3, 2019
node: fix host build fail
7ba1d75
modify patch. nodejs/node#19196 made not to use libressl headers fix to include path not to use "host/include" Signed-off-by: Hirokazu MORIKAWA <morikw2@gmail.com>

Uh oh!

Uh oh!

Conversation

jer-gentoo commented Mar 7, 2018• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

addaleax commented Mar 7, 2018

Uh oh!

jer-gentoo commented Mar 7, 2018

Uh oh!

richardlau commented Mar 7, 2018

Uh oh!

jer-gentoo commented Mar 7, 2018

Uh oh!

richardlau commented Mar 7, 2018

Uh oh!

addaleax commented Mar 7, 2018

Uh oh!

richardlau commented Mar 7, 2018

Uh oh!

addaleax commented Mar 7, 2018

Uh oh!

yhwang commented Mar 7, 2018

Uh oh!

gibfahn commented Mar 20, 2018

Uh oh!

richardlau commented Mar 21, 2018• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jer-gentoo commented Mar 21, 2018

Uh oh!

yhwang commented Mar 21, 2018• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yhwang commented Mar 22, 2018• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Martii commented Mar 25, 2018

Uh oh!

gibfahn commented Mar 25, 2018

Uh oh!

Martii commented Mar 26, 2018

Uh oh!

jer-gentoo commented Mar 26, 2018

Uh oh!

Martii commented Mar 26, 2018

Uh oh!

Martii commented Mar 26, 2018

Uh oh!

gibfahn commented Mar 27, 2018

Uh oh!

MylesBorins commented May 22, 2018

Uh oh!

MylesBorins commented Jun 14, 2018

Uh oh!

addaleax commented Jun 14, 2018

Uh oh!

Martii commented Jun 14, 2018

Uh oh!

Martii commented Jun 14, 2018

Uh oh!

yhwang commented Jun 18, 2018

Uh oh!

bnoordhuis commented Jun 22, 2018

Uh oh!

MylesBorins commented Jul 9, 2018

Uh oh!

MylesBorins commented Jul 31, 2018

Uh oh!

yhwang commented Aug 2, 2018

Uh oh!

yhwang commented Aug 2, 2018

Uh oh!

Martii commented Aug 2, 2018

Uh oh!

MylesBorins commented Aug 7, 2018

Uh oh!

Reviewers

Assignees

jer-gentoo commented Mar 7, 2018•
edited
Loading

richardlau commented Mar 21, 2018•
edited
Loading

yhwang commented Mar 21, 2018•
edited
Loading

yhwang commented Mar 22, 2018•
edited
Loading