runtime: declare time.Timer with same shape as runtime.timeTimer

[adonovan]

TIL that time.Timer is not what it seems: it looks like a struct with two fields, but it is in fact the tip of an iceberg of a runtime data structure, timeTimer, that is much larger. So long as you create a Timer using NewTimer (as is required), all is well, but if you call methods on a Timer that you have (foolishly) created yourself, then the runtime will look for its submerged part of the iceberg, and instead find... whatever is adjacent in the heap.

There's an open issue (#69186) against cmd/vet to report this, but I wonder whether a more robust change would be to declare time.Timer so that it has the same shape as the runtime timeTimer (with a bunch of blank int and pointer fields) to avoid this particular kind of corruption.

(The initTimer bool field would need to be replaced by a self-pointer, so that copying a Timer doesn't copy the "initialized" state: a copy of a self-pointer is no longer a self-pointer.)

cc: @prattmic @golang/runtime

[gh2o]

I've seen it done this way in FreeRTOS (link) but it has always looked a little difficult to maintain to me.

Also, with regards to the self-pointer, theoretically somebody could copy the timer to somewhere else, then copy it back to the original location, likely breaking stuff in the process. Perhaps the most we can do here is to make the Timer struct as small as possible (i.e. it contains C and nothing else), then add a check for a magic value in the non-user-visible portion and crash if it's wrong.

[adonovan]

it has always looked a little difficult to maintain to me.

We wouldn't implement it like that. ;-) A simpler implementation would be to either declare the shape manually and have a test that the two structures are equal, or, better, just move the real timeTimer to somewhere in GOROOT/src/internal that allows both packages to access the same declaration.

Also, with regards to the self-pointer, theoretically somebody could copy the timer to somewhere else, then copy it back to the original location, likely breaking stuff in the process.

The goal is not to prevent misuse, but to stop misuse from corrupting the heap.

[qiulaidongfeng]

See [CL 559076](https://go.dev/cl/559076)

…

---Original--- From: "Alan ***@***.***> Date: Wed, May 28, 2025 00:53 AM To: ***@***.***>; Cc: ***@***.******@***.***>; Subject: Re: [golang/go] runtime: declare time.Timer with same shape asruntime.timeTimer (Issue #73886) adonovan left a comment (golang/go#73886) it has always looked a little difficult to maintain to me. We wouldn't implement it like that. ;-) A simpler implementation would be to either declare the shape manually and have a test that the two structures are equal, or, better, just move the real timeTimer to somewhere in GOROOT/src/internal that allows both packages to access the same declaration. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[rsc]

It is very important that time.Timer does NOT include the full size of the runtime Timer, because we are making sure that the runtime timeTimer cannot be copied by arbitrary user code, which would corrupt the runtime timer heap. I seem to remember that time.Timer used to contain a *timer, not a timer, which addressed the problem better. I don't remember why we changed that but if I'm right, maybe we should change it back.

[prattmic]

I've got a CL about ready to send out that changes the init bool to a self pointer which allows detecting copies.

I agree that it seems like a pointer to the runtime structure would be more robust (then we could allow copies, as they'd be shallow). Patch set 1 of https://go.dev/cl/568339 did this. IIUC, it looks like it was changed away from that to avoid the extra allocation.

[gopherbot]

Change https://go.dev/cl/676537 mentions this issue: time: detect copies of Timer and Ticker