What if I accidentally implemented __builtin_unreachable ?

[Jorropo]

I have some code that uses some intrinsics in bits.* and it fails to BCE so to get this over I'm adding support for all of bits.* to prove.

When it comes to bits.Add64 the docs says:
> The carry input must be 0 or 1; otherwise the behavior is undefined.

so I've added:

  ft.unsignedMax(v.Args[2], 1)

to OpAdd64carry's case, however this means if the programmer makes a mistake and calls bits.Add64 with a non 0 or 1 carry this leads to a misscompilation.

This can also be exploited purposely to implement __builtin_unreachable:

//go:noinline

func empty() {}

func trustMeThisIsUnreachable() {

 s, _ := bits.Add64(0, 0, 2)

 if s == 0 { empty() }

}

Allows people to write:

func fastUint64(s []byte) uint64 {

 if len(s) < 8 { trustMeThisIsUnreachable() } // compiles to nothing

 return binary.LittleEndian.Uint64(s) // BCE ✨

}

which looks like a valid implementation of bits.Add64 given the docs.

Now that the context is out of the way, questions what is reasonable to do here ?

My current implementation feels useful but way too dangerous to live anywhere outside of unsafe (also from memory something proposing this feature in unsafe has been refused).

I guess I could do "nothing" to the input, but I don't like this option because this opens a new question:

When is it safe to compute limits of Add64 ?

Because if I compute the limits of Add64's result I want to do this math assuming carry is 0 or 1, however if then at runtime a 2 is feeded in carry the CPU might react producing a runtime value outside of the computed limits.

The most reasonable option is IMO to make Add64, Add32, Sub64, ... panic if an invalid carry is fed in.

It solves all of my problems because carry is now proven to be 0 or 1 for me by the bound check.

Because I am adding support for bits's intrinsics this won't have big performance costs, for example in a chain of Add64 only the first one needs a bound check.

It is also possible to use the type system to remove this first bound check "for free" and get back to the current state of performance but with added safety:

func FastAdd64(a, b uint64, carry bool) (sum uint64, carry bool) {

 var carryUint uint64

 if carry { carryUint = 1 } // compiles to nothing thx to CvtBoolToUint8

 sum, carryUint = bits.Add64(a, b, carryUint)

 carry = carryUint != 0 // can teach prove to simplify this to a Zext which is easily removed after optimized with whoever is consuming this

 return

}

[Keith Randall]

I think the "otherwise the behavior is undefined" part of the math/bits.Add64 spec doesn't really mean arbitrary behavior is allowed. I take it to only mean that Add64 can return a wrong answer. It's not in the same league as data races, where if you disobey the rules we really can do anything.

So I don't think we can assert its carry input to 0/1.

--
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.
To view this discussion visit https://groups.google.com/d/msgid/golang-dev/CAHWihb_sFHpxNasooJg3aycp7fnJjDi0aNavOSJOe-kPUSHyKw%40mail.gmail.com.

[Keith Randall]

As to panicking, I seem to recall we decided not to do that because of performance reasons. Maybe our prove pass is good enough now that it would remove most of the checks?

I think we could probably do that given what the docs say, but I really don't see the motivation.

[Jorropo]

Actually, the question I should have started with, why is it not bits.Add64(uint64, uint64, bool) (uint64, bool) ?

[Keith Randall]

On Mon, Mar 10, 2025 at 9:35 AM Jorropo <jorro...@gmail.com> wrote:

Actually, the question I should have started with, why is it not bits.Add64(uint64, uint64, bool) (uint64, bool) ?

Maybe we thought we would avoid zero extensions that way? I don't remember.

It could also be the constant-time thing. Do we have a means to do bool->int in constant time on all archs?

[Russ Cox]

You've discovered a compiler bug, which should be fixed.

Best,

Russ

[Jorropo]

The buggy code has never been merged nor submitted, nothing to fix.