Guarantees for package unsafe

[Matthew Dempsky]

Background: We've received a handful of bug reports against 1.7 due to handling package unsafe code differently than in 1.6:

    #16727: Converting from &struct{}{} to *T and then taking the address of T's fields causes a segmentation fault.

    #16769: Assigning to a uint64 via a *uint16 pointer leaks uninitialized memory into the other 48 bits.

    #16772: Slicing an non-nil pointer-to-array that points to unmapped memory now causes a segmentation fault.

The first and last are the same root problem: we changed from an explicit nil pointer check to an implicit one by trying to load via the pointer.  It seems feasible that we could revert this behavior, but the current users seem okay with fixing their own code after hearing an explanation why.

The second issue, I think is a legitimate regression because we stopped accepting a documented valid pattern.  Keith also agreed it should be fixed, but David disagrees.

dr2chase wrote:

I think that we should not be bound by any particular artifacts of the previous compiler's treatment of unsafe code, because the full set of those artifacts is not specified.

Agreed that just because code happened to work with an old Go release, we should not feel obligated to support it forever.

However, I don't see that happening in the bugs above.  In #16727 and #16772, we've pushed back against users to update their code first, and in both cases they accepted that.  In #16769, the code is using a pattern that's documented to be valid, and the compiler was violating that.

Aside: Package unsafe is not strictly bound by Go 1 compat rules, so it seems valid to revise the rules over time if we choose to.  I think a compiler release should faithfully implement whatever set of rules we document for it at the time though.

I also think it was a mistake to "document" this behavior for end-user use,

Disagreed: If we're not willing to document how package unsafe can be used correctly, then there's not much point to providing it at all.  App Engine Go for a long time didn't allow end user code to import package unsafe.

because people will write unsafe code that happens to pass tests with a particular Go release and believe that therefore they got it right.

That's true in general though.  E.g., when sort.Sort's algorithm or hash map iteration changed, it revealed bugs in user code, and we advised how they should fix their code.

That's exactly why we should document safe patterns: at least then we can steer users towards patterns that we're less likely to break, which is ultimately less effort for us.

We don't have a test suite for what unsafe behavior we support except for the runtime itself, and compiler writers aren't good at imaging the crazy things that people might do with unsafe code. And lacking such a test suite, for unsafe, I think it's fine for compiler-writers to push back hard on "bugs" involving unsafe code.

Agreed that lacking a comprehensive test suite for package unsafe is unfortunate.  I don't see why that makes it the users' problem though.  Tests exist to make our jobs easier/safer.

After all: if we only accepted bugs that were already covered by a test case, then unless we stopped running the tests regularly, we'd never have to accept new bugs anymore. :)

Unsafe code is generally a security violation, because no matter what we say in documentation, we don't check it mechanically. It's pretty much the whole point of actual security violations that they do an end-run on documented restrictions. Or as I asked, "what prevents?"

This goes back to the issue about documentation above.  Security violations are just a form of incorrect usage.

[David Chase]

I think we have a different understanding of what a security violation is.

One of the problems with unsafe is that it is not mechanically checked;

this means that people can make uncaught unintentional mistakes,

which are latent bugs, or they can make uncaught *intentional* "mistakes",

and those are security violations.

To me these are very separate issues, but they have the same root cause,

which is the lack of mechanical checking.

As to the usefulness, in older languages, unsafe was used quite happily

as part of the language implementation, as well as in certain bits of specialized

FFI glue code, without being well-documented (and in fact generally being

actively discouraged) for "civilian" use.  It's nice for portability, it often helps

runtime performance if crucial bits of the GC and scheduler are "just code"

that can be called without dropping into assembly language or doing some sort

of FFI nonsense.

But we *definitely* need a test suite.  We might want an anti-test suite,

as in "this happens to work now, but we hope to break it in the future".

Another thing we might want to do is figure out what "necessary" unsafe

idioms could be avoided with the proper library or (perish the thought)

generic support.

The default pattern ought to be "don't use unsafe.  No, really, don't use unsafe.

Did you benchmark it yet without unsafe? Could you file a compiler performance

bug instead?"

--
You received this message because you are subscribed to the Google Groups "golang-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Matthew Dempsky]

On Thu, Aug 18, 2016 at 2:34 PM, David Chase <drc...@google.com> wrote:

I think we have a different understanding of what a security violation is..

Possibly.  My background is in information security, so when I hear "security violation", I think of the kinds of security issues that Wikipedia's "security bugs" page discusses.

One of the problems with unsafe is that it is not mechanically checked;

this means that people can make uncaught unintentional mistakes,

which are latent bugs, or they can make uncaught *intentional* "mistakes",

and those are security violations.

Sorry, I'm afraid I don't follow.  How do you distinguish between "unintentional mistakes" and "intentional 'mistakes'"?  And what's the use of distinguishing them here?

As to the usefulness, in older languages, unsafe was used quite happily

as part of the language implementation,

I agree having package unsafe is beneficial for use by package runtime, etc.

To clarify: when I said "there's not much point to providing [package unsafe] at all" if we won't document how to use it, I meant "providing it to end users at all".  Hence the example of App Engine Go, which does still use unsafe.Pointer internally, but does not allow user code to use it.  Sorry to not have been more explicit.

The default pattern ought to be "don't use unsafe.  No, really, don't use unsafe.

Did you benchmark it yet without unsafe? Could you file a compiler performance

bug instead?"

If you mean users should strongly prefer writing safe Go code by default, then I'm in full agreement with you.  But then you still end up with situations where users do need to use package unsafe.  So they also need to know how to use it correctly, which goes back to my argument that we need to document how to do that.

If you mean users should never use package unsafe, then that goes back to my questioning why we provide package unsafe to users.

[Uli Kunitz]

Let me state first, that I rarely use unsafe. I just grepped through my code and found only one use related to an IOCTL syscall. 

But I have been surprised, how many people accepted -- even welcomed -- breaking user code that used unsafe. The actual conditions on the pointer are not described in the documentation. There is a long list of use cases and one of them mentions that the pointer must point to the allocated memory, but the semantics of allocated memory is not described and it's not clear whether this is a requirement for all use cases. Since conditions for the pointer have not been described, developers had no chance to avoid the problem. The code worked in older versions of the compiler and now it doesn't. You can't blame users for not using it correctly, if the correct way is not documented. The only defensible position is that the documentation explicitly stated that semantics might change, but then nobody should have used unsafe, because there are no guaranteed semantics. I now wonder whether I run the risk that my syscall will break at one point in the future, but there is no other way to provide the functionality. 

The other point is that I don't understand the rationale for that check. I would have never expected that a pointer conversion would result in an access to the converted pointer. Go allows nil pointers and accessing them results in a segmentation violation panic on Linux without any use of unsafe. Why shouldn't it be possible to convert uintptr(0) to a nil pointer? The 1.7 check requires now that the first byte of the pointer must be accessible during conversion. Why is only the first byte checked, when the second byte might not be accessible? The accessibility of a memory address can change at any point in time; for instance through a truncation of a memory-mapped file. So why access the pointer, if the check is only valid for a specific point in time.

My proposal is, make unsafe.Pointer a simple pointer conversion without any requirements for the pointer. If that is not possible and conditions are required, document them clearly. In any case break user code only, if it is absolutely necessary.

[Uli Kunitz]

Just to be clear my statements refer to the use of unsafe.Poiinter.

[Keith Randall]

I think you misunderstand what is going on here.  There is no check on the conversion from unsafe.Pointer to uintptr and back.  It is a simple pointer conversion.

The nil checks are coming from code like the following:

   p := unsafe.Pointer(...)

   q := (*T)(p)

   r := &q.f          // the nil check is here.

   ...read or write *r...

Where the field f is at a large offset in T.  The &q.f expression requires a nil check on q to prevent the manufacture of pointers to real memory from nil pointers.  Unsafe is not mentioned at this line, and the check has to happen regardless of any unsafeness used (or not used) to generate q.

The problem in #16727 and one of the problems in #16772 is that p isn't nil but doesn't point to readable memory.  The nil check, which in 1.7 is implemented as a load from q, fails.

[Uli Kunitz]

Thank you, Keith, for the explanation. I will have a deeper look into it and might come back with questions later.

[David Chase]

I may not understand present-day information security (i.e., unintentional leaks may be regarded as a lot more severe than I imagine), but by intentional "mistake", I meant an exploit, made to look like a hand-optimization.  With unsafe many of the conventional checks are turned off, and it's entirely possible to scribble all over memory if you can sneak your "optimization" past reviewers.

Experience with "unsafe" code is that even when users are explicitly told not to use it, they use it. This was the case with Java; Sun introduced an internal unsafe package for implementation purposes (because it really is that good for that purpose), people noticed it, and started playing with it "for performance".  The fact that Go documents it at end-users in a way that is intended to mean that people might actually use it is (to me) somewhat insane.  Java experience (and experience with unsafe in other less-successful languages) is that people overestimate their need for unsafe, overestimate their ability to understand how it works, and use unsafe to scribble bugs and technical debt all over their code for usually-minor performance gains.  And because it's not checked, those bugs can lie hidden until revealed by some change in the implementation -- for example, our decision to dereference pointers to prove their non-nilness, rather than insert an explicit test-and-conditional.  We're pretty sure the dereference is the better choice (more compact code, no branch to mispredict, such pointers tend to get dereferenced soon anyway) but it turned out to break some "working" unsafe code.  As soon as that happens to an "important" piece of code, we have a problem -- either a delayed Go release, or we turn down an optimization that benefits everyone else, never mind the cost at each release cycle to deal with "no, you missed a technicality there in your unsafe code, that really isn't a compiler bug" (the more popular Go gets, the more of these we'll encounter).

Actually documenting the restrictions on unsafe, in some way other than "don't", requires us to imagine the space of likely future implementations of Go, and the space of crazy things we might want to do to loads and stores, that present-day use of unsafe might be incompatible with.  This includes not just weird implementations for performance, but also weird implementations for checking.  I worked on one of those for C/C++ that never launched, and a constant problem was dealing with the so-called "false"-positive rate caused by various gonzo C coding practices.  A major part of the UI was finding ways for users to accurately classify huge groups of "errors" as "don't care".  The tool wasn't useful unless it could find the actual-bug-needle in the haystack of technical infractions present in most C code (especially back in the late 90s).

TLDR, as far as unsafe in user code goes, I'm a badly snake-bit dog.

[Ian Lance Taylor]

I agree that people misuse the unsafe package. But we need to have
it, if for no other reason, to communicate with C code. A JNI-style
solution is unsatisfactory. It doesn't really get us anywhere to say
that user code should avoid the unsafe package. The unsafe package
exists, there are valid reasons for using it, and it is not going to
go away. The best answer to misuse of the unsafe package is better
checks in cmd/vet.

In any case, the limited valid uses of the unsafe package have been
documented. It seems that there may be some disagreement about issue
#16769. The docs for unsafe say that it is valid to convert from *T1
to *T2 when T2 is no larger than T1 and the types "share an equivalent
memory layout." That seems to me to be true when T1 is uint64 and T2
is uint16. If we want to say that the code in 16769 is invalid, we
need to clarify the docs of the unsafe package to say why. Since we
have documented the valid uses of unsafe, and since there are real use
cases, and since we have strict backward compatibility guidelines, we
can't just give up and tell people to not use unsafe.

Issues #16727 and #16772 appear to have been doing operations that the
unsafe package docs do not permit: converting an essentially random
uintptr to unsafe.Pointer, and converting between pointer types where
the new type is larger than the original type. Also, these are
operations that can be done in other ways using safe code. So that
code was always broken in principle, and now it is broken in practice.
I don't see a problem there.

Ian

[David Chase]

Is the memory layout really "equivalent"?

That code will do different things depending on the endianness of the machine.

(Just being picky....)

Unsafe is necessary for communicating with C code, but that's not usual-case,

and I'd expect that we'd deal with it by "yeah, okay, you needed to do that, and 

here's the recipe, please follow it".  My main problem with writing a "specification"

for normal use of unsafe is that I'm not sure I can get it right, and I don't want my

mistakes to be carved into stone.

A secondary problem is that such a specification will result in a lot more unsafe code

being written, and a lot more friction with users every time we change the

implementation.

[Ian Lance Taylor]

On Fri, Aug 19, 2016 at 9:13 AM, David Chase <drc...@google.com> wrote:
>
> Unsafe is necessary for communicating with C code, but that's not
> usual-case,
> and I'd expect that we'd deal with it by "yeah, okay, you needed to do that,
> and
> here's the recipe, please follow it". My main problem with writing a
> "specification"
> for normal use of unsafe is that I'm not sure I can get it right, and I
> don't want my
> mistakes to be carved into stone.
>
> A secondary problem is that such a specification will result in a lot more
> unsafe code
> being written, and a lot more friction with users every time we change the
> implementation.

Well, there is already a specification: https://golang.org/pkg/unsafe/ .

I'm not sure we need more than that. I hope we don't need less.

Ian

[Matthew Dempsky]

On Fri, Aug 19, 2016 at 8:31 AM, David Chase <drc...@google.com> wrote:

I may not understand present-day information security (i.e., unintentional leaks may be regarded as a lot more severe than I imagine),

Yes.  For example, "Heartbleed" was a major security bug due to an unintentional information leak.  Of course, not all memory leaks are that severe by themselves, but we take them pretty seriously in security reviews.

but by intentional "mistake", I meant an exploit, made to look like a hand-optimization.  With unsafe many of the conventional checks are turned off, and it's entirely possible to scribble all over memory if you can sneak your "optimization" past reviewers.

I see.  At least on Chrome's security team, I don't think this was ever a major concern for us, despite Chromium being predominantly written in C++ (a much less safe language than Go) and being open source (i.e., accepting contributions from external untrusted developers).  Reviewers generally push back against questionable coding patterns like C++'s reinterpret_cast or Go's package unsafe and apply extra scrutiny when these are determined to be necessary.

More often the problem was unintentional mistakes due to developers not understanding the nuances of type aliasing and pointer arithmetic in C++.  I would expect less documentation about how to use them safely to only make these mistakes more common.

Experience with "unsafe" code is that even when users are explicitly told not to use it, they use it.

I agree that users are probably using package unsafe more than they should, and that's unfortunate.  My experience in security matches yours in that regards.

However, your wording keeps expressing concern that users are using package unsafe at all.  So let me establish this: do you think end users should never use package unsafe, or do you think sometimes it's appropriate for them to use?

In security, we'd love nothing more than if engineers only used languages/libraries/patterns that could be proven secure, but that's not really practical today.  Instead we often have to rely on: 1) discouraging unsafe practices as much as possible (e.g., via style guides, safe library abstractions, and code reviews), and 2) when they are necessary, educating users on how to use them safely and correctly.

[David Chase]

On Fri, Aug 19, 2016 at 2:54 PM, Matthew Dempsky <mdem...@google.com> wrote:

However, your wording keeps expressing concern that users are using package unsafe at all.  So let me establish this: do you think end users should never use package unsafe, or do you think sometimes it's appropriate for them to use?

My goal is never for performance, meaning that its only use is those cases where it is necessary for compatibility with stuff written in other languages.  The default performance model I'd like to use is that code I don't trust enough to run it is modeled as infinitely slow.  Bigger picture goal is to reduce the occurrence of unsafe code, and to make it as idiomatic as possible, and "only for interlanguage compatibility" tends strongly in that direction.

This gets squishy around memory-mapped files and things like that.  Some of this can be addressed with better interfaces (and with improved compiler optimization, for instance better inlining, so that the calls have zero cost).  Java introduced nio and ByteBuffers for this, for example.  And understand, making something like java ByteBuffers work well in Go is a real problem in the existing compiler, I'd like it to be the case that the tricksy-clever unsafe code is in a library function, that the user might pass in a closure or two, and that everything would inline itself into no calls at all, but that doesn't happen now.

And there are cases where I'm wrong about "never for performance", but if I had to derive a rule of thumb for bending the rule on "never", it would be that the each such use of unsafe would require the filing of a performance bug against the compiler/runtime, or attach a back-reference to an existing bug, so that when we had a fix for that particular performance shortcoming, we could try to get the unsafe code working around it backed out.  As a corollary, bending the rule on "never" should be rare enough that this would actually scale.

[Uli Kunitz]

I have done some experiments and I have now a better understanding of 16772. The memory check is related to the conversion of the array pointer to the slice. It is an optimization of the former nil pointer check. There is no change to the unsafe.Pointer semantics, it is actually a simple conversion as I demanded. 

I agree now that 16772 shouldn't be fixed, because it would prevent optimizations or would extremely complicate the compiler. My demand on documenting the constraints would be basically require to document all potential operations on a pointer. 

I suggest to document that the compiler may change the internal handling of pointers and therefore the Go 1 compatibility guidelines don't apply. For me the two sentences "Pointer therefore allows a program to defeat the type system and read and write arbitrary memory. It should be used with extreme care." worked in the past as I have used unsafe.Pointer only with IOCTL syscalls.

[Caleb Spare]

In case it's helpful, I looked through my own code for uses of unsafe
in order to provide some perspective from the dastardly end-user
vantage point :)

It seems like they can broadly be divided into (1) things which should
be provided by the go project but aren't, yet and (2) low-level
interactions with the machine which are hard to imagine doing in a
safe way.

In my code, I use unsafe:

1. To call msync, which hasn't been added to x/sys/unix yet
(https://golang.org/issue/8245).
2. To call ioctl.
3. To implement a JIT which writes out some code and then execs it.
This requires all kinds of unsafe hacks (obviously).
4. As part of the implementation of a column-oriented database which
loads its data unsafely from mmap-ed files.
5. To parse float64s out of []bytes without allocation; this
optimization ended up fixing an important bottleneck in an internal
system. This issue is discussed in https://golang.org/issue/2632 which
has been open for 5 years.

I look forward to the day I can remove unsafe for usages 1, 2, and 5.
For 4, the stuff that David hinted out sounds interesting; maybe in
the future there could be better ways of doing that, too.

I think 3 will always require lots of unsafe tricks and I'm fully
aware that my code is tied to details of the gc compiler (for example,
the JIT entry code would've had to change if it had existed back when
Go 1.1 changed how function calls work). Yet I think it's really great
that it was possible for me to use Go for that project, and I hope to
(ab)use Go for more experiments in machine code in the future.

-Caleb

> --
> You received this message because you are subscribed to the Google Groups
> "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an

> email to golang-dev+...@googlegroups.com.

[David Chase]

Hmm, double-interesting.  High on my list of things to get into the pipeline is changing calling conventions to use registers, which might cause your JIT a little distress.  That's another discussion, I have questions...

But I looked at that bug (2632), and I note the usefulness of knowing that a function doesn't modify the contents of a slice or *array parameter, and I was looking into enhancing the Note field of parameters (already used for escape analysis) to maybe include register information (the necessity of coexisting with "old" assembly language means mixed-convention code is likely), and it could just as easily include "does-not-modify".

> email to golang-dev+unsubscribe@googlegroups.com.

[Matthew Dempsky]

On Fri, Aug 19, 2016 at 12:20 PM, David Chase <drc...@google.com> wrote:

My goal is never for performance, meaning that its only use is those cases where it is necessary for compatibility with stuff written in other languages.

Cool, so we agree there are cases where it's necessary to use package unsafe. :)

How should users that find themselves in those infrequent scenarios know how to use package unsafe correctly?  It seems unhelpful to tell a user they have to use X to implement something, but then to refuse to tell them how to use X.  Even more so if we're going to later blame them for misusing X.

The way we do that for other features/tools is to document them.

And again: I'm not arguing that every use of package unsafe is valid, or that patterns valid today need necessarily be valid forever.  Just that we need to have at least some valid use patterns (like we currently do) and should commit to supporting those.

The default performance model I'd like to use is that code I don't trust enough to run it is modeled as infinitely slow.

Security reviewers would love if people shared this attitude.  E.g., a coworker estimated once that if Chrome accepted something like a 20% performance penalty, we could categorically prevent all future security issues from C++ memory corruption.  But that's an incredibly hard sell for a project that competes with other products on speed.  (One of the first questions I got after transferring to the Go team was literally "so why is Chrome so slow?")

This gets squishy around memory-mapped files and things like that.  Some of this can be addressed with better interfaces (and with improved compiler optimization, for instance better inlining, so that the calls have zero cost).  Java introduced nio and ByteBuffers for this, for example.  And understand, making something like java ByteBuffers work well in Go is a real problem in the existing compiler, I'd like it to be the case that the tricksy-clever unsafe code is in a library function, that the user might pass in a closure or two, and that everything would inline itself into no calls at all, but that doesn't happen now.

Agreed that providing safe higher-level abstractions is a good way to reduce how much unsafe code users need to write directly.  But the developers building those abstractions need to know how to write them correctly.

[cherry]

My goal is never for performance, meaning that its only use is those cases where it is necessary for compatibility with stuff written in other languages.  The default performance model I'd like to use is that code I don't trust enough to run it is modeled as infinitely slow. 

What about this: for each use of unsafe (except in the runtime), we insert 10 useless instructions.

[Rob Pike]

Here is a list of some to choose from: https://www.physics.ohio-state.edu/~bcd/humor/instruction.set.html

On Sat, Aug 20, 2016 at 7:57 AM, cherry <luna...@gmail.com> wrote:

My goal is never for performance, meaning that its only use is those cases where it is necessary for compatibility with stuff written in other languages.  The default performance model I'd like to use is that code I don't trust enough to run it is modeled as infinitely slow. 

What about this: for each use of unsafe (except in the runtime), we insert 10 useless instructions.

[Michael Jones]

https://en.wikipedia.org/wiki/COMEFROM

 

From: <golan...@googlegroups.com> on behalf of Rob Pike <r...@golang.org>
Date: Friday, August 19, 2016 at 2:58 PM
To: cherry <luna...@gmail.com>
Cc: David Chase <drc...@google.com>, Matthew Dempsky <mdem...@google.com>, golang-dev <golan...@googlegroups.com>
Subject: Re: [golang-dev] Guarantees for package unsafe

 

Here is a list of some to choose from: https://www.physics.ohio-state.edu/~bcd/humor/instruction.set.html

On Sat, Aug 20, 2016 at 7:57 AM, cherry <luna...@gmail.com> wrote:

 

 

My goal is never for performance, meaning that its only use is those cases where it is necessary for compatibility with stuff written in other languages.  The default performance model I'd like to use is that code I don't trust enough to run it is modeled as infinitely slow. 

 

What about this: for each use of unsafe (except in the runtime), we insert 10 useless instructions.

 

--
You received this message because you are subscribed to the Google Groups "golang-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+...@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

 

--

You received this message because you are subscribed to the Google Groups "golang-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+...@googlegroups.com.

[David Chase]

On Fri, Aug 19, 2016 at 5:28 PM, Matthew Dempsky <mdem...@google.com> wrote:

On Fri, Aug 19, 2016 at 12:20 PM, David Chase <drc...@google.com> wrote:

My goal is never for performance, meaning that its only use is those cases where it is necessary for compatibility with stuff written in other languages.

Cool, so we agree there are cases where it's necessary to use package unsafe. :)

How should users that find themselves in those infrequent scenarios know how to use package unsafe correctly?  It seems unhelpful to tell a user they have to use X to implement something, but then to refuse to tell them how to use X.  Even more so if we're going to later blame them for misusing X.

The way we do that for other features/tools is to document them.

The problem is that if we say "therefore we should include it in the spec" we get way too much unsafe code written.  "Way too much" means too much friction at dot releases, too much unsafe that complicates building other tools, too much risk of actual security bugs, that sort of thing.

If the spec can say "this applies to a particular dot release of Go, and it's all, entirely, subject to change in the future, and you should feel comfortable reading assembly code to figure out what's going on before you file a bug that involves 'unsafe'", that might make me happy.

Obviously in such a world we'd prioritize bugs in interfaces to native code (because there really is no other way), but I think this is a different sort of contract than the usual language spec, which is why I'm not comfortable just sticking it in the spec "because that's where we document things".  There's a rule of language design, that you can't imagine all the crazy-ass things that people will do with your beautiful new language, and I don't like stirring unsafe into that mix.  For most of the language, if someone figures out something we hadn't previously imagined and somehow our implementation falls short, we should make it work, because that's what the language spec says.  If it involves unsafe, I want a different contract, one where I get to say "whoops, wow, we never imagined unsafe could work like that, we need to fix the spec to ban that".

I like Cherry's idea.

[Keith Randall]

I understand the desire to limit the use of unsafe.Pointer.  But Go is a systems language.  It intentionally gives users the power to get under the covers.  I don't want to make its use harder.  To the extent that we can, we should make it safer, but I don't think throwing arbitrary roadblocks at unsafe users is productive.  People want to call mmap, talk to memory-mapped device drivers, convert []byte to []int64, etc.  We should let them do that.

We should certainly document the dangers of unsafe.  https://golang.org/pkg/unsafe/ has lots of that already.  I think it would help to add some examples of the correct use of unsafe (the runtime has a few examples of how to do it).  But I don't think we should obsess about specifying exactly what is ok and what is not.

--

[Matthew Dempsky]

On Sat, Aug 20, 2016 at 10:44 AM, David Chase <drc...@google.com> wrote:

The problem is that if we say "therefore we should include it in the spec" we get way too much unsafe code written.

Do you have evidence that documenting safe practices results in more unsafe code?

Anecdotally, my experience security reviewing C/C++ code suggests the opposite.  C/C++ have detailed explanations about when pointer arithmetic and type casting are safe, but most of the problematic unsafe code is written by people who haven't read that documentation.  On the other hand, it's the people who have read the documentation that understand their caveats and always push back against their (mis)use.

I.e., providing documentation allows the people who want to write correct code to know how to do so and to more effectively argue against incorrect usage, whereas the people who are going to write bad code are going to do so regardless of what we want/say.

If the spec can say "this applies to a particular dot release of Go, and it's all, entirely, subject to change in the future, and you should feel comfortable reading assembly code to figure out what's going on before you file a bug that involves 'unsafe'", that might make me happy.

As far as I can tell, that's the situation we have today.  The Go 1 compat doc and package unsafe both explicitly state that package unsafe is not guaranteed to continue working.  We document patterns that are supposed to work today and we hope to continue supporting in the future, but none of the wording suggests to me that those patterns are exceptions to the "not guaranteed to continue working" warnings.  I.e., if we determine some of the patterns really are too onerous to continue supporting, I would argue it's okay to change them without violating Go 1 compat.

(That's my 2c at least; one of Ian's earlier messages suggests he thinks the documented patterns are covered by Go 1 compat.)

Obviously in such a world we'd prioritize bugs in interfaces to native code (because there really is no other way), but I think this is a different sort of contract than the usual language spec, which is why I'm not comfortable just sticking it in the spec "because that's where we document things".  There's a rule of language design, that you can't imagine all the crazy-ass things that people will do with your beautiful new language, and I don't like stirring unsafe into that mix.  For most of the language, if someone figures out something we hadn't previously imagined and somehow our implementation falls short, we should make it work, because that's what the language spec says.  If it involves unsafe, I want a different contract, one where I get to say "whoops, wow, we never imagined unsafe could work like that, we need to fix the spec to ban that".

This is a straw man.  I'm not proposing to move package unsafe's documentation about valid use patterns into the language spec.  I'm just arguing against your proposal to remove that documentation.

I like Cherry's idea.

I think intentionally slowing down usage of package unsafe is punitive of our users who legitimately need to use it.  The users who unnecessarily/incorrectly use it are already punished by extra bugs / upgrade pains.

[Rob Pike]

I agree with Keith's position. I think of unsafe a bit like the memory model. Yes, you need something there but if you have to think hard about what you're doing you should consider another approach. Unsafe is necessary but should appear in very few programs and be used in a simple, easy-to-understand way. We write down what works, concisely, and don't waste time trying to define the boundary to the micron.

-rob

[Rob Pike]

Perhaps more important, we discourage programmers from depending on the precision of that boundary's location.

-rob

[David Chase]

On Sat, Aug 20, 2016 at 5:26 PM, Matthew Dempsky <mdem...@google.com> wrote:

I like Cherry's idea.

I think intentionally slowing down usage of package unsafe is punitive of our users who legitimately need to use it.  The users who unnecessarily/incorrectly use it are already punished by extra bugs / upgrade pains.

Extra nops aren't going to happen (except where they already do),

but it's certainly imaginable that functions containing unsafe code will end up less-optimized.

There's precedent for that in C (setjmp, volatile),

in Fortran (goto in/out of the range of a do loop),

more likely when you use goto to write irreducible control flow.

I've heard that the hotspot JIT would punt various 'legal" but annoying bytecode to the interpreter,

and leave it there (the example I heard of was unbalanced monitorenter/exit; I think they used to also

have expectations about nesting of exception ranges, until the obfuscator for Volano's benchmark

did something screwy and they were forced to deal with it).

For Go, who knows, maybe we'll go to implement type-based alias

analysis and discover a problem there.

And all of these might be regarded as "intentional", in the sense of

"this is a lot harder to get right if we have to deal with that screwy case, so let's just not".

Irreducible loops are a good example of this -- the reducible-loop algorithms are fast, easy

to understand, and cover the overwhelmingly common case, and generating test data to be

sure that you really got the irreducible case right is a pain.  So why not just punt?

 

The way the documentation's written now, I think is too neutral on the desirability of using unsafe.

It doesn't say "think really hard before using unsafe, it's intended to help with interface to

other languages, not to help you do a time-saving end-run around safety checks".

The other languages (e.g., Modula-3, Modula-2+, Cedar) did document unsafe

in their specs, e.g. https://www.cs.purdue.edu/homes/hosking/m3/reference/unsafe.html,

but they also made it by default unmentionable in "ordinary" code.  The mechanism

used in those languages, where "interface" and "implementation" were in two separate files,

was to allow an "unsafe" modifier on either interface or implementation --

default code (implementation) could not mention unsafe operations or unsafe interfaces,

but a safe interface could be implemented by unsafe code (in an unsafe implementation).

And despite this anti-usage speed bump, people still used it too much, and cheerfully

wrote bugs that (to a runtime+compiler guy) were transparently obvious (not naming any

of the guilty, but it includes people who later went to work for Google, and not just me).

Some of this is influenced by other issues of compiler quality -- if procedure calls are

really cheap and frequently inlined where sensible, then it costs less to stuff all the unsafe

code behind a safe interface.

And yeah, the people who do it wrong suffer the bugs that they write, but so do we,

and as we get more users we're going to get more of this each time we change something

affecting unsafe.

I'm sorry not to have written more clearly about this.

There is history, I am a snake-bit dog, and I think this has the potential to be a problem as Go becomes more widespread.