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

Variable declaration and receipe parameters crashes NeoVim #69

Closed
eshepelyuk opened this issue Jan 11, 2024 · 33 comments
Closed

Variable declaration and receipe parameters crashes NeoVim #69

eshepelyuk opened this issue Jan 11, 2024 · 33 comments

Comments

@eshepelyuk
Copy link

eshepelyuk commented Jan 11, 2024

Hello @tgross35

  • The latest commit of the plugin
  • NeoVim 0.9.5
  • justfile to reproduce
    VAR := "test"
    

NeoVim just crashes, no errors. It crashes both on opening file and on editing -when typing double quote after :=.
I can't find any logs to additionaly provide. Ready to provide any debug info if you guide me.
I was trying to go back in history for few days. looks like the bug was introduced not recently.

@eshepelyuk
Copy link
Author

eshepelyuk commented Jan 11, 2024

Adding another usecase - receipe parameter declaration with default value

test param="qwe": 
    echo {{param}}

As soon as typing double quote after = - NeoVim crashes.

@eshepelyuk eshepelyuk changed the title Variable declaration crashes NeoVim Variable declaration and receipe parameters crashes NeoVim Jan 11, 2024
@tgross35
Copy link
Collaborator

Thanks for the report, I am not sure why nvim seems to struggle so much when every other editor I test with works fine.

I think you can nvim -V9nvim.log to dump a very verbose log, but I am unsure whether anything useful will be there.

I'll take a look...

@eshepelyuk
Copy link
Author

Thanks for the report, I am not sure why nvim seems to struggle so much when every other editor I test with works fine.

I think you can nvim -V9nvim.log to dump a very verbose log, but I am unsure whether anything useful will be there.

I'll take a look...

Tried this and there's nothing useful or suspicious in logs :(

@getchoo
Copy link

getchoo commented Jan 19, 2024

i'm able to reliably reproduce this on f807ab3 with this justfile

@tgross35
Copy link
Collaborator

Thanks for the report. Are you (or anyone) able to play around with the queries? Just comment things out and see if hiding anything fixes this crash.

Unfortunately I have no way to debug this since I haven't been able to replicate it on my system.

@eshepelyuk
Copy link
Author

Thanks for the report. Are you (or anyone) able to play around with the queries? Just comment things out and see if hiding anything fixes this crash.

Unfortunately I have no way to debug this since I haven't been able to replicate it on my system.

what is your "system" ?
os \ nvim version \ minimal config

@getchoo
Copy link

getchoo commented Jan 19, 2024

after some testing, it appears this was broken in 5766e75. this commit and all after exhibit the same behavior when building against treesitter 0.20.8 on neovim v0.9.5

Are you (or anyone) able to play around with the queries? Just comment things out and see if hiding anything fixes this crash.

commenting out the following lines from the previously mentioned file lets neovim start. editing lines to not include strings (i.e., test: (rebuild "test") -> test: (rebuild)) also fixes this, which lines up with the breakage appearing with the new string scanner

image

Unfortunately I have no way to debug this since I haven't been able to replicate it on my system.

i actually came across this using my configuration found in this nix flake. if you install nix, you should be able to reproduce this by running nix shell --experimental-features 'nix-command flakes' github:getchoo/getchvim/bc1489046e0ddf3ac9460e3b227455d3cb3606e5, which will give you a shell with my configuration

@tgross35
Copy link
Collaborator

tgross35 commented Jan 19, 2024

Thanks for the report. Are you (or anyone) able to play around with the queries? Just comment things out and see if hiding anything fixes this crash.
Unfortunately I have no way to debug this since I haven't been able to replicate it on my system.

what is your "system" ? os \ nvim version \ minimal config

I most recently tested with 0.10.0-dev on aarch64 macos, and nothing in the config outside of Plug and tree-sitter. But I have also tried a few different versions, and in docker.

after some testing, it appears this was broken in 5766e75. this commit and after all exhibit the same behavior when building against treesitter 0.20.8 on neovim v0.9.5

Unfortunately that commit is about the biggest change yet since it reworked everything to do with strings :)

Are you (or anyone) able to play around with the queries? Just comment things out and see if hiding anything fixes this crash.

commenting out the following lines from the previously mentioned file lets neovim start. editing lines to not include strings (i.e., test: (rebuild "test") -> test: (rebuild)) also fixes this, which lines up with the breakage appearing with the new string scanner

Ah, sorry for the confusion. I meant editing the query files, these should be some .scm files likely located wherever Plug (or the relevant manager) downloaded these items, and tells the editor how to apply highlighting to structured grammar. I am curious whether commenting out parts of those files makes things work correctly since queries are the only thing that is unique to nvim (tree-sitter-cli and helix share the grammar, and those two tools can parse everything fine).

Unfortunately I have no way to debug this since I haven't been able to replicate it on my system.

i actually came across this using my configuration found in this nix flake. if you install nix, you should be able to reproduce this by running nix shell --experimental-features 'nix-command flakes' github:getchoo/getchvim/bc1489046e0ddf3ac9460e3b227455d3cb3606e5, which will give you a shell with my configuration

Hm, I wonder if I could get that working on CI somehow... #64 could use some help if you have any idea how to just run nvim headlessly on files

@woojiq
Copy link

woojiq commented Jan 21, 2024

FYI, I can reproduce the same thing (SIGSEGV) with Helix. I tried to update just ts revision to the latest because right now helix has 3y.o revision. Queries do not affect this in any way, without queries the crash still exists.

@tgross35
Copy link
Collaborator

FYI, I can reproduce the same thing (SIGSEGV) with Helix. I tried to update just ts revision to the latest because right now helix has 3y.o revision. Also it breaks after fixing 5766e75 for me. Queries do not affect this in any way, without queries the crash still exists.

Ah thanks, knowing it's a segfault is very helpful. Must be a bug in the external scanner then.

I should hook up a fuzzer in CI, unless somebody spots the issue (PRs welcome for either, I won't be able to get to it for a few days)

@tgross35
Copy link
Collaborator

tgross35 commented Jan 21, 2024

This could actually also be related to the Windows timeout #67 (even though that started later)

@tgross35
Copy link
Collaborator

Also I have some sorta-helpful debug statements in scanner.c that can help trace this back, either compile with -DDEBUG_PRINT or uncomment https://github.com/tgross35/tree-sitter-just/blob/3e55519c162997eb1809d9c05a0e9aa073cf754d/src/scanner.c#L13

@ievgenii-shepeliuk
Copy link

Dear all,
Unfortunately I can't help on debugging / fixing this.

But the same time the plugin is unusable, so maybe it worth rolling it back to some usable state ?

I've been trying to "freeze" to particular commit in vim-plug to some older version, but there are always problems with complilation, even with year old commits.

Maybe you can suggest other ways to use older version. Thanks in advance.

@woojiq

This comment was marked as outdated.

@woojiq
Copy link

woojiq commented Jan 21, 2024

The easiest reproduction:

echo -n '"' > /tmp/Justfile && tree-sitter parse /tmp/Justfile -0

full-valgrind-report.txt

I'd be able to investigate more on the next weekends (I hope) if no one comes up with a solution faster.

@tgross35
Copy link
Collaborator

Thanks for the report! I’ll revert this when I’m back at a computer if I don’t spot an easy fix.

@tgross35
Copy link
Collaborator

This was reverted for now #72

@eshepelyuk
Copy link
Author

This was reverted for now #72

Hello, after this update - when opening simple justfile - I receive an error

test:
    echo "test"

Error

Error detected while processing BufReadPost Autocommands for "{.,}justfile\c"..FileType Autocommands for "*":
Error executing lua callback: ...5/share/nvim/runtime/lua/vim/treesitter/languagetree.lua:582: table index is nil
stack traceback:
        ...5/share/nvim/runtime/lua/vim/treesitter/languagetree.lua:582: in function 'add_injection'
        ...5/share/nvim/runtime/lua/vim/treesitter/languagetree.lua:708: in function 'f'
        ...5/share/nvim/runtime/lua/vim/treesitter/languagetree.lua:137: in function 'tcall'
        ...5/share/nvim/runtime/lua/vim/treesitter/languagetree.lua:283: in function 'parse'
        ...r/neovim/0.9.5/share/nvim/runtime/lua/vim/treesitter.lua:89: in function '_create_parser'
        ...r/neovim/0.9.5/share/nvim/runtime/lua/vim/treesitter.lua:131: in function 'get_parser'
        ...r/neovim/0.9.5/share/nvim/runtime/lua/vim/treesitter.lua:460: in function 'start'
        ...lugged/nvim-treesitter/lua/nvim-treesitter/highlight.lua:20: in function 'attach'
        .../plugged/nvim-treesitter/lua/nvim-treesitter/configs.lua:509: in function 'attach_module'
        .../plugged/nvim-treesitter/lua/nvim-treesitter/configs.lua:532: in function 'reattach_module'
        .../plugged/nvim-treesitter/lua/nvim-treesitter/configs.lua:133: in function <.../plugged/nvim-treesitter/lua/nvim-treesitter/configs.lua:132>
Error executing lua callback: ...5/share/nvim/runtime/lua/vim/treesitter/languagetree.lua:582: table index is nil
stack traceback:
        ...5/share/nvim/runtime/lua/vim/treesitter/languagetree.lua:582: in function 'add_injection'
        ...5/share/nvim/runtime/lua/vim/treesitter/languagetree.lua:708: in function 'f'
        ...5/share/nvim/runtime/lua/vim/treesitter/languagetree.lua:137: in function 'tcall'
        ...5/share/nvim/runtime/lua/vim/treesitter/languagetree.lua:283: in function 'parse'
        ...r/neovim/0.9.5/share/nvim/runtime/lua/vim/treesitter.lua:89: in function '_create_parser'
        ...r/neovim/0.9.5/share/nvim/runtime/lua/vim/treesitter.lua:131: in function 'get_parser'
        ...r/neovim/0.9.5/share/nvim/runtime/lua/vim/treesitter.lua:460: in function 'start'
        ...lugged/nvim-treesitter/lua/nvim-treesitter/highlight.lua:20: in function 'attach'
        .../plugged/nvim-treesitter/lua/nvim-treesitter/configs.lua:509: in function 'attach_module'
        .../plugged/nvim-treesitter/lua/nvim-treesitter/configs.lua:532: in function 'reattach_module'
        .../plugged/nvim-treesitter/lua/nvim-treesitter/configs.lua:133: in function <.../plugged/nvim-treesitter/lua/nvim-treesitter/configs.lua:132>
Error executing lua callback: ...5/share/nvim/runtime/lua/vim/treesitter/languagetree.lua:582: table index is nil
stack traceback:
        ...5/share/nvim/runtime/lua/vim/treesitter/languagetree.lua:582: in function 'add_injection'
        ...5/share/nvim/runtime/lua/vim/treesitter/languagetree.lua:708: in function 'f'
        ...5/share/nvim/runtime/lua/vim/treesitter/languagetree.lua:137: in function 'tcall'
        ...5/share/nvim/runtime/lua/vim/treesitter/languagetree.lua:283: in function 'parse'
        ...r/neovim/0.9.5/share/nvim/runtime/lua/vim/treesitter.lua:89: in function '_create_parser'
        ...r/neovim/0.9.5/share/nvim/runtime/lua/vim/treesitter.lua:131: in function 'get_parser'
        ...r/neovim/0.9.5/share/nvim/runtime/lua/vim/treesitter.lua:460: in function 'start'
        ...lugged/nvim-treesitter/lua/nvim-treesitter/highlight.lua:20: in function 'attach'
        .../plugged/nvim-treesitter/lua/nvim-treesitter/configs.lua:509: in function 'attach_module'
        .../plugged/nvim-treesitter/lua/nvim-treesitter/configs.lua:532: in function 'reattach_module'
        .../plugged/nvim-treesitter/lua/nvim-treesitter/configs.lua:133: in function <.../plugged/nvim-treesitter/lua/nvim-treesitter/configs.lua:132>

@eshepelyuk
Copy link
Author

Even more, I tried to uninstall and install just with TSUninstall / TSInstall commands
And I am currently receiving error during compliation

Failed to execute the following command:
{
  cmd = <function 1>
}
"...5/share/nvim/runtime/lua/vim/treesitter/languagetree.lua:582: table index is nil"

@tgross35
Copy link
Collaborator

Does pinning an old version work? Those errors don't look like they are from this repo.

@eshepelyuk
Copy link
Author

Does pinning an old version work? Those errors don't look like they are from this repo.

As I already mentioned, I can't make any older version work, even year old ones.
So I am currently completely unable to edit justfiles from nvim. Any suggestions are appreciated.

@tgross35
Copy link
Collaborator

The readme has and nvim-treesitter has instructions for using a local repo. Follow those and play around as needed. I would recommend trying an older version (specifically if 27b2d8a works, since it is the point before reverted commits) and trying with queries commented out (starting with injections.scm since it seems like it may be struggling there).

If you find a fix then I can review a PR, but my focus needs to be on adding back what was lost in the revert.

@eshepelyuk
Copy link
Author

eshepelyuk commented Jan 22, 2024

Sorry, you probably misunderstood me.

I don't want and I am unable to manually compile something locally, I just need a way how to make this pluging work.
Now, after latest revert - I expect it to work, but it still doesn't compile by NeoVim, so apparently latest revert still has some bugs preventing the normal installation.

@tgross35
Copy link
Collaborator

Sorry, you probably misunderstood me.

I don't want and I am unable to manually compile something locally, I just need a way how to make this pluging work.

Tree-sitter grammars always get compiled locally, regardless of whether nvim does it or NPM.

Now, after latest revert - I expect it to work, but it still doesn't compile by NeoVim, so apparently latest revert still has some bugs preventing the normal installation.

Unfortunately, I really just need to ask that you either pin an older version (which others have been able to do) or help by submitting a fix to help the situation (which can also be a revert of specific commits). I am not going to dig deep into getting this temporary revert to work when it doesn't even have correct string parsing. I would much rather work on getting the original issue solved.

@eshepelyuk
Copy link
Author

eshepelyuk commented Jan 22, 2024

Hello @tgross35

Unfortunately, I really just need to ask that you either pin an older version (which others have been able to do)

Unfortunately, as I mentioned few times already, I was unable to do this - because I am receiving those compilation errors when try to pin any previous version, even one year old.

So I am asking for any suggestion how can I make older version work for me, because right now I can't edit any of my justfiles :(

@woojiq
Copy link

woojiq commented Jan 22, 2024

because right now I can't edit any of my justfiles

Then don't use tree-sitter-just. You won't have syntax highlighting, that's it.

because I am receiving those compilation errors when try to pin any previous version

99% that the problem is not in tree-sitter-just because the author, me and probably a bunch of other people can compile it (CI works too). Try to find error in your vim plugin configuratoin or idk, I personally don't use nvim.

Sorry if this sounds aggressive, but you've already been explained several times and you're still trying to convince the author to solve your problem, which he can't even reproduce.

@eshepelyuk
Copy link
Author

The problem is that author doing changes without testing and breaking the plugin functionality completely.
I am not using any specific setup, it's just regular ubuntu with latest neovim from Homebrew, i bet a lot of people are using the same.

And starting from New Year approx, I am only doing bug reports that plugin stopped working because author is not doing proper testing and alwasy just replies that he can't reproduce in his setup.

I am jsut trying to ask for getting some stable state like it was before intorducing those breaking changes
And that state jsut be tested on very widely used setup of Ubuntu + NeoVim from homebrew.

I had zero issues with this plugin before those breaking changes were introduced in the end of previous year.
So I do have a feeling that the reason is that plugin has not enough testing on simple old Ubuntu.

While I can pin any other plugin to previous version if they crash, I can't do the same with only this plugin.
And I don't even see an intention to provide any help with pinning old version.

@tgross35
Copy link
Collaborator

This is frustrating. I need to remind you of a few things:

  1. I have not been able to reproduce some of these issues. The segfault was not showing up on my machine (aarch64), leading me to believe it is primarily x86, and it does not show up with tree-sitter-cli (which is run in CI). Please appreciate how this makes these things extremely hard to track down.

  2. This project did not have any CI testing at all until I started working on it this month, before which highlighting was completely broken for plenty of things. I also do try it out on local with both Helix and neoVim but cannot guarantee 100% coverage. Please do not accuse me of not doing proper testing.

  3. The only highlighter that can be easily tested in CI is tree-sitter-cli, and this project has much more complete highlighting than many other TS grammars. I have stated multiple times that I would love to test editors in CI (to which you are more than welcome to add your configuration) but I need help getting it over the line. link.

  4. Unfortunately, as I mentioned few times already, I was unable to do this - because I am receiving those compilation errors when try to pin any previous version, even one year old.

    How do you expect me to help you? If there is no point along this project's history ever worked for you, then why do you expect top of tree to work? I want something that "just works" as much as you do, but it takes effort to get there. And so far that has been my effort.

    If you are on MacOS, pin a version after I changed scaner.cc to scanner.c Scanner (src/scanner.cc) fails to build on MacOS #23.

  5. The readme makes it clear that this project is a work in progress. You are not obligated to use the top of main. I tried to be helpful with a revert, but I am not going to start maintaining a LTS version of this plugin. If that does not work for you and you cannot pin a version, use the more stable but less correct (for Just) tree-sitter-make.

The license states Licensor provides the Work (and each Contributor provides its Contributions) on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND. This is FOSS: if you do not appreciate how I am doing my contributions, you are welcome to bring your own. I realize that sounds harsh, but I am imperfect and my time is limited so you need to lower your expectations.

I do not intend to comment more on this thread as it is nonproductive. Submit PRs, wait patiently until I get a chance to fix everything, or do not use this plugin.

@tgross35
Copy link
Collaborator

This segfault is proving extremely difficult to track down. If anyone has ideas, I'm open to them. Details:

  1. I was finally able to reproduce this issue on a different machine
  2. I reimplemented the improved string parser and have it on the next branch (permalink), but it still has this segfault
  3. This issue appears x86-only and does not reliably reproduce in CI.
  4. Error is sometimes SIGBUS sometimes SIGSEGV, makes me think that there is a misaligned read somewhere
  5. The actual error happens within tree-sitter's advance function, more details here Bus error within regexec tree-sitter/tree-sitter#2876
  6. I can't reproduce this with a simple parser, debug.c in this branch https://github.com/tgross35/tree-sitter-just/tree/1bce36f0243523a2669d22c8a29f6e7d31297d6a (just debug-build then LD_LIBRARY_PATH=tree-sitter-src/ ./debug.out '\nfoo := "abc"\n' to run).
  7. This means I can only reproduce the issue with tree-sitter test --debug-build -f 'assignment' (or an editor), which seems to test with an optimized and stripped binary despite the --debug-build flag. So my stack trace uselessly jumps directly from just.so to libc.so and I can't debug anything.

All of this is sort of pointing me to an issue that might be fixed upstream, since lexer->advance doesn't seem like it should ever be a point of failure. But I have no clue why it hasn't been reported before.

If anyone has any ideas, I could use a few 🙂

@tgross35
Copy link
Collaborator

Actually it looks like TS does indeed pass -g regardless of the debug-build flag, so I'm not sure why I have no debug info https://github.com/tree-sitter/tree-sitter/blob/660481dbf71413eba5a928b0b0ab8da50c1109e0/cli/loader/src/lib.rs#L450-L467

@tgross35
Copy link
Collaborator

tgross35 commented Mar 3, 2024

Has anybody run into this recently? The scanner changes from #79 were merged a couple weeks ago, which hopefully fixed things.

@aleprovencio
Copy link

It's working fine here, thanks for you efforts @tgross35

@tgross35
Copy link
Collaborator

tgross35 commented Mar 3, 2024

Thanks for the report :) I'll close this, please feel free to open a new issue if there are further problems

@tgross35 tgross35 closed this as completed Mar 3, 2024
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

No branches or pull requests

6 participants