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

Duplicate table key triggers assert in new solver #1540

Open
nothing1649 opened this issue Nov 20, 2024 · 4 comments
Open

Duplicate table key triggers assert in new solver #1540

nothing1649 opened this issue Nov 20, 2024 · 4 comments
Assignees
Labels
bug Something isn't working new solver This issue is specific to the new solver.

Comments

@nothing1649
Copy link

nothing1649 commented Nov 20, 2024

As of version 0.652, the following code crashes the new solver, regardless of strict mode being used:

--!strict
type dict = {
    read vals: {[string]:number}
}

local _:dict = {
    vals = {
        key = 1,
        key = 1
    }
}

image

Relevant assert:
https://github.com/luau-lang/luau/blob/7a6142e792eebae628916220b58faf7465d3ff59/Analysis/src/TableLiteralInference.cpp#L246C1-L246C57

Worth noting is that on my PC running Arch (Intel i5-10400F) and laptop (Intel i5-12450H) the code crashed luau-analyze every time, however when using the prebuilt binaries for Windows it only crashed some of the time; on my PC it crashed about 50% of the time but on my laptop it was more like 10% of the time.

@nothing1649 nothing1649 added the bug Something isn't working label Nov 20, 2024
@hgoldstein hgoldstein added the new solver This issue is specific to the new solver. label Nov 20, 2024
@hgoldstein
Copy link
Contributor

Thanks for the report! Definitely new solver, definitely a bug.

@hgoldstein hgoldstein self-assigned this Nov 20, 2024
@hgoldstein
Copy link
Contributor

It's not a race condition, the issue is that matchLiteralType (in TableLiteralInference) mutates types in place. We want to transform { key: number } into { [string]: number }. We traverse over the table members, and when we get to key = 1 we actually clip that part of the type ... but when we come back to the next key = 1 we panic (assert) because we expect all of the literal members to be represented in the type.

@nothing1649
Copy link
Author

got it, will remove mentions of it

@nothing1649 nothing1649 changed the title Duplicate table key triggers assert in new solver; possible race condition Duplicate table key triggers assert in new solver Nov 20, 2024
@hgoldstein
Copy link
Contributor

Oh no worries there! I was commenting as a note for myself (or someone else if I don't get to this). It's a good guess: the LUAU_ASSERT is guarding against memory corruption, which kind of looks like a race condition (maybe one can think of it as a race condition against the operating system).

The prebuilt binaries are built in release mode which turns off asserts. You might see similar behavior for your example (crashing some percentage of the time but not always) if you build from source on Linux and pass -DCMAKE_BUILD_TYPE=Release to cmake.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working new solver This issue is specific to the new solver.
Development

No branches or pull requests

2 participants