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

new Array(2 ** 32) makes let and const declarations that can't be initialized. #53356

Closed
1Git2Clone opened this issue Jun 5, 2024 · 18 comments
Closed
Labels
duplicate Issues and PRs that are duplicates of other issues or PRs. repl Issues and PRs related to the REPL subsystem.

Comments

@1Git2Clone
Copy link

Version

v22.2.0

Platform

Linux hutao 6.9.3-arch1-1 #1 SMP PREEMPT_DYNAMIC Fri, 31 May 2024 15:14:45 +0000 x86_64 GNU/Linux

Subsystem

No response

What steps will reproduce the bug?

 node
Welcome to Node.js v22.2.0.
Type ".help" for more information.
let x = new Array(2 ** 32); // or const, also works the same, `var` is not affected.

How often does it reproduce? Is there a required condition?

Consistently reproducable via making a variable which stores a new Array that has a length higher than the limit (2^32 - 1).

What is the expected behavior? Why is that the expected behavior?

The same way the chromium developer console handles it by disallowing the declaration of the variable all together.

let x = new Array(2**32);
Uncaught RangeError: Invalid array length
    at <anonymous>:1:9
let x = 1;
undefined
console.log(x);
1
undefined

I mean it only makes sense, especially in const where it'd be weird to have a constant that's not defined (despite it being in JavaScript).

Although to be honest this is also an issue with const values even in Firefox...

Chromium - handles it correctly because it's not allowing for undefined const values

const y = new Array(2**32);
Uncaught RangeError: Invalid array length
    at <anonymous>:1:11
typeof(y)
Uncaught ReferenceError: y is not defined
    at <anonymous>:1:1
const y = 1;
undefined
typeof(y)
'number'

Firefox - how can you have an undefined const?

const x = new Array(2**32);
Uncaught RangeError: invalid array length
    <anonymous> debugger eval code:1
const x = 1;
Uncaught SyntaxError: redeclaration of const x
    <anonymous> debugger eval code:1
typeof(x)
"undefined" 

What do you see instead?

 node
Welcome to Node.js v22.2.0.
Type ".help" for more information.
> let x = new Array(2 ** 32);
Uncaught RangeError: Invalid array length
> console.log(x)
Uncaught ReferenceError: x is not defined
> x = 1
Uncaught ReferenceError: Cannot access 'x' before initialization
> console.log(x)
Uncaught ReferenceError: x is not defined
> typeof(x)
Uncaught ReferenceError: x is not defined
> let x = 1
Uncaught SyntaxError: Identifier 'x' has already been declared
> var y = new Array(2**32);
Uncaught RangeError: Invalid array length
> const z = new Array(2**32);
Uncaught RangeError: Invalid array length
> typeof(y)
'undefined'
> typeof(z)
Uncaught ReferenceError: z is not defined
> console.log(y)
undefined
undefined
> console.log(y)
undefined
undefined
> console.log(z)
Uncaught ReferenceError: z is not defined

Undefined const variable: z as well as a declared (but inaccessible) let variable: x.

Additional information

No response

@RedYetiDev
Copy link
Member

Hi! That part of JavaScript is handled by V8, so if you have an issue with it, I'd recommend you report it there

CC @nodejs/v8 to make sure I'm giving the correct information

@1Git2Clone
Copy link
Author

Thank you for the information, I'll make sure to submit the issue there! Feel free to close the issue if there's nothing you want to reply with and have a good day/evening!

@1Git2Clone
Copy link
Author

1Git2Clone commented Jun 5, 2024

https://github.com/nodejs/v8

Wait, the repository is a public archive, where do I need to head to in order to create the issue?

Edit: Oh, it's a mirror for chromium's V8 engine... got it.

@Trott
Copy link
Member

Trott commented Jun 5, 2024

https://github.com/nodejs/v8

Wait, the repository is a public archive, where do I need to head to in order to create the issue?

https://issues.chromium.org/p/v8/issues/entry

@Trott
Copy link
Member

Trott commented Jun 5, 2024

I'm not actually sure this should be opened in the V8 repository, though. It sounds like you're suggesting that the Node.js REPL should act like the Chrome console. That's (probably) not a V8 issue but something Node.js would handle in its REPL code (or we should at least explain why we don't/can't do that).

@Trott
Copy link
Member

Trott commented Jun 5, 2024

@nodejs/repl

@Trott
Copy link
Member

Trott commented Jun 5, 2024

(Although I guess if the Firefox REPL has the same behavior Node.js does, then maybe there's a case to be made for "We're not going to worry about this until the browsers figure out what the right thing to do is." And if they never do, that's probably OK. This is definitely an edge case. I can't imagine it affects many users.)

@RedYetiDev
Copy link
Member

FWIW, it’s not REPL exclusive, and the same behavior happens in scripts like:

try{
    const x = new Array(2**32);
} catch(e) {}
x = 1
console.log(x)

@Trott
Copy link
Member

Trott commented Jun 5, 2024

FWIW, it’s not REPL exclusive, and the same behavior happens in scripts like:

try{
    const x = new Array(2**32);
} catch(e) {}
x = 1
console.log(x)

@RedYetiDev I think you're misunderstanding the bug report but maybe @1Git2Clone can confirm.

2 ** 32 exceeds the array length limit in JavaScript, so an error is expected.

The problem is the behavior in the REPL that happens after the error is thrown.

@Trott
Copy link
Member

Trott commented Jun 5, 2024

The problem is the behavior in the REPL that happens after the error is thrown.

Specifically, it prohibits you from trying to declare the const again. I'm not 100% sure I agree that this is a bug. This might just be a case where the REPL has to figure out how to handle something novel during interaction, and different REPLs might do different things, and nobody is "wrong".

I don't think the Array() part has anything to do with it. I think this is likely to happen with any assignment that throws an error. But I haven't tested.

And if I'm totally wrong and I'm the one misunderstanding things, my apologies!

@RedYetiDev
Copy link
Member

RedYetiDev commented Jun 5, 2024

Oh, I misunderstood, my bad.

Also, I can't reproduce

Welcome to Node.js v22.2.0.
Type ".help" for more information.
> let x = new Array(2 ** 32);
Uncaught RangeError: Invalid array length
> let x = 1;
Uncaught SyntaxError: Identifier 'x' has already been declared
> 

@RedYetiDev RedYetiDev added the repl Issues and PRs related to the REPL subsystem. label Jun 5, 2024
@Trott
Copy link
Member

Trott commented Jun 5, 2024

Probably a duplicate of #8309?

@Trott
Copy link
Member

Trott commented Jun 5, 2024

Also, I can't reproduce

Welcome to Node.js v22.2.0.
Type ".help" for more information.
> let x = new Array(2 ** 32);
Uncaught RangeError: Invalid array length
> let x = 1;
Uncaught SyntaxError: Identifier 'x' has already been declared
> 

You totally reproduced it, at least as I understand it!

The request here (as I understand it) is that we behave like the Chrome console, which will let you try again with a second let x if the first one throws an error. This is different from what happens in a script file, and the argument is that the interactive console should be lenient about that. (Otherwise, a typo in a declaration might mean you have to start all over again. I've certainly experienced this many times myself...)

Screenshot 2024-06-05 at 1 11 39 PM

I'm not 100% sure that's the only "right" behavior here but it does seem like "emulate Chromium console to the extent possible/useful" is a good high-level heuristic for the REPL so maybe?

@RedYetiDev
Copy link
Member

Oh, I thought it was the exact opposite haha

@RedYetiDev
Copy link
Member

RedYetiDev commented Jun 5, 2024

FWIW It's also reproducible in #52965, which means it's part of the makeContextifyScript call, so it might be a VM issue. I can try and make it a goal of the experimental REPL, but it might be a VM problem, so CC @nodejs/vm

@1Git2Clone
Copy link
Author

I want to send an update to the participants here, I've taken the report to Chromium already

https://issues.chromium.org/issues/345248266

And they marked it as a medium severity priority 2 (P2) issue. If anyone is curious they can continue tracking it on there.

Additionally, they marked the topic under Chromium > Blink > Javascript > Language (component id: 1456491).

@RedYetiDev
Copy link
Member

Thanks! I'll curious what they say!

@BridgeAR
Copy link
Member

BridgeAR commented Jun 5, 2024

There is a repl mode provided by V8 that we did not yet implement. It is quite some effort to do so and it was not a high enough priority for contributors.
Closing as a duplicate of #8309

@BridgeAR BridgeAR closed this as completed Jun 5, 2024
@RedYetiDev RedYetiDev added the duplicate Issues and PRs that are duplicates of other issues or PRs. label Jun 5, 2024
@RedYetiDev RedYetiDev closed this as not planned Won't fix, can't repro, duplicate, stale Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate Issues and PRs that are duplicates of other issues or PRs. repl Issues and PRs related to the REPL subsystem.
Projects
None yet
Development

No branches or pull requests

4 participants