Does your program work with the 1.15.3 release? Or, which releases does your program work with?

What is the GEOSGeometry type in the C code?

Does your program work with the 1.15.3 release? Or, which releases does your program work with?

@ianlancetaylor Yes, 1.15.3 was the last release that worked. In that version, in the switch statement, ifaceIndir(t) returns false instead, as t.kind = 54 (and kindDirectIface = 32), resulting in the default section of the switch to be executed:

default:
	// Value is direct, and so is the interface.
	e.word = v.ptr

In v1.16.3, t.kind = 22, resulting in ifaceIndir(t) returning true.

What is the GEOSGeometry type in the C code?

It's coming from the GEOS library, and I think it's being created somewhere in preprocessing, so as such I'm not sure exactly what it looks like. What I can do is provide steps to reproduce this:

This example requires the libgeos-dev package which can be installed on most systems using a package manager.. on Ubuntu for example:

sudo apt install -y libgeos-dev

Sample application:

package main

/*
#include "geos_c.h"
*/
import "C"
import (
	"fmt"
	"reflect"
)

func main() {
	var geom *C.GEOSGeometry
	t := reflect.ValueOf(geom).Type().Elem()
	v := reflect.New(t)
	fmt.Println("v.CanInterface(): ", v.CanInterface())
	v.Interface() // Panics in 1.15.4+
}

The program completes successfully when compiled with go1.15.3 and panics with go1.15.4+.
Examples are provided for 1.15.3, 1.15.4 and 1.16.3
Note: in all cases v.CanInterface() returns true.

go version
go version go1.15.3 linux/amd64

go run main ; echo EXITED $?
v.CanInterface():  true
EXITED 0

--

go version
go version go1.15.4 linux/amd64

go run main ; echo EXITED $?
v.CanInterface():  true
panic: bad indir

goroutine 1 [running]:
reflect.packEface(0x4a2aa0, 0x588650, 0x16, 0x0, 0x0)
	/usr/local/go/src/reflect/value.go:113 +0xed
reflect.valueInterface(0x4a2aa0, 0x588650, 0x16, 0x1, 0x2, 0x18)
	/usr/local/go/src/reflect/value.go:1046 +0xe9
reflect.Value.Interface(...)
	/usr/local/go/src/reflect/value.go:1016
main.main()
	/home/user/src/main/main.go:17 +0x1bb
exit status 2
EXITED 1

--

go version
go version go1.16.3 linux/amd64

go run main ; echo EXITED $?
v.CanInterface():  true
panic: bad indir

goroutine 1 [running]:
reflect.packEface(0x4a0a40, 0x57c660, 0x16, 0x0, 0x18)
	/usr/local/go/src/reflect/value.go:113 +0xed
reflect.valueInterface(0x4a0a40, 0x57c660, 0x16, 0x1, 0x2, 0x18)
	/usr/local/go/src/reflect/value.go:1046 +0xe9
reflect.Value.Interface(...)
	/usr/local/go/src/reflect/value.go:1016
main.main()
	/home/user/src/main/main.go:17 +0x1bb
exit status 2
EXITED 1

@ianlancetaylor let me know if there's anything else I can provide. Thanks

I believe this is due to the go:notinheap changes we made. In a copy of geos_c.h I found, we have:

typedef struct GEOSGeom_t GEOSGeometry;

GEOSGeometry is a type defined as an incomplete struct type. I believe we would make that a go:notinheap type.
Allocating such a type in Go is certain not to work - we don't know how big it is, for example. This program attempts to allocate such a type.

Did this ever work? It used to compile, probably, but did the thing you allocated here actually work correctly? Try printing t.Size() for the t in your program and tell us what it prints. Likely, you're allocating a zero-sized thing in Go. C thinks it is not zero-sized, so it is reading and writing to some random place in the Go runtime that happens to not crash everything.

I think there is something to fix here, though. The allocation shouldn't succeed - we should fail there, instead of a random later spot.

Standalone repro:

package main

// typedef struct S T;
import "C"
import "reflect"

func main() {
	var x *C.T
	t := reflect.ValueOf(x).Type().Elem()
	v := reflect.New(t)
	v.Interface()
}

Fails from 1.15.4 through tip.

Note that if you add _ = new(C.T), you get an appropriate allocation error:

./issue45451.go:12:9: _Ctype_struct_S can't be allocated in Go; it is incomplete (or unallocatable)

I think it is just the reflect allocation path that isn't being checked (because the error above comes from the compiler - we probably need a runtime check in reflect).

Thanks. I think this was caused by the fix for #42076.

Here is a standalone test case:

package main

// typedef struct unknown u;
import "C"

import "reflect"

func main() {
	var u *C.u
	t := reflect.ValueOf(u).Type().Elem()
	v := reflect.New(t)
	v.Interface()
}

I have a patch.

CC @randall77

I want to note that your example code is very unlikely to do what you want. Presumably the real code is different. This code is allocating a struct value, where the struct type is left undefined in the C header file and as such is not visible to Go. The effect is going to be to allocate a zero-typed struct, which is almost certainly not the C definition. Using this struct for anything, such as passing it to C, is quite likely to fail. It's basically the equivalent of writing, in C, "GEOSGeometry g;", for which the C compiler will give an error storage size of ‘g’ isn’t known. Since you are using reflection here, there is no facility to see a similar error in your dynamic Go code.

Because this code is unlikely to work correctly, it's not clear to me whether we should backport the fix or not. Can you say more about how your code uses the value created by reflect.New(t).Interface()? If the code, for example, passes that value to C, it's not going to work. Thanks.

Change https://golang.org/cl/308970 mentions this issue: reflect: make go:notinheap pointers indirect in New

@randall77 Sorry for the overlapping messages, for some reason I didn't see your comments.