x/crypto/curve25519: MOVQ global clobbers R15 when dynamically linking

[crawshaw]

The functions ladderstep, mul, and square load global values.

When dynamically linking on amd64, a the load uses R15 as a temporary. Unfortunately, these functions try to use R15 across these loads.

For example,

MOVQ ·REDMASK51(SB),DX

turns into MOVQ 0xABCD(IP), DX typically, but under the compiler's dynamic linking mode it becomes

MOVQ 0xABCD(IP), R15
MOVQ 0(R15), DX

The unit tests for this package (and crypto/tls which depends on this) hang under -buildmode=shared and fail under -buildmode=plugin:

curve25519_test.go:27: incorrect result: got 469423c8d7cc27bb36c7096de54d38d285740efa69594c1f98f49b113dcc7a19, want 89161fde887b2b53de549af483940106ecc114d6982daa98256de23bdf77661a

cc @mwhudson

@bradfitz Any idea who owns this code? I'd like their opinion before touching it.

[bradfitz]

@agl is the meta owner of all x/crypto.

[agl]

These values should be hidden in "C shared library" terminology, i.e. they should not go via a GOT when loading. Is there some directive that can be used to make them as such?

[crawshaw]

I do not believe we have such a mechanism. (Though I would be happy to be proven wrong.)

How performance sensitive are these functions? We could push the globals into stack slots at the start.

[agl]

I can work around it, I'm sure, but if there's no notion of symbol visibility in Go does that mean that every function call goes via the PLT and every global access goes via the GOT? That does seem at odds with traditional shared-library conventions and unlikely to give good performance.

(I would guess that it also means that every function is listed in the dynamic exports table, but that's less crazy in Go since it already has namespaces.)

[crawshaw]

I suspect @mwhudson or @ianlancetaylor can give a better answer as they have thought about it more, but my attempt would be:

(EDIT) All exported symbols in -buildmode=shared and cgo exports need to exposed so they can be dynamically linked again. Otherwise all unexported symbols like this are hidden (in the "C shared library" sense), unless the //go:cgo_export_static or //go:cgo_export_dynamic directives are used. The toolchain is missing optimizations.

[agl]

(If someone can definitively say that we're currently indirecting all global loads and thus need to avoid holding R15 during them I can start on the needed changes. Just want to make sure that this is needed though because it's unexpected to me.)

[ianlancetaylor]

In the normal case we do not indirect global loads through R15. However, we do when the code is compiled with the -dynlink option, which happens when using the go tool options -buildmode=shared, -buildmode=plugin, or -linkshared. When the compiler is invoked with -dynlink, the assembler will be invoked with -D=GOBUILDMODE_shared=1, so it is possible for you to assemble different instruction sequences if you like.

[agl]

@ianlancetaylor and there's no way to avoid the indirection when the symbol cannot be shadowed, like hidden symbols in C?

[ianlancetaylor]

That is correct: it applies to every global symbol. There is no way to mark a symbol as hidden.

[mwhudson]

Oh, good sleuthing @crawshaw

There is one way to make a symbol end up being local to the module being built, which is to use the ·foo<> form. But this is file local, which is not really appropriate here, as far as I can see.

[minux]

Perhaps the linker should treat ·foo<> as private symbols when generating shared libraries.

[rsc]

@agl, yes there's no way to avoid the indirection. R15 is clobbered by global references like this, sorry. We should probably fix this for Go 1.8, right?

[mwhudson]

Perhaps the linker should treat ·foo<> as private symbols when generating
shared libraries.

It does already. But that doesn't really help here.

It would be nice to fix this for 1.8, please. I can carry a fix as a distro patch until 1.8.1 if necessary if the timings don't work out.

[minux]

On Jan 30, 2017 6:03 PM, "Michael Hudson-Doyle" <notifications@github.com> wrote: Perhaps the linker should treat ·foo<> as private symbols when generating shared libraries. It does already. But that doesn't really help here. For private symbols, it could use rip-relative addressing mode to avoid the indirection through GOT. Or am I missing anything?

[gopherbot]

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

[gopherbot]

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

[rsc]

Fixed in subrepo. CL pending for master. Then have to cherry-pick to release branch.

[rsc]

Final CL out: https://go-review.googlesource.com/36414

[gopherbot]

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