Your preferable solution is possible, but a bit involved. The module data is put on the linked list by runtime.addmoduledata, called from an ELF .init_array. It's called with the C calling convention and doesn't have the Go machinery setup to call progToPointerMask. I think the easiest thing to do would be to have addmoduledata put the module on a pending linked list, and then have plugin_lastmoduleinit initialize gccdatamask/gccbssmask, then move the module to the main linked list.

But a (somewhat depressing) question before that: if we are concurrently reading/writing moduledata.next, does that mean we should be atomically loading/storing it?

If so, then I think while I'll keep the pending data structure used by addmouledata a linked list, the list of active modules should turn into a global slice, so all the sites were we iterate through modules only gets hit with a single atomic load.

Oh and thanks for analyzing the crash. When I wrote plugin_lastmoduleinit I saw the gcdatamask/gcbssmask uses in mbitmap.go and assumed late initialization was fine because there were no active data pointers yet. But I completely missed the data root loading in gcMarkRootPrepare.

Your preferable solution is possible, but a bit involved. The module data is put on the linked list by runtime.addmoduledata, called from an ELF .init_array. It's called with the C calling convention and doesn't have the Go machinery setup to call progToPointerMask.

Ah, no wonder I couldn't find it. Guru needs to learn assembly. :)

I think the easiest thing to do would be to have addmoduledata put the module on a pending linked list, and then have plugin_lastmoduleinit initialize gccdatamask/gccbssmask, then move the module to the main linked list.

That seems reasonable. Do you even need the list? It looks like there can only be one pending module load right now (enforced by pluginsMu and assumed by lastmoduleinit.)

But a (somewhat depressing) question before that: if we are concurrently reading/writing moduledata.next, does that mean we should be atomically loading/storing it?

Well, we need to be doing something. I suspect what you proposed above would be fine on x86 as long as the compiler isn't doing any real memory reordering. On a weak machine or if the compiler gets more aggressive with reordering, there are three hazards:

1. The initializing writes to the moduledata could be reordered to after the write that adds it to the list, making it possible to observe an uninitialized moduledata (the read side is safe because the reads are dependent).
2. The write to add it to the list could be reordered after some other write from the module that causes another CPU to walk the list, causing the other CPU to miss the module, even though it saw its effects.
3. The read-side equivalent of 2: reading the list could be reordered before some read that observed some effect of the module.

These could be solved by always accessing the list with atomics, but that's pretty annoying. Hazard 1 could be solved by making just the store to next be atomic, or by using the runtime's publicationBarrier function between the last write to the moduledata and adding it to the list.

Your global slice idea may be the way to go for 2 and 3 (and may address 1 by side effect), though note that you can't read or write a slice value atomically (since it's three words), so it can't actually be a slice, per se. You could "manually" store the pointer and length, though that would require two (carefully ordered) loads at each loop. Perhaps the best thing would be to store it as a pointer to an array with the convention that there's always a nil pointer after the last *moduledata in the array. The loops would then atomically load just this one array pointer and iterate until they reached a nil. To keep the type system happy and the loops simple, you could give the array some ridiculous bound with the knowledge that you'll never go past the nil.

For completeness, a few other solutions come to mind, but I think I like the array the best:

1. If the list were stored in reverse, so you were adding new modules to the beginning, you would only need to access the head of the list with an atomic. Since the list is usually 1 long, maybe this is still a lose, though uncontended atomic loads aren't much more expensive than regular loads (on x86 it's literally the same instruction). It looks like most loops have to go over all of the modules anyway, but some important ones don't, like bulkBarrierBitmap, so this would probably slow those down in the common case where firstmoduledata is what they're looking for.
2. After adding the moduledata to the list, use forEachP with an empty function to make the write globally visible by creating a global happens-before relation. Unfortunately, while this isn't as big of a hammer as stopping the world, it requires acquiring worldsema, which will still block until a concurrent GC cycle is over.
3. Make a convincing argument that 2 and 3 aren't a problem, or are only a problem if the program is racy anyway. It's possible that anything where another CPU really needs to observe the module in the list also creates a happens-before edge.

Thanks for the thoughts.

There are two uses of the moduledata linked list and addmoduledata. In -buildmode=plugin there only ever is one pending module, but in -buildmode=shared all modules are loaded by ld.so before program initialization, so in that case there are any number of pending modules. We could separate the two paths, but I would like to minimize architecture-dependent code in cmd/link, and the code to call addmoduledata is very arch-dependent.

I've cut a CL that I believe introduces an array to take care of this, and run all.bash a few times on darwin. I don't have a better testing strategy for this, as I haven't been able to replicate the crash.

so in that case there are any number of pending modules.

Ah, okay.

I don't have a better testing strategy for this, as I haven't been able to replicate the crash.

I can reproduce it 100% of the time on linux/amd64. I'm happy to test.

CL https://golang.org/cl/32357 mentions this issue.