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

go_binary and go_test rules do not take --@io_bazel_rules_go//go/config:linkmode=pie into account #3614

Closed
HakanSunay opened this issue Jul 4, 2023 · 1 comment · Fixed by #3627 or #3629

Comments

@HakanSunay
Copy link

HakanSunay commented Jul 4, 2023

What version of rules_go are you using?

v0.40.1

What version of gazelle are you using?

v0.31.1

What version of Bazel are you using?

Bazelisk version: development
Build label: 6.2.1
Build target: bazel-out/k8-opt/bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar
Build time: Fri Jun 2 16:59:58 2023 (1685725198)
Build timestamp: 1685725198
Build timestamp as int: 1685725198

Does this issue reproduce with the latest releases of all the above?

Yes.

What operating system and processor architecture are you using?

CentOS 7.9.2009 on x86_64

Any other potentially useful information about your toolchain?

No, this is a plain environment.

What did you do?

I built a plain hello world binary using the --@io_bazel_rules_go//go/config:linkmode=pie flag.
Afterward, I ran checksec to inspect its PIE-ness.
(the script was downloaded from https://www.trapkit.de/tools/checksec/checksec.sh)

$ bazelisk build --@io_bazel_rules_go//go/config:linkmode=pie //src/cmd:cmd
$ ./checksec.sh --file bazel-bin/src/cmd/cmd_/cmd
RELRO           STACK CANARY      NX            PIE             RPATH      RUNPATH      FILE
No RELRO        No canary found   NX enabled    No PIE          No RPATH   No RUNPATH   bazel-bin/src/cmd/cmd_/cmd

Environment

$ find .
.
./BUILD.bazel
./WORKSPACE
./src
./src/BUILD.bazel
./src/go.mod
./src/cmd
./src/cmd/main.go
./src/cmd/BUILD.bazel
./checksec.sh
# ./BUILD.bazel
# empty
# ./WORKSPACE
load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")

http_archive(
    name = "io_bazel_rules_go",
    sha256 = "51dc53293afe317d2696d4d6433a4c33feedb7748a9e352072e2ec3c0dafd2c6",
    urls = [
        "https://mirror.bazel.build/github.com/bazelbuild/rules_go/releases/download/v0.40.1/rules_go-v0.40.1.zip",
        "https://github.com/bazelbuild/rules_go/releases/download/v0.40.1/rules_go-v0.40.1.zip",
    ],
)

http_archive(
    name = "bazel_gazelle",
    sha256 = "b8b6d75de6e4bf7c41b7737b183523085f56283f6db929b86c5e7e1f09cf59c9",
    urls = [
        "https://mirror.bazel.build/github.com/bazelbuild/bazel-gazelle/releases/download/v0.31.1/bazel-gazelle-v0.31.1.tar.gz",
        "https://github.com/bazelbuild/bazel-gazelle/releases/download/v0.31.1/bazel-gazelle-v0.31.1.tar.gz",
    ],
)

load("@io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", "go_rules_dependencies")
load("@bazel_gazelle//:deps.bzl", "gazelle_dependencies")

go_rules_dependencies()

go_register_toolchains(version="1.20.5")

gazelle_dependencies()
# ./src/BUILD.bazel
load("@bazel_gazelle//:def.bzl", "gazelle")

# gazelle:prefix src
gazelle(name = "gazelle")
// ./src/go.mod
module src

go 1.17
# ./src/cmd/BUILD.bazel
load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library")

go_binary(
    name = "cmd",
    embed = [":cmd_lib"],
    visibility = ["//visibility:public"],
)

go_library(
    name = "cmd_lib",
    srcs = ["main.go"],
    importpath = "src/cmd",
    visibility = ["//visibility:private"],
)
// ./src/cmd/main.go

package main

import (
	"fmt"
)

func main() {
	fmt.Println("Hello World")
}

What did you expect to see?

I expected to see positive information as an output of checksec for the PIE field.

What did you see instead?

PIE
No PIE

Extras

When I set the linkmode attribute of go_binary to "pie", the same command (checksec) gives a positive result for PIE-ness.

# ./src/cmd/BUILD.bazel
load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library")

go_binary(
    name = "cmd",
    embed = [":cmd_lib"],
+   linkmode = "pie",
    visibility = ["//visibility:public"],
)

go_library(
    name = "cmd_lib",
    srcs = ["main.go"],
    importpath = "src/cmd",
    visibility = ["//visibility:private"],
)
$ bazelisk build //src/cmd:cmd
$ ./checksec.sh --file bazel-bin/src/cmd/cmd_/cmd
RELRO           STACK CANARY      NX            PIE             RPATH      RUNPATH      FILE
Full RELRO      No canary found   NX enabled    PIE enabled     No RPATH   No RUNPATH   bazel-bin/src/cmd/cmd_/cmd
@HakanSunay HakanSunay changed the title go_binary rules do not take --@io_bazel_rules_go//go/config:linkmode=pie into account go_binary and go_test rules do not take --@io_bazel_rules_go//go/config:linkmode=pie into account Jul 4, 2023
@HakanSunay
Copy link
Author

After applying the following patch to debug what's going on in go_binary:

diff --git a/go/private/rules/transition.bzl b/go/private/rules/transition.bzl
index 4e87e30..521b07c 100644
--- a/go/private/rules/transition.bzl
+++ b/go/private/rules/transition.bzl
@@ -68,6 +68,9 @@ def _go_transition_impl(settings, attr):
     # In any case, get_mode should mainly be responsible for reporting
     # invalid modes, since it also takes --features flags into account.

+    print("In _go_transition_impl")
+    print("Linkmode (//go/config:linkmode) in transition input settings is '%s'" % settings['//go/config:linkmode'])
+    print("Linkmode in transition input attr is '%s'" % attr.linkmode)
     original_settings = settings
     settings = dict(settings)

@@ -111,11 +114,15 @@ def _go_transition_impl(settings, attr):
     if tags:
         settings["//go/config:tags"] = _deduped_and_sorted(tags)

+    # Settings is never inspected while deciding on the final linkmode.
+    # The only way to use what is in settings (config flag) is to explicitly
+    # set "linkmode" to "" or "auto".
     linkmode = getattr(attr, "linkmode", "auto")
     if linkmode != "auto":
         if linkmode not in LINKMODES:
             fail("linkmode: invalid mode {}; want one of {}".format(linkmode, ", ".join(LINKMODES)))
         settings["//go/config:linkmode"] = linkmode
+        print("Linkmode from settings[//go/config:linkmode] is never processed")

     for key, original_key in _SETTING_KEY_TO_ORIGINAL_SETTING_KEY.items():
         old_value = original_settings[key]
@@ -136,6 +143,7 @@ def _go_transition_impl(settings, attr):
         else:
             settings[original_key] = ""

+    print("Transition returning settings['//go/config:linkmode']='%s'" % settings["//go/config:linkmode"])
     return settings

 def _request_nogo_transition(settings, _attr):

I get the following output when running the build command with --@io_bazel_rules_go//go/config:linkmode=pie:

DEBUG: io_bazel_rules_go/go/private/rules/transition.bzl:71:10: In _go_transition_impl
DEBUG: io_bazel_rules_go/go/private/rules/transition.bzl:72:10: Linkmode (//go/config:linkmode) in transition input settings is 'pie'
DEBUG: io_bazel_rules_go/go/private/rules/transition.bzl:73:10: Linkmode in transition input attr is 'normal'
DEBUG: io_bazel_rules_go/go/private/rules/transition.bzl:125:14: Linkmode from settings[//go/config:linkmode] is never processed
DEBUG: io_bazel_rules_go/go/private/rules/transition.bzl:146:10: Transition returning settings['//go/config:linkmode']='normal'

I feel like the default values of go_binary.linkmode and go_test.linkmode could be reconsidered.
Another option is to handle the presence of both values in the transition itself and decide on definitive action, given the lack of default values in any of them.

fmeum added a commit that referenced this issue Jul 17, 2023
The value of the flag was always overriden by the `linkmode` `go_binary`
attribute, even if that attribute was at its default (`normal`) value.
Instead, use a default of `auto` to distinguish this case from the case
where no attribute value has been set explicitly.

Fixes #3614
Closes #3615
fmeum added a commit that referenced this issue Jul 17, 2023
The value of the flag was always overriden by the `linkmode` `go_binary`
attribute, even if that attribute was at its default (`normal`) value.
Instead, use a default of `auto` to distinguish this case from the case
where no attribute value has been set explicitly.

Fixes #3614
Closes #3615

Co-authored-by: Hakan Halil <hhalil@vmware.com>
fmeum added a commit that referenced this issue Jul 17, 2023
The value of the flag was always overriden by the `linkmode` `go_binary`
attribute, even if that attribute was at its default (`normal`) value.
Instead, use a default of `auto` to distinguish this case from the case
where no attribute value has been set explicitly.

Fixes #3614
Closes #3615

Co-authored-by: Hakan Halil <hhalil@vmware.com>
fmeum added a commit that referenced this issue Jul 17, 2023
The value of the flag was always overriden by the `linkmode` `go_binary`
attribute, even if that attribute was at its default (`normal`) value.
Instead, use a default of `auto` to distinguish this case from the case
where no attribute value has been set explicitly.

Fixes #3614
Closes #3615

Co-authored-by: Hakan Halil <hhalil@vmware.com>
fmeum added a commit that referenced this issue Jul 17, 2023
The value of the flag was always overriden by the `linkmode` `go_binary`
attribute, even if that attribute was at its default (`normal`) value.
Instead, use a default of `auto` to distinguish this case from the case
where no attribute value has been set explicitly.

Fixes #3614
Closes #3615
fmeum added a commit that referenced this issue Jul 17, 2023
The value of the flag was always overriden by the `linkmode` `go_binary`
attribute, even if that attribute was at its default (`normal`) value.
Instead, use a default of `auto` to distinguish this case from the case
where no attribute value has been set explicitly.

Fixes #3614
Closes #3615
fmeum added a commit that referenced this issue Jul 18, 2023
The value of the flag was always overriden by the `linkmode` `go_binary`
attribute, even if that attribute was at its default (`normal`) value.
Instead, use a default of `auto` to distinguish this case from the case
where no attribute value has been set explicitly.

Fixes #3614
Closes #3615
fmeum added a commit that referenced this issue Jul 18, 2023
The value of the flag was always overriden by the `linkmode` `go_binary`
attribute, even if that attribute was at its default (`normal`) value.
Instead, use a default of `auto` to distinguish this case from the case
where no attribute value has been set explicitly.

Fixes #3614
Closes #3615
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant