[libc-commits] [libc] [libc] fix issues around stack protector (PR #74567)
via libc-commits
libc-commits at lists.llvm.org
Tue Dec 5 22:32:50 PST 2023
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-libc
Author: Schrodinger ZHU Yifan (SchrodingerZhu)
<details>
<summary>Changes</summary>
If a function is declared with stack-protector, the compiler may try to load the TLS. However, inside the certain runtime functions, TLS may not be available. This patch disables stack protectors for certain routines to fix the problem.
Closes #<!-- -->74487.
---
Full diff: https://github.com/llvm/llvm-project/pull/74567.diff
6 Files Affected:
- (modified) libc/src/__support/OSUtil/linux/quick_exit.h (+4-2)
- (modified) libc/startup/linux/aarch64/CMakeLists.txt (+1)
- (modified) libc/startup/linux/aarch64/start.cpp (+3-5)
- (modified) libc/startup/linux/riscv/start.cpp (+3-5)
- (modified) libc/startup/linux/x86_64/CMakeLists.txt (+1)
- (modified) libc/startup/linux/x86_64/start.cpp (+3-5)
``````````diff
diff --git a/libc/src/__support/OSUtil/linux/quick_exit.h b/libc/src/__support/OSUtil/linux/quick_exit.h
index c461d5f8b8fcb..43d9f82784109 100644
--- a/libc/src/__support/OSUtil/linux/quick_exit.h
+++ b/libc/src/__support/OSUtil/linux/quick_exit.h
@@ -9,7 +9,7 @@
#ifndef LLVM_LIBC_SRC___SUPPORT_OSUTIL_LINUX_QUICK_EXIT_H
#define LLVM_LIBC_SRC___SUPPORT_OSUTIL_LINUX_QUICK_EXIT_H
-#include "syscall.h" // For internal syscall function.
+#include "syscall.h" // For internal syscall function.
#include "src/__support/common.h"
@@ -17,7 +17,9 @@
namespace LIBC_NAMESPACE {
-LIBC_INLINE void quick_exit(int status) {
+// mark as no_stack_protector since TLS can be torn down before calling
+// quick_exit
+__attribute__((no_stack_protector)) LIBC_INLINE void quick_exit(int status) {
for (;;) {
LIBC_NAMESPACE::syscall_impl<long>(SYS_exit_group, status);
LIBC_NAMESPACE::syscall_impl<long>(SYS_exit, status);
diff --git a/libc/startup/linux/aarch64/CMakeLists.txt b/libc/startup/linux/aarch64/CMakeLists.txt
index b47db8eb5d23f..ccd9bbcb9b63d 100644
--- a/libc/startup/linux/aarch64/CMakeLists.txt
+++ b/libc/startup/linux/aarch64/CMakeLists.txt
@@ -13,6 +13,7 @@ add_startup_object(
libc.src.string.memory_utils.inline_memcpy
libc.src.unistd.environ
COMPILE_OPTIONS
+ -fno-stack-protector
-fno-omit-frame-pointer
-ffreestanding # To avoid compiler warnings about calling the main function.
)
diff --git a/libc/startup/linux/aarch64/start.cpp b/libc/startup/linux/aarch64/start.cpp
index b5c426866b56d..858995203d9b2 100644
--- a/libc/startup/linux/aarch64/start.cpp
+++ b/libc/startup/linux/aarch64/start.cpp
@@ -184,7 +184,7 @@ __attribute__((noinline)) static void do_start() {
app.tls.align = phdr->p_align;
}
- LIBC_NAMESPACE::TLSDescriptor tls;
+ static LIBC_NAMESPACE::TLSDescriptor tls;
LIBC_NAMESPACE::init_tls(tls);
if (tls.size != 0)
LIBC_NAMESPACE::set_thread_ptr(tls.tp);
@@ -193,6 +193,8 @@ __attribute__((noinline)) static void do_start() {
LIBC_NAMESPACE::main_thread_attrib.atexit_callback_mgr =
LIBC_NAMESPACE::internal::get_thread_atexit_callback_mgr();
+ LIBC_NAMESPACE::atexit(
+ []() { LIBC_NAMESPACE::cleanup_tls(tls.tp, tls.size); });
// We want the fini array callbacks to be run after other atexit
// callbacks are run. So, we register them before running the init
// array callbacks as they can potentially register their own atexit
@@ -208,10 +210,6 @@ __attribute__((noinline)) static void do_start() {
reinterpret_cast<char **>(app.args->argv),
reinterpret_cast<char **>(env_ptr));
- // TODO: TLS cleanup should be done after all other atexit callbacks
- // are run. So, register a cleanup callback for it with atexit before
- // everything else.
- LIBC_NAMESPACE::cleanup_tls(tls.addr, tls.size);
LIBC_NAMESPACE::exit(retval);
}
diff --git a/libc/startup/linux/riscv/start.cpp b/libc/startup/linux/riscv/start.cpp
index bf04be5ad14ad..8bd76631f402c 100644
--- a/libc/startup/linux/riscv/start.cpp
+++ b/libc/startup/linux/riscv/start.cpp
@@ -187,7 +187,7 @@ __attribute__((noinline)) static void do_start() {
app.tls.align = phdr->p_align;
}
- LIBC_NAMESPACE::TLSDescriptor tls;
+ static LIBC_NAMESPACE::TLSDescriptor tls;
LIBC_NAMESPACE::init_tls(tls);
if (tls.size != 0)
LIBC_NAMESPACE::set_thread_ptr(tls.tp);
@@ -196,6 +196,8 @@ __attribute__((noinline)) static void do_start() {
LIBC_NAMESPACE::main_thread_attrib.atexit_callback_mgr =
LIBC_NAMESPACE::internal::get_thread_atexit_callback_mgr();
+ LIBC_NAMESPACE::atexit(
+ []() { LIBC_NAMESPACE::cleanup_tls(tls.tp, tls.size); });
// We want the fini array callbacks to be run after other atexit
// callbacks are run. So, we register them before running the init
// array callbacks as they can potentially register their own atexit
@@ -211,10 +213,6 @@ __attribute__((noinline)) static void do_start() {
reinterpret_cast<char **>(app.args->argv),
reinterpret_cast<char **>(env_ptr));
- // TODO: TLS cleanup should be done after all other atexit callbacks
- // are run. So, register a cleanup callback for it with atexit before
- // everything else.
- LIBC_NAMESPACE::cleanup_tls(tls.addr, tls.size);
LIBC_NAMESPACE::exit(retval);
}
diff --git a/libc/startup/linux/x86_64/CMakeLists.txt b/libc/startup/linux/x86_64/CMakeLists.txt
index 076c0c3e444f5..aac5a0626a176 100644
--- a/libc/startup/linux/x86_64/CMakeLists.txt
+++ b/libc/startup/linux/x86_64/CMakeLists.txt
@@ -15,6 +15,7 @@ add_startup_object(
libc.src.string.memory_utils.inline_memcpy
libc.src.unistd.environ
COMPILE_OPTIONS
+ -fno-stack-protector
-fno-omit-frame-pointer
-ffreestanding # To avoid compiler warnings about calling the main function.
-fno-builtin
diff --git a/libc/startup/linux/x86_64/start.cpp b/libc/startup/linux/x86_64/start.cpp
index bc1b4f0487f31..96031723e009a 100644
--- a/libc/startup/linux/x86_64/start.cpp
+++ b/libc/startup/linux/x86_64/start.cpp
@@ -222,7 +222,7 @@ extern "C" void _start() {
app.tls.align = phdr->p_align;
}
- LIBC_NAMESPACE::TLSDescriptor tls;
+ static LIBC_NAMESPACE::TLSDescriptor tls;
LIBC_NAMESPACE::init_tls(tls);
if (tls.size != 0 && !LIBC_NAMESPACE::set_thread_ptr(tls.tp))
LIBC_NAMESPACE::syscall_impl<long>(SYS_exit, 1);
@@ -231,6 +231,8 @@ extern "C" void _start() {
LIBC_NAMESPACE::main_thread_attrib.atexit_callback_mgr =
LIBC_NAMESPACE::internal::get_thread_atexit_callback_mgr();
+ LIBC_NAMESPACE::atexit(
+ []() { LIBC_NAMESPACE::cleanup_tls(tls.tp, tls.size); });
// We want the fini array callbacks to be run after other atexit
// callbacks are run. So, we register them before running the init
// array callbacks as they can potentially register their own atexit
@@ -246,9 +248,5 @@ extern "C" void _start() {
reinterpret_cast<char **>(app.args->argv),
reinterpret_cast<char **>(env_ptr));
- // TODO: TLS cleanup should be done after all other atexit callbacks
- // are run. So, register a cleanup callback for it with atexit before
- // everything else.
- LIBC_NAMESPACE::cleanup_tls(tls.addr, tls.size);
LIBC_NAMESPACE::exit(retval);
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/74567
More information about the libc-commits
mailing list