runtime: ctxt field scanned even if it is junk

[randall77]

The ctxt argument to a function is saved unconditionally in
pkg/runtime/asm_*.s:runtime.morestack.

We queue it to be scanned in pkg/runtime/mgc0.c:addstackroots.

On functions which do not take a context argument (most of them), the context argument
contains junk.  It gets saved and then scanned as if it was a pointer.

This is imprecise for garbage collection.  We should either have different versions of
morestack for functions with and without context args, or have a flag in a Func
somewhere that tells us whether a function has a context arg.

[davecheney]

Comment 1:

Could we arrange to have the context set to nil in these cases ? This wouldn't save the
overhead of adding it as a stack root, but it might be an improvement over scanning a
random value as a root.

[randall77]

Comment 2:

That would be fine.  That's what I meant by "different version of morestack", one which
saves DX (or the arm equivalent) to ctxt and one which saves nil to ctxt.

[davecheney]

Comment 3:

Thanks for your reply Keith. I probably don't understand the subtleties here but why is
the context argument that morestack copies junk in the first place? Is it possible to
address the problem at the source. FWIW introducing morestack variants sounds like the
better fix.

[randall77]

Comment 4:

(x86-centric for a moment)
When a function takes a context, it gets passed in the register DX.  When it doesn't
take a context, DX contains some arbitrary junk.  Morestack unconditionally saves DX to
the ctxt field.  It doesn't know if it is being called from a context-taking function or
not.

[davecheney]

Comment 5:

Can the compiler arrange for DX to be nil if there is no context field or would that
require that the caller must save DX on every function invocation ?

[randall77]

Comment 6:

It would mean a MOV $0, DX in almost every function prologue (or almost every call
site), and I think that's too expensive.  Probably best to have morestack_clearDX
versions that clear DX and then jump to the regular morestacks.
It needs a better name than morestack_clearDX...

[davecheney]

Comment 7:

I see. Thank you for helping me understand.

[davecheney]

Comment 8:

Labels changed: added repo-main.

Status changed to Accepted.

[rsc]

Comment 9:

MOVL $0, DX is two bytes of code, and on Intel chips it is free, because it handled
during register renaming and not actually "executed". I think I'd rather do that than
double the set of morestack functions we have.

[rsc]

Comment 10:

I've started on the "double the morestacks" approach. I don't know what's causing the
finalizer flakiness so I am just grasping at straws addressing all the imprecise things
I know about.

Labels changed: added release-go1.3, removed release-go1.3maybe.

Status changed to Started.

[rsc]

Comment 11:

This issue was closed by revision c2dd33a.

Status changed to Fixed.