-
Notifications
You must be signed in to change notification settings - Fork 60
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
[Bug]: VSCode gives error when formatting #1218
Comments
Hello @simonjwright. Thanks for reporting this. I was not able to reproduce on Linux. I suspect that this will be specific to the aarch64 ALS build. I'll try to reproduce on a Mac. I have a couple questions though.
What exactly was compiled here? ALS?
Can you elaborate on what |
Sorry to be unclear. I don’t know how the stack dump in my bug report was produced, but - and I assume here it’s from a caught exception? - if you compile with
This is raiser's GPR
This is the source
and this is the execution and stack dump processing
|
Same issue:
|
I see a lot of Note that in parallel we will try to make GNATformat working on Ada files that do not belong to the loaded project, which would solve these issues in the long-term. Regards, |
@rongcuid Note that I can reproduce on my side on Mac OS aarch64. We'll investigate. |
I don't know why this is the case. |
Do you launch VS Code from your terminal, or outside of it? If you launch it outside of your terminal, you might want to have a look at this |
Ok. I didn't know vscode can do that now. It does remove the error about |
Great! We'll investigate the erroneous memory access on our side, keeping you posted. |
@simonjwright I confirm that it's a aarch64 Mac OS issue: I can't reproduce with the extension built for x86-64 Mac OS. I think you have worked on the aarch64 port for Mac OS right? Unfortunately at AdaCore we don't officially support this platform, so it will be hard for us to fix that issue, which looks like a pure compiler issue (the same code runs fine on Mac OS x86-64, Linux platforms and Windows). Would you be able to investigate on your side? That would greatly help us and the Ada/Mac OS community. If not we'll see internally how we can proceed. Regards, |
@AnthonyLeonardoGracio, I will certainly have a go. I’ll need to rebuild the compiler first (so I can decode those tracebacks, see GCC PR 117358). Would the GCC 14.2 compiler be an appropriate base? |
I've built ALS (25.0.0) with my fixed compiler, and it's working, but how do I get VSCode to use it? GNATFormat 25.0.0 works fine with my test code, see above. VSCode is OK so long as I configure not to use gnatformat (i.e., use gnatpp). I've managed to start ALS with
but it seems to have no effect: when I formatted the file, I tried replacing 'true' with 'maybe', and ALS refused to start up, so it's getting read. I tried |
Found the location:
I’ll put my rebuilt version in there. |
Great @simonjwright ! Is the fix already available via your personal Alire index? Because a temporary solution for us would be to use your index to build the Ada Language Server for Mac OS aarch64, until the fix gets propagated in Alire's community index: would that be ok for you? |
@AnthonyLeonardoGracio , the point of that compiler is so that I can find out where the exceptions are being raised (on macOS, the standard So it wouldn’t help anyone. However! I think I’ve found the issue; it’s in Libadalang. Using the patched compiler, I found that the exception is in
because In
(returning the object to the In the procedure just above it,
so if we pop a rebinding off the So, I patched in This change I could put in my personal Alire index. So, this would explain why the error happens with the gnatformat option in ALS, because it’s built over Libadalang - well, so is gnatpp, but the usage pattern is probably different? Why does the problem show up on the aarch64 build and not on the x86_64 build? I have no idea. If I’m right and this is a bug, it’d mean that the x86_64 build never returned a rebinding to pool, which seems strange. |
I thought to check whether there was a problem with other pointers to the now-stale rebinding; I arranged for I can see this might affect performance (how much?), but it seems to work OK for both VSCode and ada-ts-mode; no errors reported so far. |
Hello @simonjwright, If I understand correctly what you meant, this is not going in the right direction: the goal of the safety net mechanism is to detect (and error out) when a Libadalang user calls a Libadalang subprogram passing an entity to it when that entity no longer exists (root cause: because it somehow refers to a node that does not exist due to reparsing). To achieve that at a reasonable cost, rebindings records are allocated on demand, and then never deallocated. The goal of never deallocating them is to completely avoid dangling pointers (or worse: user after free+alloc). Yet to avoid leaking memory, we reuse allocated rebindings when they were disposed (i.e. when they were marked as stale), so that we do not allocate a new record if there still one that is available. The second stage for this mechanism is rejecting uses of reference to a rebinding when they are stale. To achieve this, rebindings records have a version number set to 0 initially, and then that version number is bumbed each time the rebinding is marked stale: any attempt to use this disposed (or repurposed) rebinding while the old purpose was meant can be turned into an exception. Unfortunately, both of your suggestions defeat these goals:
The fact that you are getting this error on one arch but not the other is indeed concerning, but we need to get to the bottom of this (really understand what is going on) to fix the actual bug. It would be much easier if we could reproduce this ourselves. |
If I replace the aarch64 ALS installed under ~/.vscode with the x86_64 ALS downloaded from here, I no longer have a problem. Nor do I have a problem using an Alire 25.0.0 ALS built for x86_64. So clearly there’s an arch-related problem (sorry to have doubted this :-) Immediate lines of investigation:
Since we don’t have an Ada-aware debugger for aarch64 Darwin, progress is likely to be slow, but at least users have a way ahead as I noted at the top. As a matter of interest, if a stale rebinding is reused, at what point does |
I’m not sure I understand precisely your question, let me know if you wanted to know something else: When a Libadalang function returns a node (which includes a reference to rebindings), we create a safety net for it that keeps track of the rebindings version: that’s the safety net Once this is done, the safety net in the returned node is read-only: when rebindings are invalidated, it is |
OK, things are much clearer now. Using the unpatched source (i.e. no messing with the standard safety net stuff), we have on macOS
Since lal_tools built OK after removing In at least one case I saw a claim that this switch would allow using the () aggregate syntax without generating a warning; that’s not so. I’d like to add that the only way to get the successful build was to compile with
so making people using released code override the warning seems counter-effective. |
Environment
Bug Summary and Reproducer
Bug Summary:
$ ./raiser
caught raised CONSTRAINT_ERROR : a message
Load address: 0x10261c000
Call stack traceback locations:
0x102620a7c 0x102620adc 0x10261eb54 0x10261eb7c 0x10261eb7c 0x10261eb7c 0x10261eb7c 0x10261eb7c 0x10261ebc0 0x10261eacc
$ atos -o raiser -l 0x10261c000 0x102620a7c 0x102620adc 0x10261eb54 0x10261eb7c 0x10261eb7c 0x10261eb7c 0x10261eb7c 0x10261eb7c 0x10261ebc0 0x10261eacc
ada__exceptions__complete_and_propagate_occurrence (in raiser) (a-except.adb:1128)
__gnat_raise_exception (in raiser) (a-except.adb:1165)
raiser__inner.0 (in raiser) (raiser.adb:7)
raiser__inner.0 (in raiser) (raiser.adb:9)
raiser__inner.0 (in raiser) (raiser.adb:9)
raiser__inner.0 (in raiser) (raiser.adb:9)
raiser__inner.0 (in raiser) (raiser.adb:9)
raiser__inner.0 (in raiser) (raiser.adb:9)
_ada_raiser (in raiser) (raiser.adb:12)
main (in raiser) (b~raiser.adb:227)
The text was updated successfully, but these errors were encountered: