]> Cypherpunks repositories - gostls13.git/commitdiff
runtime, syscall: use local cache for Setenv/Getenv in Plan 9
authorRichard Miller <miller.research@gmail.com>
Mon, 25 May 2020 09:30:23 +0000 (10:30 +0100)
committerDavid du Colombier <0intro@gmail.com>
Fri, 19 Jun 2020 11:28:19 +0000 (11:28 +0000)
In os.Getenv and os.Setenv, instead of directly reading and writing the
Plan 9 environment device (which may be shared with other processes),
use a local copy of environment variables cached at the start of
execution. This gives the same semantics for Getenv and Setenv as on
other operating systems which don't share the environment, making it
more likely that Go programs (for example the build tests) will be
portable to Plan 9.

This doesn't preclude writing non-portable Plan 9 Go programs which make
use of the shared environment semantics (for example to have a command
which exports variable definitions to the parent shell). To do this, use
  ioutil.ReadFile("/env/"+key) and
  ioutil.WriteFile("/env/"+key, value, 0666)
in place of os.Getenv(key) and os.Setenv(key, value) respectively.

Note that CL 5599054 previously added env cacheing, citing efficiency
as the reason. However it made the cache write-through, with Setenv
changing the shared environment as well as the cache (so not consistent
with Posix semantics), and Clearenv breaking the sharing of the
environment between the calling thread and other threads (leading to
unpredictable behaviour). Because of these inconsistencies (#8849),
CL 158970045 removed the cacheing again.

This CL restores cacheing but without write-through. The local cache is
initialised at start of execution, manipulated by the standard functions
in syscall/env_unix.go to ensure the same semantics, and exported only
when exec'ing a new program.

Fixes #34971
Fixes #25234
Fixes #19388
Updates #38772

Change-Id: I2dd15516d27414afaf99ea382f0e00be37a570c3
Reviewed-on: https://go-review.googlesource.com/c/go/+/236520
Run-TryBot: David du Colombier <0intro@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Fazlul Shahriar <fshahriar@gmail.com>
Reviewed-by: David du Colombier <0intro@gmail.com>
src/runtime/env_plan9.go
src/runtime/env_posix.go
src/runtime/env_test.go
src/runtime/os_plan9.go
src/syscall/env_plan9.go [deleted file]
src/syscall/env_unix.go

index c95b5db510d9e3112b1c2cf109293aaae300443b..b7ea863735c7d03a55d5964d2179bd9f723b2205 100644 (file)
@@ -6,45 +6,117 @@ package runtime
 
 import "unsafe"
 
-var tracebackbuf [128]byte
+const (
+       // Plan 9 environment device
+       envDir = "/env/"
+       // size of buffer to read from a directory
+       dirBufSize = 4096
+       // size of buffer to read an environment variable (may grow)
+       envBufSize = 128
+       // offset of the name field in a 9P directory entry - see syscall.UnmarshalDir()
+       nameOffset = 39
+)
 
-func gogetenv(key string) string {
-       var file [128]byte
-       if len(key) > len(file)-6 {
-               return ""
+// Goenvs caches the Plan 9 environment variables at start of execution into
+// string array envs, to supply the initial contents for os.Environ.
+// Subsequent calls to os.Setenv will change this cache, without writing back
+// to the (possibly shared) Plan 9 environment, so that Setenv and Getenv
+// conform to the same Posix semantics as on other operating systems.
+// For Plan 9 shared environment semantics, instead of Getenv(key) and
+// Setenv(key, value), one can use ioutil.ReadFile("/env/" + key) and
+// ioutil.WriteFile("/env/" + key, value, 0666) respectively.
+//go:nosplit
+func goenvs() {
+       buf := make([]byte, envBufSize)
+       copy(buf, envDir)
+       dirfd := open(&buf[0], _OREAD, 0)
+       if dirfd < 0 {
+               return
        }
+       defer closefd(dirfd)
+       dofiles(dirfd, func(name []byte) {
+               name = append(name, 0)
+               buf = buf[:len(envDir)]
+               copy(buf, envDir)
+               buf = append(buf, name...)
+               fd := open(&buf[0], _OREAD, 0)
+               if fd < 0 {
+                       return
+               }
+               defer closefd(fd)
+               n := len(buf)
+               r := 0
+               for {
+                       r = int(pread(fd, unsafe.Pointer(&buf[0]), int32(n), 0))
+                       if r < n {
+                               break
+                       }
+                       n = int(seek(fd, 0, 2)) + 1
+                       if len(buf) < n {
+                               buf = make([]byte, n)
+                       }
+               }
+               if r <= 0 {
+                       r = 0
+               } else if buf[r-1] == 0 {
+                       r--
+               }
+               name[len(name)-1] = '='
+               env := make([]byte, len(name)+r)
+               copy(env, name)
+               copy(env[len(name):], buf[:r])
+               envs = append(envs, string(env))
+       })
+}
 
-       copy(file[:], "/env/")
-       copy(file[5:], key)
+// Dofiles reads the directory opened with file descriptor fd, applying function f
+// to each filename in it.
+//go:nosplit
+func dofiles(dirfd int32, f func([]byte)) {
+       dirbuf := new([dirBufSize]byte)
 
-       fd := open(&file[0], _OREAD, 0)
-       if fd < 0 {
-               return ""
-       }
-       n := seek(fd, 0, 2)
-       if n <= 0 {
-               closefd(fd)
-               return ""
+       var off int64 = 0
+       for {
+               n := pread(dirfd, unsafe.Pointer(&dirbuf[0]), int32(dirBufSize), off)
+               if n <= 0 {
+                       return
+               }
+               for b := dirbuf[:n]; len(b) > 0; {
+                       var name []byte
+                       name, b = gdirname(b)
+                       if name == nil {
+                               return
+                       }
+                       f(name)
+               }
+               off += int64(n)
        }
+}
 
-       p := make([]byte, n)
-
-       r := pread(fd, unsafe.Pointer(&p[0]), int32(n), 0)
-       closefd(fd)
-       if r < 0 {
-               return ""
+// Gdirname returns the first filename from a buffer of directory entries,
+// and a slice containing the remaining directory entries.
+// If the buffer doesn't start with a valid directory entry, the returned name is nil.
+//go:nosplit
+func gdirname(buf []byte) (name []byte, rest []byte) {
+       if 2+nameOffset+2 > len(buf) {
+               return
        }
-
-       if p[r-1] == 0 {
-               r--
+       entryLen, buf := gbit16(buf)
+       if entryLen > len(buf) {
+               return
        }
-
-       var s string
-       sp := stringStructOf(&s)
-       sp.str = unsafe.Pointer(&p[0])
-       sp.len = int(r)
-       return s
+       n, b := gbit16(buf[nameOffset:])
+       if n > len(b) {
+               return
+       }
+       name = b[:n]
+       rest = buf[entryLen:]
+       return
 }
 
-var _cgo_setenv unsafe.Pointer   // pointer to C function
-var _cgo_unsetenv unsafe.Pointer // pointer to C function
+// Gbit16 reads a 16-bit little-endian binary number from b and returns it
+// with the remaining slice of b.
+//go:nosplit
+func gbit16(b []byte) (int, []byte) {
+       return int(b[0]) | int(b[1])<<8, b[2:]
+}
index f95ff685450785d71cc9cfa2f4a532e0c674fa63..af353bbcd9a9694f2ddf994540db22b071bb7fbd 100644 (file)
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-// +build aix darwin dragonfly freebsd js,wasm linux netbsd openbsd solaris windows
+// +build aix darwin dragonfly freebsd js,wasm linux netbsd openbsd solaris windows plan9
 
 package runtime
 
index 2399e46faa91e8bc850aa5e303687d7128ca0f35..c009d0f31e1758ce725124fad1b5e58387ccb392 100644 (file)
@@ -11,10 +11,6 @@ import (
 )
 
 func TestFixedGOROOT(t *testing.T) {
-       if runtime.GOOS == "plan9" {
-               t.Skipf("skipping plan9, it is inconsistent by allowing GOROOT to be updated by Setenv")
-       }
-
        // Restore both the real GOROOT environment variable, and runtime's copies:
        if orig, ok := syscall.Getenv("GOROOT"); ok {
                defer syscall.Setenv("GOROOT", orig)
index 2bea1058f2d86ce5b4e20eb916bd57723664e70a..9e187d2220514cdbb0a0352b0e66403e94ba2940 100644 (file)
@@ -306,9 +306,6 @@ func getRandomData(r []byte) {
        extendRandom(r, 0)
 }
 
-func goenvs() {
-}
-
 func initsig(preinit bool) {
        if !preinit {
                notify(unsafe.Pointer(funcPC(sigtramp)))
diff --git a/src/syscall/env_plan9.go b/src/syscall/env_plan9.go
deleted file mode 100644 (file)
index e403a25..0000000
+++ /dev/null
@@ -1,122 +0,0 @@
-// Copyright 2011 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-// Plan 9 environment variables.
-
-package syscall
-
-import (
-       "errors"
-)
-
-var (
-       errZeroLengthKey = errors.New("zero length key")
-       errShortWrite    = errors.New("i/o count too small")
-)
-
-func readenv(key string) (string, error) {
-       fd, err := open("/env/"+key, O_RDONLY)
-       if err != nil {
-               return "", err
-       }
-       defer Close(fd)
-       l, _ := Seek(fd, 0, 2)
-       Seek(fd, 0, 0)
-       buf := make([]byte, l)
-       n, err := Read(fd, buf)
-       if err != nil {
-               return "", err
-       }
-       if n > 0 && buf[n-1] == 0 {
-               buf = buf[:n-1]
-       }
-       return string(buf), nil
-}
-
-func writeenv(key, value string) error {
-       fd, err := create("/env/"+key, O_RDWR, 0666)
-       if err != nil {
-               return err
-       }
-       defer Close(fd)
-       b := []byte(value)
-       n, err := Write(fd, b)
-       if err != nil {
-               return err
-       }
-       if n != len(b) {
-               return errShortWrite
-       }
-       return nil
-}
-
-func Getenv(key string) (value string, found bool) {
-       if len(key) == 0 {
-               return "", false
-       }
-       v, err := readenv(key)
-       if err != nil {
-               return "", false
-       }
-       return v, true
-}
-
-func Setenv(key, value string) error {
-       if len(key) == 0 {
-               return errZeroLengthKey
-       }
-       err := writeenv(key, value)
-       if err != nil {
-               return err
-       }
-       return nil
-}
-
-func Clearenv() {
-       // Creating a new environment group using rfork(RFCENVG) can race
-       // with access to files in /env (e.g. from Setenv or Getenv).
-       // Remove all environment variables in current environment group instead.
-       fd, err := open("/env", O_RDONLY)
-       if err != nil {
-               return
-       }
-       defer Close(fd)
-       files, err := readdirnames(fd)
-       if err != nil {
-               return
-       }
-       for _, key := range files {
-               Remove("/env/" + key)
-       }
-}
-
-func Unsetenv(key string) error {
-       if len(key) == 0 {
-               return errZeroLengthKey
-       }
-       Remove("/env/" + key)
-       return nil
-}
-
-func Environ() []string {
-       fd, err := open("/env", O_RDONLY)
-       if err != nil {
-               return nil
-       }
-       defer Close(fd)
-       files, err := readdirnames(fd)
-       if err != nil {
-               return nil
-       }
-       ret := make([]string, 0, len(files))
-
-       for _, key := range files {
-               v, err := readenv(key)
-               if err != nil {
-                       continue
-               }
-               ret = append(ret, key+"="+v)
-       }
-       return ret
-}
index e80a3ff1c96872c5d5ce3cae29a2f6139e5440a5..a4bb28cc455b8ab05bede23c717cc3bc2fb47b19 100644 (file)
@@ -2,13 +2,16 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-// +build aix darwin dragonfly freebsd js,wasm linux netbsd openbsd solaris
+// +build aix darwin dragonfly freebsd js,wasm linux netbsd openbsd solaris plan9
 
 // Unix environment variables.
 
 package syscall
 
-import "sync"
+import (
+       "runtime"
+       "sync"
+)
 
 var (
        // envOnce guards initialization by copyenv, which populates env.
@@ -100,9 +103,12 @@ func Setenv(key, value string) error {
                        return EINVAL
                }
        }
-       for i := 0; i < len(value); i++ {
-               if value[i] == 0 {
-                       return EINVAL
+       // On Plan 9, null is used as a separator, eg in $path.
+       if runtime.GOOS != "plan9" {
+               for i := 0; i < len(value); i++ {
+                       if value[i] == 0 {
+                               return EINVAL
+                       }
                }
        }