From 2875743718fb23293190b5edd0f2b530977270b1 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Wed, 28 Jul 2021 22:48:21 -0400 Subject: [PATCH] Add C++ build tests MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Some urcu header files cause build failures when included in C++ programs. Add a test file that includes all exported headers (except those that are marked as deprecated) and build that test file as a C and C++ program, with and without _LGPL_SOURCE defined (to test both the static and non-static implementations). This helps ensure that the code in these headers works as both languages. This alone does not ensure full coverage, there may be code in unused macros that would need fixing. But by including the "static" headers, this already finds a few issues. The test doesn't run anything, its purpose is only to verify that things build. This catches the following build failures: - clang++ doesn't know the __transparent_union__ attribute, place some pragmas to ignore this warning around where __transparent_union__ is used. - CDS_WFS_WOULDBLOCK and CDS_WFS_END are cast to a void*. This doesn't work in C++ when assigning to a field them to typed pointer. Fix them by casting to the appropriate type. - The transparent union trick doesn't work in C++: CXX test_build_cxx.o In file included from /home/simark/src/urcu/include/urcu/wfstack.h:116, from /home/simark/src/urcu/include/urcu/cds.h:35, from /home/simark/src/urcu/tests/unit/test_build_cxx.cpp:34: /home/simark/src/urcu/include/urcu/static/wfstack.h: In function ‘cds_wfs_node* _cds_wfs_pop_with_state_blocking(cds_wfs_stack*, int*)’: /home/simark/src/urcu/include/urcu/static/wfstack.h:350:54: error: could not convert ‘s’ from ‘cds_wfs_stack*’ to ‘cds_wfs_stack_ptr_t’ 350 | retnode = ___cds_wfs_pop_with_state_blocking(s, state); | ^ | | | cds_wfs_stack* A C++ user can fall back to instantiating a cds_wfs_stack_ptr_t explicitly, assigning the right field, and passing the cds_wfs_stack_ptr_t to the function. Fix a few instances in the static headers. A follow up commit will introduce C++ API wrappers based on function overloading to provide a C++ API similar to the C API. Signed-off-by: Simon Marchi Signed-off-by: Mathieu Desnoyers Change-Id: I30adc8df69414a0a019c5ec081f64cfac64843f5 --- include/urcu/compiler.h | 6 +++ include/urcu/lfstack.h | 5 ++- include/urcu/static/lfstack.h | 8 +++- include/urcu/static/rculfqueue.h | 3 +- include/urcu/static/wfstack.h | 10 +++-- include/urcu/wfstack.h | 6 ++- tests/unit/Makefile.am | 24 ++++++++++- tests/unit/test_build.c | 69 ++++++++++++++++++++++++++++++++ tests/unit/test_build_cxx.cpp | 3 ++ 9 files changed, 124 insertions(+), 10 deletions(-) create mode 100644 tests/unit/test_build.c create mode 100644 tests/unit/test_build_cxx.cpp diff --git a/include/urcu/compiler.h b/include/urcu/compiler.h index 34eb564..1573467 100644 --- a/include/urcu/compiler.h +++ b/include/urcu/compiler.h @@ -119,4 +119,10 @@ + __GNUC_PATCHLEVEL__) #endif +#ifndef __cplusplus +#define caa_c_transparent_union __attribute__((__transparent_union__)) +#else +#define caa_c_transparent_union +#endif + #endif /* _URCU_COMPILER_H */ diff --git a/include/urcu/lfstack.h b/include/urcu/lfstack.h index 5a9bca3..43ef2a5 100644 --- a/include/urcu/lfstack.h +++ b/include/urcu/lfstack.h @@ -29,6 +29,7 @@ extern "C" { #include #include +#include /* * Lock-free stack. @@ -83,11 +84,13 @@ struct cds_lfs_stack { * The transparent union allows calling functions that work on both * struct cds_lfs_stack and struct __cds_lfs_stack on any of those two * types. + * + * Avoid complaints from clang++ not knowing this attribute. */ typedef union { struct __cds_lfs_stack *_s; struct cds_lfs_stack *s; -} __attribute__((__transparent_union__)) cds_lfs_stack_ptr_t; +} caa_c_transparent_union cds_lfs_stack_ptr_t; #ifdef _LGPL_SOURCE diff --git a/include/urcu/static/lfstack.h b/include/urcu/static/lfstack.h index 6c82b42..a05acb4 100644 --- a/include/urcu/static/lfstack.h +++ b/include/urcu/static/lfstack.h @@ -289,9 +289,11 @@ struct cds_lfs_node * _cds_lfs_pop_blocking(struct cds_lfs_stack *s) { struct cds_lfs_node *retnode; + cds_lfs_stack_ptr_t stack; _cds_lfs_pop_lock(s); - retnode = ___cds_lfs_pop(s); + stack.s = s; + retnode = ___cds_lfs_pop(stack); _cds_lfs_pop_unlock(s); return retnode; } @@ -304,9 +306,11 @@ struct cds_lfs_head * _cds_lfs_pop_all_blocking(struct cds_lfs_stack *s) { struct cds_lfs_head *rethead; + cds_lfs_stack_ptr_t stack; _cds_lfs_pop_lock(s); - rethead = ___cds_lfs_pop_all(s); + stack.s = s; + rethead = ___cds_lfs_pop_all(stack); _cds_lfs_pop_unlock(s); return rethead; } diff --git a/include/urcu/static/rculfqueue.h b/include/urcu/static/rculfqueue.h index a8e1091..ad73454 100644 --- a/include/urcu/static/rculfqueue.h +++ b/include/urcu/static/rculfqueue.h @@ -66,7 +66,8 @@ struct cds_lfq_node_rcu *make_dummy(struct cds_lfq_queue_rcu *q, { struct cds_lfq_node_rcu_dummy *dummy; - dummy = malloc(sizeof(struct cds_lfq_node_rcu_dummy)); + dummy = (struct cds_lfq_node_rcu_dummy *) + malloc(sizeof(struct cds_lfq_node_rcu_dummy)); urcu_posix_assert(dummy); dummy->parent.next = next; dummy->parent.dummy = 1; diff --git a/include/urcu/static/wfstack.h b/include/urcu/static/wfstack.h index bd7ba68..088e6e3 100644 --- a/include/urcu/static/wfstack.h +++ b/include/urcu/static/wfstack.h @@ -37,7 +37,7 @@ extern "C" { #endif -#define CDS_WFS_END ((void *) 0x1UL) +#define CDS_WFS_END ((struct cds_wfs_head *) 0x1UL) #define CDS_WFS_ADAPT_ATTEMPTS 10 /* Retry if being set */ #define CDS_WFS_WAIT 10 /* Wait 10 ms if being set */ @@ -345,9 +345,11 @@ struct cds_wfs_node * _cds_wfs_pop_with_state_blocking(struct cds_wfs_stack *s, int *state) { struct cds_wfs_node *retnode; + cds_wfs_stack_ptr_t stack; _cds_wfs_pop_lock(s); - retnode = ___cds_wfs_pop_with_state_blocking(s, state); + stack.s = s; + retnode = ___cds_wfs_pop_with_state_blocking(stack, state); _cds_wfs_pop_unlock(s); return retnode; } @@ -370,9 +372,11 @@ struct cds_wfs_head * _cds_wfs_pop_all_blocking(struct cds_wfs_stack *s) { struct cds_wfs_head *rethead; + cds_wfs_stack_ptr_t stack; _cds_wfs_pop_lock(s); - rethead = ___cds_wfs_pop_all(s); + stack.s = s; + rethead = ___cds_wfs_pop_all(stack); _cds_wfs_pop_unlock(s); return rethead; } diff --git a/include/urcu/wfstack.h b/include/urcu/wfstack.h index 1058fba..016d9f2 100644 --- a/include/urcu/wfstack.h +++ b/include/urcu/wfstack.h @@ -57,7 +57,7 @@ extern "C" { * synchronization. */ -#define CDS_WFS_WOULDBLOCK ((void *) -1UL) +#define CDS_WFS_WOULDBLOCK ((struct cds_wfs_node *) -1UL) enum cds_wfs_state { CDS_WFS_STATE_LAST = (1U << 0), @@ -95,11 +95,13 @@ struct cds_wfs_stack { * The transparent union allows calling functions that work on both * struct cds_wfs_stack and struct __cds_wfs_stack on any of those two * types. + * + * Avoid complaints from clang++ not knowing this attribute. */ typedef union { struct __cds_wfs_stack *_s; struct cds_wfs_stack *s; -} __attribute__((__transparent_union__)) cds_wfs_stack_ptr_t; +} caa_c_transparent_union cds_wfs_stack_ptr_t; #ifdef _LGPL_SOURCE diff --git a/tests/unit/Makefile.am b/tests/unit/Makefile.am index 119dc73..8e2aa8a 100644 --- a/tests/unit/Makefile.am +++ b/tests/unit/Makefile.am @@ -16,7 +16,11 @@ noinst_PROGRAMS = \ test_urcu_multiflavor_single_unit \ test_urcu_multiflavor_single_unit_cxx \ test_urcu_multiflavor_single_unit_dynlink \ - test_urcu_multiflavor_single_unit_dynlink_cxx + test_urcu_multiflavor_single_unit_dynlink_cxx \ + test_build \ + test_build_cxx \ + test_build_dynlink \ + test_build_dynlink_cxx TESTS = $(noinst_PROGRAMS) @@ -99,6 +103,24 @@ test_urcu_multiflavor_single_unit_dynlink_cxx_CXXFLAGS = -DDYNAMIC_LINK_TEST $(A test_urcu_multiflavor_single_unit_dynlink_cxx_LDADD = $(URCU_LIB) $(URCU_MB_LIB) \ $(URCU_SIGNAL_LIB) $(URCU_QSBR_LIB) $(URCU_BP_LIB) $(TAP_LIB) +test_build_SOURCES = \ + test_build.c +test_build_LDADD = $(URCU_COMMON_LIB) $(TAP_LIB) + +test_build_cxx_SOURCES = \ + test_build_cxx.cpp +test_build_cxx_LDADD = $(URCU_COMMON_LIB) $(TAP_LIB) + +test_build_dynlink_SOURCES = \ + test_build.c +test_build_dynlink_CFLAGS = -DDYNAMIC_LINK_TEST $(AM_CFLAGS) +test_build_dynlink_LDADD = $(URCU_COMMON_LIB) $(TAP_LIB) + +test_build_dynlink_cxx_SOURCES = \ + test_build_cxx.cpp +test_build_dynlink_cxx_CXXFLAGS = -DDYNAMIC_LINK_TEST $(AM_CXXFLAGS) +test_build_dynlink_cxx_LDADD = $(URCU_COMMON_LIB) $(TAP_LIB) + all-local: @if [ x"$(srcdir)" != x"$(builddir)" ]; then \ for script in $(SCRIPT_LIST); do \ diff --git a/tests/unit/test_build.c b/tests/unit/test_build.c new file mode 100644 index 0000000..deb0a73 --- /dev/null +++ b/tests/unit/test_build.c @@ -0,0 +1,69 @@ +/* + * Copyright 2021 Simon Marchi + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program 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 General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +/* + * This file is meant to verify that headers are compatible with both C and + * C++. It includes all exported headers and is compiled as C and C++ source. + */ + +#ifndef DYNAMIC_LINK_TEST +# define _LGPL_SOURCE +#endif + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "tap.h" + +int main(void) +{ + /* Need at least 1 test to make a valid TAP test plan. */ + plan_tests(1); + ok(1, "dummy"); + + return exit_status(); +} diff --git a/tests/unit/test_build_cxx.cpp b/tests/unit/test_build_cxx.cpp new file mode 100644 index 0000000..5a45b6a --- /dev/null +++ b/tests/unit/test_build_cxx.cpp @@ -0,0 +1,3 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#include "test_build.c" -- 2.34.1