code review 146510043: [release-branch.go1.3] runtime: hide cgocallback_gofunc...

[release-branch.go1.3] runtime: hide cgocallback_gofunc calling cgocallbackg from linker

The linker error at
http://build.golang.org/log/42c57ff6b57ab36f6622417108bb5697e4d36fa7
is a false positive: cgocallback_gofunc is invoked on the g0 stack,
as explained in its comments, and then it switches to the m->curg
stack to invoke cgocallbackg. Checking the stacksplit sequence
on the g0 stack doesn't make sense, nor does carrying a check
that starts on the g0 stack over to the curg stack.
Change the code to use an indirect call so that the linker
stacksplit check does not follow through it.

[rsc, 2014-09-30 00:50:00]

Hello golang-codereviews@googlegroups.com (cc: adg, iant, r),

I'd like you to review this change to
https://code.google.com/p/go/

[bradfitz, 2014-09-30 01:15:20]

LGTM


On Mon, Sep 29, 2014 at 5:50 PM, <rsc@golang.org> wrote:

> Reviewers: golang-codereviews,
>
> Message:
> Hello golang-codereviews@googlegroups.com (cc: adg, iant, r),
>
> I'd like you to review this change to
> https://code.google.com/p/go/
>
>
> Description:
> [release-branch.go1.3] runtime: hide cgocallback_gofunc calling
> cgocallbackg from linker
>
> The linker error at
> http://build.golang.org/log/42c57ff6b57ab36f6622417108bb5697e4d36fa7
> is a false positive: cgocallback_gofunc is invoked on the g0 stack,
> as explained in its comments, and then it switches to the m->curg
> stack to invoke cgocallbackg. Checking the stacksplit sequence
> on the g0 stack doesn't make sense, nor does carrying a check
> that starts on the g0 stack over to the curg stack.
> Change the code to use an indirect call so that the linker
> stacksplit check does not follow through it.
>
> Please review this at https://codereview.appspot.com/146510043/
>
> Affected files (+2, -1 lines):
>   M src/pkg/runtime/asm_amd64.s
>
>
> Index: src/pkg/runtime/asm_amd64.s
> ===================================================================
> --- a/src/pkg/runtime/asm_amd64.s
> +++ b/src/pkg/runtime/asm_amd64.s
> @@ -784,7 +784,8 @@
>         MOVQ    BP, -8(DI)
>         LEAQ    -(8+8)(DI), SP
>         MOVQ    R8, 0(SP)
> -       CALL    runtime·cgocallbackg(SB)
> +       MOVQ    $runtime·cgocallbackg(SB), AX // hide function call from
> linker
> +       CALL    AX
>         MOVQ    0(SP), R8
>
>         // Restore g->sched (== m->curg->sched) from saved values.
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-codereviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-codereviews+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>

[adg, 2014-09-30 01:16:39]

LGTM but I don't say so from a position of authority

[rsc, 2014-09-30 01:23:58]

*** Submitted as https://code.google.com/p/go/source/detail?r=f1046f988c1a ***

[release-branch.go1.3] runtime: hide cgocallback_gofunc calling cgocallbackg
from linker

The linker error at
http://build.golang.org/log/42c57ff6b57ab36f6622417108bb5697e4d36fa7
is a false positive: cgocallback_gofunc is invoked on the g0 stack,
as explained in its comments, and then it switches to the m->curg
stack to invoke cgocallbackg. Checking the stacksplit sequence
on the g0 stack doesn't make sense, nor does carrying a check
that starts on the g0 stack over to the curg stack.
Change the code to use an indirect call so that the linker
stacksplit check does not follow through it.

LGTM=bradfitz, adg
R=golang-codereviews, bradfitz, adg
CC=golang-codereviews, iant, r
https://codereview.appspot.com/146510043