FAQ
Reviewers: golang-dev_googlegroups.com,

Message:
Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://go.googlecode.com/hg/


Description:
cgo: enable cgo on openbsd

Enable cgo on OpenBSD.

The OpenBSD ld.so(1) does not currently support PT_TLS sections. Work
around this by fixing up the TCB that has been provided by librthread
and reallocating a TCB with additional space for TLS.

Requires change 6846064.

Fixes issue 3205.

Please review this at http://codereview.appspot.com/6853059/

Affected files:
M doc/progs/run
M misc/cgo/test/basic.go
M src/pkg/go/build/build.go
M src/pkg/net/cgo_openbsd.go
M src/pkg/net/cgo_unix.go
M src/pkg/os/user/lookup_unix.go
M src/pkg/runtime/cgo/gcc_openbsd_386.c
M src/pkg/runtime/cgo/gcc_openbsd_amd64.c
M src/pkg/runtime/cgo/openbsd.c


Index: doc/progs/run
===================================================================
--- a/doc/progs/run
+++ b/doc/progs/run
@@ -45,6 +45,10 @@
if [ "$goos" == "netbsd" ]; then
c_go_cgo=""
fi
+# cgo3 and cgo4 don't run on openbsd, since cgo cannot handle stdout
correctly
+if [ "$goos" == "openbsd" ]; then
+ c_go_cgo="cgo1 cgo2"
+fi

timeout="
timeout1
Index: misc/cgo/test/basic.go
===================================================================
--- a/misc/cgo/test/basic.go
+++ b/misc/cgo/test/basic.go
@@ -56,6 +56,7 @@
*/
import "C"
import (
+ "runtime"
"syscall"
"testing"
"unsafe"
@@ -119,7 +120,12 @@
func testMultipleAssign(t *testing.T) {
p := C.CString("234")
n, m := C.strtol(p, nil, 345), C.strtol(p, nil, 10)
- if n != 0 || m != 234 {
+ if runtime.GOOS == "openbsd" {
+ // Bug in OpenBSD strtol(3) - base > 36 succeeds.
+ if (n != 0 && n != 239089) || m != 234 {
+ t.Fatal("Strtol x2: ", n, m)
+ }
+ } else if n != 0 || m != 234 {
t.Fatal("Strtol x2: ", n, m)
}
C.free(unsafe.Pointer(p))
Index: src/pkg/go/build/build.go
===================================================================
--- a/src/pkg/go/build/build.go
+++ b/src/pkg/go/build/build.go
@@ -222,6 +222,8 @@
"linux/arm": true,
"netbsd/386": true,
"netbsd/amd64": true,
+ "openbsd/386": true,
+ "openbsd/amd64": true,
"windows/386": true,
"windows/amd64": true,
}
Index: src/pkg/net/cgo_openbsd.go
===================================================================
copy from src/pkg/net/cgo_netbsd.go
copy to src/pkg/net/cgo_openbsd.go
Index: src/pkg/net/cgo_unix.go
===================================================================
--- a/src/pkg/net/cgo_unix.go
+++ b/src/pkg/net/cgo_unix.go
@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

-// +build darwin freebsd linux netbsd
+// +build darwin freebsd linux netbsd openbsd

package net

Index: src/pkg/os/user/lookup_unix.go
===================================================================
--- a/src/pkg/os/user/lookup_unix.go
+++ b/src/pkg/os/user/lookup_unix.go
@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

-// +build darwin freebsd linux netbsd
+// +build darwin freebsd linux netbsd openbsd
// +build cgo

package user
Index: src/pkg/runtime/cgo/gcc_openbsd_386.c
===================================================================
copy from src/pkg/runtime/cgo/gcc_netbsd_386.c
copy to src/pkg/runtime/cgo/gcc_openbsd_386.c
--- a/src/pkg/runtime/cgo/gcc_netbsd_386.c
+++ b/src/pkg/runtime/cgo/gcc_openbsd_386.c
@@ -10,6 +10,34 @@

static void* threadentry(void*);

+#define TCB_SIZE 16
+#define TLS_SIZE 8
+
+void *__get_tcb(void);
+void __set_tcb(void *);
+
+static void
+tcb_fixup(int mainthread)
+{
+ void *newtcb, *oldtcb;
+
+ // The OpenBSD ld.so(1) does not currently support PT_TLS. As a result,
+ // we need to allocate our own TLS space while preserving the existing
+ // TCB that has been setup via librthread.
+
+ newtcb = malloc(TCB_SIZE + TLS_SIZE);
+ if(newtcb == NULL)
+ abort();
+
+ oldtcb = __get_tcb();
+ bcopy(oldtcb, newtcb + TLS_SIZE, TCB_SIZE);
+ __set_tcb(newtcb + TLS_SIZE);
+
+ // The main thread TCB is a static allocation - do not try to free it.
+ if(!mainthread)
+ free(oldtcb);
+}
+
static void
xinitcgo(G *g)
{
@@ -20,6 +48,8 @@
pthread_attr_getstacksize(&attr, &size);
g->stackguard = (uintptr)&attr - size + 4096;
pthread_attr_destroy(&attr);
+
+ tcb_fixup(1);
}

void (*initcgo)(G*) = xinitcgo;
@@ -54,6 +84,8 @@
{
ThreadStart ts;

+ tcb_fixup(0);
+
ts = *(ThreadStart*)v;
free(v);

@@ -66,7 +98,7 @@
ts.g->stackguard = (uintptr)&ts - ts.g->stackguard + 4096;

/*
- * Set specific keys. On NetBSD/ELF, the thread local storage
+ * Set specific keys. On OpenBSD/ELF, the thread local storage
* is just before %gs:0. Our dynamic 8.out's reserve 8 bytes
* for the two words g and m at %gs:-8 and %gs:-4.
*/
Index: src/pkg/runtime/cgo/gcc_openbsd_amd64.c
===================================================================
copy from src/pkg/runtime/cgo/gcc_netbsd_amd64.c
copy to src/pkg/runtime/cgo/gcc_openbsd_amd64.c
--- a/src/pkg/runtime/cgo/gcc_netbsd_amd64.c
+++ b/src/pkg/runtime/cgo/gcc_openbsd_amd64.c
@@ -10,6 +10,34 @@

static void* threadentry(void*);

+#define TCB_SIZE 32
+#define TLS_SIZE 16
+
+void *__get_tcb(void);
+void __set_tcb(void *);
+
+static void
+tcb_fixup(int mainthread)
+{
+ void *newtcb, *oldtcb;
+
+ // The OpenBSD ld.so(1) does not currently support PT_TLS. As a result,
+ // we need to allocate our own TLS space while preserving the existing
+ // TCB that has been setup via librthread.
+
+ newtcb = malloc(TCB_SIZE + TLS_SIZE);
+ if(newtcb == NULL)
+ abort();
+
+ oldtcb = __get_tcb();
+ bcopy(oldtcb, newtcb + TLS_SIZE, TCB_SIZE);
+ __set_tcb(newtcb + TLS_SIZE);
+
+ // The main thread TCB is a static allocation - do not try to free it.
+ if(!mainthread)
+ free(oldtcb);
+}
+
static void
xinitcgo(G *g)
{
@@ -20,6 +48,8 @@
pthread_attr_getstacksize(&attr, &size);
g->stackguard = (uintptr)&attr - size + 4096;
pthread_attr_destroy(&attr);
+
+ tcb_fixup(1);
}

void (*initcgo)(G*) = xinitcgo;
@@ -55,6 +85,8 @@
{
ThreadStart ts;

+ tcb_fixup(0);
+
ts = *(ThreadStart*)v;
free(v);

@@ -67,7 +99,7 @@
ts.g->stackguard = (uintptr)&ts - ts.g->stackguard + 4096;

/*
- * Set specific keys. On NetBSD/ELF, the thread local storage
+ * Set specific keys. On OpenBSD/ELF, the thread local storage
* is just before %fs:0. Our dynamic 6.out's reserve 16 bytes
* for the two words g and m at %fs:-16 and %fs:-8.
*/
Index: src/pkg/runtime/cgo/openbsd.c
===================================================================
copy from src/pkg/runtime/cgo/netbsd.c
copy to src/pkg/runtime/cgo/openbsd.c
--- a/src/pkg/runtime/cgo/netbsd.c
+++ b/src/pkg/runtime/cgo/openbsd.c
@@ -2,12 +2,17 @@
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

-// Supply environ and __progname, because we don't
-// link against the standard NetBSD crt0.o and the
-// libc dynamic library needs them.
+// Supply environ, __progname and __guard_local, because
+// we don't link against the standard OpenBSD crt0.o and
+// the libc dynamic library needs them.

char *environ[1];
char *__progname;
+long __guard_local;

#pragma dynexport environ environ
#pragma dynexport __progname __progname
+
+// This is normally marked as hidden and placed in the
+// .openbsd.randomdata section.
+#pragma dynexport __guard_local __guard_local

Search Discussions

  • Minux at Nov 16, 2012 at 5:38 pm
    what if extern c code creates a thread, and a signal is
    accidentally delivered to that thread?
  • Joel Sing at Nov 17, 2012 at 2:33 pm

    On 17 November 2012 04:38, minux wrote:
    what if extern c code creates a thread, and a signal is
    accidentally delivered to that thread?
    Good catch. Obviously we cannot rely on m being 0, which is what the
    current test is sigtramp checks. I should be able to add a canary to
    the cgo allocated TLS and we can look for that in the sigtramp - if
    cgo is enabled and the canary does not exist then we can assume Go did
    not create the thread.
  • Minux at Nov 17, 2012 at 5:06 pm

    On Sat, Nov 17, 2012 at 10:33 PM, Joel Sing wrote:
    On 17 November 2012 04:38, minux wrote:
    what if extern c code creates a thread, and a signal is
    accidentally delivered to that thread?
    Good catch. Obviously we cannot rely on m being 0, which is what the
    current test is sigtramp checks. I should be able to add a canary to
    the cgo allocated TLS and we can look for that in the sigtramp - if
    cgo is enabled and the canary does not exist then we can assume Go did
    not create the thread.
    my proposed solution is that we wrap and override pthread_create so that
    we can always allocate our own TLS when creating new threads. WDYT?
  • Joel Sing at Nov 22, 2012 at 12:37 pm

    On 18 November 2012 04:06, minux wrote:
    On Sat, Nov 17, 2012 at 10:33 PM, Joel Sing wrote:
    On 17 November 2012 04:38, minux wrote:
    what if extern c code creates a thread, and a signal is
    accidentally delivered to that thread?
    Good catch. Obviously we cannot rely on m being 0, which is what the
    current test is sigtramp checks. I should be able to add a canary to
    the cgo allocated TLS and we can look for that in the sigtramp - if
    cgo is enabled and the canary does not exist then we can assume Go did
    not create the thread.
    my proposed solution is that we wrap and override pthread_create so that
    we can always allocate our own TLS when creating new threads. WDYT?
    That would be an option, however I cannot see how this would provide
    any advantage over what has been implemented in this change. In the
    case of a non-cgo thread, we still cannot assign a g/m even if we
    allocated TLS to store it. Or am I overlooking something?
  • Minux at Nov 23, 2012 at 7:03 pm

    On Thu, Nov 22, 2012 at 8:37 PM, Joel Sing wrote:
    On 18 November 2012 04:06, minux wrote:
    On Sat, Nov 17, 2012 at 10:33 PM, Joel Sing wrote:
    my proposed solution is that we wrap and override pthread_create so that
    we can always allocate our own TLS when creating new threads. WDYT?
    That would be an option, however I cannot see how this would provide
    any advantage over what has been implemented in this change. In the
    case of a non-cgo thread, we still cannot assign a g/m even if we
    allocated TLS to store it. Or am I overlooking something?
    my proposal is this:
    we define and export a pthread_create in runtime/cgo that calls the real
    pthread_create
    in libpthread, and then allocate two more slots for TLS for the new thread
    so that
    the signal handler will read zero m & g and correctly detect the signal is
    received
    on a foreign thread.

    the question is, can we override symbols this way (i think so)?
  • Ian Lance Taylor at Nov 24, 2012 at 6:49 pm

    On Fri, Nov 23, 2012 at 11:03 AM, minux wrote:
    On Thu, Nov 22, 2012 at 8:37 PM, Joel Sing wrote:
    On 18 November 2012 04:06, minux wrote:
    On Sat, Nov 17, 2012 at 10:33 PM, Joel Sing wrote:
    my proposed solution is that we wrap and override pthread_create so that
    we can always allocate our own TLS when creating new threads. WDYT?
    That would be an option, however I cannot see how this would provide
    any advantage over what has been implemented in this change. In the
    case of a non-cgo thread, we still cannot assign a g/m even if we
    allocated TLS to store it. Or am I overlooking something?
    my proposal is this:
    we define and export a pthread_create in runtime/cgo that calls the real
    pthread_create
    in libpthread, and then allocate two more slots for TLS for the new thread
    so that
    the signal handler will read zero m & g and correctly detect the signal is
    received
    on a foreign thread.

    the question is, can we override symbols this way (i think so)?
    If the executable defines a function named pthread_create, then any
    code linked into the executable, or any code in shared libraries
    dynamically linked in, should call the pthread_create defined in the
    executable. The harder part is calling the pthread_create defined in
    libpthread. If it has an alias, e.g., __pthread_create, then it is
    easy. Otherwise the code needs to explicitly dlopen libpthread.so and
    then use dlsym.

    I assume that the problem is that OpenBSD doesn't actually support
    __thread variables?

    Ian
  • Minux at Nov 24, 2012 at 7:32 pm

    On Sun, Nov 25, 2012 at 2:49 AM, Ian Lance Taylor wrote:

    my proposal is this:
    we define and export a pthread_create in runtime/cgo that calls the real
    pthread_create
    in libpthread, and then allocate two more slots for TLS for the new thread
    so that
    the signal handler will read zero m & g and correctly detect the signal is
    received
    on a foreign thread.

    the question is, can we override symbols this way (i think so)?
    If the executable defines a function named pthread_create, then any
    code linked into the executable, or any code in shared libraries
    dynamically linked in, should call the pthread_create defined in the
    executable. The harder part is calling the pthread_create defined in
    great. that's also my understanding of ELF. thank you for the confirmation.
    libpthread. If it has an alias, e.g., __pthread_create, then it is
    easy. Otherwise the code needs to explicitly dlopen libpthread.so and
    then use dlsym.
    i think this is acceptable, and i will try to write a proof of concept
    implementation.
    I assume that the problem is that OpenBSD doesn't actually support
    __thread variables?
    no, it doesn't support __thread storage class.
  • Minux at Nov 24, 2012 at 9:03 pm
    my proposal: https://codereview.appspot.com/6855086/
    (only tested on openbsd/amd64).

    My CL doesn't need to store a special TLS_CANARY in TLS and it can also
    handle the case where the tls slot used to store the canary can't be read
    (for threads not created by Go runtime).
  • Joel Sing at Nov 26, 2012 at 3:38 pm

    On 25 November 2012 08:02, minux wrote:
    my proposal: https://codereview.appspot.com/6855086/
    (only tested on openbsd/amd64).

    My CL doesn't need to store a special TLS_CANARY in TLS and it can also
    handle the case where the tls slot used to store the canary can't be read
    (for threads not created by Go runtime).
    Thanks!

    I understood what you were proposing, however I could not see the
    benefit in doing this over what I had suggested - as you point out, it
    is possible that the memory above the TCB is inaccessible and trying
    to read it could result in a fault inside the signal trampoline. On
    one hand we would be about to terminate with a "bad signal" anyway,
    however this could result in programs that just hang, which would be
    poor form.

    I have merged your changes into my change, with a couple of minor
    differences and one key difference - when we create a thread for the
    Go runtime we do not need to use the pthread_create wrapper, hence we
    can call the system pthread_create directly and avoid the additional
    overhead. I have specifically created a library to test the external
    pthread_create and signalling of the externally created threads. This
    has been tested successfully on OpenBSD/amd64 -current, OpenBSD/amd64
    5.2 and OpenBSD/i386 5.2.

    This also ponders the question - since we are wrapping the
    pthread_create we could potentially use sigprocmask to prevent signals
    being delivered to the thread, although this may lead to some weird
    failures if the C code was signalling itself and expecting the signal
    to be delivered (if the C library is messing with signal handlers then
    we probably already have bigger problems though).

    PTAL.
  • Joel Sing at Nov 26, 2012 at 4:48 pm

    On 27 November 2012 02:31, Joel Sing wrote:
    On 25 November 2012 08:02, minux wrote:
    my proposal: https://codereview.appspot.com/6855086/
    (only tested on openbsd/amd64).

    My CL doesn't need to store a special TLS_CANARY in TLS and it can also
    handle the case where the tls slot used to store the canary can't be read
    (for threads not created by Go runtime).
    Thanks!

    I understood what you were proposing, however I could not see the
    benefit in doing this over what I had suggested - as you point out, it
    is possible that the memory above the TCB is inaccessible and trying
    to read it could result in a fault inside the signal trampoline. On
    one hand we would be about to terminate with a "bad signal" anyway,
    however this could result in programs that just hang, which would be
    poor form.

    I have merged your changes into my change, with a couple of minor
    differences and one key difference - when we create a thread for the
    Go runtime we do not need to use the pthread_create wrapper, hence we
    can call the system pthread_create directly and avoid the additional
    overhead. I have specifically created a library to test the external
    pthread_create and signalling of the externally created threads. This
    has been tested successfully on OpenBSD/amd64 -current, OpenBSD/amd64
    5.2 and OpenBSD/i386 5.2.

    This also ponders the question - since we are wrapping the
    pthread_create we could potentially use sigprocmask to prevent signals
    being delivered to the thread, although this may lead to some weird
    failures if the C code was signalling itself and expecting the signal
    to be delivered (if the C library is messing with signal handlers then
    we probably already have bigger problems though).

    PTAL.
    Hrmmm... bad news - while this works correctly for Go created threads,
    the use of pthread_create in an external library is still resulting in
    the system libpthread version being used directly.
  • Ian Lance Taylor at Nov 26, 2012 at 5:09 pm

    On Mon, Nov 26, 2012 at 8:40 AM, Joel Sing wrote:
    On 27 November 2012 02:31, Joel Sing wrote:
    On 25 November 2012 08:02, minux wrote:
    my proposal: https://codereview.appspot.com/6855086/
    (only tested on openbsd/amd64).

    My CL doesn't need to store a special TLS_CANARY in TLS and it can also
    handle the case where the tls slot used to store the canary can't be read
    (for threads not created by Go runtime).
    Thanks!

    I understood what you were proposing, however I could not see the
    benefit in doing this over what I had suggested - as you point out, it
    is possible that the memory above the TCB is inaccessible and trying
    to read it could result in a fault inside the signal trampoline. On
    one hand we would be about to terminate with a "bad signal" anyway,
    however this could result in programs that just hang, which would be
    poor form.

    I have merged your changes into my change, with a couple of minor
    differences and one key difference - when we create a thread for the
    Go runtime we do not need to use the pthread_create wrapper, hence we
    can call the system pthread_create directly and avoid the additional
    overhead. I have specifically created a library to test the external
    pthread_create and signalling of the externally created threads. This
    has been tested successfully on OpenBSD/amd64 -current, OpenBSD/amd64
    5.2 and OpenBSD/i386 5.2.

    This also ponders the question - since we are wrapping the
    pthread_create we could potentially use sigprocmask to prevent signals
    being delivered to the thread, although this may lead to some weird
    failures if the C code was signalling itself and expecting the signal
    to be delivered (if the C library is messing with signal handlers then
    we probably already have bigger problems though).

    PTAL.
    Hrmmm... bad news - while this works correctly for Go created threads,
    the use of pthread_create in an external library is still resulting in
    the system libpthread version being used directly.
    Does pthread_create appear in the dynamic symbol table of the Go
    binary? You can dump the dynamic symbol table using readelf -s and
    looking at the .dynsym section, or by using nm --dynamic.

    Ian
  • Minux at Nov 26, 2012 at 5:17 pm

    On Tue, Nov 27, 2012 at 1:02 AM, Ian Lance Taylor wrote:
    On Mon, Nov 26, 2012 at 8:40 AM, Joel Sing wrote:
    Hrmmm... bad news - while this works correctly for Go created threads,
    the use of pthread_create in an external library is still resulting in
    the system libpthread version being used directly.
    This is pretty strange.
    Does pthread_create appear in the dynamic symbol table of the Go
    binary? You can dump the dynamic symbol table using readelf -s and
    looking at the .dynsym section, or by using nm --dynamic.
    it does.
    Symbol table '.dynsym' contains 31 entries:
    Num: Value Size Type Bind Vis Ndx Name
    0: 0000000000000000 0 NOTYPE LOCAL DEFAULT UND
    1: 000000000060144f 0 OBJECT GLOBAL DEFAULT 11 crosscall2
    2: 0000000000600f10 0 FUNC GLOBAL DEFAULT 11 _cgo_allocate
    3: 0000000000600fc0 0 FUNC GLOBAL DEFAULT 11 _cgo_panic
    4: 000000000099ab60 0 OBJECT GLOBAL DEFAULT 14 environ
    5: 000000000099aa88 0 OBJECT GLOBAL DEFAULT 14 __progname
    6: 000000000099aa44 0 OBJECT GLOBAL DEFAULT 14 __guard_local
    7: 0000000000601220 0 OBJECT GLOBAL DEFAULT 11 pthread_create
  • Joel Sing at Nov 27, 2012 at 4:19 pm

    On 27 November 2012 04:10, minux wrote:
    On Tue, Nov 27, 2012 at 1:02 AM, Ian Lance Taylor wrote:
    On Mon, Nov 26, 2012 at 8:40 AM, Joel Sing wrote:
    Hrmmm... bad news - while this works correctly for Go created threads,
    the use of pthread_create in an external library is still resulting in
    the system libpthread version being used directly.
    This is pretty strange.

    Does pthread_create appear in the dynamic symbol table of the Go
    binary? You can dump the dynamic symbol table using readelf -s and
    looking at the .dynsym section, or by using nm --dynamic.
    it does.
    Symbol table '.dynsym' contains 31 entries:
    Num: Value Size Type Bind Vis Ndx Name
    0: 0000000000000000 0 NOTYPE LOCAL DEFAULT UND
    1: 000000000060144f 0 OBJECT GLOBAL DEFAULT 11 crosscall2
    2: 0000000000600f10 0 FUNC GLOBAL DEFAULT 11 _cgo_allocate
    3: 0000000000600fc0 0 FUNC GLOBAL DEFAULT 11 _cgo_panic
    4: 000000000099ab60 0 OBJECT GLOBAL DEFAULT 14 environ
    5: 000000000099aa88 0 OBJECT GLOBAL DEFAULT 14 __progname
    6: 000000000099aa44 0 OBJECT GLOBAL DEFAULT 14 __guard_local
    7: 0000000000601220 0 OBJECT GLOBAL DEFAULT 11 pthread_create
    Ugh, so I missed an important part in the merge (the dynsym export).
    Having fixed this I now get the pthread_create symbol in the .dynsym
    table and the external library is using the correct pthread_create
    function. However, we now have another issue...

    The pthread_create entry in the .dynsym table generated by Go has a
    size of 0. The libpthread entry has an actual size (currently 603
    bytes). This is also referenced in the external library. When the
    binding occurs ld.so loads compares the sizes and if they are not the
    same, it generates a warning. This means that any external code that
    calls pthread_create will result in:

    WARNING: symbol(pthread_create) size mismatch, relink your program

    The only real solution would be to make the .dynsym pthread_create
    entry generated by Go match the size up with the libpthread .dynsym
    entry, which is a pretty large hack.

    So I guess we either put up with this, or we return to the idea of a
    TLS canary slot and run the risk of faulting on read...
  • Ian Lance Taylor at Nov 27, 2012 at 4:48 pm

    On Tue, Nov 27, 2012 at 8:18 AM, Joel Sing wrote:
    The pthread_create entry in the .dynsym table generated by Go has a
    size of 0. The libpthread entry has an actual size (currently 603
    bytes). This is also referenced in the external library. When the
    binding occurs ld.so loads compares the sizes and if they are not the
    same, it generates a warning. This means that any external code that
    calls pthread_create will result in:

    WARNING: symbol(pthread_create) size mismatch, relink your program
    That warning would be appropriate for a variable, but it makes no
    sense for a function. Make sure that pthread_create has type STT_FUNC
    in your entry in the dynamic symbol table.

    If your dynamic linker is really issuing a warning about a size change
    in an STT_FUNC symbol, fix the dynamic linker.

    Ian
  • Joel Sing at Nov 27, 2012 at 5:15 pm

    On 28 November 2012 03:48, Ian Lance Taylor wrote:
    On Tue, Nov 27, 2012 at 8:18 AM, Joel Sing wrote:

    The pthread_create entry in the .dynsym table generated by Go has a
    size of 0. The libpthread entry has an actual size (currently 603
    bytes). This is also referenced in the external library. When the
    binding occurs ld.so loads compares the sizes and if they are not the
    same, it generates a warning. This means that any external code that
    calls pthread_create will result in:

    WARNING: symbol(pthread_create) size mismatch, relink your program
    That warning would be appropriate for a variable, but it makes no
    sense for a function. Make sure that pthread_create has type STT_FUNC
    in your entry in the dynamic symbol table.
    Indeed - thanks for the pointer. The Go generated binary has
    pthread_create as STT_OBJECT rather than STT_FUNC:

    7: 0000000000601220 0 OBJECT GLOBAL DEFAULT 11 pthread_create

    So it would appear that "#pragma dynexport pthread_create
    pthread_create" is resulting in the wrong symbol type. Is there
    something else that we should be adding/doing, or is this a bug in the
    Go linker?
    If your dynamic linker is really issuing a warning about a size change
    in an STT_FUNC symbol, fix the dynamic linker.
    The ld.so code appears correct - it only warns for
    ELF_ST_TYPE(*this)->st_info) != STT_FUNC.
  • Ian Lance Taylor at Nov 27, 2012 at 6:45 pm

    On Tue, Nov 27, 2012 at 9:15 AM, Joel Sing wrote:
    So it would appear that "#pragma dynexport pthread_create
    pthread_create" is resulting in the wrong symbol type. Is there
    something else that we should be adding/doing, or is this a bug in the
    Go linker?
    I think this is something that has to be changed in the Go linker
    itself, yes. Offhand I'm not sure what is involved.

    Ian
  • Minux at Nov 27, 2012 at 8:54 pm

    On Wed, Nov 28, 2012 at 2:45 AM, Ian Lance Taylor wrote:
    On Tue, Nov 27, 2012 at 9:15 AM, Joel Sing wrote:
    So it would appear that "#pragma dynexport pthread_create
    pthread_create" is resulting in the wrong symbol type. Is there
    something else that we should be adding/doing, or is this a bug in the
    Go linker?
    I think this is something that has to be changed in the Go linker
    itself, yes. Offhand I'm not sure what is involved.
    I propose we extent format of #pragma dynexport to something like this:
    #pragma dynexport pthread_create pthread_create func
    this is a backward compatible change.

    If this sounds good to you, I will prepare a CL.
  • Ian Lance Taylor at Nov 27, 2012 at 9:13 pm

    On Tue, Nov 27, 2012 at 12:53 PM, minux wrote:
    On Wed, Nov 28, 2012 at 2:45 AM, Ian Lance Taylor wrote:
    On Tue, Nov 27, 2012 at 9:15 AM, Joel Sing wrote:
    So it would appear that "#pragma dynexport pthread_create
    pthread_create" is resulting in the wrong symbol type. Is there
    something else that we should be adding/doing, or is this a bug in the
    Go linker?
    I think this is something that has to be changed in the Go linker
    itself, yes. Offhand I'm not sure what is involved.
    I propose we extent format of #pragma dynexport to something like this:
    #pragma dynexport pthread_create pthread_create func
    this is a backward compatible change.

    If this sounds good to you, I will prepare a CL.
    I'm not opposed to that but I wonder if it is necessary. We know
    whether the internal symbol is TEXT or not, don't we? Could we just
    base the decision on that?

    Ian
  • Russ Cox at Nov 27, 2012 at 9:07 pm
    I like what Ian said. If for some reason that doesn't work let's just
    make ld strcmp(name, "pthread_create") == 0 to make its decision for
    now. We still haven't actually seen this work, and it's pretty
    disgusting already.

    Russ
  • Minux at Nov 27, 2012 at 9:52 pm

    On Wed, Nov 28, 2012 at 5:06 AM, Ian Lance Taylor wrote:
    On Tue, Nov 27, 2012 at 12:53 PM, minux wrote:
    I propose we extent format of #pragma dynexport to something like this:
    #pragma dynexport pthread_create pthread_create func
    this is a backward compatible change.
    If this sounds good to you, I will prepare a CL.
    I'm not opposed to that but I wonder if it is necessary. We know
    whether the internal symbol is TEXT or not, don't we? Could we just
    base the decision on that?
    Silly me. It should be a bug in 6l. CL in a moment.
    Possibly related to issue 4267?
  • Minux at Nov 27, 2012 at 9:51 pm

    On Wed, Nov 28, 2012 at 5:45 AM, minux wrote:
    On Wed, Nov 28, 2012 at 5:06 AM, Ian Lance Taylor wrote:
    On Tue, Nov 27, 2012 at 12:53 PM, minux wrote:
    I propose we extent format of #pragma dynexport to something like this:
    #pragma dynexport pthread_create pthread_create func
    this is a backward compatible change.
    If this sounds good to you, I will prepare a CL.
    I'm not opposed to that but I wonder if it is necessary. We know
    whether the internal symbol is TEXT or not, don't we? Could we just
    base the decision on that?
    Silly me. It should be a bug in 6l. CL in a moment.
    Possibly related to issue 4267?
    This should fix the 6l for this issue, 8l could be similarly be fixed.
    I still need to audit other part of the linker for this same problem.

    diff -r ffd1e075c260 src/cmd/6l/asm.c
    --- a/src/cmd/6l/asm.c Fri Nov 23 22:15:26 2012 -0800
    +++ b/src/cmd/6l/asm.c Wed Nov 28 09:35:05 2012 +0800
    @@ -455,7 +455,7 @@
    adduint32(d, addstring(lookup(".dynstr", 0), name));
    /* type */
    t = STB_GLOBAL << 4;
    - if(s->dynexport && s->type == STEXT)
    + if(s->dynexport && (s->type&SMASK) == STEXT)
    t |= STT_FUNC;
    else
    t |= STT_OBJECT;
  • Joel Sing at Nov 28, 2012 at 12:38 am

    On 28 November 2012 08:50, minux wrote:
    On Wed, Nov 28, 2012 at 5:45 AM, minux wrote:
    On Wed, Nov 28, 2012 at 5:06 AM, Ian Lance Taylor wrote:
    On Tue, Nov 27, 2012 at 12:53 PM, minux wrote:
    I propose we extent format of #pragma dynexport to something like this:
    #pragma dynexport pthread_create pthread_create func
    this is a backward compatible change.
    If this sounds good to you, I will prepare a CL.
    I'm not opposed to that but I wonder if it is necessary. We know
    whether the internal symbol is TEXT or not, don't we? Could we just
    base the decision on that?
    Silly me. It should be a bug in 6l. CL in a moment.
    Possibly related to issue 4267?
    This should fix the 6l for this issue, 8l could be similarly be fixed.
    I still need to audit other part of the linker for this same problem.

    diff -r ffd1e075c260 src/cmd/6l/asm.c
    --- a/src/cmd/6l/asm.c Fri Nov 23 22:15:26 2012 -0800
    +++ b/src/cmd/6l/asm.c Wed Nov 28 09:35:05 2012 +0800
    @@ -455,7 +455,7 @@
    adduint32(d, addstring(lookup(".dynstr", 0), name));
    /* type */
    t = STB_GLOBAL << 4;
    - if(s->dynexport && s->type == STEXT)
    + if(s->dynexport && (s->type&SMASK) == STEXT)
    t |= STT_FUNC;
    else
    t |= STT_OBJECT;
    Yes, this fixes the issue:

    Symbol table '.dynsym' contains 29 entries:
    Num: Value Size Type Bind Vis Ndx Name
    0: 0000000000000000 0 NOTYPE LOCAL DEFAULT UND
    1: 000000000041b81f 0 FUNC GLOBAL DEFAULT 11 crosscall2
    2: 000000000041b2d0 0 FUNC GLOBAL DEFAULT 11 _cgo_allocate
    3: 000000000041b380 0 FUNC GLOBAL DEFAULT 11 _cgo_panic
    4: 0000000000520400 0 OBJECT GLOBAL DEFAULT 14 environ
    5: 00000000005203b8 0 OBJECT GLOBAL DEFAULT 14 __progname
    6: 0000000000520374 0 OBJECT GLOBAL DEFAULT 14 __guard_local
    7: 000000000041b6c0 0 FUNC GLOBAL DEFAULT 11 pthread_create

    From a quick glance there appear to be a lot of comparisons without masking...

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedNov 16, '12 at 4:07p
activeNov 28, '12 at 12:38a
posts23
users4
websitegolang.org

People

Translate

site design / logo © 2022 Grokbase