[libc] move __stack_chk_fail to src/ from startup/ #75863

nickdesaulniers · 2023-12-18T21:54:33Z

__stack_chk_fail should be provided by libc.a, not startup files.

Add __stack_chk_fail to existing linux and arm entrypoints. On Windows (when
not targeting MinGW), it seems that the corresponding function identifier is
__security_check_cookie, so no entrypoint is added for Windows. Baremetal
targets also ought to be compileable with -fstack-protector*

There is no common header for this prototype, since calls to __stack_chk_fail
are meant to be inserted by the compiler upon function return when compiled
-fstack-protector*.

__stack_chk_fail should be provided by libc.a, not startup files. Add __stack_chk_fail to existing linux and arm entrypoints. On Windows (when not targeting MinGW), it seems that the corresponding function identifier is __security_check_cookie, so no entrypoint is added for Windows. Baremetal targets also ought to be compileable with `-fstack-protector*` There is no common header for this prototype, since calls to __stack_chk_fail are meant to be inserted by the compiler upon function return when compiled `-fstack-protector*`.

llvmbot · 2023-12-18T21:55:03Z

@llvm/pr-subscribers-libc

Author: Nick Desaulniers (nickdesaulniers)

Changes

__stack_chk_fail should be provided by libc.a, not startup files.

Add __stack_chk_fail to existing linux and arm entrypoints. On Windows (when
not targeting MinGW), it seems that the corresponding function identifier is
__security_check_cookie, so no entrypoint is added for Windows. Baremetal
targets also ought to be compileable with -fstack-protector*

There is no common header for this prototype, since calls to __stack_chk_fail
are meant to be inserted by the compiler upon function return when compiled
-fstack-protector*.

Full diff: https://github.com/llvm/llvm-project/pull/75863.diff

11 Files Affected:

(modified) libc/config/baremetal/arm/entrypoints.txt (+4-1)
(modified) libc/config/baremetal/riscv/entrypoints.txt (+3)
(modified) libc/config/linux/aarch64/entrypoints.txt (+5-2)
(modified) libc/config/linux/arm/entrypoints.txt (+2-2)
(modified) libc/config/linux/riscv/entrypoints.txt (+3)
(modified) libc/config/linux/x86_64/entrypoints.txt (+3)
(modified) libc/src/CMakeLists.txt (+2-1)
(added) libc/src/compiler/CMakeLists.txt (+18)
(added) libc/src/compiler/generic/CMakeLists.txt (+11)
(added) libc/src/compiler/generic/__stack_chk_fail.cpp (+21)
(modified) libc/startup/linux/x86_64/start.cpp (-5)

diff --git a/libc/config/baremetal/arm/entrypoints.txt b/libc/config/baremetal/arm/entrypoints.txt
index a88b7aa749e565..a0779c41652aeb 100644
--- a/libc/config/baremetal/arm/entrypoints.txt
+++ b/libc/config/baremetal/arm/entrypoints.txt
@@ -17,6 +17,9 @@ set(TARGET_LIBC_ENTRYPOINTS
     libc.src.ctype.tolower
     libc.src.ctype.toupper
 
+    # compiler entrypoints (no corresponding header)
+    libc.src.compiler.__stack_chk_fail
+
     # errno.h entrypoints
     libc.src.errno.errno
 
@@ -69,7 +72,7 @@ set(TARGET_LIBC_ENTRYPOINTS
     libc.src.stdio.snprintf
     libc.src.stdio.vsprintf
     libc.src.stdio.vsnprintf
- 
+
     # stdlib.h entrypoints
     libc.src.stdlib.abs
     libc.src.stdlib.atoi
diff --git a/libc/config/baremetal/riscv/entrypoints.txt b/libc/config/baremetal/riscv/entrypoints.txt
index 3b7ca513eb0965..3e15cc8901bddf 100644
--- a/libc/config/baremetal/riscv/entrypoints.txt
+++ b/libc/config/baremetal/riscv/entrypoints.txt
@@ -17,6 +17,9 @@ set(TARGET_LIBC_ENTRYPOINTS
     libc.src.ctype.tolower
     libc.src.ctype.toupper
 
+    # compiler entrypoints (no corresponding header)
+    libc.src.compiler.__stack_chk_fail
+
     # errno.h entrypoints
     libc.src.errno.errno
 
diff --git a/libc/config/linux/aarch64/entrypoints.txt b/libc/config/linux/aarch64/entrypoints.txt
index 60e0e2b29aed36..77c9a50b8b7e5d 100644
--- a/libc/config/linux/aarch64/entrypoints.txt
+++ b/libc/config/linux/aarch64/entrypoints.txt
@@ -16,7 +16,7 @@ set(TARGET_LIBC_ENTRYPOINTS
     libc.src.ctype.toascii
     libc.src.ctype.tolower
     libc.src.ctype.toupper
-    
+
     # errno.h entrypoints
     libc.src.errno.errno
 
@@ -239,7 +239,7 @@ set(TARGET_LIBM_ENTRYPOINTS
     libc.src.math.asinf
     libc.src.math.asinhf
     libc.src.math.atanf
-    libc.src.math.atanhf    
+    libc.src.math.atanhf
     libc.src.math.copysign
     libc.src.math.copysignf
     libc.src.math.copysignl
@@ -353,6 +353,9 @@ set(TARGET_LIBM_ENTRYPOINTS
 
 if(LLVM_LIBC_FULL_BUILD)
   list(APPEND TARGET_LIBC_ENTRYPOINTS
+    # compiler entrypoints (no corresponding header)
+    libc.src.compiler.__stack_chk_fail
+
     # network.h entrypoints
     libc.src.network.htonl
     libc.src.network.htons
diff --git a/libc/config/linux/arm/entrypoints.txt b/libc/config/linux/arm/entrypoints.txt
index 123c7e33377ad1..274d5aa5a0057d 100644
--- a/libc/config/linux/arm/entrypoints.txt
+++ b/libc/config/linux/arm/entrypoints.txt
@@ -66,7 +66,7 @@ set(TARGET_LIBC_ENTRYPOINTS
     libc.src.inttypes.imaxdiv
     libc.src.inttypes.strtoimax
     libc.src.inttypes.strtoumax
- 
+
     # stdlib.h entrypoints
     libc.src.stdlib.abs
     libc.src.stdlib.atoi
@@ -88,7 +88,7 @@ set(TARGET_LIBC_ENTRYPOINTS
     libc.src.stdlib.strtoll
     libc.src.stdlib.strtoul
     libc.src.stdlib.strtoull
-    
+
     # sys/mman.h entrypoints
     libc.src.sys.mman.mmap
     libc.src.sys.mman.munmap
diff --git a/libc/config/linux/riscv/entrypoints.txt b/libc/config/linux/riscv/entrypoints.txt
index 948708e35f45d2..e389936ffca1ef 100644
--- a/libc/config/linux/riscv/entrypoints.txt
+++ b/libc/config/linux/riscv/entrypoints.txt
@@ -362,6 +362,9 @@ set(TARGET_LIBM_ENTRYPOINTS
 
 if(LLVM_LIBC_FULL_BUILD)
   list(APPEND TARGET_LIBC_ENTRYPOINTS
+    # compiler entrypoints (no corresponding header)
+    libc.src.compiler.__stack_chk_fail
+
     # assert.h entrypoints
     libc.src.assert.__assert_fail
 
diff --git a/libc/config/linux/x86_64/entrypoints.txt b/libc/config/linux/x86_64/entrypoints.txt
index 1c93063e25e90c..3adcd57d0c0849 100644
--- a/libc/config/linux/x86_64/entrypoints.txt
+++ b/libc/config/linux/x86_64/entrypoints.txt
@@ -375,6 +375,9 @@ if(LLVM_LIBC_FULL_BUILD)
     # assert.h entrypoints
     libc.src.assert.__assert_fail
 
+    # compiler entrypoints (no corresponding header)
+    libc.src.compiler.__stack_chk_fail
+
     # dirent.h entrypoints
     libc.src.dirent.closedir
     libc.src.dirent.dirfd
diff --git a/libc/src/CMakeLists.txt b/libc/src/CMakeLists.txt
index 3ab62a4f667d26..492f9c5bd50f9b 100644
--- a/libc/src/CMakeLists.txt
+++ b/libc/src/CMakeLists.txt
@@ -29,10 +29,11 @@ if(NOT LLVM_LIBC_FULL_BUILD)
 endif()
 
 add_subdirectory(assert)
+add_subdirectory(compiler)
 add_subdirectory(network)
+add_subdirectory(search)
 add_subdirectory(setjmp)
 add_subdirectory(signal)
 add_subdirectory(spawn)
 add_subdirectory(threads)
 add_subdirectory(time)
-add_subdirectory(search)
diff --git a/libc/src/compiler/CMakeLists.txt b/libc/src/compiler/CMakeLists.txt
new file mode 100644
index 00000000000000..aa59d84e08d146
--- /dev/null
+++ b/libc/src/compiler/CMakeLists.txt
@@ -0,0 +1,18 @@
+if(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${LIBC_TARGET_OS})
+  add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/${LIBC_TARGET_OS})
+else()
+  add_subdirectory(generic)
+endif()
+
+if(TARGET libc.src.compiler.${LIBC_TARGET_OS}.__stack_chk_fail)
+  set(stack_chk_fail_dep libc.src.compiler.${LIBC_TARGET_OS}.__stack_chk_fail)
+else()
+  set(stack_chk_fail_dep libc.src.compiler.generic.__stack_chk_fail)
+endif()
+
+add_entrypoint_object(
+  __stack_chk_fail
+  ALIAS
+  DEPENDS
+    ${stack_chk_fail_dep}
+)
diff --git a/libc/src/compiler/generic/CMakeLists.txt b/libc/src/compiler/generic/CMakeLists.txt
new file mode 100644
index 00000000000000..0d869b72a12cf5
--- /dev/null
+++ b/libc/src/compiler/generic/CMakeLists.txt
@@ -0,0 +1,11 @@
+add_entrypoint_object(
+  __stack_chk_fail
+  SRCS
+    __stack_chk_fail.cpp
+  HDRS
+    ../__stack_chk_fail.h
+  DEPENDS
+    libc.include.assert
+    libc.src.__support.OSUtil.osutil
+    libc.src.stdlib.abort
+)
diff --git a/libc/src/compiler/generic/__stack_chk_fail.cpp b/libc/src/compiler/generic/__stack_chk_fail.cpp
new file mode 100644
index 00000000000000..076ed351e5fe0c
--- /dev/null
+++ b/libc/src/compiler/generic/__stack_chk_fail.cpp
@@ -0,0 +1,21 @@
+//===-- Implementation of __stack_chk_fail --------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/compiler/__stack_chk_fail.h"
+#include "src/__support/OSUtil/io.h"
+#include "src/stdlib/abort.h"
+
+namespace LIBC_NAMESPACE {
+
+LLVM_LIBC_FUNCTION(void, __stack_chk_fail, (void)) {
+  LIBC_NAMESPACE::write_to_stderr("stack smashing detected");
+  LIBC_NAMESPACE::abort();
+}
+
+} // namespace LIBC_NAMESPACE
+
diff --git a/libc/startup/linux/x86_64/start.cpp b/libc/startup/linux/x86_64/start.cpp
index 496105dfd0b43a..c7d931d763a46f 100644
--- a/libc/startup/linux/x86_64/start.cpp
+++ b/libc/startup/linux/x86_64/start.cpp
@@ -25,11 +25,6 @@
 
 extern "C" int main(int, char **, char **);
 
-extern "C" void __stack_chk_fail() {
-  LIBC_NAMESPACE::write_to_stderr("stack smashing detected");
-  LIBC_NAMESPACE::abort();
-}
-
 namespace LIBC_NAMESPACE {
 
 #ifdef SYS_mmap2

github-actions · 2023-12-18T21:57:02Z

✅ With the latest revision this PR passed the C/C++ code formatter.

libc/src/compiler/generic/CMakeLists.txt

nickdesaulniers · 2023-12-18T22:13:50Z

I talked with @michaelrj-google about adding a EXPECT_DEATH for this, too.

SchrodingerZhu · 2023-12-18T22:31:24Z

As a side note, I was to move __stack_chk_fail to a separate file inside startup once #75413 merged. But it does make more sense to move it to src.

michaelrj-google

LGTM, though if possible it would be nice to add a test to check the actual stack check mechanism (i.e. overwriting the stack cookie intentionally to cause a crash)

nickdesaulniers · 2023-12-19T17:24:48Z

LGTM, though if possible it would be nice to add a test to check the actual stack check mechanism (i.e. overwriting the stack cookie intentionally to cause a crash)

@michaelrj-google done in f5e86b3

Prabhuk

LGTM

michaelrj-google

LGTM

libc/test/src/compiler/stack_chk_guard_test.cpp

nickdesaulniers · 2023-12-19T19:13:08Z

follow up fix for arm: #75962

llvmbot added the libc label Dec 18, 2023

nickdesaulniers requested review from jhuber6, lntue and michaelrj-google December 18, 2023 21:54

git format

932af76

nickdesaulniers requested a review from Prabhuk December 18, 2023 22:03

michaelrj-google reviewed Dec 18, 2023

View reviewed changes

libc/src/compiler/generic/CMakeLists.txt Show resolved Hide resolved

add header

7c26cda

nickdesaulniers marked this pull request as draft December 18, 2023 22:13

add death test

35d0759

nickdesaulniers marked this pull request as ready for review December 18, 2023 22:29

michaelrj-google approved these changes Dec 19, 2023

View reviewed changes

add explicit test for stack smashing

f5e86b3

git clang-format HEAD~

e4acf15

Prabhuk approved these changes Dec 19, 2023

View reviewed changes

michaelrj-google approved these changes Dec 19, 2023

View reviewed changes

libc/test/src/compiler/stack_chk_guard_test.cpp Outdated Show resolved Hide resolved

fix comment in test

13df494

nickdesaulniers merged commit 315a5cc into llvm:main Dec 19, 2023

nickdesaulniers deleted the stack_chk_fail branch December 19, 2023 19:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[libc] move __stack_chk_fail to src/ from startup/ #75863

[libc] move __stack_chk_fail to src/ from startup/ #75863

Uh oh!

nickdesaulniers commented Dec 18, 2023

Uh oh!

llvmbot commented Dec 18, 2023

Uh oh!

github-actions bot commented Dec 18, 2023 •

edited

Loading

Uh oh!

Uh oh!

nickdesaulniers commented Dec 18, 2023

Uh oh!

SchrodingerZhu commented Dec 18, 2023 •

edited

Loading

Uh oh!

michaelrj-google left a comment

Uh oh!

nickdesaulniers commented Dec 19, 2023

Uh oh!

Prabhuk left a comment

Uh oh!

michaelrj-google left a comment

Uh oh!

Uh oh!

nickdesaulniers commented Dec 19, 2023

Uh oh!

Uh oh!

[libc] move __stack_chk_fail to src/ from startup/ #75863

[libc] move __stack_chk_fail to src/ from startup/ #75863

Uh oh!

Conversation

nickdesaulniers commented Dec 18, 2023

Uh oh!

llvmbot commented Dec 18, 2023

Uh oh!

github-actions bot commented Dec 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

nickdesaulniers commented Dec 18, 2023

Uh oh!

SchrodingerZhu commented Dec 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

michaelrj-google left a comment

Choose a reason for hiding this comment

Uh oh!

nickdesaulniers commented Dec 19, 2023

Uh oh!

Prabhuk left a comment

Choose a reason for hiding this comment

Uh oh!

michaelrj-google left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nickdesaulniers commented Dec 19, 2023

Uh oh!

Uh oh!

github-actions bot commented Dec 18, 2023 •

edited

Loading

SchrodingerZhu commented Dec 18, 2023 •

edited

Loading