[libc-commits] [libc] 1896ee3 - [libc] Fix undefined behavior for nan functions. (#106468)
via libc-commits
libc-commits at lists.llvm.org
Wed Sep 11 11:13:36 PDT 2024
Author: lntue
Date: 2024-09-11T14:13:31-04:00
New Revision: 1896ee38898a73ea9c2894e848884c8999884ab1
URL: https://github.com/llvm/llvm-project/commit/1896ee38898a73ea9c2894e848884c8999884ab1
DIFF: https://github.com/llvm/llvm-project/commit/1896ee38898a73ea9c2894e848884c8999884ab1.diff
LOG: [libc] Fix undefined behavior for nan functions. (#106468)
Currently the nan* functions use nullptr dereferencing to crash with
SIGSEGV if the input is nullptr. Both `nan(nullptr)` and `nullptr`
dereferencing are undefined behaviors according to the C standard.
Employing `nullptr` dereference in the `nan` function implementation is
ok if users only linked against the pre-built library, but it might be
completely removed by the compilers' optimizations if it is built from
source together with the users' code.
See for instance: https://godbolt.org/z/fd8KcM9bx
This PR uses volatile load to prevent the undefined behavior if libc is
built without sanitizers, and leave the current undefined behavior if
libc is built with sanitizers, so that the undefined behavior can be
caught for users' codes.
Added:
libc/src/__support/macros/null_check.h
Modified:
libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
libc/config/config.json
libc/docs/configure.rst
libc/src/__support/CMakeLists.txt
libc/src/__support/macros/CMakeLists.txt
libc/src/__support/macros/sanitizer.h
libc/src/__support/str_to_float.h
libc/test/src/compiler/CMakeLists.txt
libc/test/src/compiler/stack_chk_guard_test.cpp
libc/test/src/math/smoke/CMakeLists.txt
libc/test/src/math/smoke/nan_test.cpp
libc/test/src/math/smoke/nanf128_test.cpp
libc/test/src/math/smoke/nanf16_test.cpp
libc/test/src/math/smoke/nanf_test.cpp
libc/test/src/math/smoke/nanl_test.cpp
utils/bazel/llvm-project-overlay/libc/BUILD.bazel
Removed:
################################################################################
diff --git a/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake b/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
index 45dfe3e63302bf..8643c9bb48ad41 100644
--- a/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
+++ b/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
@@ -75,6 +75,10 @@ function(_get_compile_options_from_config output_var)
list(APPEND config_options "-DLIBC_TYPES_TIME_T_IS_32_BIT")
endif()
+ if(LIBC_ADD_NULL_CHECKS)
+ list(APPEND config_options "-DLIBC_ADD_NULL_CHECKS")
+ endif()
+
set(${output_var} ${config_options} PARENT_SCOPE)
endfunction(_get_compile_options_from_config)
diff --git a/libc/config/config.json b/libc/config/config.json
index 2e72c0a3fd1d69..7dfbb560a36db3 100644
--- a/libc/config/config.json
+++ b/libc/config/config.json
@@ -94,5 +94,11 @@
"value": false,
"doc": "Force the size of time_t to 64 bits, even on platforms where compatibility considerations would otherwise make it 32-bit."
}
+ },
+ "general": {
+ "LIBC_ADD_NULL_CHECKS": {
+ "value": true,
+ "doc": "Add nullptr checks in the library's implementations to some functions for which passing nullptr is undefined behavior."
+ }
}
}
diff --git a/libc/docs/configure.rst b/libc/docs/configure.rst
index 54ca5d55d7b243..86875d4c975c01 100644
--- a/libc/docs/configure.rst
+++ b/libc/docs/configure.rst
@@ -30,6 +30,8 @@ to learn about the defaults for your platform and target.
- ``LIBC_CONF_KEEP_FRAME_POINTER``: Keep frame pointer in functions for better debugging experience.
* **"errno" options**
- ``LIBC_CONF_ERRNO_MODE``: The implementation used for errno, acceptable values are LIBC_ERRNO_MODE_DEFAULT, LIBC_ERRNO_MODE_UNDEFINED, LIBC_ERRNO_MODE_THREAD_LOCAL, LIBC_ERRNO_MODE_SHARED, LIBC_ERRNO_MODE_EXTERNAL, and LIBC_ERRNO_MODE_SYSTEM.
+* **"general" options**
+ - ``LIBC_ADD_NULL_CHECKS``: Add nullptr checks in the library's implementations to some functions for which passing nullptr is undefined behavior.
* **"math" options**
- ``LIBC_CONF_MATH_OPTIMIZATIONS``: Configures optimizations for math functions. Values accepted are LIBC_MATH_SKIP_ACCURATE_PASS, LIBC_MATH_SMALL_TABLES, LIBC_MATH_NO_ERRNO, LIBC_MATH_NO_EXCEPT, and LIBC_MATH_FAST.
* **"printf" options**
diff --git a/libc/src/__support/CMakeLists.txt b/libc/src/__support/CMakeLists.txt
index 9bd1e29081a801..0302ad64f8b5df 100644
--- a/libc/src/__support/CMakeLists.txt
+++ b/libc/src/__support/CMakeLists.txt
@@ -192,6 +192,9 @@ add_header_library(
libc.src.__support.CPP.optional
libc.src.__support.FPUtil.fp_bits
libc.src.__support.FPUtil.rounding_mode
+ libc.src.__support.macros.config
+ libc.src.__support.macros.null_check
+ libc.src.__support.macros.optimization
libc.src.errno.errno
)
diff --git a/libc/src/__support/macros/CMakeLists.txt b/libc/src/__support/macros/CMakeLists.txt
index bcd47c3651bf5d..99d4f640f283a4 100644
--- a/libc/src/__support/macros/CMakeLists.txt
+++ b/libc/src/__support/macros/CMakeLists.txt
@@ -27,3 +27,13 @@ add_header_library(
DEPENDS
libc.src.__support.macros.properties.compiler
)
+
+add_header_library(
+ null_check
+ HDRS
+ null_check.h
+ DEPENDS
+ .config
+ .optimization
+ .sanitizer
+)
diff --git a/libc/src/__support/macros/null_check.h b/libc/src/__support/macros/null_check.h
new file mode 100644
index 00000000000000..400f7d809db4fa
--- /dev/null
+++ b/libc/src/__support/macros/null_check.h
@@ -0,0 +1,33 @@
+//===-- Safe nullptr check --------------------------------------*- C++ -*-===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC___SUPPORT_MACROS_NULL_CHECK_H
+#define LLVM_LIBC_SRC___SUPPORT_MACROS_NULL_CHECK_H
+
+#include "src/__support/macros/config.h"
+#include "src/__support/macros/optimization.h"
+#include "src/__support/macros/sanitizer.h"
+
+#if defined(LIBC_ADD_NULL_CHECKS) && !defined(LIBC_HAS_SANITIZER)
+// Use volatile to prevent undefined behavior of dereferencing nullptr.
+// Intentionally crashing with SIGSEGV.
+#define LIBC_CRASH_ON_NULLPTR(PTR) \
+ do { \
+ if (LIBC_UNLIKELY(PTR == nullptr)) { \
+ volatile auto *crashing = PTR; \
+ [[maybe_unused]] volatile auto crash = *crashing; \
+ __builtin_trap(); \
+ } \
+ } while (0)
+#else
+#define LIBC_CRASH_ON_NULLPTR(ptr) \
+ do { \
+ } while (0)
+#endif
+
+#endif // LLVM_LIBC_SRC___SUPPORT_MACROS_NULL_CHECK_H
diff --git a/libc/src/__support/macros/sanitizer.h b/libc/src/__support/macros/sanitizer.h
index c4f8b5bce39755..c20412e0f8b69f 100644
--- a/libc/src/__support/macros/sanitizer.h
+++ b/libc/src/__support/macros/sanitizer.h
@@ -15,7 +15,25 @@
// Functions to unpoison memory
//-----------------------------------------------------------------------------
+#if LIBC_HAS_FEATURE(address_sanitizer) || defined(__SANITIZE_ADDRESS__)
+#define LIBC_HAS_ADDRESS_SANITIZER
+#endif
+
#if LIBC_HAS_FEATURE(memory_sanitizer)
+#define LIBC_HAS_MEMORY_SANITIZER
+#endif
+
+#if LIBC_HAS_FEATURE(undefined_behavior_sanitizer)
+#define LIBC_HAS_UNDEFINED_BEHAVIOR_SANITIZER
+#endif
+
+#if defined(LIBC_HAS_ADDRESS_SANITIZER) || \
+ defined(LIBC_HAS_MEMORY_SANITIZER) || \
+ defined(LIBC_HAS_UNDEFINED_BEHAVIOR_SANITIZER)
+#define LIBC_HAS_SANITIZER
+#endif
+
+#ifdef LIBC_HAS_MEMORY_SANITIZER
// Only perform MSAN unpoison in non-constexpr context.
#include <sanitizer/msan_interface.h>
#define MSAN_UNPOISON(addr, size) \
@@ -27,8 +45,7 @@
#define MSAN_UNPOISON(ptr, size)
#endif
-#if LIBC_HAS_FEATURE(address_sanitizer)
-#define LIBC_HAVE_ADDRESS_SANITIZER
+#ifdef LIBC_HAS_ADDRESS_SANITIZER
#include <sanitizer/asan_interface.h>
#define ASAN_POISON_MEMORY_REGION(addr, size) \
__asan_poison_memory_region((addr), (size))
diff --git a/libc/src/__support/str_to_float.h b/libc/src/__support/str_to_float.h
index ffd6ebf27c7726..a452b3a55fdeb4 100644
--- a/libc/src/__support/str_to_float.h
+++ b/libc/src/__support/str_to_float.h
@@ -20,6 +20,8 @@
#include "src/__support/detailed_powers_of_ten.h"
#include "src/__support/high_precision_decimal.h"
#include "src/__support/macros/config.h"
+#include "src/__support/macros/null_check.h"
+#include "src/__support/macros/optimization.h"
#include "src/__support/str_to_integer.h"
#include "src/__support/str_to_num_result.h"
#include "src/__support/uint128.h"
@@ -1208,6 +1210,8 @@ template <class T> LIBC_INLINE StrToNumResult<T> strtonan(const char *arg) {
using FPBits = typename fputil::FPBits<T>;
using StorageType = typename FPBits::StorageType;
+ LIBC_CRASH_ON_NULLPTR(arg);
+
FPBits result;
int error = 0;
StorageType nan_mantissa = 0;
diff --git a/libc/test/src/compiler/CMakeLists.txt b/libc/test/src/compiler/CMakeLists.txt
index 65a9acceb6f7f1..a45fa8c55e5128 100644
--- a/libc/test/src/compiler/CMakeLists.txt
+++ b/libc/test/src/compiler/CMakeLists.txt
@@ -7,6 +7,7 @@ add_libc_unittest(
SRCS
stack_chk_guard_test.cpp
DEPENDS
+ libc.hdr.signal_macros
libc.src.__support.macros.sanitizer
libc.src.compiler.__stack_chk_fail
libc.src.string.memset
diff --git a/libc/test/src/compiler/stack_chk_guard_test.cpp b/libc/test/src/compiler/stack_chk_guard_test.cpp
index 6b71e155fa3e4d..4ec8398c9fc95d 100644
--- a/libc/test/src/compiler/stack_chk_guard_test.cpp
+++ b/libc/test/src/compiler/stack_chk_guard_test.cpp
@@ -6,7 +6,7 @@
//
//===----------------------------------------------------------------------===//
-#include "include/llvm-libc-macros/signal-macros.h"
+#include "hdr/signal_macros.h"
#include "src/__support/macros/sanitizer.h"
#include "src/compiler/__stack_chk_fail.h"
#include "src/string/memset.h"
@@ -18,7 +18,7 @@ TEST(LlvmLibcStackChkFail, Death) {
// Disable the test when asan is enabled so that it doesn't immediately fail
// after the memset, but before the stack canary is re-checked.
-#ifndef LIBC_HAVE_ADDRESS_SANITIZER
+#ifndef LIBC_HAS_ADDRESS_SANITIZER
TEST(LlvmLibcStackChkFail, Smash) {
EXPECT_DEATH(
[] {
@@ -27,4 +27,4 @@ TEST(LlvmLibcStackChkFail, Smash) {
},
WITH_SIGNAL(SIGABRT));
}
-#endif // LIBC_HAVE_ADDRESS_SANITIZER
+#endif // LIBC_HAS_ADDRESS_SANITIZER
diff --git a/libc/test/src/math/smoke/CMakeLists.txt b/libc/test/src/math/smoke/CMakeLists.txt
index 7271e933b9311d..e943d98256a97b 100644
--- a/libc/test/src/math/smoke/CMakeLists.txt
+++ b/libc/test/src/math/smoke/CMakeLists.txt
@@ -2895,9 +2895,10 @@ add_fp_unittest(
SRCS
nanf_test.cpp
DEPENDS
- libc.include.signal
+ libc.hdr.signal_macros
libc.src.math.nanf
libc.src.__support.FPUtil.fp_bits
+ libc.src.__support.macros.sanitizer
# FIXME: The nan tests currently have death tests, which aren't supported for
# hermetic tests.
UNIT_TEST_ONLY
@@ -2910,9 +2911,10 @@ add_fp_unittest(
SRCS
nan_test.cpp
DEPENDS
- libc.include.signal
+ libc.hdr.signal_macros
libc.src.math.nan
libc.src.__support.FPUtil.fp_bits
+ libc.src.__support.macros.sanitizer
# FIXME: The nan tests currently have death tests, which aren't supported for
# hermetic tests.
UNIT_TEST_ONLY
@@ -2925,9 +2927,10 @@ add_fp_unittest(
SRCS
nanl_test.cpp
DEPENDS
- libc.include.signal
+ libc.hdr.signal_macros
libc.src.math.nanl
libc.src.__support.FPUtil.fp_bits
+ libc.src.__support.macros.sanitizer
# FIXME: The nan tests currently have death tests, which aren't supported for
# hermetic tests.
UNIT_TEST_ONLY
@@ -2940,7 +2943,7 @@ add_fp_unittest(
SRCS
nanf16_test.cpp
DEPENDS
- libc.include.signal
+ libc.hdr.signal_macros
libc.src.math.nanf16
libc.src.__support.FPUtil.fp_bits
libc.src.__support.macros.sanitizer
@@ -2956,9 +2959,10 @@ add_fp_unittest(
SRCS
nanf128_test.cpp
DEPENDS
- libc.include.signal
+ libc.hdr.signal_macros
libc.src.math.nanf128
libc.src.__support.FPUtil.fp_bits
+ libc.src.__support.macros.sanitizer
# FIXME: The nan tests currently have death tests, which aren't supported for
# hermetic tests.
UNIT_TEST_ONLY
diff --git a/libc/test/src/math/smoke/nan_test.cpp b/libc/test/src/math/smoke/nan_test.cpp
index 68c844181a1946..46b9e9aa9563ab 100644
--- a/libc/test/src/math/smoke/nan_test.cpp
+++ b/libc/test/src/math/smoke/nan_test.cpp
@@ -6,12 +6,13 @@
//
//===----------------------------------------------------------------------===//
+#include "hdr/signal_macros.h"
#include "src/__support/FPUtil/FPBits.h"
+#include "src/__support/macros/sanitizer.h"
#include "src/math/nan.h"
#include "test/UnitTest/FEnvSafeTest.h"
#include "test/UnitTest/FPMatcher.h"
#include "test/UnitTest/Test.h"
-#include <signal.h>
class LlvmLibcNanTest : public LIBC_NAMESPACE::testing::FEnvSafeTest {
public:
@@ -43,8 +44,8 @@ TEST_F(LlvmLibcNanTest, RandomString) {
run_test("123 ", 0x7ff8000000000000);
}
-#if !defined(LIBC_HAVE_ADDRESS_SANITIZER) && defined(LIBC_TARGET_OS_IS_LINUX)
+#if !defined(LIBC_HAS_ADDRESS_SANITIZER) && defined(LIBC_TARGET_OS_IS_LINUX)
TEST_F(LlvmLibcNanTest, InvalidInput) {
EXPECT_DEATH([] { LIBC_NAMESPACE::nan(nullptr); }, WITH_SIGNAL(SIGSEGV));
}
-#endif // LIBC_HAVE_ADDRESS_SANITIZER
+#endif // LIBC_HAS_ADDRESS_SANITIZER
diff --git a/libc/test/src/math/smoke/nanf128_test.cpp b/libc/test/src/math/smoke/nanf128_test.cpp
index 015cc31e4be237..25dd2ef1d5b1ca 100644
--- a/libc/test/src/math/smoke/nanf128_test.cpp
+++ b/libc/test/src/math/smoke/nanf128_test.cpp
@@ -6,7 +6,9 @@
//
//===----------------------------------------------------------------------===//
+#include "hdr/signal_macros.h"
#include "src/__support/FPUtil/FPBits.h"
+#include "src/__support/macros/sanitizer.h"
#include "src/__support/uint128.h"
#include "src/math/nanf128.h"
#include "test/UnitTest/FEnvSafeTest.h"
@@ -53,9 +55,8 @@ TEST_F(LlvmLibcNanf128Test, RandomString) {
QUIET_NAN);
}
-#if !defined(LIBC_HAVE_ADDRESS_SANITIZER) && defined(LIBC_TARGET_OS_IS_LINUX)
-#include <signal.h>
+#if !defined(LIBC_HAS_ADDRESS_SANITIZER) && defined(LIBC_TARGET_OS_IS_LINUX)
TEST_F(LlvmLibcNanf128Test, InvalidInput) {
EXPECT_DEATH([] { LIBC_NAMESPACE::nanf128(nullptr); }, WITH_SIGNAL(SIGSEGV));
}
-#endif // LIBC_HAVE_ADDRESS_SANITIZER
+#endif // LIBC_HAS_ADDRESS_SANITIZER
diff --git a/libc/test/src/math/smoke/nanf16_test.cpp b/libc/test/src/math/smoke/nanf16_test.cpp
index 81b844bf6bb59c..ec640a3b9eef92 100644
--- a/libc/test/src/math/smoke/nanf16_test.cpp
+++ b/libc/test/src/math/smoke/nanf16_test.cpp
@@ -6,6 +6,7 @@
//
//===----------------------------------------------------------------------===//
+#include "hdr/signal_macros.h"
#include "src/__support/FPUtil/FPBits.h"
#include "src/__support/macros/sanitizer.h"
#include "src/math/nanf16.h"
@@ -13,8 +14,6 @@
#include "test/UnitTest/FPMatcher.h"
#include "test/UnitTest/Test.h"
-#include <signal.h>
-
class LlvmLibcNanf16Test : public LIBC_NAMESPACE::testing::FEnvSafeTest {
public:
using StorageType = LIBC_NAMESPACE::fputil::FPBits<float16>::StorageType;
@@ -44,8 +43,8 @@ TEST_F(LlvmLibcNanf16Test, RandomString) {
run_test("123 ", 0x7e00);
}
-#if !defined(LIBC_HAVE_ADDRESS_SANITIZER) && defined(LIBC_TARGET_OS_IS_LINUX)
+#if !defined(LIBC_HAS_ADDRESS_SANITIZER) && defined(LIBC_TARGET_OS_IS_LINUX)
TEST_F(LlvmLibcNanf16Test, InvalidInput) {
EXPECT_DEATH([] { LIBC_NAMESPACE::nanf16(nullptr); }, WITH_SIGNAL(SIGSEGV));
}
-#endif // LIBC_HAVE_ADDRESS_SANITIZER
+#endif // LIBC_HAS_ADDRESS_SANITIZER
diff --git a/libc/test/src/math/smoke/nanf_test.cpp b/libc/test/src/math/smoke/nanf_test.cpp
index ff5823685225ce..dd3124ee9c5112 100644
--- a/libc/test/src/math/smoke/nanf_test.cpp
+++ b/libc/test/src/math/smoke/nanf_test.cpp
@@ -6,12 +6,13 @@
//
//===----------------------------------------------------------------------===//
+#include "hdr/signal_macros.h"
#include "src/__support/FPUtil/FPBits.h"
+#include "src/__support/macros/sanitizer.h"
#include "src/math/nanf.h"
#include "test/UnitTest/FEnvSafeTest.h"
#include "test/UnitTest/FPMatcher.h"
#include "test/UnitTest/Test.h"
-#include <signal.h>
class LlvmLibcNanfTest : public LIBC_NAMESPACE::testing::FEnvSafeTest {
public:
@@ -42,8 +43,8 @@ TEST_F(LlvmLibcNanfTest, RandomString) {
run_test("123 ", 0x7fc00000);
}
-#if !defined(LIBC_HAVE_ADDRESS_SANITIZER) && defined(LIBC_TARGET_OS_IS_LINUX)
+#if !defined(LIBC_HAS_ADDRESS_SANITIZER) && defined(LIBC_TARGET_OS_IS_LINUX)
TEST_F(LlvmLibcNanfTest, InvalidInput) {
EXPECT_DEATH([] { LIBC_NAMESPACE::nanf(nullptr); }, WITH_SIGNAL(SIGSEGV));
}
-#endif // LIBC_HAVE_ADDRESS_SANITIZER
+#endif // LIBC_HAS_ADDRESS_SANITIZER
diff --git a/libc/test/src/math/smoke/nanl_test.cpp b/libc/test/src/math/smoke/nanl_test.cpp
index de9af05100c10a..ef3f9c15dafd9f 100644
--- a/libc/test/src/math/smoke/nanl_test.cpp
+++ b/libc/test/src/math/smoke/nanl_test.cpp
@@ -6,12 +6,13 @@
//
//===----------------------------------------------------------------------===//
+#include "hdr/signal_macros.h"
#include "src/__support/FPUtil/FPBits.h"
+#include "src/__support/macros/sanitizer.h"
#include "src/math/nanl.h"
#include "test/UnitTest/FEnvSafeTest.h"
#include "test/UnitTest/FPMatcher.h"
#include "test/UnitTest/Test.h"
-#include <signal.h>
#if defined(LIBC_TYPES_LONG_DOUBLE_IS_FLOAT64)
#define SELECT_LONG_DOUBLE(val, _, __) val
@@ -70,8 +71,8 @@ TEST_F(LlvmLibcNanlTest, RandomString) {
run_test("123 ", expected);
}
-#if !defined(LIBC_HAVE_ADDRESS_SANITIZER) && defined(LIBC_TARGET_OS_IS_LINUX)
+#if !defined(LIBC_HAS_ADDRESS_SANITIZER) && defined(LIBC_TARGET_OS_IS_LINUX)
TEST_F(LlvmLibcNanlTest, InvalidInput) {
EXPECT_DEATH([] { LIBC_NAMESPACE::nanl(nullptr); }, WITH_SIGNAL(SIGSEGV));
}
-#endif // LIBC_HAVE_ADDRESS_SANITIZER
+#endif // LIBC_HAS_ADDRESS_SANITIZER
diff --git a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
index f3d3c745246af8..b86fcace5703c7 100644
--- a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
@@ -272,6 +272,16 @@ libc_support_library(
],
)
+libc_support_library(
+ name = "__support_macros_null_check",
+ hdrs = ["src/__support/macros/null_check.h"],
+ deps = [
+ ":__support_macros_config",
+ ":__support_macros_optimization",
+ ":__support_macros_sanitizer",
+ ],
+)
+
libc_support_library(
name = "__support_common",
hdrs = [
@@ -665,6 +675,7 @@ libc_support_library(
":__support_ctype_utils",
":__support_fputil_fp_bits",
":__support_fputil_rounding_mode",
+ ":__support_macros_null_check",
":__support_str_to_integer",
":__support_str_to_num_result",
":__support_uint128",
More information about the libc-commits
mailing list