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
Milestone

Comments

@davidzech
Copy link

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
Copy link
Author

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

davidzech pushed 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 pushed 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
Copy link

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

@randall77
Copy link
Contributor

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
Copy link

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

@randall77
Copy link
Contributor

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
Copy link
Author

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

@randall77
Copy link
Contributor

Sure, that's fine.

@golang golang locked and limited conversation to collaborators Feb 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants