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

LLNode Windows support #17

Open
hhellyer opened this Issue Jul 12, 2016 · 21 comments

Comments

Projects
None yet
10 participants
@hhellyer
Collaborator

hhellyer commented Jul 12, 2016

I thought I’d open this issue to discuss whether Windows support for LLNode is something anyone wants.

I’ve got and LLDB build up and running on Windows so I had a chance to try building LLNode on Windows. Running gyp_llnode worked fairly well.

The resulting Visual Studio project created by gyp required a little tweaking (mostly things I could google for). The main issue was that LLNode uses the getopt library which isn’t on Windows by default. Given that the getopt problem could probably be fixed running LLNode on Windows looks do-able but would need a bit of work and would probably break fairly quickly unless someone was using it regularly.

There’s two reasons why someone might want to do this:

  • To debug issues with versions of Node running on Windows.
  • To debug issues with core dumps generated on Linux. (LLDB is able to open Linux ELF cores even when it’s running on Mac or Windows.)

I’m not sure how important the first scenario is but the second one might be useful to developers with Windows machines who deploy to Linux. Of course you do have to get LLDB for Windows, probably by building it yourself, which isn’t hard but it is quite time consuming. (Developers with Mac’s who deploy to Linux are effectively already supported.)

Any thoughts? Does anyone else have a strong need for this? (I’m using a Mac so I’m neutral.)

@indutny

This comment has been minimized.

Show comment
Hide comment
@indutny

indutny Jul 12, 2016

Member

I think this is good addition, but we will probably need to hook it up to node.js CI, so that it will always be tested too. cc @rvagg @Fishrock123

Member

indutny commented Jul 12, 2016

I think this is good addition, but we will probably need to hook it up to node.js CI, so that it will always be tested too. cc @rvagg @Fishrock123

@rvagg

This comment has been minimized.

Show comment
Hide comment
@rvagg

rvagg Jul 13, 2016

Member

Opened an issue here to discuss the question of how a project might qualify to use our infra: nodejs/build#448, I think it might be a tough call because it'll open floodgates for projects not clearly within the scope of the foundation.

Member

rvagg commented Jul 13, 2016

Opened an issue here to discuss the question of how a project might qualify to use our infra: nodejs/build#448, I think it might be a tough call because it'll open floodgates for projects not clearly within the scope of the foundation.

@indutny

This comment has been minimized.

Show comment
Hide comment
@indutny

indutny Jul 13, 2016

Member

I don't think there is any reason for llnode to not move under Node.js Foundation. If this is a show stopper - what could be done to make this project qualify for this?

Member

indutny commented Jul 13, 2016

I don't think there is any reason for llnode to not move under Node.js Foundation. If this is a show stopper - what could be done to make this project qualify for this?

@rvagg

This comment has been minimized.

Show comment
Hide comment
@rvagg

rvagg Jul 13, 2016

Member

I have no problem with llnode to move into nodejs/, perhaps raise an issue in the diagnostics WG and ask if they'd like to take some broad responsibility for it?

Member

rvagg commented Jul 13, 2016

I have no problem with llnode to move into nodejs/, perhaps raise an issue in the diagnostics WG and ask if they'd like to take some broad responsibility for it?

@indutny

This comment has been minimized.

Show comment
Hide comment
@indutny

indutny Jul 13, 2016

Member

@rvagg absolutely!

@hhellyer will this scenario work for you, or would you prefer the project to remain independent?

Member

indutny commented Jul 13, 2016

@rvagg absolutely!

@hhellyer will this scenario work for you, or would you prefer the project to remain independent?

@indutny

This comment has been minimized.

Show comment
Hide comment
@indutny

indutny Jul 13, 2016

Member

@rvagg that was "?" not "!", sorry for confusion.

Member

indutny commented Jul 13, 2016

@rvagg that was "?" not "!", sorry for confusion.

@hhellyer

This comment has been minimized.

Show comment
Hide comment
@hhellyer

hhellyer Jul 13, 2016

Collaborator

I think the moving the project under Node.js would be great, it would probably improve the visibility and hopefully adoption and participation as well.
I'm not sure I know enough about the Node.js build infrastructure for that part of this discussion but I can see why they'd be worried about the load from adding various other projects.

Collaborator

hhellyer commented Jul 13, 2016

I think the moving the project under Node.js would be great, it would probably improve the visibility and hopefully adoption and participation as well.
I'm not sure I know enough about the Node.js build infrastructure for that part of this discussion but I can see why they'd be worried about the load from adding various other projects.

@mhdawson

This comment has been minimized.

Show comment
Hide comment
@mhdawson

mhdawson Jul 13, 2016

Member

The topic of finding a good home for work form the post-mortem WG is on the agenda for monday. See: nodejs/post-mortem#31. And more specifically this issue is to discuss the issue for work like llnode (as well as others) from the post-mortem WG: nodejs/post-mortem#30

So I was thinking it was more likely in the scope of the post-mortem WG than the diagnostics WG but there is likely some overlap between those WG's so that may not matter.

Member

mhdawson commented Jul 13, 2016

The topic of finding a good home for work form the post-mortem WG is on the agenda for monday. See: nodejs/post-mortem#31. And more specifically this issue is to discuss the issue for work like llnode (as well as others) from the post-mortem WG: nodejs/post-mortem#30

So I was thinking it was more likely in the scope of the post-mortem WG than the diagnostics WG but there is likely some overlap between those WG's so that may not matter.

@rnchamberlain

This comment has been minimized.

Show comment
Hide comment
@rnchamberlain

rnchamberlain Oct 20, 2016

Member

Trying LLDB/LLNODE on Windows. There seems to be a problem in API/SBDebugger.cpp (LLDB code). It has a hard-coded mangled name for the plugin entry point:

  // TODO: mangle this differently for your system - on OSX, the first
  // underscore needs to be removed and the second one stays
 LLDBCommandPluginInit init_func =
        (LLDBCommandPluginInit)dynlib.getAddressOfSymbol(
            "_ZN4lldb16PluginInitializeENS_10SBDebuggerE");

Not sure the comment is correct, as we have it working fine on OSX. Using Windows VS2015 the mangled name I get is "?PluginInitialize@lldb@@YA_NVSBDebugger@1@@Z". I'll do some more digging, then maybe raise and fix in LLDB.

Member

rnchamberlain commented Oct 20, 2016

Trying LLDB/LLNODE on Windows. There seems to be a problem in API/SBDebugger.cpp (LLDB code). It has a hard-coded mangled name for the plugin entry point:

  // TODO: mangle this differently for your system - on OSX, the first
  // underscore needs to be removed and the second one stays
 LLDBCommandPluginInit init_func =
        (LLDBCommandPluginInit)dynlib.getAddressOfSymbol(
            "_ZN4lldb16PluginInitializeENS_10SBDebuggerE");

Not sure the comment is correct, as we have it working fine on OSX. Using Windows VS2015 the mangled name I get is "?PluginInitialize@lldb@@YA_NVSBDebugger@1@@Z". I'll do some more digging, then maybe raise and fix in LLDB.

@rnchamberlain

This comment has been minimized.

Show comment
Hide comment
@rnchamberlain

rnchamberlain Oct 26, 2016

Member

Note for reference, re "getopt library isn’t on Windows" above. There is an alternative implementation inside LLDB, see getopt_internal() in lldb/source/Host/common/GetOptInc.cpp.

Member

rnchamberlain commented Oct 26, 2016

Note for reference, re "getopt library isn’t on Windows" above. There is an alternative implementation inside LLDB, see getopt_internal() in lldb/source/Host/common/GetOptInc.cpp.

@indutny

This comment has been minimized.

Show comment
Hide comment
@indutny

indutny Oct 26, 2016

Member

@rnchamberlain I think if getopt_internal is linked without public exposure we may end up carrying libgetopt with us.

Member

indutny commented Oct 26, 2016

@rnchamberlain I think if getopt_internal is linked without public exposure we may end up carrying libgetopt with us.

@bnoordhuis

This comment has been minimized.

Show comment
Hide comment
@bnoordhuis

bnoordhuis Jun 21, 2017

Member

@hhellyer Just curious, were you able to make progress on this?

Apropos getopt, we could vendor the implementation from freebsd or openbsd, it's just one file and it's license-compatible.

Member

bnoordhuis commented Jun 21, 2017

@hhellyer Just curious, were you able to make progress on this?

Apropos getopt, we could vendor the implementation from freebsd or openbsd, it's just one file and it's license-compatible.

@langhuihui

This comment has been minimized.

Show comment
Hide comment
@langhuihui

langhuihui Sep 22, 2017

I built lldb successfully,but llnode is not support windows now.

langhuihui commented Sep 22, 2017

I built lldb successfully,but llnode is not support windows now.

@langhuihui

This comment has been minimized.

Show comment
Hide comment
@langhuihui

langhuihui Sep 25, 2017

I built llnode on windows successfully. @rnchamberlain I use a def file

LIBRARY  llnode.dll
EXPORTS
;
; llnode exports
;
_ZN4lldb16PluginInitializeENS_10SBDebuggerE=PluginInitialize

langhuihui commented Sep 25, 2017

I built llnode on windows successfully. @rnchamberlain I use a def file

LIBRARY  llnode.dll
EXPORTS
;
; llnode exports
;
_ZN4lldb16PluginInitializeENS_10SBDebuggerE=PluginInitialize
@rnchamberlain

This comment has been minimized.

Show comment
Hide comment
@rnchamberlain

rnchamberlain Sep 25, 2017

Member

@langhuihui ah, that's clever, so you export a function name from the llnode DLL that matches that hard-coded mangled name in LLDB (which is probably a clang or gcc style mangled name). Easier than fixing it on the LLDB side!

Member

rnchamberlain commented Sep 25, 2017

@langhuihui ah, that's clever, so you export a function name from the llnode DLL that matches that hard-coded mangled name in LLDB (which is probably a clang or gcc style mangled name). Easier than fixing it on the LLDB side!

@mhdawson

This comment has been minimized.

Show comment
Hide comment
@mhdawson

mhdawson Sep 25, 2017

Member

@rnchamberlain @langhuihui great to hear. What would be the next steps in getting this in our testing/validation to make sure we keep it working ?

Member

mhdawson commented Sep 25, 2017

@rnchamberlain @langhuihui great to hear. What would be the next steps in getting this in our testing/validation to make sure we keep it working ?

@bnoordhuis

This comment has been minimized.

Show comment
Hide comment
@bnoordhuis

bnoordhuis Sep 27, 2017

Member

See #137 (comment) - postmortem metadata needs to be enabled in the node binary first.

Member

bnoordhuis commented Sep 27, 2017

See #137 (comment) - postmortem metadata needs to be enabled in the node binary first.

@mhdawson

This comment has been minimized.

Show comment
Hide comment
@mhdawson

mhdawson Nov 3, 2017

Member

@rnchamberlain @hhellyer can you help with getting the metadata added to the node binaries. Maybe at least start by adding an issue in the node repo indicating its needed.

Member

mhdawson commented Nov 3, 2017

@rnchamberlain @hhellyer can you help with getting the metadata added to the node binaries. Maybe at least start by adding an issue in the node repo indicating its needed.

@cjihrig

This comment has been minimized.

Show comment
Hide comment
@cjihrig

cjihrig Nov 3, 2017

Contributor

Is this all that's needed to add the metadata? cjihrig/node-1@6391209

Contributor

cjihrig commented Nov 3, 2017

Is this all that's needed to add the metadata? cjihrig/node-1@6391209

@joaocgreis

This comment has been minimized.

Show comment
Hide comment
@joaocgreis

joaocgreis Jun 7, 2018

Member

PR to add build support: #203 . This allows llnode to be easily built on Windows, though actually debugging will require some more work.

Member

joaocgreis commented Jun 7, 2018

PR to add build support: #203 . This allows llnode to be easily built on Windows, though actually debugging will require some more work.

@mhdawson

This comment has been minimized.

Show comment
Hide comment
@mhdawson

mhdawson Jun 7, 2018

Member

@joaocgreis great to see this come in :)

Member

mhdawson commented Jun 7, 2018

@joaocgreis great to see this come in :)

joaocgreis added a commit to joaocgreis/llnode that referenced this issue Jun 18, 2018

joyeecheung added a commit that referenced this issue Jun 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment