Avoid potential UB in loggers using composition

This cast would work fine, but any other fields added to those loggers
would be left uninitialized.

meson.build

@@ -76,7 +76,7 @@ vst2_plugin_sources = [
 vst3_plugin_sources = [
   'src/common/communication/common.cpp',
   'src/common/logging/common.cpp',
-  'src/common/logging/vst2.cpp',
+  'src/common/logging/vst3.cpp',
   'src/common/configuration.cpp',
   'src/common/plugins.cpp',
   'src/common/utils.cpp',

src/common/communication/vst2.h

@@ -254,7 +254,8 @@ class EventHandler : public AdHocSocketHandler<Thread> {
             };
 
         this->receive_multi(
-            logging ? std::optional(std::ref(logging->first)) : std::nullopt,
+            logging ? std::optional(std::ref(logging->first.logger))
+                    : std::nullopt,
             [&](boost::asio::local::stream_protocol::socket& socket) {
                 process_event(socket, true);
             },

src/common/communication/vst3.h

@@ -174,9 +174,10 @@ class Vst3MessageHandler : public AdHocSocketHandler<Thread> {
                 write_object(socket, response);
             };
 
-        this->receive_multi(
-            logging ? std::optional(std::ref(logging->first)) : std::nullopt,
-            process_message);
+        this->receive_multi(logging
+                                ? std::optional(std::ref(logging->first.logger))
+                                : std::nullopt,
+                            process_message);
     }
 };
 

src/common/logging/vst2.cpp

@@ -18,6 +18,8 @@
 
 #include <sstream>
 
+Vst2Logger::Vst2Logger(Logger& generic_logger) : logger(generic_logger) {}
+
 std::optional<std::string> opcode_to_string(bool is_dispatch, int opcode) {
     if (is_dispatch) {
         // Opcodes for a plugin's dispatch function
@@ -316,7 +318,7 @@ std::optional<std::string> opcode_to_string(bool is_dispatch, int opcode) {
 }
 
 void Vst2Logger::log_get_parameter(int index) {
-    if (BOOST_UNLIKELY(verbosity >= Verbosity::most_events)) {
+    if (BOOST_UNLIKELY(logger.verbosity >= Logger::Verbosity::most_events)) {
         std::ostringstream message;
         message << ">> getParameter() " << index;
 
@@ -325,7 +327,7 @@ void Vst2Logger::log_get_parameter(int index) {
 }
 
 void Vst2Logger::log_get_parameter_response(float value) {
-    if (BOOST_UNLIKELY(verbosity >= Verbosity::most_events)) {
+    if (BOOST_UNLIKELY(logger.verbosity >= Logger::Verbosity::most_events)) {
         std::ostringstream message;
         message << "   getParameter() :: " << value;
 
@@ -334,7 +336,7 @@ void Vst2Logger::log_get_parameter_response(float value) {
 }
 
 void Vst2Logger::log_set_parameter(int index, float value) {
-    if (BOOST_UNLIKELY(verbosity >= Verbosity::most_events)) {
+    if (BOOST_UNLIKELY(logger.verbosity >= Logger::Verbosity::most_events)) {
         std::ostringstream message;
         message << ">> setParameter() " << index << " = " << value;
 
@@ -343,7 +345,7 @@ void Vst2Logger::log_set_parameter(int index, float value) {
 }
 
 void Vst2Logger::log_set_parameter_response() {
-    if (BOOST_UNLIKELY(verbosity >= Verbosity::most_events)) {
+    if (BOOST_UNLIKELY(logger.verbosity >= Logger::Verbosity::most_events)) {
         log("   setParameter() :: OK");
     }
 }
@@ -355,7 +357,7 @@ void Vst2Logger::log_event(bool is_dispatch,
                            const EventPayload& payload,
                            float option,
                            const std::optional<EventPayload>& value_payload) {
-    if (BOOST_UNLIKELY(verbosity >= Verbosity::most_events)) {
+    if (BOOST_UNLIKELY(logger.verbosity >= Logger::Verbosity::most_events)) {
         if (should_filter_event(is_dispatch, opcode)) {
             return;
         }
@@ -442,7 +444,7 @@ void Vst2Logger::log_event_response(
     intptr_t return_value,
     const EventResultPayload& payload,
     const std::optional<EventResultPayload>& value_payload) {
-    if (BOOST_UNLIKELY(verbosity >= Verbosity::most_events)) {
+    if (BOOST_UNLIKELY(logger.verbosity >= Logger::Verbosity::most_events)) {
         if (should_filter_event(is_dispatch, opcode)) {
             return;
         }
@@ -512,7 +514,7 @@ void Vst2Logger::log_event_response(
 }
 
 bool Vst2Logger::should_filter_event(bool is_dispatch, int opcode) const {
-    if (verbosity >= Verbosity::all_events) {
+    if (logger.verbosity >= Logger::Verbosity::all_events) {
         return false;
     }
 

src/common/logging/vst2.h

@@ -33,10 +33,24 @@
 std::optional<std::string> opcode_to_string(bool is_dispatch, int opcode);
 
 /**
- * Provides VST2 specific logging functionality for debugging plugins.
+ * Wraps around `Logger` to provide VST2 specific logging functionality for
+ * debugging plugins. This way we can have all the complex initialisation be
+ * performed in one place.
  */
-class Vst2Logger : public Logger {
+class Vst2Logger {
    public:
+    Vst2Logger(Logger& generic_logger);
+
+    /**
+     * @see Logger::log
+     */
+    inline void log(const std::string& message) { logger.log(message); }
+
+    /**
+     * @see Logger::log_trace
+     */
+    inline void log_trace(const std::string& message) { logger.log(message); }
+
     // The following functions are for logging specific events, they are only
     // enabled for verbosity levels higher than 1 (i.e. `Verbosity::events`)
     void log_get_parameter(int index);
@@ -60,6 +74,11 @@ class Vst2Logger : public Logger {
         const EventResultPayload& payload,
         const std::optional<EventResultPayload>& value_payload);
 
+    /**
+     * The underlying logger instance we're wrapping.
+     */
+    Logger& logger;
+
    private:
     /**
      * Determine whether an event should be filtered based on the current

src/common/logging/vst3.cpp

@@ -0,0 +1,19 @@
+// yabridge: a Wine VST bridge
+// Copyright (C) 2020  Robbert van der Helm
+//
+// 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 3 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, see <https://www.gnu.org/licenses/>.
+
+#include "vst3.h"
+
+Vst3Logger::Vst3Logger(Logger& generic_logger) : logger(generic_logger) {}

src/common/logging/vst3.h

@@ -19,9 +19,25 @@
 #include "common.h"
 
 /**
- * Provides VST3-specific logging functionality for debugging plugins.
+ * Wraps around `Logger` to provide VST3 specific logging functionality for
+ * debugging plugins. This way we can have all the complex initialisation be
+ * performed in one place.
  */
-class Vst3Logger : public Logger {
+class Vst3Logger {
    public:
+    Vst3Logger(Logger& generic_logger);
+
+    /**
+     * @see Logger::log
+     */
+    inline void log(const std::string& message) { logger.log(message); }
+
+    /**
+     * @see Logger::log_trace
+     */
+    inline void log_trace(const std::string& message) { logger.log(message); }
+
     // TODO: Logging interface for VST3 plugins
+
+    Logger& logger;
 };

src/plugin/bridges/vst2.cpp

@@ -55,9 +55,7 @@ Vst2PluginBridge::Vst2PluginBridge(audioMasterCallback host_callback)
       // bridge will crash otherwise
       plugin(),
       host_callback_function(host_callback),
-      // TODO: This is UB, use composition with `generic_logger` instead
-      logger(static_cast<Vst2Logger&&>(Logger::create_from_environment(
-          create_logger_prefix(sockets.base_dir)))) {
+      logger(generic_logger) {
     log_init_message();
 
     // This will block until all sockets have been connected to by the Wine VST

src/plugin/bridges/vst3.cpp

@@ -27,9 +27,7 @@ Vst3PluginBridge::Vst3PluginBridge()
                                              .string()),
                   true);
           }),
-      // TODO: This is UB, use composition with `generic_logger` instead
-      logger(static_cast<Vst3Logger&&>(Logger::create_from_environment(
-          create_logger_prefix(sockets.base_dir)))) {
+      logger(generic_logger) {
     log_init_message();
 
     // This will block until all sockets have been connected to by the Wine VST