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/cgo: incomplete Clang detection #10453

Closed
trombonehero opened this issue Apr 14, 2015 · 4 comments

Comments

@trombonehero
Copy link

commented Apr 14, 2015

When cgo invokes Clang, it needs to set -ferror-limit=0 in order to see a complete set of error messages (rather than a truncated list of 20). This works whenever cgo can tell that it's using Clang rather than GCC, but the detection logic is incomplete on modern versions of FreeBSD.

The current check, located at line 765 of cmd/cgo/gcc.go, tests:

if strings.Contains(c[0], "clang") {

but on FreeBSD, /usr/bin/cc is Clang. We could detect this with OS-specific checks, such as:

if strings.Contains(c[0], "clang") || runtime.GOOS == "freebsd" {

but I'm not sure how to distinguish an older-but-still-supported version of FreeBSD (where /usr/bin/cc is GCC) with a simple string comparison. A more general approach would be to invoke cc --version to see what comes back:

func ccIsClang(cc string) bool {
    if strings.Contains(cc, "clang") {
        return true
    }
    out, _ := exec.Command(cc, "--version").Output()
    return strings.Contains(string(out), "clang")
}

This is a more heavyweight approach, but given that we're going to execute the C compiler anyway, maybe the performance cost is worth the increase in robustness? The extra exec could be guarded with some runtime.GOOS == "freebsd" logic, but as Clang gets adopted in more and more places, it might make sense to just Do The Right Thing rather than rely on heuristics.

@ianlancetaylor ianlancetaylor changed the title cgo: incomplete Clang detection cmd/cgo: incomplete Clang detection Apr 14, 2015

@ianlancetaylor ianlancetaylor added this to the Go1.5 milestone Apr 14, 2015

@mdempsky

This comment has been minimized.

Copy link
Member

commented Apr 16, 2015

I would expect the overhead from unconditionally running "$CC -v" or "$CC --version" would be minimal, so that seems like the easiest way to fix this.

If we did want to avoid invoking the compiler unnecessarily though, we could add "-v" in gccDefines and check if the first line of stderr contains "clang version". (I'd need to double check that this string works for historical Clang versions too, but I would expect so.) gccDefines is the first place we run the compiler, and it doesn't currently need any clang-specific logic.

@mdempsky mdempsky self-assigned this Apr 18, 2015

@mdempsky

This comment has been minimized.

Copy link
Member

commented Apr 18, 2015

@trombonehero Can you see if https://go-review.googlesource.com/#/c/9090/ helps with cc==clang on FreeBSD?

After implementing that fix, I noticed that cmd/go and cmd/internal/ld also have similar strings.Contains(gcc, "clang") checks. Those should probably be fixed too, but at least they only control diagnostic output formatting and additional warnings, so hopefully CL 9090 is still an improvement.

@mdempsky mdempsky closed this in 9340238 Apr 18, 2015

@davidchisnall

This comment has been minimized.

Copy link

commented May 7, 2015

This issue will also manifest on OS X, where cc is clang.

@mdempsky

This comment has been minimized.

Copy link
Member

commented May 7, 2015

@davidchisnall If there's a specific problem you're aware of on OS X with cc==clang, please file a new issue.

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