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

CGO Unsetenv does not work on macOS 10.15.2 #36705

Closed
davidzech opened this issue Jan 23, 2020 · 7 comments
Closed

CGO Unsetenv does not work on macOS 10.15.2 #36705

davidzech opened this issue Jan 23, 2020 · 7 comments
Assignees
Milestone

Comments

@davidzech
Copy link

@davidzech davidzech commented Jan 23, 2020

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

$ go version
go version go1.13.4 darwin/amd64

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

GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/zech/Library/Caches/go-build"
GOENV="/Users/zech/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/zech/gocode"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/Users/zech/.gvm/gos/go1.13.4"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/Users/zech/.gvm/gos/go1.13.4/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/vc/ws9fc9l10jnb_fg364m5y3340000gn/T/go-build778170232=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

package main

// #include <stdlib.h>
// #include <unistd.h>
import "C"

import "fmt"
import "os"

func main() {
	os.Setenv("FOO", "bar")
	os.Unsetenv("FOO")
	s := C.GoString(C.getenv(C.CString("FOO")))
	fmt.Printf("Native GO unsetenv \"%s\"\n", s)

	// native behavior
	C.unsetenv(C.CString("FOO"))
	s = C.GoString(C.getenv(C.CString("FOO")))
	fmt.Printf("CGO unsetenv \"%s\"\n", s)
}

What did you expect to see?

Print out empty string in both C.getenv calls

What did you see instead?

I see "bar" as a result of the first C.getenv call

Native GO unsetenv "bar"
CGO unsetenv ""

It looks like the argument to unsetenv is corrupted when called from os.Unsetenv

Take a look at the argument dump from breaking on getenv() originating from os.Unsetenv and C.unsetenv

Messages Image(1310911872)

@davidzech

This comment has been minimized.

Copy link
Author

@davidzech davidzech commented Jan 23, 2020

It seems the argument passed https://golang.org/src/runtime/cgo/gcc_setenv.c is incorrect

davidzech added a commit to davidzech/go that referenced this issue Jan 23, 2020
x_cgo_unsetenv did not properly deference the arg pointer when calling unsetenv.

Fixes golang#36705
davidzech added a commit to davidzech/go that referenced this issue Jan 23, 2020
x_cgo_unsetenv did not properly deference the arg pointer when calling unsetenv.

Fixes golang#36705
@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Jan 23, 2020

Change https://golang.org/cl/215941 mentions this issue: runtime/cgo: pass proper argument to unsetenv

@randall77

This comment has been minimized.

Copy link
Contributor

@randall77 randall77 commented Jan 23, 2020

Yep, this is indeed a bug. I don't think this ever worked.
It's been broken since at least 1.11 on both darwin and linux.
@davidzech You're right, x_cgo_unsetenv is wrong. Fix coming.

@randall77 randall77 added this to the Go1.15 milestone Jan 23, 2020
@randall77 randall77 self-assigned this Jan 23, 2020
@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Jan 23, 2020

Change https://golang.org/cl/215942 mentions this issue: runtime/cgo: fix unsetenv wrapper

@randall77

This comment has been minimized.

Copy link
Contributor

@randall77 randall77 commented Jan 23, 2020

Simultaneous CLs, oops.
Your CL needs a test. Feel free to grab the test from my CL and add it to your CL if you want. Or just review my CL.

@davidzech

This comment has been minimized.

Copy link
Author

@davidzech davidzech commented Jan 23, 2020

@randall77 It's probably easier if I just hand off the process to your team, right?

@randall77

This comment has been minimized.

Copy link
Contributor

@randall77 randall77 commented Jan 23, 2020

Sure, that's fine.

@gopherbot gopherbot closed this in 4f074b5 Feb 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants
You can’t perform that action at this time.