Merge pull request #3363 from ebms03:fix-resolve-plugin-segfault

PiperOrigin-RevId: 947628510
Change-Id: Ied0fcd3bb30267a133f1c6f6d6799c9236bc6845

src/user/user_model.cc

@@ -5962,15 +5962,14 @@ void mjCModel::ResolvePlugin(mjCBase* obj, const std::string& plugin_name,
   else if (!*plugin_instance) {
     *plugin_instance =
       static_cast<mjCPlugin*>(FindObject(mjOBJ_PLUGIN, plugin_instance_name));
-    (*plugin_instance)->plugin_slot = plugin_slot;
     if (!*plugin_instance) {
       throw mjCError(
               obj, "unrecognized name '%s' for plugin instance", plugin_instance_name.c_str());
     }
+    (*plugin_instance)->plugin_slot = plugin_slot;
     if (plugin_slot != -1 && plugin_slot != (*plugin_instance)->plugin_slot) {
       throw mjCError(
               obj, "'plugin' attribute does not match that of the instance");
     }
-    plugin_slot = (*plugin_instance)->plugin_slot;
   }
 }

test/user/CMakeLists.txt

@@ -15,6 +15,9 @@
 mujoco_test(
   user_model_test
   ADDITIONAL_LINK_LIBRARIES absl::str_format compare_model
+  PROPERTIES
+  ENVIRONMENT
+  "MUJOCO_PLUGIN_DIR=$<TARGET_FILE_DIR:actuator>"
 )
 
 mujoco_test(user_objects_test)

test/user/user_model_test.cc

@@ -872,6 +872,43 @@ TEST_F(LengthRangeTest, LengthRangeThreading) {
   mj_deleteSpec(spec);
 }
 
+
+TEST_F(MujocoTest, ResolvePluginMissingInstanceThrowsError) {
+  // Instance name="my_pid_config" dos not match actuator plugin's instance="pid_config",
+  // this should throw an appropriate error
+  static constexpr char xml_mismatch[] = R"(
+  <mujoco>
+    <extension>
+      <plugin plugin="mujoco.pid">
+        <instance name="my_pid_config" />
+      </plugin>
+    </extension>
+    <worldbody>
+      <body name="block" pos="0 0 0.5">
+        <joint name="slide_z" type="slide" axis="0 0 1" />
+        <geom type="box" size="0.1 0.1 0.1" mass="1.0" rgba="0 0.7 0 1"/>
+      </body>
+    </worldbody>
+    <actuator>
+      <plugin name="pid_actuator" joint="slide_z" plugin="mujoco.pid" instance="pid_config" />
+    </actuator>
+  </mujoco>
+  )";
+
+  std::array<char, 1024> error_buffer;
+  mjSpec* spec = mj_parseXMLString(xml_mismatch, 0, error_buffer.data(), error_buffer.size());
+  ASSERT_THAT(spec, NotNull()) << error_buffer.data();
+
+  mjModel* model = mj_compile(spec, nullptr);
+  EXPECT_THAT(model, IsNull());
+
+  std::string error_msg = mjs_getError(spec);
+  EXPECT_THAT(error_msg, HasSubstr("unrecognized name 'pid_config' for plugin instance"));
+
+  if (model) mj_deleteModel(model);
+  mj_deleteSpec(spec);
+}
+
 // ----------------------------- test modeldir  --------------------------------
 
 TEST_F(MujocoTest, Modeldir) {