-
Notifications
You must be signed in to change notification settings - Fork 74
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
gen/nvml: add --export-dynamic
linker flag
#79
Conversation
Signed-off-by: braydonk <[email protected]>
Thanks for taking the time to dig into this. I'm happy to merge this given your detailed explanation of the root cause and the minimal change required to fix things. However, I strangely still don't see this behaviour on my system on a fresh checkout of
|
Reading through your issue against the golang repo more closely, it's likely because my system has
You mention that the bug surfaces with |
Thank you for taking a look @klueska! |
Closes #36
Investigation on the Go side golang/go#63264
When built with
go1.21.x
,go-nvml
panics. This is due to a change in howgo tool link
executes the external linker. Previously,-rdynamic
was always passed to the external linker, however this was changed to only happen in certain circumstances, such as when linking to previously built CGO shared objects, or when the flag to export specific dynamic symbols isn't supported. The justification for the change is in golang/go#53579.The resolution I propose for
go-nvml
is to pass the--export-dynamic
flag intoLDFLAGS
explicitly. This LDFLAG is required because if it is not present, the symbols fromnvml.h
are not added to the PLT, meaning each call to a symbol fromnvml.h
will have an address of0x0
instead of a PLT offset (at least in ELF AMD64, though I assume some equivalent occurs in other setups).Passing this flag explicitly means whether it's built with a Go version that always applies
-rdynamic
or not, this required flag will be present.I tested this PR by uncommenting the commented out test command from
make test
. On the main branch, it passes alldl
tests, then panics.On this branch, all tests pass.
I also tested with
go1.20.8
and the tests all pass.