SpiderMonkey upgrade #291

Ms2ger · 2016-08-23T09:14:08Z

Recreated the PR to aid collaboration.

This change is

Ms2ger · 2016-08-23T10:36:39Z

Funny story, that. The presence of invalid Cargo.tomls in m-c make cargo fail to compile here.

metajack · 2016-08-23T16:36:15Z

Why are there invalid Cargo.tomls in m-c?

cc @larsbergstrom

fitzgen · 2016-08-23T20:40:09Z

./mozjs/python/mozbuild/mozbuild/test/frontend/data/crate-build-dependency-absolute-path/Cargo.toml:14:bogus = { version = "0.1.0", path = "/path/to/somewhere" }
./mozjs/python/mozbuild/mozbuild/test/frontend/data/crate-dependency-absolute-path/Cargo.toml:12:bogus = { version = "0.1.0", path = "/path/to/somewhere" }

fitzgen · 2016-08-23T20:54:59Z

I'm just deleting all of m-c's test Cargo.toml files in servo/mozjs

fitzgen · 2016-08-23T21:09:34Z

By deleting the test Cargo.toml files, and commit d0b0f5a, rust-mozjs builds successfully for me. We'll see what CI thinks...

fitzgen · 2016-08-24T00:20:17Z

Travis CI OSX builds failing due to having an old clang/llvm? Do we need to update the .travis.yml config to use new OSX images?

ERROR: Only clang/llvm 3.6 or newer is supported.

Just pushed linux 64 bindings as well. Anyone care to help out with linux 32 and windows?

Ms2ger · 2016-08-24T14:06:18Z

CC @nox @vvuk: could you generate the missing bindings?

fitzgen · 2016-08-24T17:14:32Z

Actually hold up a minute. This upgrade caught the big cx/rt refactoring that jandem did, so cargo test is rather unhappy at the moment -- kind of surprised that cargo build didn't blow up too.

fitzgen · 2016-08-24T22:03:24Z

Looks like this upgrade also caught the move of roots from the runtime to zones...

fitzgen · 2016-08-25T00:15:44Z

Ok, linux64 is passing in Travis CI.

I updated the macos64 bindings locally and tested them and they are all good, but I expect them to fail on Travis CI again due to the llvm issues I mentioned before. Anyone have any ideas? I can choose a random OSX image that is new enough, but I'm not sure if there is a more principled approach you;d like to take...

@nox @vvuk I think we are ready for windows and linux32 bindings updates now!

fitzgen · 2016-08-26T22:22:09Z

Rebased.

I updated the macos64 bindings locally and tested them and they are all good, but I expect them to fail on Travis CI again due to the llvm issues I mentioned before. Anyone have any ideas? I can choose a random OSX image that is new enough, but I'm not sure if there is a more principled approach you;d like to take...

@Ms2ger @nox any opinion on this ^?

bors-servo · 2016-08-28T12:53:01Z

☔ The latest upstream changes (presumably #298) made this pull request unmergeable. Please resolve the merge conflicts.

fitzgen · 2016-08-30T19:08:14Z

Ok, I think all the bindings need to be regenerated again, now that we aren't using unsafe_no_drop_flag anymore.

…g stacks move to Zone

nox · 2016-08-31T08:17:13Z

Ok, I think all the bindings need to be regenerated again, now that we aren't using unsafe_no_drop_flag anymore.

You can just do /unsafe_no_drop_flag/d on them.

Ms2ger · 2016-08-31T08:23:01Z

@metajack do you have opinions on the llvm thing mentioned above?

metajack · 2016-08-31T15:17:13Z

If recent OS X images provided by Travis have the correct llvm, I think it's fine to udpate to those. Otherwise, it should be possible to install the required llvm version via brew.

fitzgen · 2016-09-02T00:43:24Z

I regenerated bindings for linux64 and now cargo test is segfaulting in a very strange way. It seems like the DWARF debugging info is garbage when I compare what gdb reports for variable values vs looking at the disassembly, so digging in is more annoying than usual.

I am 95% sure this is a miscompilation by gdb. It is passing the first argument to the current function as the this parameter in a method call on what should be a stack allocated object that never is actually allocated. Completely nonsensical.

I'll bet this works on Travis with a (hopefully) different compiler.

fitzgen · 2016-09-02T01:23:53Z

FWIW, filed https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77448 for the gcc codegen issues I was seeing.

fitzgen · 2016-09-02T22:24:13Z

Either rustc or g++ is not following the x86-64 ABI here. They're disagreeing on whether JS::CallArgs should be classified as MEMORY and therefore have the stack address of the return value passed as an implicit first parameter in rdi or not. g++ is classifying it as MEMORY and using the implicit stack pointer, while rustc is classifying it as INTEGER and splitting it across rax and rdx.

fitzgen · 2016-09-02T22:25:39Z

I tried to make a reduced test case, however now rustc is treating CallArgs as MEMORY in this example and passing the stack pointer in through rdi!

fitzgen · 2016-09-02T22:34:46Z

From the most up to date draft of the x86-64 ABI I could find: AMD64 ABI Draft 0.99.7, Section 3.2.3 Parameter Passing, p. 19:

If the size of the aggregate exceeds two eightbytes and the first eight-
byte isn’t SSE or any other eightbyte isn’t SSEUP, the whole argument
is passed in memory

That seems fairly damning to rustc's behavior, here. (sizeof(JS::CallArgs) is 24)

Ms2ger · 2016-09-05T13:15:32Z

File an issue on rustc?

nox · 2016-09-06T08:07:17Z

File an issue on rustc?

Nah the bindings were just badly generated, right @fitzgen?

fitzgen · 2016-09-06T16:39:50Z

Nah the bindings were just badly generated, right @fitzgen?

Yes, I managed to put non-DEBUG bindings in the _debug.rs file. I'm writing a patch to make it so that no one will repeat this mistake because it was a huge time sink figuring out what was going on.

Travis CI is now green for OSX and linux 64. Just need linux 32 and windows now.

vvuk · 2016-09-06T18:08:36Z

I'll work on win32 right now.

vvuk · 2016-09-06T23:57:57Z

Trying to build this version (smup branch of mozjs-sys, commit b5b7c84990325cde5226) with MSVC I get:

Unified_cpp_js_src3.cpp
c:/proj/r/mozjs/mozjs/js/src/builtin/Intl.cpp(859): error C2971: 'ScopedICUObject': template parameter 'Delete': 'uenum_close': a variable with non-static storage duration cannot be used as a non-type argument
c:/proj/r/mozjs/mozjs/js/src/builtin/Intl.cpp(615): note: see declaration of 'ScopedICUObject'
c:/proj/r/mozjs/mozjs/js/src/builtin/Intl.cpp(128): note: see declaration of 'uenum_close'

This is pretty strange, since that uenum_close on line 128 is a stub (because intl is disabled) declared as static void uenum_close() { ... }

vvuk · 2016-09-07T00:04:47Z

I can fix this if I change

static void
uenum_close(UEnumeration* en)
{
    MOZ_CRASH("uenum_close: Intl API disabled");
}

to

namespace {
void
uenum_close(UEnumeration* en)
{
    MOZ_CRASH("uenum_close: Intl API disabled");
}
}

(and for other similar stub functions).

I'm guessing that this was never tested with --without-intl-api on Windows. I'll prepare a patch and put up a PR for the smup branch.

vvuk · 2016-09-07T19:57:01Z

The other issue is that currently, the SM/Gecko build is broken with anything but MSVC or clang on Windows, because of https://bugzilla.mozilla.org/show_bug.cgi?id=1288313#c12 (configure fails). I'm working on a workaround.

vvuk · 2016-09-08T04:25:24Z

Okay, for windows:
rust-bindgen fix: rust-lang/rust-bindgen#54
mozjs-sys fixes: servo/mozjs#99
updated bindings: #303

Note that without servo/mozjs/pull/99, mozjs will not be buildable on Windows (gcc has the WINNT issue, msvc has the anon namespace static issue).

Update Windows Bindings

nox · 2016-09-23T14:03:57Z

Proper union support landed in rust-bindgen. Should we account for this in that smup?

fitzgen · 2016-10-10T21:24:21Z

Okay, for windows:

So does this mean we need to rebase this PR and then the appveyor CI tests should start passing?

Also, I think the docopt changes got backed out of bindgen and haven't relanded so we may need to update some scripts...

fitzgen · 2016-10-11T23:32:24Z

The etc/bindings.sh script no longer works with the latest servo/rust-bindgen.

vvuk · 2016-10-12T01:12:12Z

Oh wow, didn't realize this hadn't landed yet. What's wrong with the bindings.sh script?

fitzgen · 2016-10-12T18:04:47Z

What's wrong with the bindings.sh script?

bindgen's backout of docopt made it so that the args aren't getting parsed correctly anymore for some reason. Still digging in. Additionally, I don't think the SpiderMonkey bindings have been regenerated since bindgen was completely rewritten, so maybe some fallout from there.

fitzgen · 2016-10-12T21:02:36Z

Biggest issue with the bindings.sh script atm: tons of --match, which is no longer a supported flag. Working on it.

vvuk · 2016-10-12T21:10:48Z

Oh wow, okay. Give me a shout on irc if you need help with any Windows
stuff.

On Oct 12, 2016 5:03 PM, "Nick Fitzgerald" [email protected] wrote:

Biggest issue with the bindings.sh script atm: tons of --match, which is
no longer a supported flag. Working on it.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#291 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAL5lTMBPoByFoMpGo39fOEDPLSi6Gkcks5qzUrtgaJpZM4JqtfP
.

fitzgen · 2016-10-13T01:14:01Z

My current plan of action:

Land the mozjs-sys crate in m-c now because that is working and green in taskcluster
Redo bindings generation from a clean slate because our current setup is completely hosed
Land that in m-c once its fixed + building in taskcluster
Then, whenever we want to smup, take known-working snapshots of spidermonkey and bindings into servo/mozjs or servo/rust-mozjs (wouldn't really need separate repos anymore)

This stuff will mostly happen in https://bugzilla.mozilla.org/show_bug.cgi?id=1277338

nox · 2016-10-13T10:35:41Z

@fitzgen Don't know if I told you that before, but if we could use the new support for unions in our bindgen, we can remove a few ifdef RUST_BINDGEN stuff from our SM fork.

bors-servo · 2016-10-25T16:50:38Z

☔ The latest upstream changes (presumably #310) made this pull request unmergeable. Please resolve the merge conflicts.

jdm · 2017-01-27T19:41:57Z

Closing since the activity is happening elsewhere.

Ms2ger mentioned this pull request Aug 23, 2016

[do not merge] smup #290

Closed

fitzgen force-pushed the smup branch from d0b0f5a to 77970fd Compare August 23, 2016 21:22

fitzgen force-pushed the smup branch 2 times, most recently from 4e5bd84 to 1db7623 Compare August 25, 2016 00:13

fitzgen force-pushed the smup branch from 1db7623 to 7e0e77a Compare August 26, 2016 22:20

fitzgen and others added 3 commits August 30, 2016 13:58

Update friendly rustic API for the great rt -> cx refactor and rootin…

75ace6b

…g stacks move to Zone

Update the bindings for macos 64

501273c

TEST

06d8705

fitzgen force-pushed the smup branch from 7e0e77a to 06d8705 Compare August 30, 2016 21:10

Force Xcode 7 on Travis for OS X

f6e30b1

nox force-pushed the smup branch 2 times, most recently from 2d76ded to 3d7704c Compare September 1, 2016 13:47

Install CMake on OS X

1ac258a

nox force-pushed the smup branch from 2c8efb9 to 1ac258a Compare September 1, 2016 14:18

Update the bindings for linux 64

eff454f

fitzgen force-pushed the smup branch from e01a844 to eff454f Compare September 2, 2016 23:20

vvuk added 2 commits September 8, 2016 00:19

Update README for Windows, and create build-windows-bindings.sh

d7ab415

Update Windows bindings

b9a888c

Merge pull request #303 from vvuk/smup

9bd284d

Update Windows Bindings

jdm closed this Jan 27, 2017

SpiderMonkey upgrade #291

SpiderMonkey upgrade #291

Conversation

Ms2ger commented Aug 23, 2016 • edited by larsbergstrom Loading

Ms2ger commented Aug 23, 2016

metajack commented Aug 23, 2016

fitzgen commented Aug 23, 2016

fitzgen commented Aug 23, 2016

fitzgen commented Aug 23, 2016

fitzgen commented Aug 24, 2016

Ms2ger commented Aug 24, 2016

fitzgen commented Aug 24, 2016

fitzgen commented Aug 24, 2016

fitzgen commented Aug 25, 2016

fitzgen commented Aug 26, 2016

bors-servo commented Aug 28, 2016

fitzgen commented Aug 30, 2016

nox commented Aug 31, 2016

Ms2ger commented Aug 31, 2016

metajack commented Aug 31, 2016

fitzgen commented Sep 2, 2016

fitzgen commented Sep 2, 2016

fitzgen commented Sep 2, 2016

fitzgen commented Sep 2, 2016

fitzgen commented Sep 2, 2016 • edited Loading

Ms2ger commented Sep 5, 2016

nox commented Sep 6, 2016 • edited Loading

fitzgen commented Sep 6, 2016

vvuk commented Sep 6, 2016

vvuk commented Sep 6, 2016

vvuk commented Sep 7, 2016

vvuk commented Sep 7, 2016

vvuk commented Sep 8, 2016

nox commented Sep 23, 2016

fitzgen commented Oct 10, 2016

fitzgen commented Oct 11, 2016

vvuk commented Oct 12, 2016

fitzgen commented Oct 12, 2016

fitzgen commented Oct 12, 2016

vvuk commented Oct 12, 2016

fitzgen commented Oct 13, 2016

nox commented Oct 13, 2016

bors-servo commented Oct 25, 2016

jdm commented Jan 27, 2017

Ms2ger commented Aug 23, 2016 •

edited by larsbergstrom

Loading

fitzgen commented Sep 2, 2016 •

edited

Loading

nox commented Sep 6, 2016 •

edited

Loading