I'm not entirely sure why this wouldn't work for you:

type int32holder struct {
    p *atomic.Int32
}
func (h *int32holder) Load() int32 {
    v := h.p.Load()
    runtime.KeepAlive(h)
    return v
}
// Same for store
var variable1 *int32holder
func init() {
    p := (*atomic.Int32)(mmap(...))
    h := &int32holder{p}
    runtime.SetFinalizer(h, func(h *int32holder) { munmap(h.p) })
    variable1 = h
    // or pass h around wherever
}

CC @golang/runtime

FWIW this has come up between a few of us before (CC @dr2chase) and there are some benefits to direct support vs. @randall77's approach (which would also work). Mainly, it just smooths over some friction with interacting with non-Go managed memory, including C values, and makes them a little less error-prone to work with.

Aside from that, I don't like the name TrackPointers very much because it might make the caller think it tracks pointers inside the provided memory, which it very much does not. You absolutely cannot write Go pointers into that memory. What about runtime.AddForeignCleanup ("foreign" in the FFI sense), which matches the new proposal-accepted runtime.AddCleanup?

Lastly, I wonder if this is something that should go in the unsafe package? I am concerned about giving any user a false sense of security about the memory passed to this function.

Leaving aside the question of whether this should go in unsafe for sense-of-security reasons, is there some reason that this couldn't just be part of AddCleanup itself? i.e., AddCleanup would allow passing non-Go pointers?

@prattmic It almost could, but AddCleanup isn't sufficiently general enough. To pass a "size" parameter you'd need to model your value in the type system, otherwise the runtime has no idea what the actual span of memory it needs to cover is. (Slices also won't be supported to begin with, and it seems a little weird to use the slice bounds as the cleanup bounds.)

tracking pointers to the [mmapped memory] and unmapping the underlying memory if there are no more references

I believe that proposed feature is valuable beyond ebpf use cases as well.

The challenge with Mmap/MmapPtr is that it is inherently unsafe. A dangling pointer left behind faults on access if we are lucky. If the address space was reused in meantime, we may end up corrupting something else.

It is true that Golang has unsafe anyway and there are many ways to sabotage oneself. Unsafe requires extreme caution though. It is easier to use correctly if the context we have to reason about is small. That's why packages normally offer safe interfaces, even if leveraging unsafe internally.

What if a package needs to expose data in mmap-ed memory? Making copies is rarely an option (overhead). Wrappers do not interface well with existing packages. If it was possible to expose *T or []T backed by mmap safely, it would be a tremendous improvement.

(For a concrete example, consider memory-mapped AF_PACKET and AF_XDP sockets in Linux. It is essentially a ring buffer shared between user space and the kernel. If we build a reusable package for interfacing with this ring buffer, we aren't going to handle queued packets internally, but rather expose them to the package user.)

@randall77 Thanks for the suggestion, but as you'd expect I glossed over a lot of the details that led us here. We've prototyped nearly every API we could come up with, but we keep running into potential memory safety issues. Check the two PRs I linked from if you're interested.

My first attempt looked a little like what you proposed, but since this is library code, it's hard to build something flexible enough to be useful to the majority of users. The underlying types represented by this memory can be any C type, including structs, including ones with fields accessed atomically, etc. Initially we had a bunch of functions returning atomic.(U)int* types embedded into some unexported types (to prevent the caller coaxing out the real pointers), but this got messy fast and still had the potential for use-after-free.

Also, a mapping represents a datasec, so potentially contains n amount of variables. We could implement a refcount mechanism on the *Memory managed by finalizers, but then the caller must be very careful not to copy any of the pointers, which is equally dangerous. In the end, we concluded that the only way to make this both safe and ergonomic and fast was to rely on the GC to free the underlying mapping, so we ended up with something like this:

// Memory is just a []byte and a bool as a readonly flag.
func MemoryPointer[T any](mm *Memory, off uint64) (*T, error)

// A Variable is carved out of a Memory at a given offset.
func VariablePointer[T any](v *Variable) (*T, error)

Another thing I tried was returning a **T so I could set a finalizer holding a *Memory reference, but I mean...

u32, _ := VariablePointer[uint32](v)
(**u32) += 1

// or

a64, _ := VariablePointer[atomic.Uint64](v)
(*a64).Add(1)

This technically works, but it's clunky and requires more unnecessary pointer chasing. If we ever end up in a situation where we can return *T instead of **T, we'd need to break the API, so I'd rather hold off.

@mknyszek Thanks for the input!

Aside from that, I don't like the name TrackPointers very much [...] What about runtime.AddForeignCleanup ("foreign" in the FFI sense), which matches the new proposal-accepted runtime.AddCleanup?

This is still a rough proposal, naming should probably be decided by someone more knowledgeable about the runtime/allocator/GC. 😉 runtime.AddForeignCleanup() sounds good, but it doesn't accurately describe 'what' that 'Foreign' is supposed to represent. (memory, not Go objects) runtime.AddForeignMemoryCleanup()? That's getting long. runtime.AddPointerCleanup()? Let's take some time to mull this over. 🙂

because it might make the caller think it tracks pointers inside the provided memory, which it very much does not. You absolutely cannot write Go pointers into that memory.

Actually... 😉 Linux' bpf uapi is now frozen, which means no new map types will be introduced solely for bringing new data types to bpf. Going forward, new shared user/kernel datatypes will need to be implemented on top of a so-called 'bpf arena', which is a 4GiB range of memory that can be mapped into user space, exactly like the array map I demonstrated above. These contain pages allocated dynamically by bpf programs. Either side can write pointers into this arena, and as long as they point to somewhere inside the arena itself, the kernel will translate the pointers between kernel and user address space. (Note: it knows which values are pointers, but that's another story) Any pointers pointing outside the arena cause an exception when dereferenced from within a bpf program.

Just to add some nuance to your statement, 'cannot' should really be 'should not'. The user indeed shouldn't expect the GC to follow an off-heap pointer into the Go heap when scanning for references, so when designing these data types, care needs to be taken not to take a caller-provided Go pointer and stuff it somewhere off-heap. This property is definitely something we should document if/when working on implementing this proposal. As I understand it, the Go Arenas concept is stalled for similar reasons, though in reverse. (Go pointers into an arena can become dangling when the arena is freed)

Lastly, I wonder if this is something that should go in the unsafe package? I am concerned about giving any user a false sense of security about the memory passed to this function.

Makes sense, but nothing in the unsafe package currently modifies runtime state, so I'm not sure it fits. I'd argue there's nothing inherently unsafe about this API, since it doesn't need to dereference the given pointer and doesn't necessarily sidestep Go's type system. If anything, it aims to enhance memory safety.

@mejedi floated making this an implicit part of unix.Mmap, which made me think of a potentially-unintended interaction. unix.Mmap maintains a strong (unsafe.Pointer) reference to the underlying memory in some sort of internal cache. This isn't really documented and requires some spelunking through sys.mmapper. It would act as a permanent reference if used as follows, causing the finalizer to never run:

b, _ := unix.Mmap(...)
runtime.TrackPointers(unsafe.SliceData(b), len(b), func (ptr unsafe.Pointer) {
  unix.Munmap(unsafe.Slice(ptr, len(b)))
})

Using unix.MmapPtr instead of unix.Mmap would be necessary to make this work. Integrating this as part of unix.Mmap would make sense, but would technically be a breaking change. Although one could argue that if you lose the reference to b, you clearly have a memory leak. (barring bugs in uintptr() conversions, though this is a well-known footgun of unsafe)

This is still a rough proposal, naming should probably be decided by someone more knowledgeable about the runtime/allocator/GC. 😉 runtime.AddForeignCleanup() sounds good, but it doesn't accurately describe 'what' that 'Foreign' is supposed to represent. (memory, not Go objects) runtime.AddForeignMemoryCleanup()? That's getting long. runtime.AddPointerCleanup()? Let's take some time to mull this over. 🙂

AddForeignMemoryCleanup is not bad. I think it's OK for it to be long. While useful, this is a fairly niche concept that I suspect most Go developers will not need to interact with, so being clear in the API seems more important than being concise here.

Actually... 😉 Linux' bpf uapi is now frozen, which means no new map types will be introduced solely for bringing new data types to bpf. Going forward, new shared user/kernel datatypes will need to be implemented on top of a so-called 'bpf arena', which is a 4GiB range of memory that can be mapped into user space, exactly like the array map I demonstrated above. These contain pages allocated dynamically by bpf programs. Either side can write pointers into this arena, and as long as they point to somewhere inside the arena itself, the kernel will translate the pointers between kernel and user address space. (Note: it knows which values are pointers, but that's another story) Any pointers pointing outside the arena cause an exception when dereferenced from within a bpf program.

I mean that's fine. That's just writing a non-Go pointer into that memory. Maybe I misunderstand your point.

Just to add some nuance to your statement, 'cannot' should really be 'should not'.

Er, sure. You can pin the memory. This is closely related to the cgo pointer rules, so what I really meant to say was you absolutely cannot write unpinned Go pointers into C memory.

Lastly, I wonder if this is something that should go in the unsafe package? I am concerned about giving any user a false sense of security about the memory passed to this function.

Makes sense, but nothing in the unsafe package currently modifies runtime state, so I'm not sure it fits. I'd argue there's nothing inherently unsafe about this API, since it doesn't need to dereference the given pointer and doesn't necessarily sidestep Go's type system. If anything, it aims to enhance memory safety.

That is a fair point; it's just an idea. Mainly what I'm trying to get across is that there should be alarm bells ringing inside anyone's head when they see this function being used that something subtle is happening. The runtime APIs all tend to be more memory safe than this. One option is to put it in runtime/cgo even though it doesn't strictly require cgo, it does certainly help. (And I think it fits OK alongside something like runtime/cgo.Handle too.)

@mejedi floated making this an implicit part of unix.Mmap, which made me think of a potentially-unintended interaction.

Yeah, that would be a more specific subset of this functionality. I'm pretty sure this has also come up before a bunch of times, though I'm not certain there's an issue open for it (wouldn't be surprised if there is).

Thing is, if we have mechanism in the runtime for unix.Mmap, it's really trivial to extend that to the API proposed here. EDIT: This is actually a substantially easier problem, because the minimum size is a 4 KiB aligned region. This is much easier to represent in the runtime's existing data structures.

One thing not discussed in this issue yet is the actual implementation of this functionality. That part is easy if we place restrictions on the size and shape of these regions (must be at least one physical page in size). If we want to support arbitrarily-sized things, like anything that comes from malloc, that becomes substantially harder, because we need full on byte-level tracking (although this would certainly be the right thing to do).