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

gollvm: some built-in libgo tests failed #51648

Closed
AblakatovMikhail opened this issue Mar 14, 2022 · 10 comments
Closed

gollvm: some built-in libgo tests failed #51648

AblakatovMikhail opened this issue Mar 14, 2022 · 10 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@AblakatovMikhail
Copy link

AblakatovMikhail commented Mar 14, 2022

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

$ go version
go version unknown linux/amd64
llvm-project/llvm/tools/gollvm$ git log --oneline | head -n 1
9bc0905 gollvm: fix libgo cmake rules to include new C source

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env

What did you do?

Ran all built-in libgo tests:

build-release$ for target in $(ninja -t targets | grep check_libgo_ | sed -e 's/\(.*\):.*/\1/') ; do ninja $target 2>&1 | grep "FAILED:" ; done

What did you expect to see?

Nothing, all tests pass

What did you see instead?

FAILED: tools/gollvm/libgo/CMakeFiles/check_libgo_internal_fmtsort
FAILED: tools/gollvm/libgo/CMakeFiles/check_libgo_math_cmplx
FAILED: tools/gollvm/libgo/CMakeFiles/check_libgo_mime
FAILED: tools/gollvm/libgo/CMakeFiles/check_libgo_os_signal
FAILED: tools/gollvm/libgo/CMakeFiles/check_libgo_runtime
FAILED: tools/gollvm/libgo/CMakeFiles/check_libgo_runtime_debug
FAILED: tools/gollvm/libgo/CMakeFiles/check_libgo_runtime_pprof

Mikhail Ablakatov

@heschi
Copy link
Contributor

heschi commented Mar 14, 2022

cc @thanm

@heschi heschi added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 14, 2022
@heschi heschi added this to the Unreleased milestone Mar 14, 2022
@thanm
Copy link
Contributor

thanm commented Mar 15, 2022

Thanks for that investigation.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/393295 mentions this issue: gollvm: fold deref(addr(X)) => X

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/393595 mentions this issue: gollvm: fix an unary minus operations effect on expressions of complex types

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/394474 mentions this issue: gollvm: add a check for the existence of strsignal()

lazyparser pushed a commit to plctlab/gollvm that referenced this issue May 7, 2022
The existing implementation folds addr(deref(X)) => X only. This
change makes it also work the opposite way.

Updates golang/go#51648

Change-Id: Idc1996d431827fb2c410e4d9fdd99625c1d2dfa6
Reviewed-on: https://go-review.googlesource.com/c/gollvm/+/393295
Trust: Eli Bendersky‎ <eliben@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
lazyparser pushed a commit to plctlab/gollvm that referenced this issue May 7, 2022
…x types

The current implementation translates '-EXPR' into
'ZERO_EXPR - EXPR'. It handles EXPR of floating-point types as a
special case for which ZERO_EXPR is -0.0. This change applies the
same approach to EXPR of complex types so that '-CMPLX_EXPR'
translates into '(-0.0-0.0i) - CMPLX_EXPR' instead of
'(0.0+0.0i) - CMPLX_EXPR'.

Updates golang/go#51648

Change-Id: I66bcfaa9864a83d108c2719fe6dce59975e1c205
Reviewed-on: https://go-review.googlesource.com/c/gollvm/+/393595
Reviewed-by: Than McIntosh <thanm@google.com>
Trust: Bryan Mills <bcmills@google.com>
lazyparser pushed a commit to plctlab/gollvm that referenced this issue May 7, 2022
Signame() checks for HAVE_STRSIGNAL but the current
implementation never defines it. This change adds a
check for strsignal() existence to cmake rules.

Updates golang/go#51648

Change-Id: I404e6e6903ae31fdcceafecfae77ac5993549c03
Reviewed-on: https://go-review.googlesource.com/c/gollvm/+/394474
Reviewed-by: Than McIntosh <thanm@google.com>
Trust: Martin Möhrmann <martin@golang.org>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/419574 mentions this issue: libgo: add image/png to mime testdata

realqhc added a commit to realqhc/gofrontend that referenced this issue Aug 4, 2022
similar to https://go-review.googlesource.com/c/gofrontend/+/140517/ skipping a test that is not working on gollvm.

For golang/go#51648

Change-Id: Iec36c27a4f4f965dff512468326354686763ea22
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/421314 mentions this issue: runtime: skip TestPanicOnFault for gollvm

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/421442 mentions this issue: mime: remove test ordering dependency

realqhc pushed a commit to realqhc/gofrontend that referenced this issue Aug 6, 2022
Arrange for tests that call setMimeInit to fully restore the old values,
by clearing the sync.Once that controls initialization.

Once we've done that, call initMime in initMimeUnixTest because
otherwise the test types loaded there will be cleared by the call to
initMime that previously was not being done.

For golang/go#51648

Change-Id: I8bf92b305fc4499337db06113817c9decdc5aedb
realqhc pushed a commit to realqhc/gofrontend that referenced this issue Aug 6, 2022
Arrange for tests that call setMimeInit to fully restore the old values,
by clearing the sync.Once that controls initialization.

Once we've done that, call initMime in initMimeUnixTest because
otherwise the test types loaded there will be cleared by the call to
initMime that previously was not being done.

For golang/go#51648

Change-Id: I8bf92b305fc4499337db06113817c9decdc5aedb
gopherbot pushed a commit that referenced this issue Aug 8, 2022
Arrange for tests that call setMimeInit to fully restore the old values,
by clearing the sync.Once that controls initialization.

Once we've done that, call initMime in initMimeUnixTest because
otherwise the test types loaded there will be cleared by the call to
initMime that previously was not being done.

For #51648

Change-Id: I8bf92b305fc4499337db06113817c9decdc5aedb
Reviewed-on: https://go-review.googlesource.com/c/go/+/421442
Reviewed-by: Than McIntosh <thanm@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
jproberts pushed a commit to jproberts/go that referenced this issue Aug 10, 2022
Arrange for tests that call setMimeInit to fully restore the old values,
by clearing the sync.Once that controls initialization.

Once we've done that, call initMime in initMimeUnixTest because
otherwise the test types loaded there will be cleared by the call to
initMime that previously was not being done.

For golang#51648

Change-Id: I8bf92b305fc4499337db06113817c9decdc5aedb
Reviewed-on: https://go-review.googlesource.com/c/go/+/421442
Reviewed-by: Than McIntosh <thanm@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
@seankhliao seankhliao modified the milestones: Unreleased, gollvm Aug 20, 2022
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/483117 mentions this issue: mime: remove test ordering dependency

gopherbot pushed a commit to golang/gofrontend that referenced this issue Apr 7, 2023
Backport CL 421442 from upstream.

Original description:

Arrange for tests that call setMimeInit to fully restore the old values,
by clearing the sync.Once that controls initialization.

Once we've done that, call initMime in initMimeUnixTest because
otherwise the test types loaded there will be cleared by the call to
initMime that previously was not being done.

For golang/go#51648

Change-Id: Ie21bb1e891479d2d842e831dc5225a9386ec6ef1
Reviewed-on: https://go-review.googlesource.com/c/gofrontend/+/483117
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
@ianlancetaylor
Copy link
Contributor

I think that everything here is fixed now. Thanks for reporting the problems.

nstester pushed a commit to nstester/gcc that referenced this issue Apr 7, 2023
Backport CL 421442 from upstream.

Original description:

Arrange for tests that call setMimeInit to fully restore the old values,
by clearing the sync.Once that controls initialization.

Once we've done that, call initMime in initMimeUnixTest because
otherwise the test types loaded there will be cleared by the call to
initMime that previously was not being done.

For golang/go#51648

Reviewed-on: https://go-review.googlesource.com/c/gofrontend/+/483117
@golang golang locked and limited conversation to collaborators Apr 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge 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

6 participants