Skip to content

Commit

Permalink
Use NSCAssert() in react_native_assert instead of C assert() (#36177)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #36177

react_native_assert calls C `assert()`, where XCode does not have a built-in breakpoint navigator to hook to assertion failures (though you can add a symbolic breakpoint to "abort()" to get the effect). This changes the Apple implemented of `react_native_assert()` to use `NSCAssert` under the hood. This is safe to use in C functions, but will be trapped by the default XCode exceptions breakpoint navigator.

Changelog:
[iOS][Fixed] - Use NSCAssert() in react_native_assert instead of C assert()

Reviewed By: cipolleschi

Differential Revision: D43275024

fbshipit-source-id: 43c4e4f1ae6b99f32634d4b1880bce712c3ae8f6
  • Loading branch information
NickGerleman authored and facebook-github-bot committed Feb 22, 2023
1 parent 96b2ca4 commit c5bc3f1
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 42 deletions.
3 changes: 2 additions & 1 deletion ReactCommon/React-Fabric.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,8 @@ Pod::Spec.new do |s|
s.subspec "debug_core" do |ss|
ss.dependency folly_dep_name, folly_version
ss.compiler_flags = folly_compiler_flags
ss.source_files = "react/debug/**/*.{m,mm,cpp,h}"
ss.source_files = "react/debug/*.h",
"react/debug/ios/**/*.{m,mm,cpp,h}"
ss.exclude_files = "react/debug/tests"
ss.header_dir = "react/debug"
end
Expand Down
6 changes: 2 additions & 4 deletions ReactCommon/react/debug/BUCK
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,6 @@ APPLE_COMPILER_FLAGS = get_apple_compiler_flags()

rn_xplat_cxx_library(
name = "debug",
srcs = glob(
["**/*.cpp"],
exclude = glob(["tests/**/*.cpp"]),
),
headers = glob(
["**/*.h"],
exclude = glob(["tests/**/*.h"]),
Expand All @@ -40,8 +36,10 @@ rn_xplat_cxx_library(
# for android react_native_assert
"-llog",
],
fbandroid_srcs = glob(["android/**/*.cpp"]),
fbobjc_compiler_flags = APPLE_COMPILER_FLAGS,
fbobjc_preprocessor_flags = get_preprocessor_flags_for_build_mode() + get_apple_inspector_flags(),
fbobjc_srcs = glob(["ios/**/*.mm"]),
force_static = True,
labels = [
"pfh:ReactNative_CommonInfrastructurePlaceholder",
Expand Down
2 changes: 1 addition & 1 deletion ReactCommon/react/debug/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ add_compile_options(
-DLOG_TAG=\"Fabric\")


file(GLOB react_debug_SRC CONFIGURE_DEPENDS *.cpp)
file(GLOB react_debug_SRC CONFIGURE_DEPENDS android/*.cpp)
add_library(react_debug SHARED ${react_debug_SRC})

target_include_directories(react_debug PUBLIC ${REACT_COMMON_DIR})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,10 @@
* LICENSE file in the root directory of this source tree.
*/

#ifdef __ANDROID__

#include <android/log.h>
#include <react/debug/react_native_assert.h>

// Provide a prototype to silence missing prototype warning in release
// mode.
extern "C" void react_native_assert_fail(
const char *func,
const char *file,
int line,
const char *expr);
#ifdef REACT_NATIVE_DEBUG

extern "C" void react_native_assert_fail(
const char *func,
Expand All @@ -42,4 +35,4 @@ extern "C" void react_native_assert_fail(
expr);
}

#endif // __ANDROID__
#endif
27 changes: 27 additions & 0 deletions ReactCommon/react/debug/ios/react_native_assert.mm
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

#import <Foundation/NSException.h>
#import <glog/logging.h>
#import <react/debug/react_native_assert.h>

#ifdef REACT_NATIVE_DEBUG

extern "C" void react_native_assert_fail(const char *func, const char *file, int line, const char *expr)
{
// flush logs because some might be lost on iOS if an assert is hit right after
// this. If you are trying to debug something actively and have added lots of
// LOG statements to track down an issue, there is race between flushing the
// final logs and stopping execution when the assert hits. Thus, if we know an
// assert will fail, we force flushing to happen right before the assert.
LOG(ERROR) << "react_native_assert failure: " << expr;
google::FlushLogFiles(google::GLOG_INFO /*min_severity*/);

NSCAssert(false, @"%s:%d: function %s: assertion failed (%s)", file, line, func, expr);
}

#endif
28 changes: 2 additions & 26 deletions ReactCommon/react/debug/react_native_assert.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,8 @@

#else // REACT_NATIVE_DEBUG

#ifdef __ANDROID__

#include <android/log.h>
#define react_native_assert(e) \
((e) ? (void)0 : react_native_assert_fail(__func__, __FILE__, __LINE__, #e))

#ifdef __cplusplus
extern "C" {
Expand All @@ -44,27 +43,4 @@ void react_native_assert_fail(
}
#endif // __cpusplus

#define react_native_assert(e) \
((e) ? (void)0 : react_native_assert_fail(__func__, __FILE__, __LINE__, #e))

#else // __ANDROID__

#include <glog/logging.h>
#include <cassert>

// For all platforms, but iOS+Xcode especially: flush logs because some might be
// lost on iOS if an assert is hit right after this. If you are trying to debug
// something actively and have added lots of LOG statements to track down an
// issue, there is race between flushing the final logs and stopping execution
// when the assert hits. Thus, if we know an assert will fail, we force flushing
// to happen right before the assert.
#define react_native_assert(cond) \
if (!(cond)) { \
LOG(ERROR) << "react_native_assert failure: " << #cond; \
google::FlushLogFiles(google::GLOG_INFO); \
assert(cond); \
}

#endif // platforms besides __ANDROID__

#endif // REACT_NATIVE_DEBUG

0 comments on commit c5bc3f1

Please sign in to comment.