Skip to content
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

V8 test failure on PPC - recent backport? #25540

Closed
mhdawson opened this issue Jan 16, 2019 · 12 comments
Closed

V8 test failure on PPC - recent backport? #25540

mhdawson opened this issue Jan 16, 2019 · 12 comments
Assignees

Comments

@mhdawson
Copy link
Member

mhdawson commented Jan 16, 2019

  • Version: master (and possibly 11)
  • Platform: PPC
  • Subsystem: wasm

Looks like a test is failing in the v8 tests on master:

https://ci.nodejs.org/view/All/job/node-test-commit-v8-linux/2014/nodes=ppcle-ubuntu1404,v8test=v8test/testReport/

stdout:
/home/iojs/build/workspace/node-test-commit-v8-linux/nodes/ppcle-ubuntu1404/v8test/v8test/deps/v8/test/mjsunit/wasm/jsapi-harness.js:102: Error loading file
load("test/wasm-js/test/harness/wasm-constants.js");
^
Command: /home/iojs/build/workspace/node-test-commit-v8-linux/nodes/ppcle-ubuntu1404/v8test/v8test/deps/v8/out.gn/ppc64.release/d8 --test /home/iojs/build/workspace/node-test-commit-v8-linux/nodes/ppcle-ubuntu1404/v8test/v8test/deps/v8/test/mjsunit/mjsunit.js /home/iojs/build/workspace/node-test-commit-v8-linux/nodes/ppcle-ubuntu1404/v8test/v8test/deps/v8/test/mjsunit/wasm/jsapi-harness.js --random-seed=-1386363578 --nohard-abort --expose-wasm --allow-natives-syntax

It might be a recent V8 backport?

@mhdawson
Copy link
Member Author

@sam-github could you help investigate to see if this is a failure do to a recent backport or something else.

@bcoe
Copy link
Contributor

bcoe commented Jan 16, 2019

@hashseed I've debugged this, the problem is that d8 runs from the root of the Node.js directory in our test suite, and the includes expect that you're running from the root directory v8:

Benjamins-MacBook-Pro:node benjamincoe$ ./deps/v8/out.gn/x64.release/d8 --test deps/v8/test/mjsunit/mjsunit.js deps/v8/test/mjsunit/wasm/jsapi-harness.js
deps/v8/test/mjsunit/wasm/jsapi-harness.js:102: Error loading file
load("test/wasm-js/test/harness/wasm-constants.js");

The problem is corrected if you use:

load("deps/v8/test/wasm-js/test/harness/wasm-constants.js");

which seems ugly.

@bcoe
Copy link
Contributor

bcoe commented Jan 16, 2019

please note that I recreated this locally on OSX, by simply running:

./deps/v8/out.gn/x64.release/d8 --test deps/v8/test/mjsunit/mjsunit.js deps/v8/test/mjsunit/wasm/jsapi-harness.js

note: currently I'm trying a build with this patch 3fc3f44

☝️ this experiment did not work.

@sam-github
Copy link
Contributor

@bcoe the problem you describe in #25540 (comment) sounds like it would effect all platforms, yet this is just failing on ubuntu-ppcle, do you have any insight into that?

What happened with your experiment?

@bcoe
Copy link
Contributor

bcoe commented Jan 17, 2019

@sam-github I could reproduce a similar error on my OSX machine, and it was related to the working directory. I don't understand why the RHEL and Intel Ubuntu builds are passing.

@sam-github
Copy link
Contributor

@bcoe thanks for the info. I've no special skills here, but I'll take a crack at figuring out what is going on.

@sam-github
Copy link
Contributor

@bcoe I'm doing a ppcle build on a ci machine now. My intention was to repro the failure, then bisect to find what introduced it, but since the build is quite slow (much less the testing, no idea how long that will take), I went through the history.

Your #25429 is the only thing that touched v8 on the day the builds started to fail, which I guess explains why you are looking at the failure, too. ;-)

v8 cherry-picks usually reference a bug. As a non-v8 engineer, I don't really have any idea what that PR was supposed to have done. Was there something that did not work, then after #25429, did work?

@sam-github
Copy link
Contributor

I ran test-v8 from master on test-osuosl-ubuntu1404-ppc64--le-1, it passed. Hm.

I'm doing a ci run: https://ci.nodejs.org/view/All/job/node-test-commit-v8-linux/2023/

For future reference, I hacked up the the execute scripts to run locally. This did the trick, when run from the root of a node checkout. It depends on the machine setup, it uses specific versions of gcc, binutils, and corresponding ccache setup.

#/bin/bash -xe

# SAM - add these to env
# For ccache

export PATH=/usr/lib/ccache:$PATH

# For select-compiler.sh
export NODE_NAME=ppc64_le

# SAM clone this, its needed below
git clone https://chromium.googlesource.com/chromium/tools/depot_tools.git

# SAM skip this
#  # required because otherwise we get clang errors ?
#  rm -rf deps/v8/tools/clang || true


export WORKSPACE=$(pwd)
export PATH=$WORKSPACE/depot_tools:$PATH
export CCACHE_BASEDIR=$(pwd)

# needed to work with latest depot tools
export VPYTHON_BYPASS="manually managed python not supported by chrome operations"

RUN_TESTS="RUN"
SKIP_MESSAGE="ok # skipped s390 not supported for versions less than 6, skipping"

if [ "X$DESTCPU" = "X" ]; then
  cd tools
  echo "from utils import *" >getArch.py
  echo "print GuessArchitecture()" >> getArch.py
  # assume 64 bit unless set specifically
  DESTCPU=`python getArch.py | sed 's/ia32/x64/' | sed 's/ppc/ppc64/' | sed 's/arm/arm64/' | sed 's/s390/s390x/'`
  cd ..
fi
echo [DESTCPU=$DESTCPU]

if [ "X$DESTCPU" = "XNone" ]; then
  # we only support running v8 tests on recognized platforms.  For example
  # if you run this for 4.X on s390 the platform will not be recognized
  # as 4.X does not support s390.
  RUN_TESTS="DONT_RUN"
fi

if [ "X$DESTCPU" = "Xppc64" ]; then
  # select the appropriate compiler
  rm -rf build
  git clone https://github.com/nodejs/build.git
  . ./build/jenkins/scripts/select-compiler.sh
   
  if [ "$NODEJS_MAJOR_VERSION" -gt "9" ]; then
    # temporarily patch make-v8.sh until we upstream the required changes
     cp /home/iojs/build-tools/make-v8.sh tools/make-v8.sh -f
  fi
  
  echo "import platform" > getMachine.py
  echo "print platform.machine()" >> getMachine.py
  ENDIAN=`python getMachine.py | sed 's/ppc64//g'`
  if [ "X$ENDIAN" != "Xle" ]; then
    # ICU tests are diabled for BE platforms because the ICU included by V8 is le only
    # This is not an issue when we build in Node as we include the proper data files for BE but
    # it does mean that we need to build/test without ICU when running the v8 tests.
    DISABLE_V8_I18N_OPTION="DISABLE_V8_I18N=1"
  fi
fi

if [ "X$DESTCPU" = "Xarm64" ]; then
  # arm seems to be using clang even though the version downloaded from google is for linux
  # which obviously does not work force it to use g++/gcc instead
  ADDITIONAL_CLANG_OPTIONS='GYPFLAGS="-Dhost_clang=0"'
fi

if [ "X$DESTCPU" = "Xs390x" ]; then
 # ICU tests are diabled for BE platforms because the ICU included by V8 is le only
 # This is not an issue when we build in Node as we include the proper data files for BE but
 # it does mean that we need to build/test without ICU when running the v8 tests.
 DISABLE_V8_I18N_OPTION="DISABLE_V8_I18N=1"
 
 # select the appropriate compiler
 rm -rf build
 git clone https://github.com/nodejs/build.git
 . ./build/jenkins/scripts/select-compiler.sh

  # 'master' (v12 @ d0e593e11e) has the needed code for s390x
 if [ "$NODEJS_MAJOR_VERSION" -lt "12" ] && [ "$NODEJS_MAJOR_VERSION" -gt "9" ]; then
   # temporarily patch make-v8.sh until we upstream the required changes
   cp -f $HOME/build-tools/make-v8.patch tools/make-v8.sh
 fi
 
 # 390 is only supported on v6 and later for now
 if [ "$NODEJS_MAJOR_VERSION" -lt "6" ]; then
   RUN_TESTS="DONT_RUN"
 fi
fi


if [ "$RUN_TESTS" = "RUN" ]; then
  #make -j $(getconf _NPROCESSORS_ONLN) v8 $DISABLE_V8_I18N_OPTION DESTCPU=$DESTCPU ARCH=$DESTCPU.release $ADDITIONAL_CLANG_OPTIONS

  # when run under jenkins the d8-os.js test hangs because signals don't set to the process when a os.process times out
  # it works just fine when run from the command line so its not an issue with v8 itself.  I'd like to figure out why
  # it fails under jenkins but so far have not managed to so remove this test before running
  rm deps/v8/test/mjsunit/d8-os.js || true
  ./configure
  make -j $(getconf _NPROCESSORS_ONLN) test-v8 V=1 $DISABLE_V8_I18N_OPTION DESTCPU=$DESTCPU ARCH=$DESTCPU.release $ADDITIONAL_CLANG_OPTIONS ENABLE_V8_TAP=True V8_EXTRA_TEST_OPTIONS="--progress=dots --timeout=120"
else
  echo $SKIP_MESSAGE
  # fake out so we don't get failures
  echo '<testsuite tests="1"><testcase name="none/none.none test were skipped" time="0.001"/></testsuite>' > v8-tap.xml
fi

# SAM - skip this
# clean up so that the v8 directory is always fresh
#  rm -rf deps/v8  || true

@bcoe
Copy link
Contributor

bcoe commented Jan 18, 2019

@sam-github anything I can help with on my end, wondering if you've made any progress. You can find me on Twitter, if you want to talk real-time.

@sam-github
Copy link
Contributor

@bcoe looks like the server's jenkins working directory just needed to be deleted so the job could re-create it. I have 2 green builds in a row, and just kicked another off. This might be fixed.

@sam-github
Copy link
Contributor

OK, no reall explanation for what was going on, but it seems to be good now.

@mhdawson
Copy link
Member Author

@sam-github I guess this can be closed now?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants