Page MenuHomeFreeBSD

rtld: actually resolve memcpy plt
ClosedPublic

Authored by rlibby on Jul 5 2024, 10:22 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Oct 8, 9:47 PM
Unknown Object (File)
Sep 22 2024, 5:09 AM
Unknown Object (File)
Sep 13 2024, 2:52 PM
Unknown Object (File)
Sep 13 2024, 10:39 AM
Unknown Object (File)
Sep 8 2024, 10:44 PM
Unknown Object (File)
Sep 8 2024, 12:34 PM
Unknown Object (File)
Sep 8 2024, 1:34 AM
Unknown Object (File)
Sep 7 2024, 11:33 PM
Subscribers

Details

Summary

The call to memcpy() meant to cause plt resolution in _thr_rtld_init()
was getting optimized by the compiler. Tell the compiler not to use its
memcpy() builtin in thr_rtld.c.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 58536
Build 55424: arc lint + arc unit

Event Timeline

Should we disable all built-ins for the file, for the same reasoning?

This revision is now accepted and ready to land.Jul 5 2024, 10:32 PM
In D45891#1046235, @kib wrote:

Should we disable all built-ins for the file, for the same reasoning?

That makes sense to me, for future insurance. I checked just now that -fno-builtin didn't reveal any additional cases. Do you prefer to make that change?

kib: just disable all builtins

This revision now requires review to proceed.Jul 6 2024, 1:10 AM
kib added inline comments.
lib/libthr/Makefile
35

I would add a comment there, noting that this flag is there for functional purpose, instead of suppressing some warning.

This revision is now accepted and ready to land.Jul 6 2024, 2:33 AM
This revision now requires review to proceed.Jul 6 2024, 4:54 AM
This revision is now accepted and ready to land.Jul 6 2024, 7:52 AM
This revision was automatically updated to reflect the committed changes.

No sure how much the builtins optimization matters here but if it does the other workaround would be:

void* memcpy_for_plt(void*, const void*, size_t) asm("memcpy")

And then call that.

No sure how much the builtins optimization matters here but if it does the other workaround would be:

void* memcpy_for_plt(void*, const void*, size_t) asm("memcpy")

And then call that.

Thanks for the info. I think @kib could speak better to whether any code in this file could be important in terms of performance. In any case, I believe the one memcpy was the only site actually affected when I examined the codegen for amd64.

No sure how much the builtins optimization matters here but if it does the other workaround would be:

void* memcpy_for_plt(void*, const void*, size_t) asm("memcpy")

And then call that.

Thanks for the info. I think @kib could speak better to whether any code in this file could be important in terms of performance. In any case, I believe the one memcpy was the only site actually affected when I examined the codegen for amd64.

lock_acquire/release are the only functions which performance is critical for runtime.

That said, I do believe that correctness trumps speed.