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/compile: reject var main in package main #21256

Closed
tfausak opened this issue Aug 1, 2017 · 14 comments
Closed

cmd/compile: reject var main in package main #21256

tfausak opened this issue Aug 1, 2017 · 14 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@tfausak
Copy link

tfausak commented Aug 1, 2017

What version of Go are you using?

1.8

What operating system and processor architecture are you using?

The Go Playground

What did you do?

https://play.golang.org/p/HBKmQFBAuv

package main

var main = func() {}

What did you expect to see?

Either a compile-time error telling me that main must be a function (not a variable), or for it to work. The spec says:

The main package must have package name main and declare a function main that takes no arguments and returns no value.

I didn't expect func() main {} and var main = func() {} to behave differently, but I understand that the latter isn't actually a function declaration.

What did you see instead?

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0xffffffff addr=0x0 pc=0xd0940]

goroutine 1 [running]:
panic(0x9b520, 0xd0998)
	/usr/local/go/src/runtime/panic.go:531 +0x220
runtime.panicmem()
	/usr/local/go/src/runtime/panic.go:63 +0x80
runtime.sigpanic()
	/usr/local/go/src/runtime/os_nacl.go:60 +0x60
runtime.main()
	/usr/local/go/src/runtime/proc.go:194 +0x2e0
runtime.goexit()
	/usr/local/go/src/runtime/asm_amd64p32.s:1014 +0x1
@ALTree ALTree changed the title var main panics cmd/compile: var main = func() { } panics at runtime Aug 1, 2017
@ALTree ALTree added this to the Go1.10 milestone Aug 1, 2017
@ALTree
Copy link
Member

ALTree commented Aug 1, 2017

This is enough to trigger a runtime panic:

package main

var main int

I guess the question here is if we want the compiler to catch bad main names at compile time, or just let it crash at run time.

@ALTree ALTree changed the title cmd/compile: var main = func() { } panics at runtime runtime: var main = func() { } panics at runtime Aug 1, 2017
@ianlancetaylor
Copy link
Contributor

I think the compiler should catch this. The language spec says that the package main is required to declare a function main.

@ianlancetaylor ianlancetaylor changed the title runtime: var main = func() { } panics at runtime cmd/compile: var main = func() { } panics at runtime Aug 1, 2017
@ALTree ALTree added the NeedsFix The path to resolution is known, but the work has not been done. label Aug 2, 2017
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/53450 mentions this issue: Guard to panic when main = func() { } was declared

@griesemer griesemer changed the title cmd/compile: var main = func() { } panics at runtime runtime: var main = func() { } panics at runtime Aug 8, 2017
@griesemer griesemer changed the title runtime: var main = func() { } panics at runtime cmd/link: var main = func() { } panics at runtime Aug 8, 2017
@griesemer
Copy link
Contributor

griesemer commented Aug 8, 2017

@ALTree, @ianlancetaylor I don't believe that is correct: The spec says that a program is created by linking a single, unimported package main and that that package must contain a function main. It doesn't say that a package main cannot have variables, types, or constants called main. In fact, it is possible to have a library called main. For instance, given two files/packages tmp/a/a.go:

package main

var main int // <<< this is ok!

func F() { println("main.F") }

and tmp/x.go:

package main

import a "./a/a"

func main() {
	a.F()
}

it is possible to compile, link, and execute:

cd tmp
cd a
go tool compile a.go
cd ..
go tool compile x.go
go tool link x.o
./a.out

produces

main.F

as output.

This is not a compiler problem, this is a linker issue. We (or I) have long maintained (from day one in fact) that the main package shouldn't be special from the language's point of view; it's just a package. Similarly, identifiers called main shouldn't be special either.

Only when we link does it matter because we need to get to an entry point.

@ianlancetaylor
Copy link
Contributor

I think that is less true these days than it used to be. For example, the compiler rejects import "main". It also rejects

package main

func main() int { return 0 }

@griesemer
Copy link
Contributor

@ianlancetaylor Fair enough, but I'm not convinced yet that we should erode this further. There's a lot of implicit assumptions here that are not really written down in the spec. I also don't see much technical reason. It's basically an additional special case in the compiler where there doesn't need to be one as far as I can tell.

@mdempsky
Copy link
Contributor

mdempsky commented Aug 8, 2017

I was thinking the same objection as @griesemer yesterday: according to the Go spec, a package named "main" is not necessary the "main package".

Though seeing as cmd/go doesn't actually support packages named "main" and the compiler already treats packages named "main" as the main package for purposes of validating the "main" function's signature, I'm sort of inclined to just accept adding a compiler error against declaring main.main as a non-function.

@timakin
Copy link
Contributor

timakin commented Aug 8, 2017

I respect @griesemer 's idea, but honestly, as @mdempsky says, I also regard main as a little bit different definition from others.
localpkg.Name or node named main is often used for validation on compiler, so I was not so hesitated to add lines to validate var/const/type declarations whether they include main .

@griesemer
Copy link
Contributor

@mdempsky I'm not per se against that but it would be an error that is not backed by the spec. As it is, the implementation is inconsistent. My suggestion: Let's see if the linker fix is trivial and if so, let's do it there. Otherwise, let's go back to the compiler. The main goal is to provide an good error message to the user rather than a crash. Also, this is really an esoteric issue, so not worth spending a lot of engineering complexity on it.

@timakin
Copy link
Contributor

timakin commented Aug 9, 2017

@griesemer Hi, I tried to fix this issue from the aspect of cmd/link.
Could you please check this commit?
https://go-review.googlesource.com/c/54330

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/54330 mentions this issue: cmd/link: changed var main = func() { } to alert on build

@zigo101
Copy link

zigo101 commented Sep 5, 2017

btw, for good or bad, the issue makes it is possible that a Go program can run okay without a main entry function.

package main

import (
    "fmt"
    "time"
)

var main int

func init() {
	for {
		time.Sleep(time.Second)
		fmt.Println("aaa")
	}
}

@rsc
Copy link
Contributor

rsc commented Nov 22, 2017

The compiler should be the one to reject this. As Ian noted it already checks func main's signature in package main. Package main is special.

@rsc rsc changed the title cmd/link: var main = func() { } panics at runtime cmd/compile: reject var main in package main Nov 22, 2017
@rsc rsc modified the milestones: Go1.10, Go1.11 Nov 22, 2017
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/79435 mentions this issue: cmd/compile: error if main.main is not a function

@golang golang locked and limited conversation to collaborators Nov 22, 2018
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

9 participants