Skip to content

Commit 1896ee3

Browse files
lntuelntue
authored
[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.
1 parent 8625eb0 commit 1896ee3

File tree

17 files changed

+124
-26
lines changed

17 files changed

+124
-26
lines changed

‎libc/cmake/modules/LLVMLibCCompileOptionRules.cmake

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,10 @@ function(_get_compile_options_from_config output_var)
7575
list(APPEND config_options "-DLIBC_TYPES_TIME_T_IS_32_BIT")
7676
endif()
7777

78+
if(LIBC_ADD_NULL_CHECKS)
79+
list(APPEND config_options "-DLIBC_ADD_NULL_CHECKS")
80+
endif()
81+
7882
set(${output_var} ${config_options} PARENT_SCOPE)
7983
endfunction(_get_compile_options_from_config)
8084

‎libc/config/config.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,5 +94,11 @@
9494
"value": false,
9595
"doc": "Force the size of time_t to 64 bits, even on platforms where compatibility considerations would otherwise make it 32-bit."
9696
}
97+
},
98+
"general": {
99+
"LIBC_ADD_NULL_CHECKS": {
100+
"value": true,
101+
"doc": "Add nullptr checks in the library's implementations to some functions for which passing nullptr is undefined behavior."
102+
}
97103
}
98104
}

‎libc/docs/configure.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ to learn about the defaults for your platform and target.
3030
- ``LIBC_CONF_KEEP_FRAME_POINTER``: Keep frame pointer in functions for better debugging experience.
3131
* **"errno" options**
3232
- ``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.
33+
* **"general" options**
34+
- ``LIBC_ADD_NULL_CHECKS``: Add nullptr checks in the library's implementations to some functions for which passing nullptr is undefined behavior.
3335
* **"math" options**
3436
- ``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.
3537
* **"printf" options**

‎libc/src/__support/CMakeLists.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,9 @@ add_header_library(
192192
libc.src.__support.CPP.optional
193193
libc.src.__support.FPUtil.fp_bits
194194
libc.src.__support.FPUtil.rounding_mode
195+
libc.src.__support.macros.config
196+
libc.src.__support.macros.null_check
197+
libc.src.__support.macros.optimization
195198
libc.src.errno.errno
196199
)
197200

‎libc/src/__support/macros/CMakeLists.txt

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,3 +27,13 @@ add_header_library(
2727
DEPENDS
2828
libc.src.__support.macros.properties.compiler
2929
)
30+
31+
add_header_library(
32+
null_check
33+
HDRS
34+
null_check.h
35+
DEPENDS
36+
.config
37+
.optimization
38+
.sanitizer
39+
)

‎libc/src/__support/macros/null_check.h

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
//===-- Safe nullptr check --------------------------------------*- C++ -*-===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#ifndef LLVM_LIBC_SRC___SUPPORT_MACROS_NULL_CHECK_H
10+
#define LLVM_LIBC_SRC___SUPPORT_MACROS_NULL_CHECK_H
11+
12+
#include "src/__support/macros/config.h"
13+
#include "src/__support/macros/optimization.h"
14+
#include "src/__support/macros/sanitizer.h"
15+
16+
#if defined(LIBC_ADD_NULL_CHECKS) && !defined(LIBC_HAS_SANITIZER)
17+
// Use volatile to prevent undefined behavior of dereferencing nullptr.
18+
// Intentionally crashing with SIGSEGV.
19+
#define LIBC_CRASH_ON_NULLPTR(PTR) \
20+
do { \
21+
if (LIBC_UNLIKELY(PTR == nullptr)) { \
22+
volatile auto *crashing = PTR; \
23+
[[maybe_unused]] volatile auto crash = *crashing; \
24+
__builtin_trap(); \
25+
} \
26+
} while (0)
27+
#else
28+
#define LIBC_CRASH_ON_NULLPTR(ptr) \
29+
do { \
30+
} while (0)
31+
#endif
32+
33+
#endif // LLVM_LIBC_SRC___SUPPORT_MACROS_NULL_CHECK_H

‎libc/src/__support/macros/sanitizer.h

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,25 @@
1515
// Functions to unpoison memory
1616
//-----------------------------------------------------------------------------
1717

18+
#if LIBC_HAS_FEATURE(address_sanitizer) || defined(__SANITIZE_ADDRESS__)
19+
#define LIBC_HAS_ADDRESS_SANITIZER
20+
#endif
21+
1822
#if LIBC_HAS_FEATURE(memory_sanitizer)
23+
#define LIBC_HAS_MEMORY_SANITIZER
24+
#endif
25+
26+
#if LIBC_HAS_FEATURE(undefined_behavior_sanitizer)
27+
#define LIBC_HAS_UNDEFINED_BEHAVIOR_SANITIZER
28+
#endif
29+
30+
#if defined(LIBC_HAS_ADDRESS_SANITIZER) || \
31+
defined(LIBC_HAS_MEMORY_SANITIZER) || \
32+
defined(LIBC_HAS_UNDEFINED_BEHAVIOR_SANITIZER)
33+
#define LIBC_HAS_SANITIZER
34+
#endif
35+
36+
#ifdef LIBC_HAS_MEMORY_SANITIZER
1937
// Only perform MSAN unpoison in non-constexpr context.
2038
#include <sanitizer/msan_interface.h>
2139
#define MSAN_UNPOISON(addr, size) \
@@ -27,8 +45,7 @@
2745
#define MSAN_UNPOISON(ptr, size)
2846
#endif
2947

30-
#if LIBC_HAS_FEATURE(address_sanitizer)
31-
#define LIBC_HAVE_ADDRESS_SANITIZER
48+
#ifdef LIBC_HAS_ADDRESS_SANITIZER
3249
#include <sanitizer/asan_interface.h>
3350
#define ASAN_POISON_MEMORY_REGION(addr, size) \
3451
__asan_poison_memory_region((addr), (size))

‎libc/src/__support/str_to_float.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
#include "src/__support/detailed_powers_of_ten.h"
2121
#include "src/__support/high_precision_decimal.h"
2222
#include "src/__support/macros/config.h"
23+
#include "src/__support/macros/null_check.h"
24+
#include "src/__support/macros/optimization.h"
2325
#include "src/__support/str_to_integer.h"
2426
#include "src/__support/str_to_num_result.h"
2527
#include "src/__support/uint128.h"
@@ -1208,6 +1210,8 @@ template <class T> LIBC_INLINE StrToNumResult<T> strtonan(const char *arg) {
12081210
using FPBits = typename fputil::FPBits<T>;
12091211
using StorageType = typename FPBits::StorageType;
12101212

1213+
LIBC_CRASH_ON_NULLPTR(arg);
1214+
12111215
FPBits result;
12121216
int error = 0;
12131217
StorageType nan_mantissa = 0;

‎libc/test/src/compiler/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ add_libc_unittest(
77
SRCS
88
stack_chk_guard_test.cpp
99
DEPENDS
10+
libc.hdr.signal_macros
1011
libc.src.__support.macros.sanitizer
1112
libc.src.compiler.__stack_chk_fail
1213
libc.src.string.memset

‎libc/test/src/compiler/stack_chk_guard_test.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
//
77
//===----------------------------------------------------------------------===//
88

9-
#include "include/llvm-libc-macros/signal-macros.h"
9+
#include "hdr/signal_macros.h"
1010
#include "src/__support/macros/sanitizer.h"
1111
#include "src/compiler/__stack_chk_fail.h"
1212
#include "src/string/memset.h"
@@ -18,7 +18,7 @@ TEST(LlvmLibcStackChkFail, Death) {
1818

1919
// Disable the test when asan is enabled so that it doesn't immediately fail
2020
// after the memset, but before the stack canary is re-checked.
21-
#ifndef LIBC_HAVE_ADDRESS_SANITIZER
21+
#ifndef LIBC_HAS_ADDRESS_SANITIZER
2222
TEST(LlvmLibcStackChkFail, Smash) {
2323
EXPECT_DEATH(
2424
[] {
@@ -27,4 +27,4 @@ TEST(LlvmLibcStackChkFail, Smash) {
2727
},
2828
WITH_SIGNAL(SIGABRT));
2929
}
30-
#endif // LIBC_HAVE_ADDRESS_SANITIZER
30+
#endif // LIBC_HAS_ADDRESS_SANITIZER

‎libc/test/src/math/smoke/CMakeLists.txt

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2895,9 +2895,10 @@ add_fp_unittest(
28952895
SRCS
28962896
nanf_test.cpp
28972897
DEPENDS
2898-
libc.include.signal
2898+
libc.hdr.signal_macros
28992899
libc.src.math.nanf
29002900
libc.src.__support.FPUtil.fp_bits
2901+
libc.src.__support.macros.sanitizer
29012902
# FIXME: The nan tests currently have death tests, which aren't supported for
29022903
# hermetic tests.
29032904
UNIT_TEST_ONLY
@@ -2910,9 +2911,10 @@ add_fp_unittest(
29102911
SRCS
29112912
nan_test.cpp
29122913
DEPENDS
2913-
libc.include.signal
2914+
libc.hdr.signal_macros
29142915
libc.src.math.nan
29152916
libc.src.__support.FPUtil.fp_bits
2917+
libc.src.__support.macros.sanitizer
29162918
# FIXME: The nan tests currently have death tests, which aren't supported for
29172919
# hermetic tests.
29182920
UNIT_TEST_ONLY
@@ -2925,9 +2927,10 @@ add_fp_unittest(
29252927
SRCS
29262928
nanl_test.cpp
29272929
DEPENDS
2928-
libc.include.signal
2930+
libc.hdr.signal_macros
29292931
libc.src.math.nanl
29302932
libc.src.__support.FPUtil.fp_bits
2933+
libc.src.__support.macros.sanitizer
29312934
# FIXME: The nan tests currently have death tests, which aren't supported for
29322935
# hermetic tests.
29332936
UNIT_TEST_ONLY
@@ -2940,7 +2943,7 @@ add_fp_unittest(
29402943
SRCS
29412944
nanf16_test.cpp
29422945
DEPENDS
2943-
libc.include.signal
2946+
libc.hdr.signal_macros
29442947
libc.src.math.nanf16
29452948
libc.src.__support.FPUtil.fp_bits
29462949
libc.src.__support.macros.sanitizer
@@ -2956,9 +2959,10 @@ add_fp_unittest(
29562959
SRCS
29572960
nanf128_test.cpp
29582961
DEPENDS
2959-
libc.include.signal
2962+
libc.hdr.signal_macros
29602963
libc.src.math.nanf128
29612964
libc.src.__support.FPUtil.fp_bits
2965+
libc.src.__support.macros.sanitizer
29622966
# FIXME: The nan tests currently have death tests, which aren't supported for
29632967
# hermetic tests.
29642968
UNIT_TEST_ONLY

‎libc/test/src/math/smoke/nan_test.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,13 @@
66
//
77
//===----------------------------------------------------------------------===//
88

9+
#include "hdr/signal_macros.h"
910
#include "src/__support/FPUtil/FPBits.h"
11+
#include "src/__support/macros/sanitizer.h"
1012
#include "src/math/nan.h"
1113
#include "test/UnitTest/FEnvSafeTest.h"
1214
#include "test/UnitTest/FPMatcher.h"
1315
#include "test/UnitTest/Test.h"
14-
#include <signal.h>
1516

1617
class LlvmLibcNanTest : public LIBC_NAMESPACE::testing::FEnvSafeTest {
1718
public:
@@ -43,8 +44,8 @@ TEST_F(LlvmLibcNanTest, RandomString) {
4344
run_test("123 ", 0x7ff8000000000000);
4445
}
4546

46-
#if !defined(LIBC_HAVE_ADDRESS_SANITIZER) && defined(LIBC_TARGET_OS_IS_LINUX)
47+
#if !defined(LIBC_HAS_ADDRESS_SANITIZER) && defined(LIBC_TARGET_OS_IS_LINUX)
4748
TEST_F(LlvmLibcNanTest, InvalidInput) {
4849
EXPECT_DEATH([] { LIBC_NAMESPACE::nan(nullptr); }, WITH_SIGNAL(SIGSEGV));
4950
}
50-
#endif // LIBC_HAVE_ADDRESS_SANITIZER
51+
#endif // LIBC_HAS_ADDRESS_SANITIZER

‎libc/test/src/math/smoke/nanf128_test.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@
66
//
77
//===----------------------------------------------------------------------===//
88

9+
#include "hdr/signal_macros.h"
910
#include "src/__support/FPUtil/FPBits.h"
11+
#include "src/__support/macros/sanitizer.h"
1012
#include "src/__support/uint128.h"
1113
#include "src/math/nanf128.h"
1214
#include "test/UnitTest/FEnvSafeTest.h"
@@ -53,9 +55,8 @@ TEST_F(LlvmLibcNanf128Test, RandomString) {
5355
QUIET_NAN);
5456
}
5557

56-
#if !defined(LIBC_HAVE_ADDRESS_SANITIZER) && defined(LIBC_TARGET_OS_IS_LINUX)
57-
#include <signal.h>
58+
#if !defined(LIBC_HAS_ADDRESS_SANITIZER) && defined(LIBC_TARGET_OS_IS_LINUX)
5859
TEST_F(LlvmLibcNanf128Test, InvalidInput) {
5960
EXPECT_DEATH([] { LIBC_NAMESPACE::nanf128(nullptr); }, WITH_SIGNAL(SIGSEGV));
6061
}
61-
#endif // LIBC_HAVE_ADDRESS_SANITIZER
62+
#endif // LIBC_HAS_ADDRESS_SANITIZER

‎libc/test/src/math/smoke/nanf16_test.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,14 @@
66
//
77
//===----------------------------------------------------------------------===//
88

9+
#include "hdr/signal_macros.h"
910
#include "src/__support/FPUtil/FPBits.h"
1011
#include "src/__support/macros/sanitizer.h"
1112
#include "src/math/nanf16.h"
1213
#include "test/UnitTest/FEnvSafeTest.h"
1314
#include "test/UnitTest/FPMatcher.h"
1415
#include "test/UnitTest/Test.h"
1516

16-
#include <signal.h>
17-
1817
class LlvmLibcNanf16Test : public LIBC_NAMESPACE::testing::FEnvSafeTest {
1918
public:
2019
using StorageType = LIBC_NAMESPACE::fputil::FPBits<float16>::StorageType;
@@ -44,8 +43,8 @@ TEST_F(LlvmLibcNanf16Test, RandomString) {
4443
run_test("123 ", 0x7e00);
4544
}
4645

47-
#if !defined(LIBC_HAVE_ADDRESS_SANITIZER) && defined(LIBC_TARGET_OS_IS_LINUX)
46+
#if !defined(LIBC_HAS_ADDRESS_SANITIZER) && defined(LIBC_TARGET_OS_IS_LINUX)
4847
TEST_F(LlvmLibcNanf16Test, InvalidInput) {
4948
EXPECT_DEATH([] { LIBC_NAMESPACE::nanf16(nullptr); }, WITH_SIGNAL(SIGSEGV));
5049
}
51-
#endif // LIBC_HAVE_ADDRESS_SANITIZER
50+
#endif // LIBC_HAS_ADDRESS_SANITIZER

‎libc/test/src/math/smoke/nanf_test.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,13 @@
66
//
77
//===----------------------------------------------------------------------===//
88

9+
#include "hdr/signal_macros.h"
910
#include "src/__support/FPUtil/FPBits.h"
11+
#include "src/__support/macros/sanitizer.h"
1012
#include "src/math/nanf.h"
1113
#include "test/UnitTest/FEnvSafeTest.h"
1214
#include "test/UnitTest/FPMatcher.h"
1315
#include "test/UnitTest/Test.h"
14-
#include <signal.h>
1516

1617
class LlvmLibcNanfTest : public LIBC_NAMESPACE::testing::FEnvSafeTest {
1718
public:
@@ -42,8 +43,8 @@ TEST_F(LlvmLibcNanfTest, RandomString) {
4243
run_test("123 ", 0x7fc00000);
4344
}
4445

45-
#if !defined(LIBC_HAVE_ADDRESS_SANITIZER) && defined(LIBC_TARGET_OS_IS_LINUX)
46+
#if !defined(LIBC_HAS_ADDRESS_SANITIZER) && defined(LIBC_TARGET_OS_IS_LINUX)
4647
TEST_F(LlvmLibcNanfTest, InvalidInput) {
4748
EXPECT_DEATH([] { LIBC_NAMESPACE::nanf(nullptr); }, WITH_SIGNAL(SIGSEGV));
4849
}
49-
#endif // LIBC_HAVE_ADDRESS_SANITIZER
50+
#endif // LIBC_HAS_ADDRESS_SANITIZER

‎libc/test/src/math/smoke/nanl_test.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,13 @@
66
//
77
//===----------------------------------------------------------------------===//
88

9+
#include "hdr/signal_macros.h"
910
#include "src/__support/FPUtil/FPBits.h"
11+
#include "src/__support/macros/sanitizer.h"
1012
#include "src/math/nanl.h"
1113
#include "test/UnitTest/FEnvSafeTest.h"
1214
#include "test/UnitTest/FPMatcher.h"
1315
#include "test/UnitTest/Test.h"
14-
#include <signal.h>
1516

1617
#if defined(LIBC_TYPES_LONG_DOUBLE_IS_FLOAT64)
1718
#define SELECT_LONG_DOUBLE(val, _, __) val
@@ -70,8 +71,8 @@ TEST_F(LlvmLibcNanlTest, RandomString) {
7071
run_test("123 ", expected);
7172
}
7273

73-
#if !defined(LIBC_HAVE_ADDRESS_SANITIZER) && defined(LIBC_TARGET_OS_IS_LINUX)
74+
#if !defined(LIBC_HAS_ADDRESS_SANITIZER) && defined(LIBC_TARGET_OS_IS_LINUX)
7475
TEST_F(LlvmLibcNanlTest, InvalidInput) {
7576
EXPECT_DEATH([] { LIBC_NAMESPACE::nanl(nullptr); }, WITH_SIGNAL(SIGSEGV));
7677
}
77-
#endif // LIBC_HAVE_ADDRESS_SANITIZER
78+
#endif // LIBC_HAS_ADDRESS_SANITIZER

‎utils/bazel/llvm-project-overlay/libc/BUILD.bazel

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,16 @@ libc_support_library(
272272
],
273273
)
274274

275+
libc_support_library(
276+
name = "__support_macros_null_check",
277+
hdrs = ["src/__support/macros/null_check.h"],
278+
deps = [
279+
":__support_macros_config",
280+
":__support_macros_optimization",
281+
":__support_macros_sanitizer",
282+
],
283+
)
284+
275285
libc_support_library(
276286
name = "__support_common",
277287
hdrs = [
@@ -665,6 +675,7 @@ libc_support_library(
665675
":__support_ctype_utils",
666676
":__support_fputil_fp_bits",
667677
":__support_fputil_rounding_mode",
678+
":__support_macros_null_check",
668679
":__support_str_to_integer",
669680
":__support_str_to_num_result",
670681
":__support_uint128",

0 commit comments

Comments
 (0)