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.

Sign up for GitHub

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

Jump to bottom

cmd/cgo: incomplete Clang detection #10453

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

cmd/cgo: incomplete Clang detection #10453

trombonehero opened this issue Apr 14, 2015 · 4 comments
Labels
FrozenDueToAge
Milestone
Go1.5

Comments

@trombonehero
Copy link

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.
This was referenced Apr 14, 2015
Closed
Closed
@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
Copy link
Contributor

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
Copy link
Contributor

@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.

@davidchisnall
Copy link

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

@mdempsky
Copy link
Contributor

mdempsky 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.

@ianlancetaylor ianlancetaylor mentioned this issue Jun 16, 2015
Closed
@crawshaw crawshaw mentioned this issue Jul 11, 2015
Closed
@kisscool kisscool mentioned this issue Aug 25, 2015
Closed
@golang golang locked and limited conversation to collaborators Jun 25, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.5
Development

No branches or pull requests
5 participants
@mdempsky @trombonehero @davidchisnall @ianlancetaylor @gopherbot