proposal: add built-in function "formallyRead" for preventing elimination of "dead" stores #33325

nsajko · 2019-07-27T23:22:38Z

Dead stores are writes to variables that are not followed by corresponding reads. While dead store elimination is mostly a desirable optimization, some of the time it is essential for program correctness that dead store elimination (or the unreachable code elimination that it sometimes implies) does not happen.

The two examples on my mind are:

Secure zeroization of sensitive data (also known as cleansing, wiping, etc.), where the store (to computer memory) itself is essential, and it should not legitimately be followed by a read.
Benchmarking, where the problem is the elimination of code that is meant to be benchmarked

In C these problems might be solved by using memset_s, volatile types or inline assembly tricks.

In current Go user code, convoluted efforts to prevent dead store elimination for wiping memory and to prevent dead code elimination for benchmarking ultimately rely only on the lack of optimizations in current Go implementations, instead of on formal correctness, which this proposal aims to enable in addition to simplification of user code.

The new built-in "formallyRead" is meant to return nothing and either

takes a pointer or an arbitrary number of pointers
takes a slice or an arbitrary number of slices
if package "unsafe" is imported it additionally can take an unsafe.Pointer and a uintptr (to indicate base and size of a memory object)

The pointer arguments to formallyRead indicate objects that must be considered as having formally been read at the point of the formallyRead invocation. Slice arguments indicate the same for the slice's underlying array in the range up to len(slice).

The following minimal example is meant to show the effects of formallyRead :

package main

func main() {
                ...

                var key [32]byte

                // Do cryptographic operations with the key
                ...

                for i := range key {
                                key[i] = 0
                }
                // Pretend to read the key so the compiler would not optimize out the preceding loop.
                formallyRead(&key) // or formallyRead(key[:])

                ...
}

The following example showcases the cutting down of benchmark code that formallyRead would enable, because use of sink package level variables is no longer necessary:

var GlobalF float64

func BenchmarkAcos(b *testing.B) {
	x := 0.0
	for i := 0; i < b.N; i++ {
		x = Acos(.5)
	}
	GlobalF = x
}

turns into:

func BenchmarkAcos(b *testing.B) {
	for i := 0; i < b.N; i++ {
		x := Acos(.5)
		formallyRead(&x)
	}
}

See also:

Regarding benchmarking:

go/src/math/all_test.go

Line 3109 in 80f9d32

// Global exported variables are used to store the

https://groups.google.com/forum/#!topic/golang-dev/eCPwwvqs2Cg

A more ambitious and ambiguous proposal regarding the zeroization use case:

#21374

Discusses some ugly hacks that are done in the name of zeroization:

#21865

The text was updated successfully, but these errors were encountered:

ianlancetaylor · 2019-07-27T23:36:50Z

My initial thought is that a builtin function zero that zeros out the memory associated with a pointer or slice would be easier to understand than formallyRead.

The benefits to benchmarking seem less interesting to me. Using global variables is easy enough to understand, and it doesn't seem likely to fail any time soon.

nsajko · 2019-07-27T23:45:52Z

The benefits to benchmarking seem less interesting to me. Using global variables is easy enough to understand, and it doesn't seem likely to fail any time soon.

But you said in the linked mailing list discussion that:

Note that gccgo will already discard assignments to an unexported variable that is never read, among other optimizations.

Also, from the same discussion it seems like both you and rsc think that using sink variables complicates benchmarking code, but you say that formallyRead is not easy to understand ... Maybe the main problem is that formallyRead is not a good name?

Edit: i was confused about the "unexported variable" part and thus misunderstood what GCC optimizes and what does it not optimize, sorry.

ianlancetaylor · 2019-07-27T23:51:24Z

Maybe the main problem is that formallyRead is not a good name?

Yes, maybe. For the benchmarking case, perhaps we could repurpose runtime.KeepAlive. Perhaps it already works.

randall77 · 2019-07-28T01:04:01Z

I think runtime.KeepAlive should work as currently implemented (for both uses discussed here).

For benchmarking particularly, perhaps there should be a method on testing.B which could delegate to runtime.KeepAlive for now, but have a more evocative name.

awnumar · 2019-07-29T13:42:15Z

func wipe(key []byte) {
    for i := range key {
        key[i] = 0
    }
    runtime.KeepAlive(key)
}

What's to stop the compiler from optimizing out the entire function? KeepAlive is currently implemented as:

func KeepAlive(x interface{}) {
    // Introduce a use of x that the compiler can't eliminate.
    // This makes sure x is alive on entry. We need x to be alive
    // on entry for "defer runtime.KeepAlive(x)"; see issue 21402.
    if cgoAlwaysFalse {
        println(x)
    }
}

I think the branch is there to confuse it into thinking there's a possibility of the value being needed?

randall77 · 2019-07-29T15:18:34Z

runtime.KeepAlive cannot be optimized away.
Or perhaps more precisely, if we come up with an optimization that causes runtime.KeepAlive to be optimized away, we will change the implementation of runtime.KeepAlive to avoid that optimization.

I think the branch is there to confuse it into thinking there's a possibility of the value being needed?

Correct.

nsajko · 2019-07-30T00:23:26Z

@randall77 A question: Suppose one passes a slice to runtime.KeepAlive (like in awnumar's example above). Does runtime.KeepAlive then protect just the slice structure (the one that has the pointer to the backing array, length and capacity; I think) or does it also protect the slice's backing array?

I am asking because I think println does not need to read the slice's backing array.

randall77 · 2019-07-30T01:17:06Z

@nsajko Both. runtime.KeepAlive keeps its argument live directly (the slice structure). Anything that is kept live will also keep all its referents live, so the backing store is thus kept live also. If the slice backing store contained pointers, anything those pointed to would also be kept live, etc.

nsajko · 2019-07-30T11:48:12Z

@randall77 Sorry for bothering you again, but, in your last message, did you mean "live" just in the GC sense, or also in the "prevents dead code elimination" sense?

randall77 · 2019-07-30T15:17:55Z

I mean in the "live" sense. Dead code elimination isn't observable in the language.

awnumar · 2019-07-30T20:05:43Z

@randall77 So when you said,

I think runtime.KeepAlive should work as currently implemented (for both uses discussed here).

How does this hold?

randall77 · 2019-07-30T20:29:58Z

Sorry, I was talking about the language semantics. Yes, runtime.KeepAlive also makes sure its argument is computed.

Anything passed to runtime.KeepAlive must be computed and available at the runtime.KeepAlive call. As if someone might read it and check its contents. Whether that hypothetical someone is the GC or another goroutine.

ianlancetaylor · 2019-08-27T21:30:37Z

Based on the discussion above, runtime.KeepAlive already does what the proposed new builtin function would do. So, closing.

gopherbot added this to the Proposal milestone Jul 27, 2019

gopherbot added the Proposal label Jul 27, 2019

ianlancetaylor added v2 A language change or incompatible library change LanguageChange labels Jul 27, 2019

nsajko mentioned this issue Jul 27, 2019

Various fixes awnumar/memguard#106

Merged

This was referenced Jul 31, 2019

math: replace exported variables with runtime.KeepAlive in benchmarks #33394

Closed

cmd/compile: runtime.KeepAlive causes useless extra run-time overhead, even though it is an intrinsic? #33442

Open

ianlancetaylor closed this as completed Aug 27, 2019

griesemer mentioned this issue Aug 28, 2019

proposal: Go 2 review meeting minutes #33892

Open

cespare mentioned this issue Oct 25, 2019

testing: document best practices for avoiding compiler optimizations in benchmarks #27400

Open

golang locked and limited conversation to collaborators Aug 26, 2020

gopherbot added the FrozenDueToAge label Aug 26, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

proposal: add built-in function "formallyRead" for preventing elimination of "dead" stores #33325

proposal: add built-in function "formallyRead" for preventing elimination of "dead" stores #33325

nsajko commented Jul 27, 2019

ianlancetaylor commented Jul 27, 2019

nsajko commented Jul 27, 2019 •

edited

ianlancetaylor commented Jul 27, 2019

randall77 commented Jul 28, 2019

awnumar commented Jul 29, 2019

randall77 commented Jul 29, 2019 •

edited

nsajko commented Jul 30, 2019 •

edited

randall77 commented Jul 30, 2019

nsajko commented Jul 30, 2019 •

edited

randall77 commented Jul 30, 2019

awnumar commented Jul 30, 2019

randall77 commented Jul 30, 2019

ianlancetaylor commented Aug 27, 2019

proposal: add built-in function "formallyRead" for preventing elimination of "dead" stores #33325

proposal: add built-in function "formallyRead" for preventing elimination of "dead" stores #33325

Comments

nsajko commented Jul 27, 2019

ianlancetaylor commented Jul 27, 2019

nsajko commented Jul 27, 2019 • edited

ianlancetaylor commented Jul 27, 2019

randall77 commented Jul 28, 2019

awnumar commented Jul 29, 2019

randall77 commented Jul 29, 2019 • edited

nsajko commented Jul 30, 2019 • edited

randall77 commented Jul 30, 2019

nsajko commented Jul 30, 2019 • edited

randall77 commented Jul 30, 2019

awnumar commented Jul 30, 2019

randall77 commented Jul 30, 2019

ianlancetaylor commented Aug 27, 2019

nsajko commented Jul 27, 2019 •

edited

randall77 commented Jul 29, 2019 •

edited

nsajko commented Jul 30, 2019 •

edited

nsajko commented Jul 30, 2019 •

edited