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/go: ignore *.syso files in -msan mode #21884

Open
rsc opened this Issue Sep 14, 2017 · 5 comments

Comments

Projects
None yet
6 participants
@rsc
Contributor

rsc commented Sep 14, 2017

Packages with syso files currently fail in -msan mode because the syso file calls out to a routine like memcmp which then falsely reports uninitialized memory.

It's not possible to prepare a syso file that works regardless of memory-sanitizer setting, at least not if the code makes any memory writes, which most code does. Instead you'd have to prepare both a standard syso and an msan-enabled syso, and the go command conventions provide no way to tell those apart.

I suggest we simply ignore syso files in -msan mode. That will result in missing link symbols, but then package authors can revise their packages to avoid the syso in -msan mode. That gives package authors at least one way to write a package that has a syso and still works with -msan. Today there are zero ways.

A more complex change would be to introduce a new class of objects like *.syso.msan or *_msan.syso, but let's not do that until there's a compelling reason. In the case where this came up, the "ignore syso files" is good enough.

@rsc rsc added this to the Go1.10 milestone Sep 14, 2017

@gopherbot

This comment has been minimized.

gopherbot commented Sep 14, 2017

Change https://golang.org/cl/63917 mentions this issue: [dev.boringcrypto] cmd/go: exclude SysoFiles when using -msan

gopherbot pushed a commit that referenced this issue Sep 18, 2017

[dev.boringcrypto] cmd/go: exclude SysoFiles when using -msan
There's no way for a *.syso file to be compiled to work both in
normal mode and in msan mode. Assume they are compiled in
normal mode and drop them in msan mode.

Packages with syso files currently fail in -msan mode because
the syso file calls out to a routine like memcmp which then
falsely reports uninitialized memory. After this CL, they will fail
in -msan with link errors, because the syso will not be used.
But then it will at least be possible for package authors to write
fallback code in the package that avoids the syso in -msan mode,
so that the package with the syso can at least run in both modes.
Without this CL, that's not possible.

See #21884.

Change-Id: I77340614c4711325032484e65fa9c3f8332741d5
Reviewed-on: https://go-review.googlesource.com/63917
Reviewed-by: Adam Langley <agl@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@gopherbot

This comment has been minimized.

gopherbot commented Sep 22, 2017

Change https://golang.org/cl/65488 mentions this issue: [dev.boringcrypto.go1.8] cmd/go: exclude SysoFiles when using -msan

gopherbot pushed a commit that referenced this issue Sep 22, 2017

[dev.boringcrypto.go1.8] cmd/go: exclude SysoFiles when using -msan
There's no way for a *.syso file to be compiled to work both in
normal mode and in msan mode. Assume they are compiled in
normal mode and drop them in msan mode.

Packages with syso files currently fail in -msan mode because
the syso file calls out to a routine like memcmp which then
falsely reports uninitialized memory. After this CL, they will fail
in -msan with link errors, because the syso will not be used.
But then it will at least be possible for package authors to write
fallback code in the package that avoids the syso in -msan mode,
so that the package with the syso can at least run in both modes.
Without this CL, that's not possible.

See #21884.

Change-Id: I77340614c4711325032484e65fa9c3f8332741d5
Reviewed-on: https://go-review.googlesource.com/63917
Reviewed-by: Adam Langley <agl@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-on: https://go-review.googlesource.com/65488
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>

@rsc rsc modified the milestones: Go1.10, Go1.11 Dec 1, 2017

@FiloSottile

This comment has been minimized.

Member

FiloSottile commented May 18, 2018

@rsc any reason not to cherry-pick on master (with docs)?

@ManPython

This comment has been minimized.

ManPython commented May 29, 2018

There is instruction how to add .icon based on .syso and not working
https://github.com/therecipe/qt/wiki/Setting-the-Application-Icon#setting-the-icon-on-windows

In any case of .syso looks not working
go1.10.2.windows-amd64
w7 x64

@ianlancetaylor ianlancetaylor modified the milestones: Go1.11, Go1.12 Jul 6, 2018

@bcmills bcmills added the GoCommand label Nov 14, 2018

@bcmills bcmills modified the milestones: Go1.12, Go1.13 Nov 14, 2018

@rsc

This comment has been minimized.

Contributor

rsc commented Nov 20, 2018

This is OK to cherry pick to master.

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