[libc-commits] [PATCH] D78611: [libc] Add a library of low level utils.

[Siva Chandra via Phabricator via libc-commits]

sivachandra marked 2 inline comments as done.
sivachandra added a comment.

I did not finish this patch, but wrote just enough to make it understandable so that the main discussion about sanitizer instrumentation can proceed smoothly.

In D78611#1996187 <https://reviews.llvm.org/D78611#1996187>, @abrachet wrote:

> Do these all belong in a linux specific directory? Wouldn't the two which aren't x86 specific be fine to put into libc/src/__support/?


The `__builtin*` functions are really clang/gcc specific. That is why I put them in a linux specific directory. May be in future they have to be grouped differently.

> Also, we should be weary of LTO inlining. I'm not entirely sure of the details here or how to force the linker to not inline these functions. It would certainly break all of these so it's worth looking into.

Thanks for bringing this up. So yes, we need to add the `noinline` attribute to guarantee that these functions don't get inlined even under LTO. Will do this in the next update.



================
Comment at: libc/config/linux/platform_lowlevel.cpp:13
+
+uintptr_t get_current_pc() {
+  return reinterpret_cast<uintptr_t>(
----------------
Add noinline attribute.


================
Comment at: libc/config/linux/x86_64/machine_lowlevel.cpp:13
+
+uintptr_t get_current_sp() {
+  uintptr_t *bp = reinterpret_cast<uintptr_t *>(__builtin_frame_address(0));
----------------
Add noinline attribute.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78611/new/

https://reviews.llvm.org/D78611