rules_go icon indicating copy to clipboard operation
rules_go copied to clipboard

fix: properly expand absolute path placeholder for link calls

Open voxeljorge opened this issue 4 months ago • 6 comments

What type of PR is this?

Bug fix

What does this PR do? Why is it needed?

PR #4009 added a mechanism that uses a sentinel value as a placeholder for paths which need to be expanded to absolutes for compile operations. These paths end up embedded in .a files and come back to the linker during link operations. The previous PR replaces the CC tool with a pointer to builder cc which in turn substitutes the placeholder value for actual paths. Unfortunately this wasn't done for the linker, so the linker is receiving parameters like --sysroot=__GO_BAZEL_CC_PLACEHOLDER__external/_main~_repo_rules~ubuntu_20_04_sysroot/ which is not valid. When a sysroot is configured this means that internally the linker won't be able to find the sysroot in some cases.

In almost comically bad luck, this only affects the go tool link calls to linkerFlagSupported because for some reason linkerFlagSupported puts the -extldflags at the beginning of the linker command, while the main host link command has them at the end. So this means the linker detects all flags as unsupported but then proceeds to run the linker "correctly" which results in really confusing linker errors. In my case this resulted in the linker trying to run with -rdynamic incorrectly.

Which issues(s) does this PR fix?

Fixes #3886

It could be reasonable to open a bug against go tool link describing the difference between the host link and the flag checks. I suspect it's not really intended behavior and was just sort of happenstance that things ended up that way.

I also really have no idea how to write a test for this. I'm willing to give it a shot but would really appreciate a suggestion as to where/how.

voxeljorge avatar Oct 15 '24 15:10 voxeljorge