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

go/internal/gccgoimporter: panic encountering type conversion in slice element type #30628

Closed
thanm opened this issue Mar 6, 2019 · 2 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@thanm
Copy link
Contributor

thanm commented Mar 6, 2019

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

$ go version
go version devel +7d13c1b887 Fri Mar 1 13:46:39 2019 -0500 linux/amd64

Does this issue reproduce with the latest release?

Yes

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

linux/amd64

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/usr/local/google/home/thanm/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/ssd2/go1"
GOPROXY=""
GORACE=""
GOROOT="/ssd2/go"
GOTMPDIR=""
GOTOOLDIR="/ssd2/go/pkg/tool/linux_amd64"
GCCGO="/ssd/gcc-trunk/cross/bin/gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build535123708=/tmp/go-build -gno-record-gcc-switches"
 

What did you do?

For this package:

package rwmulock

import (
	"os"
	"sync"
)

const numR = int32(os.O_TRUNC + 5)

type Apple struct {
	hey sync.RWMutex
	x   int
	RQ  [numR]struct {
		Count    uintptr
		NumBytes uintptr
		Last     uintptr
	}
}

When I compile it with tip gccgo, then do something that imports the type information from the object file, the gccgo importer asserts:

panic: import error <input>:1:11 (byte offset = 10): expected Int, got "$" () [recovered]
	panic: import error <input>:1:11 (byte offset = 10): expected Int, got "$" ()

I spent a little while digging around in the export data. It looks as though the offending construct is the array type for the struct field RQ. Here is the export data:

type 1 "Apple" <type 2>
type 2 struct { .rwmulock.hey <type 3>; .rwmulock.x <type -11>; RQ <type 11>; }
type 3 "sync.RWMutex" <type 7>
 func (rw <type 4>) Lock ()
 func (rw <esc:0x12> <type 4>) RLocker () ($ret8 <type 5>)
 func (rw <type 4>) RUnlock ()
 func (rw <type 4>) Unlock ()
 func (rw <type 4>) RLock ()
type 4 *<type 3>
type 5 "sync.Locker" <type 6>
type 6 interface { Lock (); Unlock (); }
type 7 struct { .sync.w <type 8>; .sync.writerSem <type -7>; .sync.readerSem <type -7>; .sync.readerCount <type -3>; .sync.readerWait <type -3>; }
type 8 "sync.Mutex" <type 10>
 func (m <type 9>) Unlock ()
 func (m <type 9>) Lock ()
type 9 *<type 8>
type 10 struct { .sync.state <type -3>; .sync.sema <type -7>; }
type 11 [$convert(<type -3>, 517 )] <type 12>
type 12 struct { Count <type -13>; NumBytes <type -13>; Last <type -13>; }

Note the type 11 [$convert(<type -3>, 517 )] <type 12> -- the importer parser is not expecting a conversion operation here.

I think it might make sense to fix this in the exporter (strip the conversion there) as opposed to teaching the importer to deal with conversions.

What did you expect to see?

Successful import

What did you see instead?

panic (as outlined above)

@thanm thanm added this to the Gccgo milestone Mar 6, 2019
@thanm thanm self-assigned this Mar 6, 2019
@gopherbot
Copy link

Change https://golang.org/cl/165739 mentions this issue: go/internal/gccgoimporter: test case for issue 30628

@ianlancetaylor ianlancetaylor added the NeedsFix The path to resolution is known, but the work has not been done. label Mar 6, 2019
@gopherbot
Copy link

Change https://golang.org/cl/165741 mentions this issue: compiler: emit underlying constant in array_type length export

gopherbot pushed a commit that referenced this issue Mar 6, 2019
Test case for a panic/crash in gccgoimporter caused by incorrect gccgo
export data emission. Note that the *.gox file checked in contains the
correct export data; the main intent of the test is to make sure that
gccgo produces the right export data when run on the Go source.

Updates #30628

Change-Id: I29c0c17b81a43f92ff64fbfcdc58fdb46a5be370
Reviewed-on: https://go-review.googlesource.com/c/go/+/165739
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
kraj pushed a commit to kraj/gcc that referenced this issue Mar 7, 2019
    
    In Array_type::do_export, when emitting a concrete array length,
    evaluate the length expression to an integer constant and emit that
    constant, instead of calling the more general method for emitting
    expressions. This is to avoid the possibility that we will need
    to emit a conversion, which could confuse the gccgoimporter.
    
    Fixes golang/go#30628.
    
    Reviewed-on: https://go-review.googlesource.com/c/gofrontend/+/165741


git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@269443 138bc75d-0d04-0410-961f-82ee72b054a4
@golang golang locked and limited conversation to collaborators Mar 5, 2020
@rsc rsc unassigned thanm Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

3 participants