nodejs / llnode

An lldb plugin for Node.js and V8, which enables inspection of JavaScript states for insights into Node.js processes and their core dumps.
Other
1.15k stars 99 forks source link

src: don't crash on invalid script positions #426

Closed kvakil closed 1 year ago

kvakil commented 1 year ago

This "fixes" the segfault reported in #422, in the sense that you no longer get a segfault. However the printing does not actually work, i.e. you currently get an error like this:

(lldb) v8 i -s 0x2196b1a09a29
error: Invalid source range, start_pos=3108, len=-3098, source_len=10

I'm deeming this better than segfaulting. We should really never be segfaulting as the coredump might be incomplete/partially corrupted. (Also, we already know function printing on v16 doesn't work right now.)

codecov-commenter commented 1 year ago

Codecov Report

Base: 73.65% // Head: 73.59% // Decreases project coverage by -0.05% :warning:

Coverage data is based on head (4a950e7) compared to base (eb969e1). Patch coverage: 40.00% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #426 +/- ## ========================================== - Coverage 73.65% 73.59% -0.06% ========================================== Files 34 34 Lines 4995 4999 +4 ========================================== Hits 3679 3679 - Misses 1316 1320 +4 ``` | [Impacted Files](https://codecov.io/gh/nodejs/llnode/pull/426?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | | |---|---|---| | [src/llv8.cc](https://codecov.io/gh/nodejs/llnode/pull/426/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2xsdjguY2M=) | `71.25% <40.00%> (-0.38%)` | :arrow_down: | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

tony-go commented 1 year ago

Thanks ^^

Definitely better than segfault. 😃

But we don't really know why that is happening. I'm correct?

No9 commented 1 year ago

LGTM to so merging