Skip to content

Commit 1896ee3

Browse files
lntuelntue
authored
[libc] Fix undefined behavior for nan functions. (llvm#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

0 commit comments

Comments
 (0)