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

Panic when using ripgrep output in the v8 codebase #58

Closed
doubledup opened this issue Feb 2, 2024 · 6 comments
Closed

Panic when using ripgrep output in the v8 codebase #58

doubledup opened this issue Feb 2, 2024 · 6 comments
Labels
bug Something isn't working

Comments

@doubledup
Copy link

Using zf to filter ripgrep output in v8 exits with a panic & SIGABRT:

$ rg . . | zf
thread 708434 panic: reached unreachable code
fish: Process 30749, 'zf' from job 1, 'rg . . | zf' terminated by signal SIGABRT (Abort)
@natecraddock
Copy link
Owner

natecraddock commented Feb 2, 2024

Hmm, it looks like it is failing in the ziglyph unicode normalizing.

❯ rg . . | zf
thread 6355044 panic: reached unreachable code
???:?:?: 0x1053e903f in _segmenter.CodePoint.CodePointIterator.next (???)
???:?:?: 0x1053ee72b in _normalizer.Normalizer.nfd (???)
???:?:?: 0x1053e11c1 in _main.main (???)
Unwind error at address `zf:0x1053e11c1` (error.InvalidUnwindInfo), trace may be incomplete

???:?:?: 0x7ff7bab1ff8f in ??? (???)
fish: Process 58497, 'zf' from job 1, 'rg . . | zf' terminated by signal SIGABRT (Abort)

Thank you for reporting, I will look into this more

@natecraddock natecraddock added the bug Something isn't working label Feb 2, 2024
@natecraddock
Copy link
Owner

Alright, it seems like part of the rg . . output is not valid unicode and ziglyph is assuming the input is valid

❯ rg . . | isutf8 
(standard input): line 80163, char 101, byte 5832237: Expecting bytes in the following ranges: 00..7F C2..F4.

One problematic file is third_party/test262-harness/LICENSE. There are some other files that contain invalid utf8.

Are you building zf from source? Or do you use it packaged from some repository? I'll make a fix to catch this case. If you are using it from one of the repos I'll make a quick patch release

@natecraddock
Copy link
Owner

natecraddock commented Feb 6, 2024

I also want to note that rg . . outputs over 6 million lines. At least for me (on a decently fast machine) zf is quite slow for filtering this. I do have plans for making zf faster in the future though

While zf does function as a general purpose fuzzy finder, it is specifically designed for filepath matching. Maybe another tool is better for filtering the output of rg . . (which isn't filepaths)? fzf worked well for me

natecraddock added a commit that referenced this issue Feb 7, 2024
The input is passed to normalizer.nfd. Previously it was assumed that
the input was valid utf8. If the input contained invalid bytes, zf would
panic and exit.

This is a temporary workaround. A future commit will remove zyglyph
entirely and this will no longer be an issue.

Closes #58
@natecraddock
Copy link
Owner

I made a new release that contains this fix https://github.com/natecraddock/zf/releases/tag/0.9.1

@doubledup
Copy link
Author

doubledup commented Feb 7, 2024

Are you building zf from source?

@natecraddock I'm using the Homebrew package, looks like it's already got the new release. Just updated and the case above is working! Thanks for the quick fix! 🙏

zf ... is specifically designed for filepath matching

I'll bear this in mind, have used fzf for ages but I was exploring other options.

@natecraddock I've found another crash, this time when search for 262licen. Happy to open another issue if you want, but also happy to leave it if you'd rather stick to using zf for filepath matching only.

@natecraddock
Copy link
Owner

natecraddock commented Feb 7, 2024

I've found another crash, this time when search for 262licen. Happy to open another issue if you want, but also happy to leave it if you'd rather stick to using zf for filepath matching only.

@doubledup Yes please report. I haven't personally experienced crashes with zf in over a year, so I'm always interested to know what might be broken. And just because you aren't using zf for a filepath doesn't mean I shouldn't fix it. I do want to support zf as a general-purpose fuzzy finder

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants