Skip to content

Commit

Permalink
cmd/cgo: add -fno-stack-protector to CFLAGS (again)
Browse files Browse the repository at this point in the history 
Add -fno-stack-protector back to the default set of CFLAGS for cgo, so
as to avoid problems with internal linking locating the library
containing the "__stack_chk_fail_local" support function that some
compilers emit (the specific archive can vary based on GOOS).

Updates #52919.
Updates #54313.
Updates #57261.
Updates #58385.

Change-Id: I4591bfb15501f04b7afe1fcd50c4fb93c86db63d
Reviewed-on: https://go-review.googlesource.com/c/go/+/466935
Reviewed-by: Ian Lance Taylor <[email protected]>
Run-TryBot: Than McIntosh <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
  • Loading branch information
@thanm
thanm committed Feb 10, 2023
1 parent 7d57a9c commit 48f4728
Showing 1 changed file with 3 additions and 1 deletion.
4 changes: 3 additions & 1 deletion src/runtime/cgo/cgo.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ package cgo
#cgo solaris LDFLAGS: -lxnet
#cgo solaris LDFLAGS: -lsocket
#cgo CFLAGS: -Wall -Werror
// Use -fno-stack-protector to avoid problems locating the
// proper support functions. See issues #52919, #54313, #58385.
#cgo CFLAGS: -Wall -Werror -fno-stack-protector
#cgo solaris CPPFLAGS: -D_POSIX_PTHREAD_SEMANTICS
Expand Down

2 comments on commit 48f4728

@nmeum
Copy link

@nmeum nmeum commented on 48f4728 Feb 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why this was re-added. Just compiling the cgo runtime with -fno-stack-protector is not sufficient as all cgo code (i.e. not just the run-time) will need to be compiled with -fno-stack-protector if you don't want to link against the library providing __stack_chk_fail_local. Or am I missing something here?

@thanm
Copy link
Contributor Author

@thanm thanm commented on 48f4728 Feb 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just FYI, few people read comments on github commits, probably best to comment on Gerrit instead.

Regular user programs that use CGO will default to external linking, meaning that it's the external linker that has to resolve __stack_chk_fail_local (this is the normal case).

For the case where someone builds a vanilla Go program (no CGO) with -race, we'll be using internal linking (no external linker involved). In this case the Go linker has to resolve __stack_chk_fail_local, and as is clear from this sequence of bugs, it can come from different places depending on the OS. Better to go back to turning off stack protector for the Go CGO-related code than to have to fix other issues down the line as people use "go build -race" on future linux distributions that put things in different places.

Please sign in to comment.