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

Add anytype resolution based on call references #1067

Merged
merged 6 commits into from
Mar 31, 2023
Merged

Conversation

SuperAuguste
Copy link
Member

This branch should've been a more comprehensive analysis rewrite but I got burnt out and decided to just pile on some more code debt instead.

@SuperAuguste SuperAuguste marked this pull request as draft March 15, 2023 15:37
@SuperAuguste
Copy link
Member Author

Here's an example:

Screenshot 2023-03-15 225018

@SuperAuguste SuperAuguste marked this pull request as ready for review March 17, 2023 19:25
@SuperAuguste
Copy link
Member Author

Going to merge this as-is because it's just really useful and I'll add hover/goto/deduplication when I have time in the future :)

@SuperAuguste SuperAuguste added the pr:fuzz Attach to a PR to start fuzzing / continually fuzz (please do this before merging!) label Mar 17, 2023
@leecannon
Copy link
Member

Still trying to get a nice simple replication.

Segmentation fault at address 0x7f2717bda5b0
???:?:?: 0x81bd70 in ??? (???)
/home/lee/src/zls/src/analysis.zig:1183:33: 0x44f4c1 in getAllTypesWithHandlesArrayList (zls)
            .either => |e| for (e) |i| try i.type_with_handle.getAllTypesWithHandlesArrayList(arena, all_types),
                                ^
/home/lee/src/zls/src/analysis.zig:1183:94: 0x44f4f4 in getAllTypesWithHandlesArrayList (zls)
            .either => |e| for (e) |i| try i.type_with_handle.getAllTypesWithHandlesArrayList(arena, all_types),
                                                                                             ^
/home/lee/src/zls/src/analysis.zig:1177:47: 0x44f6a6 in getAllTypesWithHandles (zls)
        try ty.getAllTypesWithHandlesArrayList(arena, &all_types);
                                              ^
/home/lee/src/zls/src/analysis.zig:1429:98: 0x452a3d in getFieldAccessType (zls)
                        const current_type_nodes = try deref_type.getAllTypesWithHandles(analyser.arena);
                                                                                                 ^
/home/lee/src/zls/src/inlay_hints.zig:207:72: 0x541ea7 in writeCallNodeHint (zls)
            if (try builder.analyser.getFieldAccessType(handle, rhs_loc.end, &tokenizer)) |result| {
                                                                       ^
/home/lee/src/zls/src/inlay_hints.zig:251:34: 0x4c49d1 in writeNodeInlayHint (zls)
            try writeCallNodeHint(builder, call);
                                 ^
/home/lee/src/zls/src/ast.zig:1535:25: 0x5d6ba0 in recursive_callback (zls)
            try callback(ctx, ast, child_node);
                        ^
/home/lee/src/zls/src/ast.zig:1292:56: 0x54308e in iterateChildren__anon_49331 (zls)
            try callback(context, tree, node_data[node].lhs);
                                                       ^
/home/lee/src/zls/src/ast.zig:1536:32: 0x5d6bd1 in recursive_callback (zls)
            try iterateChildren(ast, child_node, ctx, Error, recursive_callback);
                               ^
/home/lee/src/zls/src/ast.zig:1292:56: 0x54308e in iterateChildren__anon_49331 (zls)
            try callback(context, tree, node_data[node].lhs);
                                                       ^
/home/lee/src/zls/src/ast.zig:1536:32: 0x5d6bd1 in recursive_callback (zls)
            try iterateChildren(ast, child_node, ctx, Error, recursive_callback);
                               ^
/home/lee/src/zls/src/ast.zig:1442:47: 0x5445dd in iterateChildren__anon_49331 (zls)
            try callback(context, tree, if_ast.then_expr);
                                              ^
/home/lee/src/zls/src/ast.zig:1536:32: 0x5d6bd1 in recursive_callback (zls)
            try iterateChildren(ast, child_node, ctx, Error, recursive_callback);
                               ^
/home/lee/src/zls/src/ast.zig:1311:29: 0x543322 in iterateChildren__anon_49331 (zls)
                try callback(context, tree, child);
                            ^
/home/lee/src/zls/src/ast.zig:1536:32: 0x5d6bd1 in recursive_callback (zls)
            try iterateChildren(ast, child_node, ctx, Error, recursive_callback);
                               ^
/home/lee/src/zls/src/ast.zig:1442:47: 0x5445dd in iterateChildren__anon_49331 (zls)
            try callback(context, tree, if_ast.then_expr);
                                              ^
/home/lee/src/zls/src/ast.zig:1536:32: 0x5d6bd1 in recursive_callback (zls)
            try iterateChildren(ast, child_node, ctx, Error, recursive_callback);
                               ^
/home/lee/src/zls/src/ast.zig:1292:56: 0x54308e in iterateChildren__anon_49331 (zls)
            try callback(context, tree, node_data[node].lhs);
                                                       ^
/home/lee/src/zls/src/ast.zig:1536:32: 0x5d6bd1 in recursive_callback (zls)
            try iterateChildren(ast, child_node, ctx, Error, recursive_callback);
                               ^
/home/lee/src/zls/src/ast.zig:1442:47: 0x5445dd in iterateChildren__anon_49331 (zls)
            try callback(context, tree, if_ast.then_expr);
                                              ^
/home/lee/src/zls/src/ast.zig:1536:32: 0x5d6bd1 in recursive_callback (zls)
            try iterateChildren(ast, child_node, ctx, Error, recursive_callback);
                               ^
/home/lee/src/zls/src/ast.zig:1311:29: 0x543322 in iterateChildren__anon_49331 (zls)
                try callback(context, tree, child);
                            ^
/home/lee/src/zls/src/ast.zig:1536:32: 0x5d6bd1 in recursive_callback (zls)
            try iterateChildren(ast, child_node, ctx, Error, recursive_callback);
                               ^
/home/lee/src/zls/src/ast.zig:1423:50: 0x5442f0 in iterateChildren__anon_49331 (zls)
            try callback(context, tree, while_ast.then_expr);
                                                 ^
/home/lee/src/zls/src/ast.zig:1536:32: 0x5d6bd1 in recursive_callback (zls)
            try iterateChildren(ast, child_node, ctx, Error, recursive_callback);
                               ^
/home/lee/src/zls/src/ast.zig:1311:29: 0x543322 in iterateChildren__anon_49331 (zls)
                try callback(context, tree, child);
                            ^
/home/lee/src/zls/src/ast.zig:1536:32: 0x5d6bd1 in recursive_callback (zls)
            try iterateChildren(ast, child_node, ctx, Error, recursive_callback);
                               ^
/home/lee/src/zls/src/ast.zig:1465:60: 0x544a51 in iterateChildren__anon_49331 (zls)
                try callback(context, tree, node_data[node].rhs);
                                                           ^
/home/lee/src/zls/src/ast.zig:1540:24: 0x4c4cf7 in iterateChildrenRecursive__anon_38552 (zls)
    try iterateChildren(tree, node, context, Error, RecursiveContext.recursive_callback);
                       ^
/home/lee/src/zls/src/inlay_hints.zig:307:48: 0x42c8e0 in writeRangeInlayHint (zls)
        try ast.iterateChildrenRecursive(handle.tree, child, &builder, error{OutOfMemory}, writeNodeInlayHint);
                                               ^
/home/lee/src/zls/src/Server.zig:2737:16: 0x3f2829 in inlayHintHandler (zls)
        &server.analyser,
               ^
/home/lee/src/zls/src/Server.zig:3172:37: 0x3ad963 in processMessage (zls)
            const response = handler(server, params) catch |err| {
                                    ^
/home/lee/src/zls/src/Server.zig:3026:26: 0x35ac7a in processJsonRpc (zls)
    server.processMessage(message) catch |err| switch (message) {
                         ^
/home/lee/src/zls/src/main.zig:80:30: 0x35a43b in loop (zls)
        server.processJsonRpc(json_message);
                             ^
/home/lee/src/zls/src/main.zig:395:13: 0x35b808 in main (zls)
    try loop(server, record_file, replay_file);
            ^
/home/lee/zig/0.11.0-dev.1987+a2c6ecd6d/files/lib/std/start.zig:617:37: 0x343c5e in posixCallMainAndExit (zls)
            const result = root.main() catch |err| {
                                    ^
/home/lee/zig/0.11.0-dev.1987+a2c6ecd6d/files/lib/std/start.zig:376:5: 0x3436c1 in _start (zls)
    @call(.never_inline, posixCallMainAndExit, .{});
    ^

@SuperAuguste
Copy link
Member Author

Oh god, I got that one too - I think this is the stack overflow I ran into; thought I fixed it by disabling crossrefs. Thanks for the heads up!

@leecannon
Copy link
Member

leecannon commented Mar 17, 2023

I'm not 100% sure this is actually a stack overflow (unless it is very deep) as even setting the stack size to 1GiB still hits this issue.

Unless exe.stack_size = 1 * 1024 * 1024 * 1024; // 1 GiB in build.zig doesn't work?

Checked with getrlimit and it confirms that the stack size is set to 1GiB but the issue still occurs.

@leecannon
Copy link
Member

Valgrind isn't happy about this:

==29565== Invalid read of size 1
==29565==    at 0x81BD70: memcpy (in /home/lee/src/zls/zig-out/bin/zls)
==29565==    by 0x44F4C1: analysis.TypeWithHandle.getAllTypesWithHandlesArrayList (analysis.zig:1183)
==29565==    by 0x44F4F4: analysis.TypeWithHandle.getAllTypesWithHandlesArrayList (analysis.zig:1183)
==29565==    by 0x44F6A6: analysis.TypeWithHandle.getAllTypesWithHandles (analysis.zig:1177)
==29565==    by 0x452A3D: analysis.getFieldAccessType (analysis.zig:1429)
==29565==    by 0x541EA7: inlay_hints.writeCallNodeHint (inlay_hints.zig:207)
==29565==    by 0x4C49D1: inlay_hints.writeNodeInlayHint (inlay_hints.zig:251)
==29565==    by 0x5D6BA0: ast.iterateChildrenRecursive__anon_38552.RecursiveContext.recursive_callback (ast.zig:1535)
==29565==    by 0x54308E: ast.iterateChildren__anon_49331 (ast.zig:1292)
==29565==    by 0x5D6BD1: ast.iterateChildrenRecursive__anon_38552.RecursiveContext.recursive_callback (ast.zig:1536)
==29565==    by 0x54308E: ast.iterateChildren__anon_49331 (ast.zig:1292)
==29565==    by 0x5D6BD1: ast.iterateChildrenRecursive__anon_38552.RecursiveContext.recursive_callback (ast.zig:1536)
==29565==  Address 0x525e090 is not stack'd, malloc'd or (recently) free'd

@Techatrix
Copy link
Member

I'm think this is a use after free that I've introduced in #1062. resolved_nodes caches the result of resolveTypeOfNode which may store []const EitherEntry that got allocated with the arena allocator. Once its get reset, you will read from freed memory when the cached result is re-used. NOTE I didn't verify this so I may be wrong about this.
The easiest fix I could think of would be that the Analyser uses its own arena that only gets reset once Analyser.invalidate is called.

@leecannon
Copy link
Member

After rebasing this on master and correcting the import in analysis.zig, I'm strugling to recreate the crash from before.

@SuperAuguste
Copy link
Member Author

Hey @leecannon, could you push those changes onto this branch? Thanks :)

I'll look into your changes and how they interact with mine in a bit @Techatrix, thanks for the heads up!

@leecannon
Copy link
Member

Sorry spoke too soon, still getting it :(

@leecannon
Copy link
Member

I'm strugling to make a simple repro of this, the only file I get this issue with is groups.zig and the error only occurs when I scroll.

@SuperAuguste
Copy link
Member Author

I'm going to take on deduplication before merging this :)

@SuperAuguste
Copy link
Member Author

SuperAuguste commented Mar 30, 2023

Tada! Ready to merge (pending review).

Also multi gotodef works now:
image

@SuperAuguste SuperAuguste requested a review from leecannon March 30, 2023 20:34
@SuperAuguste SuperAuguste requested a review from Techatrix March 30, 2023 20:34
Copy link
Member

@leecannon leecannon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Features like this fill me with joy.

src/features/references.zig Outdated Show resolved Hide resolved
@SuperAuguste SuperAuguste merged commit 8b5c649 into master Mar 31, 2023
@SuperAuguste SuperAuguste deleted the nouvelle-analyse branch March 31, 2023 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:fuzz Attach to a PR to start fuzzing / continually fuzz (please do this before merging!)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants