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.

Sign up for GitHub

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

Jump to bottom

v8 inspect crashes on Linux with invalid options #41

Closed
hhellyer opened this issue Oct 24, 2016 · 1 comment
Closed

v8 inspect crashes on Linux with invalid options #41

hhellyer opened this issue Oct 24, 2016 · 1 comment

Comments

@hhellyer
Copy link
Contributor

If you run v8 inspect -k 0x0 on Linux then llnode seg faults.
This is a problem with the handling of the command arguments in CommandBase::ParseInspectOptions.
@hhellyer hhellyer mentioned this issue Oct 24, 2016
Merged
@hhellyer
Copy link
Contributor Author

I've pushed a fix, it looks like there's a difference between how the two implementations of getopt_long report errors. On Linux the first argument of the arguments array passed to getopt_long is reported in the error message. On Mac (I think!) it's the first argument in the global argv variable, which is always the program name.

This is easier to explain with the fix from the above PR, which sets element 0 to llnode, and a bad option.
lldb on Linux reports:

(lldb) v8 inspect -k 0x0
llnode: invalid option -- 'k'
<Smi: 0>

On Mac:

(lldb) v8 inspect -k 0x0
lldb: invalid option -- k
<Smi: 0>

(Technically this is probably a bug in getopt_long on Mac.)

hhellyer added a commit that referenced this issue Oct 27, 2016
@hhellyer
llnode.cc: Fix linux options parsing crash.
aee4e66 
Fix up the arguments array to contain a first element rather than
subtract 1 from the lldb arguments and pretend they start at
args[1].
Linux prints args[0] in it's error message so it must point to a real
array element.
(Mac OSX appears to be hardcoded to use the program name in argv[0]
and ignores the first element of the arguments array.)

Fixes: #41
Review: #42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@hhellyer