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

gccgo: duplicate definition error from inlining function with two locals of same name #33158

Closed
thanm opened this issue Jul 17, 2019 · 2 comments
Milestone

Comments

@thanm
Copy link
Contributor

thanm commented Jul 17, 2019

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

$ go version
go version go1.12.2 gccgo (GCC) 10.0.0 20190703 (experimental) linux/amd64

Does this issue reproduce with the latest release?

yes

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

linux/amd64

What did you do?

Compile these packages (first "a" and then "b"):

Package "a":

package a

import "os"

func M() string {
	if s := os.Getenv("Fred"); s != "" {
		return s
	}
	if s := os.Getenv("Joe"); s != "" {
		return s
	}
	return string("Alex")
}

Package "b":

package b

import "a"

func B() string {
	return a.M()
}

What did you expect to see?

Clean build

What did you see instead?

Compile failure:

$ go build b
# b
 /ssd2/go1/src/a/a.go:9: error: redefinition of ‘s’
 /ssd2/go1/src/a/a.go:6: note: previous definition of ‘s’ was here

Looks as though we are treating "s" as a duplicate while importing "a.M'. Here is the export data:

func M () ($ret0 <type -16>) <inl:379>
// /ssd2/go1/src/a/a.go:5
{ //6
var s <type -16> = <p1>Getenv("Fred") //6
if (s != "") { //6
{ //7
$ret0 = s //7
return //7
} //0
} //6
} //8
{ //9
var s <type -16> = <p1>Getenv("Joe") //9
if (s != "") { //9
{ //10
$ret0 = s //10
return //10
} //0
} //9
} //11
{ //12
$ret0 = $convert(<type -16>, "Alex") //12
return //12
} //0

I think the problem area is in Variable_declaration_statement::do_import, statements.cc line 473:

  Named_object* no = ifb->block()->bindings()->add_variable(id, NULL, var);
  return Statement::make_variable_declaration(no);

This is using a single block associated with the Import_function_body object, when in fact I think what we want is to keep a stack of the current block nesting. I will look into a fix.

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

Change https://golang.org/cl/186717 mentions this issue: test: new testcase for gccgo bug

@gopherbot
Copy link

Change https://golang.org/cl/186757 mentions this issue: compiler: fix bug in importing blocks from inline functions

gopherbot pushed a commit that referenced this issue Jul 18, 2019
Updates #33158.

Change-Id: Id87eb00ddcb104ba8e7a0d2f6cf324a77a84f4a9
Reviewed-on: https://go-review.googlesource.com/c/go/+/186717
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
kraj pushed a commit to kraj/gcc that referenced this issue Jul 18, 2019
    
    This patch fixes a buglet in the function body importer. Add hooks for
    keeping a stack of blocks corresponding to the block nesting in the
    imported function. This ensures that local variables and temps wind up
    correctly scoped and don't introduce collisions.
    
    New test case for this problem in CL 186717.
    
    Fixes golang/go#33158.
    
    Reviewed-on: https://go-review.googlesource.com/c/gofrontend/+/186757


git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@273577 138bc75d-0d04-0410-961f-82ee72b054a4
@golang golang locked and limited conversation to collaborators Jul 17, 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.
Projects
None yet
Development

No branches or pull requests

2 participants