From 15aad5d3e6d97627345586e5ee92a896667bb33a Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 6 Aug 2015 16:04:39 -0700 Subject: [PATCH 1/2] make binary: do not ignore unresolved symbols TL;DR: stop building static binary that may fail Linker flag --unresolved-symbols=ignore-in-shared-libs was added in commit 06d0843 two years ago for the static build case, presumably to avoid dealing with problem of missing libraries. For the record, this is what ld(1) man page says: > --unresolved-symbols=method > Determine how to handle unresolved symbols. There are four > possible values for method: > ......... > ignore-in-shared-libs > Report unresolved symbols that come from regular object files, > but ignore them if they come from shared libraries. This can > be useful when creating a dynamic binary and it is known that > all the shared libraries that it should be referencing are > included on the linker's command line. Here, the flag is not used for its purpose ("creating a dynamic binary") and does more harm than good. Instead of complaining about missing symbols as it should do if some libraries are missing from LIBS/LDFLAGS, it lets ld create a binary with unresolved symbols, ike this: $ readelf -s bundles/1.7.1/binary/docker-1.7.1 | grep -w UND ........ 21029: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND dlopen ......... Such binary is working just fine -- until code calls one of those functions, then it crashes (for apparently no reason, i.e. it is impossible to tell why from the diagnistics printed). In other words, adding this flag allows to build a static binary with missing libraries, hiding the problem from both a developer (who forgot to add a library to #cgo: LDFLAGS -- I was one such developer a few days ago when I was working on ploop graphdriver) and from a user (who expects the binary to work without crashing, and it does that until the code calls a function in one of those libraries). Removing the flag immediately unveils the problem (as it should): /usr/lib/gcc/x86_64-linux-gnu/4.8/../../../x86_64-linux-gnu/libsqlite3.a(sqlite3.o): In function `unixDlError': (.text+0x20971): undefined reference to `dlerror' /usr/lib/gcc/x86_64-linux-gnu/4.8/../../../x86_64-linux-gnu/libsqlite3.a(sqlite3.o): In function `unixDlClose': (.text+0x8814): undefined reference to `dlclose' The problem is, gosqlite package says: #cgo LDFLAGS: -lsqlite3 which is enough for dynamic linking, as indirect dependencies (i.e. libraries required by libsqlite3.so) are listed in .so file and will be resolved dynamically by ldd upon executing the binary. For static linking though, one has to list all the required libraries, both direct and indirect. For libraries with pkgconfig support the list of required libraries can be obtained with pkg-config: $ pkg-config --libs sqlite3 # dynamic linking case -lsqlite3 $ pkg-config --libs --static sqlite3 # static case -lsqlite3 -ldl -lpthread It seems that all one has to do is to fix gosqlite this way: -#cgo LDFLAGS: -lsqlite3 +#cgo pkg-config: sqlite3 Unfortunately, cmd/go doesn't know that it needs to pass --static flag to pkg-config in case of static linking (see https://github.com/golang/go/issues/12058). So, for one, one has to do one of these things: 1. Patch sqlite.go like this: -#cgo LDFLAGS: -lsqlite3 +#cgo pkg-config: --static sqlite3 (this is exactly what I do in goploop, see https://github.com/kolyshkin/goploop/commit/e9aa072f51) 2. Patch sqlite.go like this: -#cgo LDFLAGS: -lsqlite3 +#cgo LDFLAGS: -lsqlite3 -ldl -lpthread (I would submit this patch to gosqlite but it seems that https://code.google.com/p/gosqlite/ is deserted and not maintained, and patching it here is not right as it is "vendored") 3. Explicitly add -ldl for the static link case. This is what this patch does. 4. Fork sqlite to github and maintain it there. Personally I am not ready for that, as I'm neither a Go expert nor gosqlite user. Now, #3 doesn't look like a clear solution, but nevertheless it makes the build much better than it was before. Signed-off-by: Kir Kolyshkin --- hack/make.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hack/make.sh b/hack/make.sh index 1eef426790..b27d1e7131 100755 --- a/hack/make.sh +++ b/hack/make.sh @@ -152,7 +152,7 @@ TESTFLAGS+=" -test.timeout=${TIMEOUT}" # A few more flags that are specific just to building a completely-static binary (see hack/make/binary) # PLEASE do not use these anywhere else. -EXTLDFLAGS_STATIC_DOCKER="$EXTLDFLAGS_STATIC -lpthread -Wl,--unresolved-symbols=ignore-in-object-files" +EXTLDFLAGS_STATIC_DOCKER="$EXTLDFLAGS_STATIC -lpthread -ldl" LDFLAGS_STATIC_DOCKER=" $LDFLAGS_STATIC -extldflags \"$EXTLDFLAGS_STATIC_DOCKER\" From 46df9e4ec3453dd6f46261e0eeb276965af20880 Mon Sep 17 00:00:00 2001 From: Jessica Frazelle Date: Fri, 4 Sep 2015 13:15:09 -0700 Subject: [PATCH 2/2] update sqlite3 vendor with fix for static builds Signed-off-by: Jessica Frazelle --- hack/make.sh | 2 +- hack/vendor.sh | 2 +- .../src/github.com/mattn/go-sqlite3/README.md | 4 ++ .../github.com/mattn/go-sqlite3/sqlite3.go | 35 +++++------------ .../mattn/go-sqlite3/sqlite3_icu.go | 13 +++++++ .../go-sqlite3/sqlite3_load_extension.go | 39 +++++++++++++++++++ .../go-sqlite3/sqlite3_omit_load_extension.go | 19 +++++++++ 7 files changed, 87 insertions(+), 27 deletions(-) create mode 100644 vendor/src/github.com/mattn/go-sqlite3/sqlite3_icu.go create mode 100644 vendor/src/github.com/mattn/go-sqlite3/sqlite3_load_extension.go create mode 100644 vendor/src/github.com/mattn/go-sqlite3/sqlite3_omit_load_extension.go diff --git a/hack/make.sh b/hack/make.sh index b27d1e7131..b992ec9786 100755 --- a/hack/make.sh +++ b/hack/make.sh @@ -143,7 +143,7 @@ fi EXTLDFLAGS_STATIC='-static' # ORIG_BUILDFLAGS is necessary for the cross target which cannot always build # with options like -race. -ORIG_BUILDFLAGS=( -a -tags "netgo static_build $DOCKER_BUILDTAGS" -installsuffix netgo ) +ORIG_BUILDFLAGS=( -a -tags "netgo static_build sqlite_omit_load_extension $DOCKER_BUILDTAGS" -installsuffix netgo ) # see https://github.com/golang/go/issues/9369#issuecomment-69864440 for why -installsuffix is necessary here BUILDFLAGS=( $BUILDFLAGS "${ORIG_BUILDFLAGS[@]}" ) # Test timeout. diff --git a/hack/vendor.sh b/hack/vendor.sh index 8d470407e8..a06e1d9396 100755 --- a/hack/vendor.sh +++ b/hack/vendor.sh @@ -13,7 +13,7 @@ clone git github.com/go-check/check 64131543e7896d5bcc6bd5a76287eb75ea96c673 clone git github.com/gorilla/context 14f550f51a clone git github.com/gorilla/mux e444e69cbd clone git github.com/kr/pty 5cf931ef8f -clone git github.com/mattn/go-sqlite3 b4142c444a8941d0d92b0b7103a24df9cd815e42 +clone git github.com/mattn/go-sqlite3 v1.1.0 clone git github.com/microsoft/hcsshim 7f646aa6b26bcf90caee91e93cde4a80d0d8a83e clone git github.com/mistifyio/go-zfs v2.1.1 clone git github.com/tchap/go-patricia v2.1.0 diff --git a/vendor/src/github.com/mattn/go-sqlite3/README.md b/vendor/src/github.com/mattn/go-sqlite3/README.md index 04ddfd0969..efd0046e28 100644 --- a/vendor/src/github.com/mattn/go-sqlite3/README.md +++ b/vendor/src/github.com/mattn/go-sqlite3/README.md @@ -30,6 +30,10 @@ FAQ Use `go build --tags "libsqlite3 linux"` +* Want to build go-sqlite3 with icu extension. + + Use `go build --tags "icu"` + * Can't build go-sqlite3 on windows 64bit. > Probably, you are using go 1.0, go1.0 has a problem when it comes to compiling/linking on windows 64bit. diff --git a/vendor/src/github.com/mattn/go-sqlite3/sqlite3.go b/vendor/src/github.com/mattn/go-sqlite3/sqlite3.go index 233e7e9107..e2a379626e 100644 --- a/vendor/src/github.com/mattn/go-sqlite3/sqlite3.go +++ b/vendor/src/github.com/mattn/go-sqlite3/sqlite3.go @@ -48,21 +48,21 @@ _sqlite3_bind_blob(sqlite3_stmt *stmt, int n, void *p, int np) { #include static int -_sqlite3_exec(sqlite3* db, const char* pcmd, long* rowid, long* changes) +_sqlite3_exec(sqlite3* db, const char* pcmd, long long* rowid, long long* changes) { int rv = sqlite3_exec(db, pcmd, 0, 0, 0); - *rowid = (long) sqlite3_last_insert_rowid(db); - *changes = (long) sqlite3_changes(db); + *rowid = (long long) sqlite3_last_insert_rowid(db); + *changes = (long long) sqlite3_changes(db); return rv; } static int -_sqlite3_step(sqlite3_stmt* stmt, long* rowid, long* changes) +_sqlite3_step(sqlite3_stmt* stmt, long long* rowid, long long* changes) { int rv = sqlite3_step(stmt); sqlite3* db = sqlite3_db_handle(stmt); - *rowid = (long) sqlite3_last_insert_rowid(db); - *changes = (long) sqlite3_changes(db); + *rowid = (long long) sqlite3_last_insert_rowid(db); + *changes = (long long) sqlite3_changes(db); return rv; } @@ -243,7 +243,7 @@ func (c *SQLiteConn) exec(cmd string) (driver.Result, error) { pcmd := C.CString(cmd) defer C.free(unsafe.Pointer(pcmd)) - var rowid, changes C.long + var rowid, changes C.longlong rv := C._sqlite3_exec(c.db, pcmd, &rowid, &changes) if rv != C.SQLITE_OK { return nil, c.lastError() @@ -355,23 +355,8 @@ func (d *SQLiteDriver) Open(dsn string) (driver.Conn, error) { conn := &SQLiteConn{db: db, loc: loc, txlock: txlock} if len(d.Extensions) > 0 { - rv = C.sqlite3_enable_load_extension(db, 1) - if rv != C.SQLITE_OK { - return nil, errors.New(C.GoString(C.sqlite3_errmsg(db))) - } - - for _, extension := range d.Extensions { - cext := C.CString(extension) - defer C.free(unsafe.Pointer(cext)) - rv = C.sqlite3_load_extension(db, cext, nil, nil) - if rv != C.SQLITE_OK { - return nil, errors.New(C.GoString(C.sqlite3_errmsg(db))) - } - } - - rv = C.sqlite3_enable_load_extension(db, 0) - if rv != C.SQLITE_OK { - return nil, errors.New(C.GoString(C.sqlite3_errmsg(db))) + if err := conn.loadExtensions(d.Extensions); err != nil { + return nil, err } } @@ -536,7 +521,7 @@ func (s *SQLiteStmt) Exec(args []driver.Value) (driver.Result, error) { C.sqlite3_clear_bindings(s.s) return nil, err } - var rowid, changes C.long + var rowid, changes C.longlong rv := C._sqlite3_step(s.s, &rowid, &changes) if rv != C.SQLITE_ROW && rv != C.SQLITE_OK && rv != C.SQLITE_DONE { err := s.c.lastError() diff --git a/vendor/src/github.com/mattn/go-sqlite3/sqlite3_icu.go b/vendor/src/github.com/mattn/go-sqlite3/sqlite3_icu.go new file mode 100644 index 0000000000..4c5492bc23 --- /dev/null +++ b/vendor/src/github.com/mattn/go-sqlite3/sqlite3_icu.go @@ -0,0 +1,13 @@ +// Copyright (C) 2014 Yasuhiro Matsumoto . +// +// Use of this source code is governed by an MIT-style +// license that can be found in the LICENSE file. +// +build icu + +package sqlite3 + +/* +#cgo LDFLAGS: -licuuc -licui18n +#cgo CFLAGS: -DSQLITE_ENABLE_ICU +*/ +import "C" diff --git a/vendor/src/github.com/mattn/go-sqlite3/sqlite3_load_extension.go b/vendor/src/github.com/mattn/go-sqlite3/sqlite3_load_extension.go new file mode 100644 index 0000000000..0251016b1c --- /dev/null +++ b/vendor/src/github.com/mattn/go-sqlite3/sqlite3_load_extension.go @@ -0,0 +1,39 @@ +// Copyright (C) 2014 Yasuhiro Matsumoto . +// +// Use of this source code is governed by an MIT-style +// license that can be found in the LICENSE file. +// +build !sqlite_omit_load_extension + +package sqlite3 + +/* +#include +#include +*/ +import "C" +import ( + "errors" + "unsafe" +) + +func (c *SQLiteConn) loadExtensions(extensions []string) error { + rv := C.sqlite3_enable_load_extension(c.db, 1) + if rv != C.SQLITE_OK { + return errors.New(C.GoString(C.sqlite3_errmsg(c.db))) + } + + for _, extension := range extensions { + cext := C.CString(extension) + defer C.free(unsafe.Pointer(cext)) + rv = C.sqlite3_load_extension(c.db, cext, nil, nil) + if rv != C.SQLITE_OK { + return errors.New(C.GoString(C.sqlite3_errmsg(c.db))) + } + } + + rv = C.sqlite3_enable_load_extension(c.db, 0) + if rv != C.SQLITE_OK { + return errors.New(C.GoString(C.sqlite3_errmsg(c.db))) + } + return nil +} diff --git a/vendor/src/github.com/mattn/go-sqlite3/sqlite3_omit_load_extension.go b/vendor/src/github.com/mattn/go-sqlite3/sqlite3_omit_load_extension.go new file mode 100644 index 0000000000..a80cf8745c --- /dev/null +++ b/vendor/src/github.com/mattn/go-sqlite3/sqlite3_omit_load_extension.go @@ -0,0 +1,19 @@ +// Copyright (C) 2014 Yasuhiro Matsumoto . +// +// Use of this source code is governed by an MIT-style +// license that can be found in the LICENSE file. +// +build sqlite_omit_load_extension + +package sqlite3 + +/* +#cgo CFLAGS: -DSQLITE_OMIT_LOAD_EXTENSION +*/ +import "C" +import ( + "errors" +) + +func (c *SQLiteConn) loadExtensions(extensions []string) error { + return errors.New("Extensions have been disabled for static builds") +}