Skip to content

VS Code: After installation, diag showed on first token #718

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

Closed
strager opened this issue May 6, 2022 · 7 comments
Closed

VS Code: After installation, diag showed on first token #718

strager opened this issue May 6, 2022 · 7 comments
Assignees
Labels
false positive Severe bug: quick-lint-js rejects valid code

Comments

@strager
Copy link
Collaborator

strager commented May 6, 2022

https://clips.twitch.tv/SparklingDignifiedCougarResidentSleeper-RHRLrGb3lgZtUhVu

@strager strager added the false positive Severe bug: quick-lint-js rejects valid code label May 6, 2022
@strager strager self-assigned this May 20, 2022
@strager
Copy link
Collaborator Author

strager commented May 20, 2022

I couldn't reproduce the issue on VS Code 1.66.0 or 1.67.2 on Linux x86_64.

@SidneyNemzer
Copy link

SidneyNemzer commented May 20, 2022

I think this is a minimal repro (different error):

qljs-repro.mp4
  1. Create a file with the content const a = a;
  2. Verify that QLJS errors on the last a
  3. Reload the window
  4. Actual: QLJS errors on const until an update is triggered
    Expected: the error is still on the last a

I used F1 > Start Extension Bisect to verify that other extensions didn't influence the bug. Not sure if there's an easy way to disable all extensions except one.

Version: 1.67.2 (system setup)
Commit: c3511e6c69bb39013c4a4b7b9566ec1ca73fc4d5
Date: 2022-05-17T18:15:52.058Z
Electron: 17.4.1
Chromium: 98.0.4758.141
Node.js: 16.13.0
V8: 9.8.177.13-electron.0
OS: Windows_NT x64 10.0.19043

@strager
Copy link
Collaborator Author

strager commented May 20, 2022

I can repro on Windows with VS Code 1.67.2 (x64) using @SidneyNemzer's method.

@strager
Copy link
Collaborator Author

strager commented May 20, 2022

I tracked it down to a call to napi_create_double creating a strange napi_value. More investigation needed.

@strager
Copy link
Collaborator Author

strager commented May 21, 2022

The xmm1 register (double parameter of napi_create_double) gets corrupted by a function called by __delayLoadHelper2. The trampoline generated by GNU binutils' dlltool does not preserve xmm1, despite xmm1 being a volatile register according to the Windows x64 ABI.

I think the fix is to save xmm1 and other volatile SSE and AVX registers before calling __delayLoadHelper2. I have tried to build binutils, but it looks like the latest version generates archives which crash at run-time. I don't know the exact reasons for the crash yet.

@strager
Copy link
Collaborator Author

strager commented May 21, 2022

Patching dlltool fixed the issue. I emailed my tentative patch to upstream.

We still need to apply a workaround in quick-lint-js, since we won't get a newer dlltool for a while.

@strager
Copy link
Collaborator Author

strager commented May 22, 2022

Fixed in commit 40d1291.

@strager strager closed this as completed May 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
false positive Severe bug: quick-lint-js rejects valid code
Projects
None yet
Development

No branches or pull requests

2 participants