google / starlark-go

Starlark in Go: the Starlark configuration language, implemented in Go
BSD 3-Clause "New" or "Revised" License
2.27k stars 204 forks source link

Requests a 'large" amount of virtual memory addresses, which may fail on some operating systems. #382

Closed ghost closed 2 years ago

ghost commented 2 years ago

I am trying to use a piece of software that relies on starlark-go on a system that rigourously enforces ulimits (in particular on virtual memory). Which means that I have to patch out the the line 62 in int_posix64.go, which tries to allocate 4Gb of memory, which the machine does not possess.

var smallints = reserveAddresses(1 << 32)

func reserveAddresses(len int) uintptr {
    b, err := unix.Mmap(-1, 0, len, unix.PROT_READ, unix.MAP_PRIVATE|unix.MAP_ANON)
    if err != nil {
        log.Fatalf("mmap: %v", err)
    }
    return uintptr(unsafe.Pointer(&b[0]))
}

Please, consider implementing proper memory management, instead of "just allocating 4Gb of RAM".

adonovan commented 2 years ago

This function doesn't allocate 4GB of RAM, it allocates 4GB of virtual address space. They're just locations in memory that a Go program can point to, but there's no actual RAM there. Loading or storing through those pointers would cause a segmentation fault.

ghost commented 2 years ago

You cannot assume that the OS memory manager will give you so much virtual address space in general.

In particular, this code always returns "mmap: cannot allocate memory" on OpenBSD (which is mentioned at the top of the file) machines with less that 4Gb of ram.

ghost commented 2 years ago

You can simulate the same behaviour on Linux with ulimit -S -v 1000000

adonovan commented 2 years ago

You cannot assume that the OS memory manager will give you so much virtual address space in general.

The OS memory manager may deny the request, either because VM is exhausted (which is exceedingly unlikely: 2^64 is a large number) or because the request would exceed the current ulimit. I read than OpenBSD imposes a default ulimit for virtual memory of about 1.5GB, in contrast to Linux and Darwin (no limit). (Could you confirm what value you see from ulimit -v? Thanks.) In any case, the behavior really has nothing to do with the amount of actual RAM.

If OpenBSD's default ulimit is the problem, can you confirm whether ulimit -v 5000000 is an effective workaround? If so, then I am inclined to make the error message on OpenBSD more informative. Another possibility is to make OpenBSD use the generic int representation, at a significant performance cost.

ghost commented 2 years ago

Could you confirm what value you see from ulimit -v? Thanks

>ulimit -v
1052672

It's GNU bash, so the value is in kb.

can you confirm whether ulimit -v 5000000 is an effective workaround?

>ulimit -S -v 5000000
-bash: ulimit: virtual memory: cannot modify limit: Invalid argument
>ulimit -H -v 5000000
-bash: ulimit: virtual memory: cannot modify limit: Invalid argument

I think the limit for ordinary users is 1052672 and something about 1552672 for the staff login class, and it is not possible to go above that.

Another possibility is to make OpenBSD use the generic int representation, at a significant performance cost.

I am by no means a guru of OpenBSD, but speed has never been its selling point. Security and portability were. If this issue is allowed to stay for a day or two, I can email the community and ask for suggestions, but I am inclined to say that a generic representation would be a better choice.

adonovan commented 2 years ago

If this issue is allowed to stay for a day or two, I can email the community and ask for suggestions, but I am inclined to say that a generic representation would be a better choice.

Sure, no rush. To use the generic representation (which is used on all 32-bit machines and non-POSIX OSs), all you need to do is alter these two boolean formulas, one being the negation of the other:

https://github.com/google/starlark-go/blob/master/starlark/int_posix64.go#L1-L2 https://github.com/google/starlark-go/blob/master/starlark/int_generic.go#L1

falsifian commented 2 years ago

Why doesn't this code just use invalid pointers for small int32 values? E.g. odd addresses, assuming the alignment of big.Int is an even number. Then you don't need to reserve address space.

adonovan commented 2 years ago

Why doesn't this code just use invalid pointers for small int32 values? E.g. odd addresses, assuming the alignment of big.Int is an even number. Then you don't need to reserve address space.

That's a nice idea. Care to send a PR?

falsifian commented 2 years ago

I don't use skylark (or go) so am probably not the best person to do it. Maybe this will work: