Skip to content

Commit 342c8f8

Browse files
committed
Refactor the Capstone assertions.
It changes now to the following behavior: - Debug && !defined(CAPSTONE_ASSERTION_WARNINGS) - `assert(expr)` code is included, warning is not printed if hit. - Debug && defined(CAPSTONE_ASSERTION_WARNINGS) - `assert(expr)` code is included, warning is printed if hit. - Release && !defined(CAPSTONE_ASSERTION_WARNINGS) - `assert(expr)` code is not included, warning is not printed if hit. - Release && defined(CAPSTONE_ASSERTION_WARNINGS) - `assert(expr)` code is not included, warning is printed if hit. This also fixes the inconsistent (buggy) behavior from before. The variants of CS_ASSERT_RET(_VAL) did not return if the assertions were disabled (release build). So the semantics were different.
1 parent c66f877 commit 342c8f8

File tree

5 files changed

+98
-33
lines changed

5 files changed

+98
-33
lines changed

.github/workflows/CITest.yml

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,17 +54,17 @@ jobs:
5454
build_type: 'Debug'
5555
}
5656
- {
57-
name: 'ubuntu-22.04 x64 release - assert warn',
57+
name: 'ubuntu-22.04 x64 debug - assert warn',
5858
os: ubuntu-22.04,
5959
arch: x64,
6060
build-system: 'cmake',
6161
diet-build: 'OFF',
6262
enable-asan: 'OFF',
63-
build_type: 'Release',
63+
build_type: 'Debug',
6464
build_options: '-DCAPSTONE_ASSERTION_WARNINGS=ON'
6565
}
6666
- {
67-
name: 'ubuntu-22.04 x64 release - no asserts',
67+
name: 'ubuntu-22.04 x64 release - no assert warnings',
6868
os: ubuntu-22.04,
6969
arch: x64,
7070
build-system: 'cmake',
@@ -73,6 +73,16 @@ jobs:
7373
build_type: 'Release',
7474
build_options: '-DCAPSTONE_ASSERTION_WARNINGS=OFF'
7575
}
76+
- {
77+
name: 'ubuntu-22.04 x64 release - assert warn',
78+
os: ubuntu-22.04,
79+
arch: x64,
80+
build-system: 'cmake',
81+
diet-build: 'OFF',
82+
enable-asan: 'OFF',
83+
build_type: 'Release',
84+
build_options: '-DCAPSTONE_ASSERTION_WARNINGS=ON'
85+
}
7686
- {
7787
name: 'ubuntu-24.04 x64 ASAN',
7888
os: ubuntu-24.04,
@@ -112,10 +122,10 @@ jobs:
112122
run: |
113123
mkdir build && cd build
114124
# build static library
115-
cmake -DCAPSTONE_INSTALL=1 -DCMAKE_INSTALL_PREFIX=/usr -DENABLE_ASAN=${asan} -DCAPSTONE_BUILD_DIET=${diet_build} ${build_option} ..
125+
cmake -DCMAKE_BUILD_TYPE=${build_type} -DCAPSTONE_INSTALL=1 -DCMAKE_INSTALL_PREFIX=/usr -DENABLE_ASAN=${asan} -DCAPSTONE_BUILD_DIET=${diet_build} ${build_option} ..
116126
cmake --build . --config ${build_type}
117127
# build shared library
118-
cmake -DCAPSTONE_INSTALL=1 -DCAPSTONE_BUILD_SHARED_LIBS=1 -DCMAKE_INSTALL_PREFIX=/usr -DCAPSTONE_BUILD_CSTEST=ON -DENABLE_ASAN=${asan} ${build_option} ..
128+
cmake -DCMAKE_BUILD_TYPE=${build_type} -DCAPSTONE_INSTALL=1 -DCAPSTONE_BUILD_SHARED_LIBS=1 -DCMAKE_INSTALL_PREFIX=/usr -DCAPSTONE_BUILD_CSTEST=ON -DENABLE_ASAN=${asan} ${build_option} ..
119129
sudo cmake --build . --config ${build_type} --target install
120130
121131
- name: Lower number of KASL randomized address bits

CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ option(CAPSTONE_USE_DEFAULT_ALLOC "Use default memory allocation functions" ON)
7979
option(CAPSTONE_USE_ARCH_REGISTRATION "Use explicit architecture registration" OFF)
8080
option(CAPSTONE_ARCHITECTURE_DEFAULT "Whether architectures are enabled by default" ON)
8181
option(CAPSTONE_DEBUG "Whether to enable extra debug assertions (enabled with CMAKE_BUILD_TYPE=Debug)" OFF)
82-
option(CAPSTONE_ASSERTION_WARNINGS "Warns about hit assertions in release builds." OFF)
82+
option(CAPSTONE_ASSERTION_WARNINGS "Disables some asserts and warns instead when hit. Does not disable all asserts." OFF)
8383
option(CAPSTONE_INSTALL "Generate install target" ${PROJECT_IS_TOP_LEVEL})
8484
option(ENABLE_ASAN "Enable address sanitizer" OFF)
8585
option(ENABLE_COVERAGE "Enable test coverage" OFF)

cs_priv.h

Lines changed: 41 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -107,11 +107,28 @@ extern cs_realloc_t cs_mem_realloc;
107107
extern cs_free_t cs_mem_free;
108108
extern cs_vsnprintf_t cs_vsnprintf;
109109

110-
/// By defining CAPSTONE_DEBUG assertions can be used.
111-
/// For the release build the @expr is not included.
112-
#ifdef CAPSTONE_DEBUG
110+
/// Capstone assert macros. They can be configured to print warnings
111+
/// when the `expr` is false.
112+
/// This can be enabled by defining CAPSTONE_ASSERTION_WARNINGS.
113+
/// Debug builds will always include an `assert(expr)` and hard fail
114+
/// if `!expr`.
115+
/// Release builds will not have `assert(expr)` code.
116+
117+
/// An simple assert.
118+
#if defined(CAPSTONE_DEBUG) && !defined(CAPSTONE_ASSERTION_WARNINGS)
113119
#define CS_ASSERT(expr) assert(expr)
114-
#elif CAPSTONE_ASSERTION_WARNINGS
120+
#elif defined(CAPSTONE_DEBUG) && defined(CAPSTONE_ASSERTION_WARNINGS)
121+
#define CS_ASSERT(expr) \
122+
do { \
123+
if (!(expr)) { \
124+
fprintf(stderr, \
125+
"Capstone hit the assert: \"" #expr \
126+
"\": %s:%" PRIu32 "\n", \
127+
__FILE__, __LINE__); \
128+
assert(expr) \
129+
} \
130+
} while (0)
131+
#elif defined(CAPSTONE_ASSERTION_WARNINGS)
115132
#define CS_ASSERT(expr) \
116133
do { \
117134
if (!(expr)) { \
@@ -125,42 +142,44 @@ extern cs_vsnprintf_t cs_vsnprintf;
125142
#define CS_ASSERT(expr)
126143
#endif
127144

128-
/// If compiled in debug mode it will assert(@expr).
129-
/// In the release build it will check the @expr and return @val if false.
130-
#ifdef CAPSTONE_DEBUG
145+
/// An assert which returns the value in release builds if `!expr`.
146+
#if defined(CAPSTONE_DEBUG) && !defined(CAPSTONE_ASSERTION_WARNINGS)
131147
#define CS_ASSERT_RET_VAL(expr, val) assert(expr)
132-
#elif CAPSTONE_ASSERTION_WARNINGS
148+
#elif defined(CAPSTONE_ASSERTION_WARNINGS)
133149
#define CS_ASSERT_RET_VAL(expr, val) \
134150
do { \
135151
if (!(expr)) { \
136-
fprintf(stderr, \
137-
"Capstone hit the assert: \"" #expr \
138-
"\": %s:%" PRIu32 "\n", \
139-
__FILE__, __LINE__); \
152+
CS_ASSERT(expr); \
140153
return val; \
141154
} \
142155
} while (0)
143156
#else
144-
#define CS_ASSERT_RET_VAL(expr, val)
157+
#define CS_ASSERT_RET_VAL(expr, val) \
158+
do { \
159+
if (!(expr)) { \
160+
return val; \
161+
} \
162+
} while (0)
145163
#endif
146164

147-
/// If compiled in debug mode it will assert(@expr).
148-
/// In the release build it will check the @expr and return if false.
149-
#ifdef CAPSTONE_DEBUG
165+
/// An assert which returns in release builds if `!expr`.
166+
#if defined(CAPSTONE_DEBUG) && !defined(CAPSTONE_ASSERTION_WARNINGS)
150167
#define CS_ASSERT_RET(expr) assert(expr)
151-
#elif CAPSTONE_ASSERTION_WARNINGS
168+
#elif defined(CAPSTONE_ASSERTION_WARNINGS)
152169
#define CS_ASSERT_RET(expr) \
153170
do { \
154171
if (!(expr)) { \
155-
fprintf(stderr, \
156-
"Capstone hit the assert: \"" #expr \
157-
"\": %s:%" PRIu32 "\n", \
158-
__FILE__, __LINE__); \
172+
CS_ASSERT(expr); \
159173
return; \
160174
} \
161175
} while (0)
162176
#else
163-
#define CS_ASSERT_RET(expr)
177+
#define CS_ASSERT_RET(expr) \
178+
do { \
179+
if (!(expr)) { \
180+
return; \
181+
} \
182+
} while (0)
164183
#endif
165184

166185
#endif

tests/integration/CMakeLists.txt

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,13 @@ add_test(NAME integration_compat_headers
3434
)
3535

3636
set(INTEGRATION_TEST_SRC test_litbase.c)
37+
if(CMAKE_BUILD_TYPE STREQUAL "Release")
38+
set(INTEGRATION_TEST_SRC ${INTEGRATION_TEST_SRC} test_assert_macros.c)
39+
endif()
40+
3741
foreach(TSRC ${INTEGRATION_TEST_SRC})
38-
string(REGEX REPLACE ".c$" "" TBIN ${TSRC})
39-
add_executable(${TBIN} "${TESTS_INTEGRATION_DIR}/${TSRC}")
40-
target_link_libraries(${TBIN} PRIVATE capstone)
41-
add_test(NAME "integration_${TBIN}" COMMAND ${TBIN})
42-
endforeach()
42+
string(REGEX REPLACE ".c$" "" TBIN ${TSRC})
43+
add_executable(${TBIN} "${TESTS_INTEGRATION_DIR}/${TSRC}")
44+
target_link_libraries(${TBIN} PRIVATE capstone)
45+
add_test(NAME "integration_${TBIN}" COMMAND ${TBIN})
46+
endforeach()
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
// SPDX-FileCopyrightText: 2025 Rot127 <[email protected]>
2+
// SPDX-License-Identifier: LGPL-3.0-only
3+
4+
// Has to be built as Release build or with CAPSTONE_ASSERTION_WARNINGS.
5+
6+
#include <capstone/platform.h>
7+
#include <capstone/capstone.h>
8+
9+
// Test: https://github.com/capstone-engine/capstone/issues/2791
10+
static bool test_cs_reg_null_case()
11+
{
12+
csh handle;
13+
if (cs_open(CS_ARCH_AARCH64, (cs_mode)0, &handle) != CS_ERR_OK) {
14+
printf("Open failed\n");
15+
return false;
16+
}
17+
// Invalid register id 0 should return NULL.
18+
if (cs_reg_name(handle, 0) != NULL) {
19+
printf("NULL check failed\n");
20+
return false;
21+
}
22+
cs_close(&handle);
23+
return true;
24+
}
25+
26+
int main()
27+
{
28+
bool result = true;
29+
result &= test_cs_reg_null_case();
30+
31+
return result ? 0 : -1;
32+
}

0 commit comments

Comments
 (0)