Welcome to the main channel on the development of MoarVM, a virtual machine for NQP and Rakudo (moarvm.org). This channel is being logged for historical purposes.
Set by lizmat on 24 May 2021.
01:06 MasterDuke joined 01:55 MasterDuke left 07:54 nine left, nine joined
lizmat m: my @a; @a[99999999999999]=42 10:19
camelia MoarVM panic: Memory allocation failed; could not allocate 800000000000000 bytes
lizmat I wonder whether this could be made into a catchable error ? 10:20
m: my @a; CATCH { dd $_ }; @a[99999999999999]=42
camelia MoarVM panic: Memory allocation failed; could not allocate 800000000000000 bytes
lizmat it currently isn't
research instigated by github.com/rakudo/rakudo/issues/5835 10:21
or maybe even better: make all panics somehow catchable? 10:25
I realize that's probably not possible
but it feels to me that the memory allocation failed would not necessarily need to be a panic? 10:26
nine Only because in this case it's just a stupid request. In any kind of sensible code a failure to allocate memory will most likely be thrown because the system really is out of memory. That means that any allocations will fail including those done for throwing and handling the exception. 10:37
Now because our allocations are done in the nursery and don't actually involve an OS allocation, you might think that we can still succeed and yes, there's a non-zero chance. But the nursery might also be full in which case we would start the GC which would then have to allocate its internal structures and a new nursery. 10:38
Handling OOM is something you can realistically only do when you have very tight control over allocations. That's not something we have, as we're dealing with arbitrary user code. 10:39
Panics in general are thrown in cases where something is broken in a major way and it's highly unlikely that we can continue in any sensible way.
lizmat so essentially, you're saying that this is an unsolvable issue 10:41
nine Yes.
The array allocation case is a bit of a corner case. We could set a limit for what we consider a sensible array size above which we don't even try to allocate. But then every single array allocation pays the cost of this check. And what would you set the limit to? 10:42
lizmat ok, then I suggest we resort to not allowing bigints as indices in the core's AT-POS and ASSIGN-POS implementations
nine You mean the Int:D $pos candidates? 10:44
lizmat I think so, yes 10:48
nine I think those exist more for convenience of not having to convert types explicitly 10:50
jnthn1 My hazy recollection is that if you try to use a truly big int there (something that won't fit into 64 bits), you will get a catchable error that you can't unbox something that big into the 64-bit integer that we need for the memory access. 11:08
nine Yes, we've seen that
lizmat m: my @a; @a[999999999999999999999999999999999]=42 11:09
camelia Cannot unbox 110 bit wide bigint into native integer. Did you mix int and Int or literals?
in block <unit> at <tmp> line 1
lizmat but that's *way* bigger :-)
m: my @a; @a[4294967295]=42 11:10
camelia MoarVM panic: Memory allocation failed; could not allocate 34359738368 bytes
lizmat that's be the max for uint32
nine Which may be too limiting 11:14
~> rakudo -e 'my int8 @a; @a[2 ** 34] = 1; say "survived"' 11:16
survived
lizmat yeah, it survived for me as well
m: my @a = <a b c d>; my uint $i = -2; say $i; say @a[$i] #hmmm that feels wrong :-) 11:20
camelia 18446744073709551614
c
lizmat meh, it looks like nqp::atpos is interpreting large uints as negatives: 11:35
m: my @a = <a b c d>; use nqp; my uint $i = -2; say $i; say nqp::atpos(nqp::getattr(@a,List,q/$!reified/),$i)
camelia 18446744073709551614
c
lizmat same for native arrays: 11:36
m: my str @a = <a b c d>; use nqp; my uint $i = -2; say nqp::atpos_s(@a,$i)
camelia c
lizmat which implies that uint candidates in the setting still need to check for < 0 :-( 11:37
and thus it doesn't make sense to have uint candidates, the int candidates will already DTRT 11:41
nine Isn't that the inverse conclusion? To me it'd make more sense to fix the places where we treat a uint pos as a signed one 11:42
lizmat well, that would mean introducing mmd on nqp::atpos ? 11:45
nine why? 11:46
lizmat because nqp::atpos cannot see whether the index values comes from a int or a uint ? 11:47
nine So declare that it *must* always be an uint and fix the places that rely on the wraparound
lizmat thing is, at several places in core, and probably in the ecosystem, there's code that depends on -1 being interpreted as "from the end" 11:50
so we can't change the behaviour of nqp::atpos, as it is expected to always succeed, also for negative values 11:51
and a sufficiently large uint value is interpreted as negative
nine core can be fixed, can't it? And nqp has never been a guaranteed interface. 11:58
lizmat so you're saying the -1 check should be moved to nqp::atpos ? 12:05
s/-1/< 0/
nine No, to the code that uses nqp::atpos 12:13
lizmat which is what I proposed: remove the uint candidates, replace them by int candidates 12:14
with a < 0 check 12:15
nine Why would you remove the uint candidates? 12:17
uint is by definition positive. There's no need for a check. That's quite optimal. It's what we want
lizmat that's what I also thought, but it's not true for uint values with the MSB set being passed on to nqp::atpos and friends 12:39
which will interpret them as negative values, and thus index from end
nine But if we fix nqp::atpos to not do that, it's exactly as it shoud be 12:40
lizmat right, but I thought we wouldn't fix nqp::atpos and friends ?
nine My proposal is to remove the auto-wrapping from nqp::atpos and fix all callers in core to either prevent negative values from being passed or do wrapping manually where needed. 12:42
lizmat that would also need quite a few fixes in NQP, I'm afraod 12:45
*afraid
nine sure 12:50
to me that's just part of core
lizmat i guess that would just be an nqp::die, which X::AdHoc should then make into a nicer error message 12:53
hmmm... the more I look at this, it feels we would need a Great List Index refactor :( 13:05
nine While there are a great many callers of nqp::atpos, I'm quite sure that only a comparatively few of them rely on the wrapping behavior 13:21
E.g. more than 10 % of the atpos calls in core.c are in the BUILDPLAN executor and those are really just static indices 13:22
like nqp::atpos($task,1) 13:23
All of them work just fine and unchanged if we remove the auto-wrapping
lizmat yeah, and all of that BUILDPLAN will go sooner or later 13:28
meanwhile, nqp::atpos just wraps to nqp::atpos_o
so I guess it would need to make it a more complcated mapping in the MAST process ? 13:29
afk&
nine I don't see the need for that. I'd just remove the index += body->elems in places like github.com/MoarVM/MoarVM/blob/main...ray.c#L247 try to build NQP, see where it blows up, fix that place, rince and repeat 13:40
The vast majority of users is just supplying fixed, positive indexes or is in a loop iterating over the known indexes. 13:41
There's obvious cases like nqp::getattr_s(nqp::atpos($cstack,-1), $?CLASS, '$!name')
in Cursor which needs to be nqp::getattr_s(nqp::atpos($cstack, nqp::elems($cstack) - 1), $?CLASS, '$!name') 13:42
IIRC there are a few places where we need to adjust codegen in QASTRegexCompilerMAST 13:43
lizmat fair enough, I'll give that a stab after the 2025.04 release 16:27
nine ++lizmat 16:37
timo i've got a bit of WIP towards making set_size_internal from VMArray not panic for stuff that's too large 17:24
in the process i found a small area where we can actually get mimalloc to fail an internal assertion and take the whole process down, that's always fun 17:25
github.com/microsoft/mimalloc/issues/1068 17:28
18:16 lizmat left 18:22 lizmat joined 23:20 guifa joined

Powered by Cro and the Raku® Programming Language.

Please report any issues / comments / feature requests as an issue on App::Raku::Log.

Thank you!