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

cmd/compile: == and != comparisons of addresses of symbols not handled consistently #24503

Closed
bryanpkc opened this issue Mar 23, 2018 · 7 comments

Comments

Projects
None yet
8 participants
@bryanpkc
Copy link
Contributor

commented Mar 23, 2018

What version of Go are you using (go version)?

1.9, 1.10, tip

Does this issue reproduce with the latest release?

Yes, but not with 1.8 or older releases.

What operating system and processor architecture are you using (go env)?

linux amd64

What did you do?

https://play.golang.org/p/KyovZxzxQfS

What did you expect to see?

Either "equal" or "unequal" should be printed. Go 1.8 printed "equal".

What did you see instead?

No output.

It seems that the problem is caused by CL 38338, which added a SSA rule to simplify a comparison of the addresses of two symbols (even when those addresses are numerically identical):

(EqPtr (Addr {a} x) (Addr {b} x)) -> (ConstBool [b2i(a == b)])

However, the negated form of this rule is missing, so the compiler still emits a numeric comparison of the two addresses for the second comparison in the code. Adding this rule optimizes away that comparison and fixes the program output (to "unequal"):

(NeqPtr (Addr {a} x) (Addr {b} x)) -> (ConstBool [b2i(a != b)])
@gopherbot

This comment has been minimized.

Copy link

commented Mar 23, 2018

Change https://golang.org/cl/102275 mentions this issue: cmd/compile/internal/ssa: consistent constant pointer comparisons

@ALTree ALTree changed the title compile: == and != comparisons of addresses of symbols not handled consistently cmd/compile: == and != comparisons of addresses of symbols not handled consistently Mar 23, 2018

@bcmills bcmills added this to the Go1.11 milestone Mar 23, 2018

@andybons

This comment has been minimized.

Copy link
Member

commented Mar 23, 2018

@andybons andybons added the NeedsFix label Mar 23, 2018

@randall77

This comment has been minimized.

Copy link
Contributor

commented Mar 23, 2018

I don't think this is a bug.
The language spec says:

Pointers to distinct zero-size variables may or may not be equal.

I admit that it's a bit weird that they are both equal and not equal at the same time. But a valid program should not be relying on the result in any case.

That said, I'm not adverse to the fix you suggested. It seems harmless enough.

@bryanpkc

This comment has been minimized.

Copy link
Contributor Author

commented Mar 24, 2018

I have read that line in the language spec, but have always understood it as saying that the equality may change from one Go release to the next; I didn't think of it as possibly causing inconsistent behaviour. The risk is that a program using an imported type may not necessarily know that the type is zero-width, and could be surprised by this behaviour.

Alternatively, instead of treating this as a bug fix, it could be considered an improvement to CL 38338, by optimizing the other kind of comparison as well.

@josharian

This comment has been minimized.

Copy link
Contributor

commented Mar 24, 2018

I admit that it's a bit weird that they are both equal and not equal at the same time. But a valid program should not be relying on the result in any case.

I guess, although I think (a == b) || (a != b) should always evaluate to true, whether guaranteed by the spec or not.

@cznic

This comment has been minimized.

Copy link
Contributor

commented Mar 24, 2018

I guess, although I think (a == b) || (a != b) should always evaluate to true, whether guaranteed by the spec or not.

I think this assumption is similar to the one that a == a should always evaluate to true. But it does not. Neither does your particular example always evaluate to true in, for example, sql (when a and/or b is NULL).

@mdempsky

This comment has been minimized.

Copy link
Member

commented Mar 28, 2018

I'll weigh in for team bug.

The spec defines !p to mean "not p", and u == v and u != v to mean "u equals v" and "u not equals v", respectively. By composition, it follows that !(u == v) also means "u not equals v", which should then be the same as u != v. (This is basically @josharian's (a == b) || (a != b) argument framed differently.)

I argue this is a bug because it violates this constraint. This program should either print not equal 1\nnot equal 2\n, or nothing at all: https://play.golang.org/p/cPI8UBx-GBB

I acknowledge the spec could be more precise in its description of comparing pointers to zero-width variables, but I think the simplest interpretation is that "equal" and "not equal" are exclusive relations that hold indefinitely for any two values. I think if we really wanted to admit the current behavior, we should revise the spec to explicitly state that results may by different for each comparison (of pointer to zero-width variables).

@gopherbot gopherbot closed this in 625f2dc Mar 31, 2018

@golang golang locked and limited conversation to collaborators Mar 31, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.