Use config.h file to fix use of #ifdef in headers

Ideally we would not use custom #ifdef in headers at all but this is a
bigger project for the future...

the bmv2 library with BMDEBUG_ON defined but within a separate project
(simple_switch_grpc) without BMDEBUG_ON defined, which was causing the
bm::Field class to have a different layout in the library and in the
application. This was causing different segfaults based on the other
preprocessor flags and compiler flags used and was difficult to diagnose
(wasn't caught by Valgrind).

To avoid this issue, we now rely on the config.h file generated from
configure.ac. This file needs to be installed because it is required by
installed bmv2 headers. To avoid interfering with other libraries, we
use AX_PREFIX_CONFIG_H to install config.h under @includedir@/bm and to
prefix all defined macros with BM_.

Fixes #715

Doxyfile

@@ -1935,7 +1935,7 @@ INCLUDE_FILE_PATTERNS  =
 # recursively expanded use the := operator instead of the = operator.
 # This tag requires that the tag ENABLE_PREPROCESSING is set to YES.
 
-PREDEFINED             = BMELOG_ON BMLOG_DEBUG_ON BMLOG_TRACE_ON
+PREDEFINED             = BM_ELOG_ON BM_LOG_DEBUG_ON BM_LOG_TRACE_ON
 
 # If the MACRO_EXPANSION and EXPAND_ONLY_PREDEF tags are set to YES then this
 # tag can be used to specify a list of macro names that should be expanded. The

autogen.sh

@@ -1,3 +1,6 @@
 #!/bin/sh
 
-exec autoreconf -fi
+# generates config.h.in
+autoheader
+
+autoreconf -fi

configure.ac

@@ -45,8 +45,6 @@ AC_ARG_WITH([pdfixed],
 
 AM_CONDITIONAL([COND_PDFIXED], [test "$want_pdfixed" = yes])
 
-MY_CPPFLAGS=""
-
 AC_ARG_WITH([nanomsg],
     AS_HELP_STRING([--with-nanomsg], [Build Nanomsg RPC service, if disabled then you must have some other way of controlling the switch]),
     [want_nanomsg="$withval"], [want_nanomsg=yes])
@@ -59,7 +57,7 @@ AC_ARG_ENABLE([debugger],
 AS_IF([test "x$enable_debugger" = "xyes"], [
     AS_IF([test "$want_nanomsg" = "yes"], [
         debugger_enabled=yes
-        MY_CPPFLAGS="$MY_CPPFLAGS -DBMDEBUG_ON"
+        AC_DEFINE([DEBUG_ON], [], [Enable debugger])
     ], [
         AC_MSG_ERROR([Cannot use debugger without nanomsg])
     ])
@@ -71,10 +69,11 @@ AC_ARG_ENABLE([logging_macros],
                    [Disable compile time debug and trace logging macros]))
 AS_IF([test "x$enable_logging_macros" != "xno"], [
     logging_macros_enabled=yes
-    MY_CPPFLAGS="$MY_CPPFLAGS -DBMLOG_DEBUG_ON -DBMLOG_TRACE_ON"
+    AC_DEFINE([LOG_DEBUG_ON], [], [Enable compile-time macro for debug logging])
+    AC_DEFINE([LOG_TRACE_ON], [], [Enable compile-time macro for trace logging])
 ])
 
-# BMELOG_ON is defined by default, since it is required for some tests
+# BM_ELOG_ON is defined by default, since it is required for some tests
 elogger_enabled=no
 AC_ARG_ENABLE([elogger],
     AS_HELP_STRING([--disable-elogger],
@@ -83,7 +82,7 @@ AC_ARG_ENABLE([elogger],
 AS_IF([test "x$enable_elogger" != "xno"], [
     AS_IF([test "$want_nanomsg" = "yes"], [
         elogger_enabled=yes
-        MY_CPPFLAGS="$MY_CPPFLAGS -DBMELOG_ON"
+        AC_DEFINE([ELOG_ON], [], [Enable nanomsg event logger])
     ], [
         AC_MSG_WARN([Cannot use elogger without nanomsg])
     ])
@@ -118,7 +117,7 @@ AC_ARG_ENABLE([WP4-16-stacks],
     [enable_WP4_16_stacks="$enableval"], [enable_WP4_16_stacks=yes])
 
 AS_IF([test "$enable_WP4_16_stacks" = "yes"],
-      [MY_CPPFLAGS="$MY_CPPFLAGS -DBM_WP4_16_STACKS"])
+      [AC_DEFINE([WP4_16_STACKS], [], [Implement stacks as per P4_16 spec])])
 
 # Checks for programs.
 AC_PROG_CXX
@@ -150,17 +149,17 @@ AS_IF([test "$want_thrift" = yes], [
     AS_IF([test "$want_p4thrift" = yes], [
         AC_PATH_PROG([THRIFT], [p4thrift], [])
         AC_SUBST([THRIFT_LIB], ["-lp4thrift"])
-        MY_CPPFLAGS="$MY_CPPFLAGS -DP4THRIFT"
+        AC_DEFINE([P4THRIFT], [], [Use P4.org Thrift fork])
         AC_CHECK_HEADER([p4thrift/P4Thrift.h], [], [AC_MSG_ERROR([P4Thrift headers not found. Install P4Thrift from http://github.com/p4lang/thrift/])])
     ], [
         AC_PATH_PROG([THRIFT], [thrift], [])
         AC_SUBST([THRIFT_LIB], ["-lthrift"])
         AC_CHECK_HEADER([thrift/Thrift.h], [], [AC_MSG_ERROR([Thrift headers not found. Install Thrift from http://thrift.apache.org/docs/install/])])
     ])
     AS_IF([test x"$THRIFT" = x], [AC_MSG_ERROR([cannot find thrift])])
-    MY_CPPFLAGS="$MY_CPPFLAGS -DBMTHRIFT_ON"
+    AC_DEFINE([THRIFT_ON], [], [Enable Thrift support])
     AC_CHECK_HEADER([thrift/stdcxx.h], [
-        MY_CPPFLAGS="$MY_CPPFLAGS -DHAVE_THRIFT_STDCXX_H"
+        AC_DEFINE([HAVE_THRIFT_STDCXX_H], [], [Found Thrift stdcxx wrapper])
     ], [])
 ])
 
@@ -178,7 +177,7 @@ utility vector], [], [AC_MSG_ERROR([Missing header file])])
 
 AS_IF([test "$want_nanomsg" = yes], [
     AC_CHECK_LIB([nanomsg], [nn_errno], [], [AC_MSG_ERROR([Missing libnanomsg])])
-    MY_CPPFLAGS="$MY_CPPFLAGS -DBMNANOMSG_ON"
+    AC_DEFINE([NANOMSG_ON], [], [Enable Nanomsg support])
 ])
 
 # Check for pthread, libjudy, libgmp, libpcap
@@ -222,7 +221,8 @@ AS_IF([test "x$enable_modules" != "xno"], [
   AC_MSG_CHECKING(for dlopen())
   AC_CHECK_HEADERS(dlfcn.h, [
     AC_SEARCH_LIBS([dlopen], [dl], [
-      MY_CPPFLAGS="$MY_CPPFLAGS -DBM_HAVE_DLOPEN -DBM_ENABLE_MODULES"
+      AC_DEFINE([HAVE_DLOPEN], [], [Found dlopen])
+      AC_DEFINE([ENABLE_MODULES], [], [Enable dynamic loading of modules])
       modules_enabled=yes
     ], [
       AC_MSG_RESULT(no)
@@ -252,8 +252,8 @@ AC_CHECK_HEADER([boost/program_options.hpp], [], [AC_MSG_ERROR([Missing boost pr
 AC_CHECK_HEADER([boost/functional/hash.hpp], [], [AC_MSG_ERROR([Missing boost functional hash header])])
 AC_CHECK_HEADER([boost/filesystem.hpp], [], [AC_MSG_ERROR([Missing boost filesystem header])])
 
-AC_SUBST([AM_CPPFLAGS], ["$MY_CPPFLAGS \
-                          -I\$(top_srcdir)/include \
+AC_SUBST([AM_CPPFLAGS], ["-I\$(top_srcdir)/include \
+                          -I\$(top_builddir)/include \
                           -isystem\$(top_srcdir)/third_party/jsoncpp/include \
                           -isystem\$(top_srcdir)/third_party/spdlog"])
 AC_SUBST([AM_CFLAGS], ["$PTHREAD_CFLAGS"])
@@ -308,6 +308,8 @@ AC_CONFIG_FILES([tests/utils.cpp
 AC_CONFIG_FILES([targets/simple_switch/tests/CLI_tests/run_one_test.py],
                 [chmod +x targets/simple_switch/tests/CLI_tests/run_one_test.py])
 
+AX_PREFIX_CONFIG_H([include/bm/config.h], [BM])
+
 AC_OUTPUT
 
 AS_ECHO("")

include/Makefile.am

@@ -1,6 +1,11 @@
 ACLOCAL_AMFLAGS = ${ACLOCAL_FLAGS} -I m4
 
-nobase_include_HEADERS =
+nobase_include_HEADERS = \
+bm/config.h
+
+distclean-local: distclean-ax-prefix-config-h
+distclean-ax-prefix-config-h:
+	rm -f bm/config.h
 
 if COND_NANOMSG
 nobase_include_HEADERS += \

include/bm/bm_runtime/bm_runtime.h

@@ -1,7 +1,9 @@
 #ifndef _BM_RUNTIME_BM_RUNTIME_H_
 #define _BM_RUNTIME_BM_RUNTIME_H_
 
-#ifdef P4THRIFT
+#include <bm/config.h>
+
+#ifdef BM_P4THRIFT
 #include <p4thrift/processor/TMultiplexedProcessor.h>
 
 namespace thrift_provider = p4::thrift;
@@ -36,4 +38,4 @@ int start_server(bm::SwitchWContexts *sw, int port);
 
 }
 
-#endif
+#endif  // _BM_RUNTIME_BM_RUNTIME_H_

include/bm/bm_sim/debugger.h

@@ -21,11 +21,11 @@
 #ifndef BM_BM_SIM_DEBUGGER_H_
 #define BM_BM_SIM_DEBUGGER_H_
 
+#include <bm/config.h>
+
 #include <limits>
 #include <string>
 
-// #define BMDEBUG_ON
-
 namespace bm {
 
 using device_id_t = uint64_t;
@@ -145,7 +145,7 @@ class DebuggerIface {
   virtual std::string get_addr_() const = 0;
 };
 
-#ifdef BMDEBUG_ON
+#ifdef BM_DEBUG_ON
 #define DEBUGGER_NOTIFY_UPDATE(packet_id, id, bytes, nbits) \
   Debugger::get()->notify_update(packet_id, id, bytes, nbits);
 #define DEBUGGER_NOTIFY_UPDATE_V(packet_id, id, v) \

include/bm/bm_sim/dev_mgr.h

@@ -35,6 +35,8 @@
 #ifndef BM_BM_SIM_DEV_MGR_H_
 #define BM_BM_SIM_DEV_MGR_H_
 
+#include <bm/config.h>
+
 #include <functional>
 #include <string>
 #include <map>
@@ -170,7 +172,7 @@ class DevMgr : public PacketDispatcherIface {
   // wait before starting to process packets.
   void set_dev_mgr_files(unsigned wait_time_in_seconds);
 
-#ifdef BMNANOMSG_ON
+#ifdef BM_NANOMSG_ON
   // if enforce ports is set to true, packets coming in on un-registered ports
   // are dropped
   void set_dev_mgr_packet_in(

include/bm/bm_sim/event_logger.h

@@ -23,6 +23,8 @@
 #ifndef BM_BM_SIM_EVENT_LOGGER_H_
 #define BM_BM_SIM_EVENT_LOGGER_H_
 
+#include <bm/config.h>
+
 #include <string>
 #include <memory>
 
@@ -124,7 +126,7 @@ class EventLogger {
 //! // packet processing
 //! BMELOG(packet_out, packet);
 //! @endcode
-#ifdef BMELOG_ON
+#ifdef BM_ELOG_ON
 #define BMELOG(fn, ...) bm::EventLogger::get()->fn(__VA_ARGS__)
 #else
 #define BMELOG(fn, ...)

include/bm/bm_sim/fields.h

@@ -23,6 +23,8 @@
 #ifndef BM_BM_SIM_FIELDS_H_
 #define BM_BM_SIM_FIELDS_H_
 
+#include <bm/config.h>
+
 #include <algorithm>  // for std::copy
 
 #include <cassert>
@@ -143,7 +145,7 @@ class Field : public Data {
 
   void reset_VL();
 
-#ifdef BMDEBUG_ON
+#ifdef BM_DEBUG_ON
   void set_id(uint64_t id) { my_id = id; }
   void set_packet_id(const Debugger::PacketId *id) { packet_id = id; }
 #else
@@ -189,7 +191,7 @@ class Field : public Data {
   Bignum mask{1};
   Bignum max{1};
   Bignum min{1};
-#ifdef BMDEBUG_ON
+#ifdef BM_DEBUG_ON
   uint64_t my_id{};
   const Debugger::PacketId *packet_id{&Debugger::dummy_PacketId};
 #endif

include/bm/bm_sim/headers.h

@@ -23,6 +23,8 @@
 #ifndef BM_BM_SIM_HEADERS_H_
 #define BM_BM_SIM_HEADERS_H_
 
+#include <bm/config.h>
+
 #include <vector>
 #include <string>
 #include <set>
@@ -265,7 +267,7 @@ class Header : public NamedP4Object {
   // same value.
   bool cmp(const Header &other) const;
 
-#ifdef BMDEBUG_ON
+#ifdef BM_DEBUG_ON
   void set_packet_id(const Debugger::PacketId *id);
 #else
   void set_packet_id(const Debugger::PacketId *) { }
@@ -313,7 +315,7 @@ class Header : public NamedP4Object {
   int nbytes_packet{0};
   std::unique_ptr<ArithExpression> VL_expr;
   std::unique_ptr<UnionMembership> union_membership{nullptr};
-#ifdef BMDEBUG_ON
+#ifdef BM_DEBUG_ON
   const Debugger::PacketId *packet_id{&Debugger::dummy_PacketId};
 #endif
 };

include/bm/bm_sim/logger.h

@@ -33,6 +33,7 @@
 #ifndef BM_BM_SIM_LOGGER_H_
 #define BM_BM_SIM_LOGGER_H_
 
+#include <bm/config.h>
 #include <bm/spdlog/spdlog.h>
 
 #include <string>
@@ -106,17 +107,17 @@ class Logger {
 
 }  // namespace bm
 
-#ifdef BMLOG_DEBUG_ON
+#ifdef BM_LOG_DEBUG_ON
 //! Preferred way (because can be disabled at compile time) to log a debug
-//! message. Is enabled by preprocessor BMLOG_DEBUG_ON.
+//! message. Is enabled by preprocessor BM_LOG_DEBUG_ON.
 #define BMLOG_DEBUG(...) bm::Logger::get()->debug(__VA_ARGS__);
 #else
 #define BMLOG_DEBUG(...)
 #endif
 
-#ifdef BMLOG_TRACE_ON
+#ifdef BM_LOG_TRACE_ON
 //! Preferred way (because can be disabled at compile time) to log a trace
-//! message. Is enabled by preprocessor BMLOG_TRACE_ON.
+//! message. Is enabled by preprocessor BM_LOG_TRACE_ON.
 #define BMLOG_TRACE(...) bm::Logger::get()->trace(__VA_ARGS__);
 #else
 #define BMLOG_TRACE(...)

include/bm/bm_sim/transport.h

@@ -21,6 +21,8 @@
 #ifndef BM_BM_SIM_TRANSPORT_H_
 #define BM_BM_SIM_TRANSPORT_H_
 
+#include <bm/config.h>
+
 #include <string>
 #include <initializer_list>
 #include <memory>
@@ -59,7 +61,7 @@ class TransportIface {
     return send_msgs_(msgs);
   }
 
-#ifdef BMNANOMSG_ON
+#ifdef BM_NANOMSG_ON
   static std::unique_ptr<TransportIface> make_nanomsg(const std::string &addr);
 #endif
   static std::unique_ptr<TransportIface> make_dummy();

include/bm/thrift/stdcxx.h

@@ -1,7 +1,9 @@
 #ifndef _BM_STDCXX_H_
 #define _BM_STDCXX_H_
 
-#ifdef HAVE_THRIFT_STDCXX_H
+#include <bm/config.h>
+
+#ifdef BM_HAVE_THRIFT_STDCXX_H
 #include <thrift/stdcxx.h>
 namespace stdcxx = thrift_provider::stdcxx;
 #else