Fix crash when running the Gazebo GUI (#108)

The crash was caused by an access to an invalid address within Qt when trying to access the command line arguments to the application. These are passed as argc and argv to QApplication, but for argc, we were passing a reference to a local. Signed-off-by: Addisu Z. Taddese <[email protected]>
gazebosim · Apr 28, 2020 · d9e7c88 · d9e7c88
1 parent b01b33a
commit d9e7c88
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 12 deletions.
diff --git a/include/ignition/gazebo/gui/Gui.hh b/include/ignition/gazebo/gui/Gui.hh
@@ -34,19 +34,23 @@ namespace gui
 {
   /// \brief Run GUI application
   /// \param[in] _argc Number of command line arguments (Used when running
-  /// without ign-tools. Set to 0 if using ign-tools)
+  /// without ign-tools. Set to 1 if using ign-tools). Note: The object
+  /// referenced by this variable must continue to exist for the lifetime of the
+  /// application.
   /// \param[in] _argv Command line arguments (Used when running without
-  /// ign-tools. Set to nullptr if using ign-tools)
+  /// ign-tools. Set to the name of the application if using ign-tools)
   /// \param[in] _guiConfig The GUI configuration file. If nullptr, the default
   /// configuration from IGN_HOMEDIR/.ignition/gazebo/gui.config will be used.
-  IGNITION_GAZEBO_VISIBLE int runGui(int _argc, char **_argv,
+  IGNITION_GAZEBO_VISIBLE int runGui(int &_argc, char **_argv,
                                      const char *_guiConfig);
 
   /// \brief Create a Gazebo GUI application
   /// \param[in] _argc Number of command line arguments (Used when running
-  /// without ign-tools. Set to 0 if using ign-tools)
+  /// without ign-tools. Set to 1 if using ign-tools). Note: The object
+  /// referenced by this variable must continue to exist for the lifetime of the
+  /// application.
   /// \param[in] _argv Command line arguments (Used when running without
-  /// ign-tools. Set to nullptr if using ign-tools)
+  /// ign-tools. Set to the name of the application if using ign-tools)
   /// \param[in] _guiConfig The GUI configuration file. If nullptr, the default
   /// configuration from IGN_HOMEDIR/.ignition/gazebo/gui.config will be used.
   /// \param[in] _defaultGuiConfig The default GUI configuration file. If no
@@ -57,7 +61,7 @@ namespace gui
   /// \param[in] _loadPluginsFromSdf If true, plugins specified in the world
   /// SDFormat file will get loaded.
   IGNITION_GAZEBO_VISIBLE std::unique_ptr<ignition::gui::Application> createGui(
-      int _argc, char **_argv, const char *_guiConfig,
+      int &_argc, char **_argv, const char *_guiConfig,
       const char *_defaultGuiConfig = nullptr, bool _loadPluginsFromSdf = true);
 
 }  // namespace gui

diff --git a/src/gui/Gui.cc b/src/gui/Gui.cc
@@ -40,7 +40,7 @@ namespace gui
 {
 
 std::unique_ptr<ignition::gui::Application> createGui(
-    int _argc, char **_argv, const char *_guiConfig,
+    int &_argc, char **_argv, const char *_guiConfig,
     const char *_defaultGuiConfig, bool _loadPluginsFromSdf)
 {
   ignition::common::SignalHandler sigHandler;
@@ -52,13 +52,15 @@ std::unique_ptr<ignition::gui::Application> createGui(
 
   ignmsg << "Ignition Gazebo GUI    v" << IGNITION_GAZEBO_VERSION_FULL
          << std::endl;
-  // Temporary transport interface
-  auto tmp = std::make_unique<ignition::gazebo::TmpIface>();
 
   // Initialize Qt app
   auto app = std::make_unique<ignition::gui::Application>(_argc, _argv);
   app->AddPluginPath(IGN_GAZEBO_GUI_PLUGIN_INSTALL_DIR);
 
+  // Temporary transport interface
+  auto tmp = new ignition::gazebo::TmpIface();
+  tmp->setParent(app->Engine());
+
   auto guiFileHandler = new ignition::gazebo::gui::GuiFileHandler();
   guiFileHandler->setParent(app->Engine());
 
@@ -87,7 +89,7 @@ std::unique_ptr<ignition::gui::Application> createGui(
 
   // Let QML files use TmpIface' functions and properties
   auto context = new QQmlContext(app->Engine()->rootContext());
-  context->setContextProperty("TmpIface", tmp.get());
+  context->setContextProperty("TmpIface", tmp);
   context->setContextProperty("GuiFileHandler", guiFileHandler);
 
   // Instantiate GazeboDrawer.qml file into a component
@@ -257,7 +259,7 @@ std::unique_ptr<ignition::gui::Application> createGui(
 }
 
 //////////////////////////////////////////////////
-int runGui(int _argc, char **_argv, const char *_guiConfig)
+int runGui(int &_argc, char **_argv, const char *_guiConfig)
 {
   auto app = gazebo::gui::createGui(_argc, _argv, _guiConfig);
   if (nullptr != app)

diff --git a/src/ign.cc b/src/ign.cc
@@ -282,5 +282,16 @@ extern "C" IGNITION_GAZEBO_VISIBLE int runServer(const char *_sdfString,
 //////////////////////////////////////////////////
 extern "C" IGNITION_GAZEBO_VISIBLE int runGui(const char *_guiConfig)
 {
-  return ignition::gazebo::gui::runGui(0, nullptr, _guiConfig);
+  // argc and argv are going to be passed to a QApplication. The Qt
+  // documentation has a warning about these:
+  //  "Warning: The data referred to by argc and argv must stay valid for the
+  //  entire lifetime of the QApplication object. In addition, argc must be
+  //  greater than zero and argv must contain at least one valid character
+  //  string."
+  int argc = 1;
+  // Converting a string literal to char * is forbidden as of C++11. It can only
+  // be converted to a const char *. The const cast is here to prevent a warning
+  // since we do need to pass a char* to runGui
+  char *argv = const_cast<char *>("ign-gazebo-gui");
+  return ignition::gazebo::gui::runGui(argc, &argv, _guiConfig);
 }