Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

cmd/go/internal/work: CGO builds don't link all required libraries when pkg-config files use Requires.private #70209

Closed
stanhu opened this issue Nov 5, 2024 · 5 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.

Comments

@stanhu
Copy link

stanhu commented Nov 5, 2024

Go version

go version go version go1.23.1 darwin/arm64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='arm64'
GOBIN='/Users/stanhu/.local/share/mise/installs/go/1.23.1/bin'
GOCACHE='/Users/stanhu/Library/Caches/go-build'
GOENV='/Users/stanhu/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/stanhu/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/stanhu/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/Users/stanhu/.local/share/mise/installs/go/1.23.1'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/Users/stanhu/.local/share/mise/installs/go/1.23.1/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.23.1'
GODEBUG=''
GOTELEMETRY='local'
GOTELEMETRYDIR='/Users/stanhu/Library/Application Support/go/telemetry'
GCCGO='gccgo'
GOARM64='v8.0'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/dev/null'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/k4/mbnh565n4wg6xpbqv45tv91h0000gn/T/go-build3575868181=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

On macOS with libicu 76, I tried to compile this cgo program (source below):

PKG_CONFIG_PATH=/opt/homebrew/Cellar/icu4c@76/76.1_1/lib/pkgconfig go run test.go
# command-line-arguments
/Users/stanhu/.local/share/mise/installs/go/1.23.1/pkg/tool/darwin_arm64/link: running clang failed: exit status 1
/opt/homebrew/opt/ccache/libexec/clang -arch arm64 -Wl,-S -Wl,-x -o $WORK/b001/exe/test -Qunused-arguments /var/folders/k4/mbnh565n4wg6xpbqv45tv91h0000gn/T/go-link-139099345/go.o /var/folders/k4/mbnh565n4wg6xpbqv45tv91h0000gn/T/go-link-139099345/000000.o /var/folders/k4/mbnh565n4wg6xpbqv45tv91h0000gn/T/go-link-139099345/000001.o /var/folders/k4/mbnh565n4wg6xpbqv45tv91h0000gn/T/go-link-139099345/000002.o /var/folders/k4/mbnh565n4wg6xpbqv45tv91h0000gn/T/go-link-139099345/000003.o /var/folders/k4/mbnh565n4wg6xpbqv45tv91h0000gn/T/go-link-139099345/000004.o /var/folders/k4/mbnh565n4wg6xpbqv45tv91h0000gn/T/go-link-139099345/000005.o /var/folders/k4/mbnh565n4wg6xpbqv45tv91h0000gn/T/go-link-139099345/000006.o /var/folders/k4/mbnh565n4wg6xpbqv45tv91h0000gn/T/go-link-139099345/000007.o /var/folders/k4/mbnh565n4wg6xpbqv45tv91h0000gn/T/go-link-139099345/000008.o /var/folders/k4/mbnh565n4wg6xpbqv45tv91h0000gn/T/go-link-139099345/000009.o /var/folders/k4/mbnh565n4wg6xpbqv45tv91h0000gn/T/go-link-139099345/000010.o /var/folders/k4/mbnh565n4wg6xpbqv45tv91h0000gn/T/go-link-139099345/000011.o -O2 -g -L/opt/homebrew/Cellar/icu4c@76/76.1_1/lib -licui18n -O2 -g -framework CoreFoundation -lresolv
Undefined symbols for architecture arm64:
  "_u_strFromUTF8_76", referenced from:
      _utf8ToUpper in 000001.o
  "_u_strToUTF8_76", referenced from:
      __cgo_476e3331fcdb_Cfunc_u_strToUTF8 in 000001.o
  "_u_strToUpper_76", referenced from:
      _utf8ToUpper in 000001.o
ld: symbol(s) not found for architecture arm64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

Source code:

package main

/*
#cgo pkg-config: icu-i18n
#include <unicode/ustring.h>
#include <unicode/utypes.h>
#include <stdlib.h>
#include <string.h>

// Convert UTF-8 string to uppercase using ICU, returning NULL on failure
UChar* utf8ToUpper(const char* src, int32_t* utf16Len) {
    UErrorCode status = U_ZERO_ERROR;
    int32_t srcLength = strlen(src);
    int32_t utf16Capacity = srcLength * 2;

    UChar* utf16Str = (UChar*)malloc(utf16Capacity * sizeof(UChar));
    if (utf16Str == NULL) {
        return NULL;
    }

    // Convert to UTF-16
    u_strFromUTF8(utf16Str, utf16Capacity, utf16Len, src, srcLength, &status);
    if (U_FAILURE(status)) {
        free(utf16Str);
        return NULL;
    }

    // Convert UTF-16 string to uppercase
    u_strToUpper(utf16Str, utf16Capacity, utf16Str, *utf16Len, NULL, &status);
    if (U_FAILURE(status)) {
        free(utf16Str);
        return NULL;
    }

    return utf16Str;
}
*/
import "C"
import (
    "fmt"
    "unsafe"
)


func ToUpper(input string) (string, error) {
    cInput := C.CString(input)
    defer C.free(unsafe.Pointer(cInput))

    var utf16Len C.int32_t

    // Call the C function to convert to uppercase
    utf16Str := C.utf8ToUpper(cInput, &utf16Len)
    if utf16Str == nil {
        return "", fmt.Errorf("ICU error: failed to convert string to uppercase")
    }
    defer C.free(unsafe.Pointer(utf16Str))

    // Allocate buffer for UTF-8 output
    utf8Capacity := utf16Len * 4
    utf8Buffer := make([]byte, utf8Capacity)
    var status C.UErrorCode = C.U_ZERO_ERROR
    C.u_strToUTF8((*C.char)(unsafe.Pointer(&utf8Buffer[0])), utf8Capacity, nil, utf16Str, utf16Len, &status)

    if status != C.U_ZERO_ERROR {
        return "", fmt.Errorf("ICU error during UTF-16 to UTF-8 conversion: %d", status)
    }

    return string(utf8Buffer), nil
}

func main() {
    result, err := ToUpper("Hello, World!")
    if err != nil {
        fmt.Println("Error:", err)
        return
    }
    fmt.Println("Uppercase:", result)
}

What did you see happen?

This worked fine with libicu 75:

% PKG_CONFIG_PATH=/opt/homebrew/Cellar/icu4c@75/75.1/lib/pkgconfig go run test.go
Uppercase: HELLO, WORLD!

This difference is due to the introduction of Requires.private instead of Requires in libicu-i18n.pc: unicode-org/icu@199bc82

In libicu 75:

% grep Requires /opt/homebrew/Cellar/icu4c@75/75.1/lib/pkgconfig/icu-i18n.pc
Requires: icu-uc

In Iibicu 76, I see:

% grep Requires /opt/homebrew/Cellar/icu4c@76/76.1_1/lib/pkgconfig/icu-i18n.pc
Requires.private: icu-uc

What's the difference between Requires and Requires.private? The man pkg-config says:

       Requires.private:
              A list of packages required by this package. The difference from
              Requires  is that the packages listed under Requires.private are
              not taken into account when a flag list is computed for  dynami-
              cally linked executable (i.e., when --static was not specified).
              In the situation where each .pc file corresponds to  a  library,
              Requires.private shall be used exclusively to specify the depen-
              dencies between the libraries.

You can see that the pkg-config --libs output omits these Requires.private libraries:

% PKG_CONFIG_PATH=/opt/homebrew/Cellar/icu4c@76/76.1_1/lib/pkgconfig pkg-config --libs -- icu-i18n
-L/opt/homebrew/Cellar/icu4c@76/76.1_1/lib -licui18n
% PKG_CONFIG_PATH=/opt/homebrew/Cellar/icu4c@75/75.1/lib/pkgconfig pkg-config --libs -- icu-i18n
-L/opt/homebrew/Cellar/icu4c@75/75.1/lib -licui18n -licuuc -licudata

If I add --static, I see even more libraries:

PKG_CONFIG_PATH=/opt/homebrew/Cellar/icu4c@76/76.1_1/lib/pkgconfig pkg-config --static --libs -- icu-i18n
-L/opt/homebrew/Cellar/icu4c@76/76.1_1/lib -licui18n -licuuc -licudata -lpthread -lm

Should Go add the --static flag here?

out, err = sh.runOut(p.Dir, nil, b.PkgconfigCmd(), "--libs", pcflags, "--", pkgs)

What did you expect to see?

See above.

@stanhu
Copy link
Author

stanhu commented Nov 6, 2024

It seems this issue was discussed in #12058 and #26492.

@dr2chase
Copy link
Contributor

dr2chase commented Nov 8, 2024

@ianlancetaylor

@dr2chase dr2chase added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. compiler/runtime Issues related to the Go compiler and/or runtime. labels Nov 8, 2024
@ianlancetaylor
Copy link
Member

I don't see anything showing that you are requesting, or doing, a static link. So I don't understand why you are getting this error.

I think you can work around the problem by changing your file to say

#cgo pkg-config: --static icu-i18n

but I don't understand why that should be necessary. Do you know? Thanks.

@stanhu
Copy link
Author

stanhu commented Nov 8, 2024

Yes, I did that; actually I just added this to be explicit:

// #cgo pkg-config: icu-i18n icu-uc

but I don't understand why that should be necessary.

As I mentioned, the update to libicu moved icu-uc into Requires.private, so icu-uc was no longer automatically linked.

I was wondering whether pkg-config should have included --static by default. It looks like the various issues I linked have also brought that up, so we can close this issue in favor of #26492.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

4 participants