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

test: update flag in test #3

Merged
merged 1 commit into from
Dec 20, 2016
Merged

Conversation

fhinkel
Copy link

@fhinkel fhinkel commented Dec 20, 2016

Previously used flag is deprecated in V8, use a flag
that still exists.

Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

test

Description of change

Previously used flag is deprecated in V8, use a flag that still exists.

Previously used flag is deprecated in V8, use a flag
that still exists.
@fhinkel fhinkel merged commit 5b247b7 into v8:vee-eight-lkgr Dec 20, 2016
@ofrobots
Copy link
Collaborator

ofrobots commented Dec 20, 2016 via email

@fhinkel
Copy link
Author

fhinkel commented Dec 20, 2016

@hashseed wanted to merge the PR. How does he get permission?

@ofrobots
Copy link
Collaborator

Hmm.. I'm not a repo admin for this repo either. Perhaps @s3ththompson or @natorion can make some of us admins?

@s3ththompson
Copy link

The best solution is for @hashseed to join @v8/node-js. In the meantime, @ofrobots I've given you admin access to this repo.

@hashseed
Copy link
Member

Applied for membership for @v8/node-js

@natorion
Copy link
Member

Added @hashseed to the team and promoted him to team maintainer. You should now be able to add/remove the roster. Did the same thing with @ofrobots btw.

hashseed pushed a commit that referenced this pull request Nov 14, 2017
Currently when running the test without an internet connection there are
two JavaScript test failures and one cctest. The cctest only fails on
Mac as far as I know. (I've only tested using Mac and Linux thus far).

This commit moves the two JavaScript tests to test/internet.

The details for test_inspector_socket_server.cc:

[ RUN      ] InspectorSocketServerTest.FailsToBindToNodejsHost
make[1]: *** [cctest] Segmentation fault: 11
make: *** [test] Error 2

lldb output:

[ RUN      ] InspectorSocketServerTest.FailsToBindToNodejsHost
Process 63058 stopped
* thread #1: tid = 0x7b175, 0x00007fff96d04384
* libsystem_info.dylib`_gai_simple + 87, queue =
* 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1,
* address=0x0)
    frame #0: 0x00007fff96d04384 libsystem_info.dylib`_gai_simple + 87
libsystem_info.dylib`_gai_simple:
->  0x7fff96d04384 <+87>: movw   (%rdx), %ax
    0x7fff96d04387 <+90>: movw   %ax, -0x2a(%rbp)
    0x7fff96d0438b <+94>: movq   %r13, -0x38(%rbp)
    0x7fff96d0438f <+98>: movq   0x18(%rbp), %rcx

(lldb) bt
* thread #1: tid = 0x7b175, 0x00007fff96d04384
* libsystem_info.dylib`_gai_simple + 87, queue =
* 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1,
* address=0x0)
  * frame #0: 0x00007fff96d04384 libsystem_info.dylib`_gai_simple + 87
    frame #1: 0x00007fff96cfe98b libsystem_info.dylib`search_addrinfo +
179
    frame #2: 0x00007fff96cfafef libsystem_info.dylib`si_addrinfo + 2255
    frame #3: 0x00007fff96cfa67b libsystem_info.dylib`getaddrinfo + 179
    frame #4: 0x00000001017d8888
cctest`uv__getaddrinfo_work(w=0x00007fff5fbfe210) + 72 at
getaddrinfo.c:102
    frame #5: 0x00000001017d880e
cctest`uv_getaddrinfo(loop=0x000000010287cb80, req=0x00007fff5fbfe1c8,
cb=0x0000000000000000, hostname="nodejs.org", service="0",
hints=0x00007fff5fbfe268) + 734 at getaddrinfo.c:192
    frame #6: 0x000000010171f781
cctest`node::inspector::InspectorSocketServer::Start(this=0x00007fff5fbfe658)
+ 801 at inspector_socket_server.cc:398
    frame #7: 0x00000001016ed590
cctest`InspectorSocketServerTest_FailsToBindToNodejsHost_Test::TestBody(this=0x0000000105001fd0)
+ 288 at test_inspector_socket_server.cc:593

I'm not sure about the exact cause for this but when using a standalone
c program to simulate this it seems like when the ai_flags
`AI_NUMERICSERV` is set, which is done in inspector_socket_server.cc
line 394, the servname (the port in the FailsToBindToNodejsHost test) is
expected to be a numeric port string to avoid looking it up in
/etc/services. When the port is 0 as is it was before this commit the
segment fault occurs but not if it is non-zero.

PR-URL: nodejs#16255
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
hashseed pushed a commit to hashseed/node that referenced this pull request Jan 5, 2018
Remove a pointless adapter frame  by fixing up the function's formal
parameter count.  Before:

    frame #0: 0x000033257ea446d5 onParserExecute(...)
    frame v8#1: 0x000033257ea3b93f <adaptor>
    frame v8#2: 0x000033257ea41959 <internal>
    frame v8#3: 0x000033257e9840ff <entry>

After:

    frame #0: 0x00000956287446d5 onParserExecute(...)
    frame v8#1: 0x0000095628741959 <internal>
    frame v8#2: 0x00000956286840ff <entry>

PR-URL: nodejs#17693
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Khaidi Chu <[email protected]>
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

Successfully merging this pull request may close these issues.

5 participants