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

sync: need a way to guarantee that WaitGroup.state1 fields are always 4-bytes aligned #27577

Open
go101 opened this Issue Sep 9, 2018 · 5 comments

Comments

Projects
None yet
4 participants
@go101
Copy link

go101 commented Sep 9, 2018

I have noticed and am glad that the suggestion mentioned in #19149 is adopted. But I feel the current new implementation is still not perfect.

The current implementation assumes that the WaitGroup.state1 field is always 4-bytes aligned, but I can't find any such guarantee is written down in any official Go documentation.

So I suggest that we should add a method like the following for *WaitGroup. The method will never be called, it just gives compilers a hint that the WaitGroup.state1 field needs to be 4-bytes aligned.

func (wg *WaitGroup) _() {
	_ = atomic.LoadUint32(&wg.state1[0])
}
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

ianlancetaylor commented Sep 9, 2018

I don't mind adding a compiler hint but I would prefer it to be one that the compiler might actually recognize. I think it's unlikely that the compiler will ever recognize your suggestion.

Note that it's more or less OK for the standard library to assume behavior that is not guaranteed, because if the behavior changes, the standard library can change at the same time. This is not true for packages outside the standard library.

@randall77

This comment has been minimized.

Copy link
Contributor

randall77 commented Sep 9, 2018

WaitGroup.state1 is a [3]uint32. It will be 4-byte aligned on all platforms we support.
It's not guaranteed as we don't talk about alignment in the spec. But we align all Go data types to their natural alignment up to a platform-specific maximum alignment, which is at least 4 on all platforms. It is very likely to remain at least 4 because pointers need to be aligned (to avoid shearing pointers).

@go101

This comment has been minimized.

Copy link

go101 commented Sep 9, 2018

I understand the facts. I just want to make it perfect in theory. :)
Consider there is a third-party compiler which is neither gc or gccgo, ...

@CAFxX

This comment has been minimized.

Copy link
Contributor

CAFxX commented Sep 18, 2018

Can't this be solved simply by adding a test that will make all.bash fail if the field doesn't have the expected alignment?

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

ianlancetaylor commented Sep 18, 2018

@CAFxX A sync.WaitGroup can appear in user code that all.bash will never see, but it still has to be aligned correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment