Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

cmd/cgo/internal/testshared: gccgo tests fail on some linux/ppc64le gccgo compilers #60798

Open
pmur opened this issue Jun 14, 2023 · 10 comments
Labels
arch-ppc64x GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@pmur
Copy link
Contributor

pmur commented Jun 14, 2023

What version of Go are you using (go version)?

I have observed this using the fedora 36 system gcc and gccgo. It also happens using IBM's advance-toolchain 15 and newer.

$ go version
go version devel go1.21-3aea422e2c Wed Jun 14 18:53:13 2023 +0000 linux/ppc64le
$ gccgo -dumpversion
12
$ gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/ppc64le-redhat-linux/12/lto-wrapper
OFFLOAD_TARGET_NAMES=nvptx-none
OFFLOAD_TARGET_DEFAULT=1
Target: ppc64le-redhat-linux
Configured with: ../configure --enable-bootstrap --enable-languages=c,c++,fortran,objc,obj-c++,ada,go,lto --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=http://bugzilla.redhat.com/bugzilla --enable-shared --enable-threads=posix --enable-checking=release --enable-targets=powerpcle-linux --disable-multilib --with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions --enable-gnu-unique-object --enable-linker-build-id --with-gcc-major-version-only --enable-libstdcxx-backtrace --with-linker-hash-style=gnu --enable-plugin --enable-initfini-array --with-isl=/builddir/build/BUILD/gcc-12.2.1-20221121/obj-ppc64le-redhat-linux/isl-install --enable-offload-targets=nvptx-none --without-cuda-driver --enable-offload-defaulted --enable-gnu-indirect-function --enable-secureplt --with-long-double-128 --with-long-double-format=ieee --with-cpu-32=power8 --with-tune-32=power8 --with-cpu-64=power8 --with-tune-64=power8 --build=ppc64le-redhat-linux --with-build-config=bootstrap-lto --enable-link-serialization=1
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 12.2.1 20221121 (Red Hat 12.2.1-4) (GCC) 

What did you do?

$ cd $GOROOT/src/cmd/cgo/internal/testshared
$ go test -v -test.run=TestGoPathShlibGccgo 
--- FAIL: TestGoPathShlibGccgo (0.22s)
    shared_test.go:783: executing /home/murp/git/go/bin/go install -compiler=gccgo -linkshared ./exe failed exit status 2:
        panic: runtime error: invalid memory address or nil pointer dereference
        [signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0x4b074c]
        
        goroutine 1 [running]:
        debug/elf.(*Section).Open(0x0?)
        	/home/murp/git/go/src/debug/elf/file.go:138 +0x2c
        debug/elf.(*Section).Data(0x0)
        	/home/murp/git/go/src/debug/elf/file.go:119 +0x30
        cmd/go/internal/work.readpkglist({0xc00033c180?, 0x13d464?})
        	/home/murp/git/go/src/cmd/go/internal/work/action.go:387 +0x320
        cmd/go/internal/work.(*Builder).linkSharedAction.func1()
        	/home/murp/git/go/src/cmd/go/internal/work/action.go:786 +0x6c
        cmd/go/internal/work.(*Builder).cacheAction(0xc00034a000, {0xc00032e2a0, 0x24}, 0x0, 0xc000307558)
        	/home/murp/git/go/src/cmd/go/internal/work/action.go:418 +0xa8
        cmd/go/internal/work.(*Builder).linkSharedAction(0xc00034a000, 0x2, 0x2, {0xc00033c180, 0x5e}, 0x0)
        	/home/murp/git/go/src/cmd/go/internal/work/action.go:782 +0x128
        cmd/go/internal/work.(*Builder).addTransitiveLinkDeps(0xc0003078e8?, 0xc0003122c0, 0xc000312420, {0x0, 0x0})
        	/home/murp/git/go/src/cmd/go/internal/work/action.go:731 +0x2e0
        cmd/go/internal/work.(*Builder).LinkAction.func1()
        	/home/murp/git/go/src/cmd/go/internal/work/action.go:604 +0x390
        cmd/go/internal/work.(*Builder).cacheAction(0xc00034a000, {0x66d69f, 0x4}, 0xc00039c000, 0xc0003079a8)
        	/home/murp/git/go/src/cmd/go/internal/work/action.go:418 +0xa8
        cmd/go/internal/work.(*Builder).LinkAction(0xc00034a000, 0x1, 0x662500?, 0xc00031a001?)
        	/home/murp/git/go/src/cmd/go/internal/work/action.go:571 +0x84
        cmd/go/internal/work.(*Builder).AutoAction(0x0?, 0x0?, 0xc000324090?, 0x15?)
        	/home/murp/git/go/src/cmd/go/internal/work/action.go:427 +0x68
        cmd/go/internal/work.InstallPackages({0x781038, 0xb12760}, {0xc0000200e0, 0x1, 0x1}, {0xc00031a048, 0x1, 0x1})
        	/home/murp/git/go/src/cmd/go/internal/work/build.go:794 +0x5b8
        cmd/go/internal/work.runInstall({0x781038, 0xb12760}, 0xc000024480?, {0xc0000200e0?, 0x1, 0x1})
        	/home/murp/git/go/src/cmd/go/internal/work/build.go:722 +0x2e0
        main.invoke(0xad1b20, {0xc0000200b0, 0x4, 0x4})
        	/home/murp/git/go/src/cmd/go/main.go:264 +0x57c
        main.main()
        	/home/murp/git/go/src/cmd/go/main.go:182 +0x808
FAIL
exit status 1
FAIL	cmd/cgo/internal/testshared	1.270s

cmd/go crashes because it assumes .go_export section is present within the shared library built by gccgo, but it is missing.

@gopherbot gopherbot added this to the Gccgo milestone Jun 14, 2023
@ianlancetaylor
Copy link
Contributor

I suppose that cmd/go shouldn't crash, but why is the .go_export section missing? It should always be generated by gccgo. Is something removing it?

@ianlancetaylor ianlancetaylor added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. GoCommand cmd/go labels Jun 14, 2023
@gopherbot
Copy link

Change https://go.dev/cl/503496 mentions this issue: cmd/go: check for errors reading gccgo package list

gopherbot pushed a commit that referenced this issue Jun 14, 2023
Previously if there was something invalid about the package list
cmd/go would crash rather than reporting a useful error.

For #60798

Change-Id: I502facf41442ab49217405b5b1874fff52a6d416
Reviewed-on: https://go-review.googlesource.com/c/go/+/503496
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
@pmur
Copy link
Contributor Author

pmur commented Jun 14, 2023

It seems to be getting lost when linking the shared library. It exists in the input archive depBase.a, but is not in the resulting shared library, when the following runs:

/opt/at15.0/bin/gccgo -o $WORK/b004/libtestshared-depBase.so "-Wl,-(" -Wl,--whole-archive /tmp/shared_test3084291572/gopath/pkg/gccgo_linux_ppc64le_fPIC/testshared/depBase.a -Wl,--no-whole-archive "-Wl,-)" -Wl,--build-id=0x38416f4b645851384a4f5a346e564948596539682f38416f4b645851384a4f5a346e56494859653968 -zdefs -shared -nostdlib -lgo -lgcc_s -lgcc -lc -fPIC

@ianlancetaylor
Copy link
Contributor

What linker are you using?

@pmur
Copy link
Contributor Author

pmur commented Jun 15, 2023

In the above case GNU ld (GNU Binutils) 2.37.20220122.

@pmur
Copy link
Contributor Author

pmur commented Jun 15, 2023

Hrm, I suspect the issue is the .go_export section has the exclude flag set.

Looking at the output of gccgo, I see:

.section        .go_export,"ae",@progbits

Poking around the gccgo code, I wonder if the macro TARGET_AIX is not evaluating as expected.

@ianlancetaylor
Copy link
Contributor

I do see, in gcc/config/rs6000/linux64.h:

#undef	TARGET_AIX
#define	TARGET_AIX TARGET_64BIT

That dates back to 2023 and looks like a problem here. There is similar code in freebsd64.h.

Perhaps we can change the code in go-backend.cc to test OBJECT_FORMAT_COFF rather than TARGET_AIX. Want to give that a try?

@pmur
Copy link
Contributor Author

pmur commented Jun 16, 2023

I posted a couple of patches to gcc to update the usage of TARGET_AIX to TARGET_AIX_OS. This seems to preserve the original intent, while fixing this bug. I also updated the usage in a couple other places.

wangliu-iscas pushed a commit to plctlab/patchwork-gcc that referenced this issue Jun 16, 2023
TARGET_AIX is defined to a non-zero value on linux and maybe other
powerpc64le targets.  This leads to unexpected behavior such as
dropping the .go_export section when linking a shared library
on linux/powerpc64le.

Instead, use TARGET_AIX_OS to toggle AIX specific behavior.

Fixes golang/go#60798.

gcc/go/ChangeLog:

	* go-backend.cc [TARGET_AIX]: Rename and update usage to
	TARGET_AIX_OS.
	* go-lang.cc: Likewise.
wangliu-iscas pushed a commit to plctlab/patchwork-gcc that referenced this issue Jun 16, 2023
TARGET_AIX is defined to a non-zero value on linux and maybe other
powerpc64le targets.  This leads to unexpected behavior such as
dropping the .go_export section when linking a shared library
on linux/powerpc64le.

Instead, use TARGET_AIX_OS to toggle AIX specific behavior.

Fixes golang/go#60798.

gcc/go/ChangeLog:

	* go-backend.cc [TARGET_AIX]: Rename and update usage to
	TARGET_AIX_OS.
	* go-lang.cc: Likewise.
@gopherbot
Copy link

Change https://go.dev/cl/504095 mentions this issue: cmd/cgo/internal/testshared: disable gccgo tests on PPC64

@gopherbot
Copy link

Change https://go.dev/cl/503495 mentions this issue: cmd/cgo/internal/testshared: strip newline from gccgo -dumpversion

gopherbot pushed a commit that referenced this issue Jun 16, 2023
Otherwise, gccgo tests may be quietly skipped.

For #60798

Change-Id: Iaad6b53c95ad4d407b917d8f62978979b82a4e81
Reviewed-on: https://go-review.googlesource.com/c/go/+/503495
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Paul Murphy <murp@ibm.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopherbot pushed a commit that referenced this issue Jun 16, 2023
Disable PPC64 gccgo tests until issue #60798 is fixed in gcc.

Change-Id: I4d330e3d0ab6d4174ee6d8ff2cbedf53c8c4e3d9
Reviewed-on: https://go-review.googlesource.com/c/go/+/504095
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Paul Murphy <murp@ibm.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
nstester pushed a commit to nstester/gcc that referenced this issue Jun 22, 2023
TARGET_AIX is defined to a non-zero value on linux and maybe other
powerpc64le targets.  This leads to unexpected behavior such as
dropping the .go_export section when linking a shared library
on linux/powerpc64le.

Instead, use TARGET_AIX_OS to toggle AIX specific behavior.

Fixes golang/go#60798.

2023-06-22  Paul E. Murphy  <murphyp@linux.ibm.com>

gcc/go/
	* go-backend.cc [TARGET_AIX]: Rename and update usage to TARGET_AIX_OS.
	* go-lang.cc: Likewise.
sebhub pushed a commit to RTEMS/gnu-mirror-gcc that referenced this issue Jun 28, 2023
TARGET_AIX is defined to a non-zero value on linux and maybe other
powerpc64le targets.  This leads to unexpected behavior such as
dropping the .go_export section when linking a shared library
on linux/powerpc64le.

Instead, use TARGET_AIX_OS to toggle AIX specific behavior.

Fixes golang/go#60798.

2023-06-22  Paul E. Murphy  <murphyp@linux.ibm.com>

gcc/go/
	* go-backend.cc [TARGET_AIX]: Rename and update usage to TARGET_AIX_OS.
	* go-lang.cc: Likewise.

(cherry picked from commit b76cd1e)
kraj pushed a commit to kraj/gcc that referenced this issue Jun 28, 2023
TARGET_AIX is defined to a non-zero value on linux and maybe other
powerpc64le targets.  This leads to unexpected behavior such as
dropping the .go_export section when linking a shared library
on linux/powerpc64le.

Instead, use TARGET_AIX_OS to toggle AIX specific behavior.

Fixes golang/go#60798.

2023-06-22  Paul E. Murphy  <murphyp@linux.ibm.com>

gcc/go/
	* go-backend.cc [TARGET_AIX]: Rename and update usage to TARGET_AIX_OS.
	* go-lang.cc: Likewise.

(cherry picked from commit b76cd1e)
kraj pushed a commit to kraj/gcc that referenced this issue Jun 29, 2023
TARGET_AIX is defined to a non-zero value on linux and maybe other
powerpc64le targets.  This leads to unexpected behavior such as
dropping the .go_export section when linking a shared library
on linux/powerpc64le.

Instead, use TARGET_AIX_OS to toggle AIX specific behavior.

Fixes golang/go#60798.

2023-06-22  Paul E. Murphy  <murphyp@linux.ibm.com>

gcc/go/
	* go-backend.c [TARGET_AIX]: Rename and update usage to TARGET_AIX_OS.
	* go-lang.c: Likewise.

(cherry picked from commit b76cd1e)
kraj pushed a commit to kraj/gcc that referenced this issue Jun 29, 2023
TARGET_AIX is defined to a non-zero value on linux and maybe other
powerpc64le targets.  This leads to unexpected behavior such as
dropping the .go_export section when linking a shared library
on linux/powerpc64le.

Instead, use TARGET_AIX_OS to toggle AIX specific behavior.

Fixes golang/go#60798.

2023-06-22  Paul E. Murphy  <murphyp@linux.ibm.com>

gcc/go/
	* go-backend.c [TARGET_AIX]: Rename and update usage to TARGET_AIX_OS.
	* go-lang.c: Likewise.

(cherry picked from commit b76cd1e)
rsh-raj pushed a commit to rsh-raj/gcc that referenced this issue Oct 6, 2023
TARGET_AIX is defined to a non-zero value on linux and maybe other
powerpc64le targets.  This leads to unexpected behavior such as
dropping the .go_export section when linking a shared library
on linux/powerpc64le.

Instead, use TARGET_AIX_OS to toggle AIX specific behavior.

Fixes golang/go#60798.

2023-06-22  Paul E. Murphy  <murphyp@linux.ibm.com>

gcc/go/
	* go-backend.cc [TARGET_AIX]: Rename and update usage to TARGET_AIX_OS.
	* go-lang.cc: Likewise.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-ppc64x GoCommand cmd/go 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

5 participants
@ianlancetaylor @bcmills @gopherbot @pmur and others