From 791151d0b8f0314496cf18c822c071b1dd5791ea Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Wed, 12 Nov 2014 15:16:08 -0500 Subject: [PATCH] RCU dereference check if within read-side critical section Need to link with liburcu-common in addition to urcu flavor. Signed-off-by: Mathieu Desnoyers --- Makefile.am | 5 +-- tests/benchmark/Makefile.am | 12 +++---- tests/regression/Makefile.am | 12 +++---- tests/unit/Makefile.am | 12 +++---- urcu-bp.h | 4 +-- urcu-checker.c | 68 ++++++++++++++++++++++++++++++++++++ urcu-pointer.h | 7 +++- urcu-qsbr.h | 3 ++ urcu.c | 2 ++ urcu.h | 3 ++ urcu/static/urcu-bp.h | 3 ++ urcu/static/urcu-qsbr.h | 3 ++ urcu/static/urcu.h | 3 ++ urcu/urcu-checker.h | 32 +++++++++++++++++ 14 files changed, 146 insertions(+), 23 deletions(-) create mode 100644 urcu-checker.c create mode 100644 urcu/urcu-checker.h diff --git a/Makefile.am b/Makefile.am index 752510d..7ead5f0 100644 --- a/Makefile.am +++ b/Makefile.am @@ -24,7 +24,7 @@ nobase_dist_include_HEADERS = urcu/compiler.h urcu/hlist.h urcu/list.h \ $(top_srcdir)/urcu/map/*.h \ $(top_srcdir)/urcu/static/*.h \ urcu/rand-compat.h \ - urcu/tls-compat.h + urcu/tls-compat.h urcu/urcu-checker.h nobase_nodist_include_HEADERS = urcu/arch.h urcu/uatomic.h urcu/config.h dist_noinst_HEADERS = urcu-die.h urcu-wait.h @@ -57,7 +57,8 @@ lib_LTLIBRARIES = liburcu-common.la \ # liburcu-common contains wait-free queues (needed by call_rcu) as well # as futex fallbacks. # -liburcu_common_la_SOURCES = wfqueue.c wfcqueue.c wfstack.c $(COMPAT) +liburcu_common_la_SOURCES = wfqueue.c wfcqueue.c wfstack.c $(COMPAT) \ + urcu-checker.c liburcu_la_SOURCES = urcu.c urcu-pointer.c $(COMPAT) liburcu_la_LIBADD = liburcu-common.la diff --git a/tests/benchmark/Makefile.am b/tests/benchmark/Makefile.am index 85d454d..b8cf35d 100644 --- a/tests/benchmark/Makefile.am +++ b/tests/benchmark/Makefile.am @@ -23,12 +23,12 @@ noinst_PROGRAMS = test_urcu test_urcu_dynamic_link test_urcu_timing \ test_urcu_lfs_rcu_dynlink URCU_COMMON_LIB=$(top_builddir)/liburcu-common.la -URCU_LIB=$(top_builddir)/liburcu.la -URCU_QSBR_LIB=$(top_builddir)/liburcu-qsbr.la -URCU_MB_LIB=$(top_builddir)/liburcu-mb.la -URCU_SIGNAL_LIB=$(top_builddir)/liburcu-signal.la -URCU_BP_LIB=$(top_builddir)/liburcu-bp.la -URCU_CDS_LIB=$(top_builddir)/liburcu-cds.la +URCU_LIB=$(top_builddir)/liburcu.la $(URCU_COMMON_LIB) +URCU_QSBR_LIB=$(top_builddir)/liburcu-qsbr.la $(URCU_COMMON_LIB) +URCU_MB_LIB=$(top_builddir)/liburcu-mb.la $(URCU_COMMON_LIB) +URCU_SIGNAL_LIB=$(top_builddir)/liburcu-signal.la $(URCU_COMMON_LIB) +URCU_BP_LIB=$(top_builddir)/liburcu-bp.la $(URCU_COMMON_LIB) +URCU_CDS_LIB=$(top_builddir)/liburcu-cds.la $(URCU_COMMON_LIB) DEBUG_YIELD_LIB=$(builddir)/../common/libdebug-yield.la diff --git a/tests/regression/Makefile.am b/tests/regression/Makefile.am index 8dfe542..354ad1a 100644 --- a/tests/regression/Makefile.am +++ b/tests/regression/Makefile.am @@ -13,12 +13,12 @@ noinst_PROGRAMS = test_urcu_fork \ noinst_HEADERS = rcutorture.h URCU_COMMON_LIB=$(top_builddir)/liburcu-common.la -URCU_LIB=$(top_builddir)/liburcu.la -URCU_QSBR_LIB=$(top_builddir)/liburcu-qsbr.la -URCU_MB_LIB=$(top_builddir)/liburcu-mb.la -URCU_SIGNAL_LIB=$(top_builddir)/liburcu-signal.la -URCU_BP_LIB=$(top_builddir)/liburcu-bp.la -URCU_CDS_LIB=$(top_builddir)/liburcu-cds.la +URCU_LIB=$(top_builddir)/liburcu.la $(URCU_COMMON_LIB) +URCU_QSBR_LIB=$(top_builddir)/liburcu-qsbr.la $(URCU_COMMON_LIB) +URCU_MB_LIB=$(top_builddir)/liburcu-mb.la $(URCU_COMMON_LIB) +URCU_SIGNAL_LIB=$(top_builddir)/liburcu-signal.la $(URCU_COMMON_LIB) +URCU_BP_LIB=$(top_builddir)/liburcu-bp.la $(URCU_COMMON_LIB) +URCU_CDS_LIB=$(top_builddir)/liburcu-cds.la $(URCU_COMMON_LIB) test_urcu_fork_SOURCES = test_urcu_fork.c test_urcu_fork_LDADD = $(URCU_LIB) diff --git a/tests/unit/Makefile.am b/tests/unit/Makefile.am index 8cc4acb..cd40b39 100644 --- a/tests/unit/Makefile.am +++ b/tests/unit/Makefile.am @@ -10,12 +10,12 @@ noinst_PROGRAMS = test_uatomic \ noinst_HEADERS = test_urcu_multiflavor.h URCU_COMMON_LIB=$(top_builddir)/liburcu-common.la -URCU_LIB=$(top_builddir)/liburcu.la -URCU_QSBR_LIB=$(top_builddir)/liburcu-qsbr.la -URCU_MB_LIB=$(top_builddir)/liburcu-mb.la -URCU_SIGNAL_LIB=$(top_builddir)/liburcu-signal.la -URCU_BP_LIB=$(top_builddir)/liburcu-bp.la -URCU_CDS_LIB=$(top_builddir)/liburcu-cds.la +URCU_LIB=$(top_builddir)/liburcu.la $(URCU_COMMON_LIB) +URCU_QSBR_LIB=$(top_builddir)/liburcu-qsbr.la $(URCU_COMMON_LIB) +URCU_MB_LIB=$(top_builddir)/liburcu-mb.la $(URCU_COMMON_LIB) +URCU_SIGNAL_LIB=$(top_builddir)/liburcu-signal.la $(URCU_COMMON_LIB) +URCU_BP_LIB=$(top_builddir)/liburcu-bp.la $(URCU_COMMON_LIB) +URCU_CDS_LIB=$(top_builddir)/liburcu-cds.la $(URCU_COMMON_LIB) test_uatomic_SOURCES = test_uatomic.c test_uatomic_LDADD = $(URCU_COMMON_LIB) diff --git a/urcu-bp.h b/urcu-bp.h index 4718f3b..8704ddb 100644 --- a/urcu-bp.h +++ b/urcu-bp.h @@ -88,8 +88,8 @@ extern "C" { * See LGPL-only urcu/static/urcu-pointer.h for documentation. */ -extern void rcu_read_lock(void); -extern void rcu_read_unlock(void); +extern void test_rcu_read_lock(void); +extern void test_rcu_read_unlock(void); extern int rcu_read_ongoing(void); extern void *rcu_dereference_sym_bp(void *p); diff --git a/urcu-checker.c b/urcu-checker.c new file mode 100644 index 0000000..234bd9b --- /dev/null +++ b/urcu-checker.c @@ -0,0 +1,68 @@ +/* + * urcu-checker.c + * + * Userspace RCU library checker + * + * Copyright (c) 2014 Mathieu Desnoyers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include +#include +#include +#include +#include +#include + +#define URCU_DEBUG_STACK_LEN 10 + +struct urcu_debug_entry { + void *ip; + int depth; +}; + +struct urcu_debug_stack { + struct urcu_debug_entry stack[URCU_DEBUG_STACK_LEN]; + int stackend; +}; + +static DEFINE_URCU_TLS(struct urcu_debug_stack, rcu_debug_stack); + +void rcu_read_lock_debug(void) +{ + struct urcu_debug_stack *r = &URCU_TLS(rcu_debug_stack); + + r->stack[r->stackend++].ip = __builtin_return_address(0); +} + +void rcu_read_unlock_debug(void) +{ + struct urcu_debug_stack *r = &URCU_TLS(rcu_debug_stack); + + assert(r->stackend != 0); + r->stack[--r->stackend].ip = NULL; +} + +void rcu_read_ongoing_check_debug(const char *func) +{ + struct urcu_debug_stack *r = &URCU_TLS(rcu_debug_stack); + + if (r->stackend == 0) { + fprintf(stderr, "URCU LOCKED CHECK failure: %p\n", + __builtin_return_address(0)); + abort(); + } +} diff --git a/urcu-pointer.h b/urcu-pointer.h index 5be986c..6c79c58 100644 --- a/urcu-pointer.h +++ b/urcu-pointer.h @@ -29,6 +29,7 @@ #include #include #include +#include #ifdef __cplusplus extern "C" { @@ -44,7 +45,11 @@ extern "C" { * Fetch a RCU-protected pointer. Typically used to copy the variable ptr to a * local variable. */ -#define rcu_dereference _rcu_dereference +#define rcu_dereference(p) \ + ({ \ + rcu_read_ongoing_check_debug(__func__); \ + _rcu_dereference(p); \ + }) /* * type *rcu_cmpxchg_pointer(type **ptr, type *new, type *old) diff --git a/urcu-qsbr.h b/urcu-qsbr.h index b4a28a7..60f6171 100644 --- a/urcu-qsbr.h +++ b/urcu-qsbr.h @@ -108,6 +108,9 @@ extern void rcu_read_unlock(void); #endif /* !RCU_DEBUG */ +#define test_rcu_read_lock rcu_read_lock_qsbr +#define test_rcu_read_unlock rcu_read_unlock_qsbr + extern int rcu_read_ongoing(void); extern void rcu_quiescent_state(void); extern void rcu_thread_offline(void); diff --git a/urcu.c b/urcu.c index ae3490f..d86cb6c 100644 --- a/urcu.c +++ b/urcu.c @@ -441,6 +441,8 @@ int rcu_read_ongoing(void) return _rcu_read_ongoing(); } + + void rcu_register_thread(void) { URCU_TLS(rcu_reader).tid = pthread_self(); diff --git a/urcu.h b/urcu.h index b8ca700..0a1dd4b 100644 --- a/urcu.h +++ b/urcu.h @@ -33,6 +33,9 @@ #include #include +#include +#include +#include /* * See urcu-pointer.h and urcu/static/urcu-pointer.h for pointer diff --git a/urcu/static/urcu-bp.h b/urcu/static/urcu-bp.h index b6d5f13..0bdefff 100644 --- a/urcu/static/urcu-bp.h +++ b/urcu/static/urcu-bp.h @@ -39,6 +39,7 @@ #include #include #include +#include /* * This code section can only be included in LGPL 2.1 compatible source code. @@ -155,6 +156,7 @@ static inline void _rcu_read_lock(void) { unsigned long tmp; + rcu_read_lock_debug(); if (caa_unlikely(!URCU_TLS(rcu_reader))) rcu_bp_register(); /* If not yet registered. */ cmm_barrier(); /* Ensure the compiler does not reorder us with mutex */ @@ -175,6 +177,7 @@ static inline void _rcu_read_unlock(void) cmm_smp_mb(); _CMM_STORE_SHARED(URCU_TLS(rcu_reader)->ctr, URCU_TLS(rcu_reader)->ctr - RCU_GP_COUNT); cmm_barrier(); /* Ensure the compiler does not reorder us with mutex */ + rcu_read_unlock_debug(); } /* diff --git a/urcu/static/urcu-qsbr.h b/urcu/static/urcu-qsbr.h index 8f2ca32..1ef830a 100644 --- a/urcu/static/urcu-qsbr.h +++ b/urcu/static/urcu-qsbr.h @@ -43,6 +43,7 @@ #include #include #include +#include #ifdef __cplusplus extern "C" { @@ -132,6 +133,7 @@ static inline enum rcu_state rcu_reader_state(unsigned long *ctr) */ static inline void _rcu_read_lock(void) { + rcu_read_lock_debug(); rcu_assert(URCU_TLS(rcu_reader).ctr); } @@ -144,6 +146,7 @@ static inline void _rcu_read_lock(void) */ static inline void _rcu_read_unlock(void) { + rcu_read_unlock_debug(); } /* diff --git a/urcu/static/urcu.h b/urcu/static/urcu.h index b5fc09f..0fe32e8 100644 --- a/urcu/static/urcu.h +++ b/urcu/static/urcu.h @@ -42,6 +42,7 @@ #include #include #include +#include #ifdef __cplusplus extern "C" { @@ -219,6 +220,7 @@ static inline void _rcu_read_lock(void) { unsigned long tmp; + rcu_read_lock_debug(); cmm_barrier(); tmp = URCU_TLS(rcu_reader).ctr; _rcu_read_lock_update(tmp); @@ -255,6 +257,7 @@ static inline void _rcu_read_unlock(void) tmp = URCU_TLS(rcu_reader).ctr; _rcu_read_unlock_update_and_wakeup(tmp); cmm_barrier(); /* Ensure the compiler does not reorder us with mutex */ + rcu_read_unlock_debug(); } /* diff --git a/urcu/urcu-checker.h b/urcu/urcu-checker.h new file mode 100644 index 0000000..cba6a0d --- /dev/null +++ b/urcu/urcu-checker.h @@ -0,0 +1,32 @@ +#ifndef _URCU_CHECKER_H +#define _URCU_CHECKER_H + +/* + * urcu-checker.h + * + * Userspace RCU checker header + * + * Copyright (c) 2014 Mathieu Desnoyers + * + * LGPL-compatible code should include this header with : + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +void rcu_read_lock_debug(void); +void rcu_read_unlock_debug(void); +void rcu_read_ongoing_check_debug(const char *func); + +#endif /* _URCU_CHECKER_H */ -- 2.34.1