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

runtime/cgo: fatalf is not declared as "noreturn" #64553

Closed
fukanchik opened this issue Dec 5, 2023 · 9 comments
Closed

runtime/cgo: fatalf is not declared as "noreturn" #64553

fukanchik opened this issue Dec 5, 2023 · 9 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@fukanchik
Copy link

Go version

go version go1.18.1 linux/amd64

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

GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/fuxx/.cache/go-build"
GOENV="/home/fuxx/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/fuxx/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/fuxx/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go-1.18"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go-1.18/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.18.1"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build2893647507=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Run go build of a cgo project under clang's scan-build for static analysis.

What did you expect to see?

No static analizer errors.

What did you see instead?

In my case /usr/lib/go-1.18/src/runtime/cgo/gcc_linux_amd64.c produces:

attr = (pthread_attr_t*)malloc(sizeof *attr);
1	Value assigned to 'attr'
if (attr == NULL) {
2 Assuming 'attr' is equal to NULL	
3 	Taking true branch	
fatalf("malloc failed: %s", strerror(errno));
}
pthread_attr_init(attr);
4 Null pointer passed to 1st parameter expecting 'nonnull

this is because fatalf is not declared as [[noreturn]]:

/*
 * Prints error then calls abort. For linux and android.
 */
void fatalf(const char* format, ...);
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Dec 5, 2023
@qiulaidongfeng
Copy link
Contributor

Which version of clang?
Does the latest version of go have the same problem?

@fukanchik
Copy link
Author

Latest version still does not have [[noreturn]] : https://github.com/golang/go/blob/master/src/runtime/cgo/libcgo.h#L79

clang version is Ubuntu clang version 14.0.0-1ubuntu1.1

As for gcc_linux_amd64.c g->stacklo, in the latest version it is calculated differently.

@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 6, 2023
@cagedmantis cagedmantis added this to the Backlog milestone Dec 6, 2023
@cagedmantis
Copy link
Contributor

cc @golang/runtime

@cherrymui
Copy link
Member

clang's scan-build for static analysis

What analysis, specifically, what command did you run and what is the error message? Thanks.

@fukanchik
Copy link
Author

Error message is in the section "What did you see instead?" of the ticket description.

I run the following command:

scan-build go build main.go

@mauri870
Copy link
Member

The issue description shows more errors, but that is with go 1.18, which is EOL.

This only happens for Go <= 1.21, the changes from CL519457 made the only warning go away. Since this is just a warning I don't think it is worth backporting.

$ scan-build go1.21.5 build main.go
scan-build: Using '/usr/lib/llvm-14/bin/clang' for static analysis
# runtime/cgo
gcc_linux_amd64.c:45:2: warning: Null pointer passed to 1st parameter expecting 'nonnull' [core.NonNullParamChecker]
        pthread_attr_init(attr);
        ^~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
scan-build: No bugs found.
$ scan-build gotip build main.go
scan-build: Using '/usr/lib/llvm-14/bin/clang' for static analysis
scan-build: No bugs found.

@cherrymui
Copy link
Member

Thanks @mauri870 . Since there is no more warnings we can close this.

If one wants to add a noreturn attribute or dummy return statements after fatalf, feel free to send a CL. Thanks.

@fukanchik
Copy link
Author

It's ok to close this, however even though go 1.18 is EOL, it is the actual version for Ubuntu 22.04.3 LTS. That is one of the most used distributions.

@gopherbot
Copy link

Change https://go.dev/cl/550115 mentions this issue: runtime/cgo: mark fatalf as noreturn

gopherbot pushed a commit that referenced this issue Feb 13, 2024
Fixes #64553

Change-Id: I7860cd9ba74d70a7d988538ea4df8e122f94cde6
GitHub-Last-Rev: 0616437
GitHub-Pull-Request: #64727
Reviewed-on: https://go-review.googlesource.com/c/go/+/550115
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Run-TryBot: Mauri de Souza Meneguzzo <mauri870@gmail.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
ezz-no pushed a commit to ezz-no/go-ezzno that referenced this issue Feb 18, 2024
Fixes golang#64553

Change-Id: I7860cd9ba74d70a7d988538ea4df8e122f94cde6
GitHub-Last-Rev: 0616437
GitHub-Pull-Request: golang#64727
Reviewed-on: https://go-review.googlesource.com/c/go/+/550115
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Run-TryBot: Mauri de Souza Meneguzzo <mauri870@gmail.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants