From 1ff74ba8107f0783570d6e4c80a82fdc8a876427 Mon Sep 17 00:00:00 2001 From: Yuval Tassa Date: Thu, 11 Dec 2025 04:33:33 -0800 Subject: [PATCH] Change the type of state signature arguments from `unsigned int` to `int` PiperOrigin-RevId: 843162492 Change-Id: I51d3324f088e8055898e115bdd32267c5950a412 --- doc/changelog.rst | 7 +++- doc/includes/references.h | 12 +++--- include/mujoco/mujoco.h | 12 +++--- python/mujoco/introspect/functions.py | 12 +++--- src/engine/engine_support.c | 53 ++++++++++++++++++++++----- src/engine/engine_support.h | 12 +++--- test/engine/engine_support_test.cc | 10 ++++- unity/Runtime/Bindings/MjBindings.cs | 10 ++--- wasm/codegen/generated/bindings.cc | 10 ++--- 9 files changed, 92 insertions(+), 46 deletions(-) diff --git a/doc/changelog.rst b/doc/changelog.rst index 7d8c9bb9..8c0adb30 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -7,10 +7,15 @@ Upcoming version (not yet released) General ^^^^^^^ +- Non-breaking ABI changes: + + - The type of the ``sig`` (signature) argument of :ref:`mj_stateSize` and related functions has been changed from + ``unsigned int`` to ``int``. Before this change, invalid negative arguments passed to this function would result in + a silent implicit cast, now negativity will trigger an error. MJX ^^^ -1. Added ``actuator_length``, ``cdof`` and ``cdof_dof`` fields to ``mjx.Data``. +- Added ``actuator_length``, ``cdof`` and ``cdof_dof`` fields to ``mjx.Data``. Bug fixes ^^^^^^^^^ diff --git a/doc/includes/references.h b/doc/includes/references.h index cb9013d1..632a575c 100644 --- a/doc/includes/references.h +++ b/doc/includes/references.h @@ -3178,12 +3178,12 @@ void mj_projectConstraint(const mjModel* m, mjData* d); void mj_referenceConstraint(const mjModel* m, mjData* d); void mj_constraintUpdate(const mjModel* m, mjData* d, const mjtNum* jar, mjtNum cost[1], int flg_coneHessian); -int mj_stateSize(const mjModel* m, unsigned int sig); -void mj_getState(const mjModel* m, const mjData* d, mjtNum* state, unsigned int sig); -void mj_extractState(const mjModel* m, const mjtNum* src, unsigned int srcsig, - mjtNum* dst, unsigned int dstsig); -void mj_setState(const mjModel* m, mjData* d, const mjtNum* state, unsigned int sig); -void mj_copyState(const mjModel* m, const mjData* src, mjData* dst, unsigned int sig); +int mj_stateSize(const mjModel* m, int sig); +void mj_getState(const mjModel* m, const mjData* d, mjtNum* state, int sig); +void mj_extractState(const mjModel* m, const mjtNum* src, int srcsig, + mjtNum* dst, int dstsig); +void mj_setState(const mjModel* m, mjData* d, const mjtNum* state, int sig); +void mj_copyState(const mjModel* m, const mjData* src, mjData* dst, int sig); void mj_setKeyframe(mjModel* m, const mjData* d, int k); int mj_addContact(const mjModel* m, mjData* d, const mjContact* con); int mj_isPyramidal(const mjModel* m); diff --git a/include/mujoco/mujoco.h b/include/mujoco/mujoco.h index 1d517431..d3a286fe 100644 --- a/include/mujoco/mujoco.h +++ b/include/mujoco/mujoco.h @@ -471,20 +471,20 @@ MJAPI void mj_constraintUpdate(const mjModel* m, mjData* d, const mjtNum* jar, //---------------------------------- Support ------------------------------------------------------- // Return size of state signature. -MJAPI int mj_stateSize(const mjModel* m, unsigned int sig); +MJAPI int mj_stateSize(const mjModel* m, int sig); // Get state. -MJAPI void mj_getState(const mjModel* m, const mjData* d, mjtNum* state, unsigned int sig); +MJAPI void mj_getState(const mjModel* m, const mjData* d, mjtNum* state, int sig); // Extract a subset of components from a state previously obtained via mj_getState. -MJAPI void mj_extractState(const mjModel* m, const mjtNum* src, unsigned int srcsig, - mjtNum* dst, unsigned int dstsig); +MJAPI void mj_extractState(const mjModel* m, const mjtNum* src, int srcsig, + mjtNum* dst, int dstsig); // Set state. -MJAPI void mj_setState(const mjModel* m, mjData* d, const mjtNum* state, unsigned int sig); +MJAPI void mj_setState(const mjModel* m, mjData* d, const mjtNum* state, int sig); // Copy state from src to dst. -MJAPI void mj_copyState(const mjModel* m, const mjData* src, mjData* dst, unsigned int sig); +MJAPI void mj_copyState(const mjModel* m, const mjData* src, mjData* dst, int sig); // Copy current state to the k-th model keyframe. MJAPI void mj_setKeyframe(mjModel* m, const mjData* d, int k); diff --git a/python/mujoco/introspect/functions.py b/python/mujoco/introspect/functions.py index 4110a182..f0a25c93 100644 --- a/python/mujoco/introspect/functions.py +++ b/python/mujoco/introspect/functions.py @@ -2461,7 +2461,7 @@ FUNCTIONS: Mapping[str, FunctionDecl] = dict([ ), FunctionParameterDecl( name='sig', - type=ValueType(name='unsigned int'), + type=ValueType(name='int'), ), ), doc='Return size of state signature.', @@ -2491,7 +2491,7 @@ FUNCTIONS: Mapping[str, FunctionDecl] = dict([ ), FunctionParameterDecl( name='sig', - type=ValueType(name='unsigned int'), + type=ValueType(name='int'), ), ), doc='Get state.', @@ -2515,7 +2515,7 @@ FUNCTIONS: Mapping[str, FunctionDecl] = dict([ ), FunctionParameterDecl( name='srcsig', - type=ValueType(name='unsigned int'), + type=ValueType(name='int'), ), FunctionParameterDecl( name='dst', @@ -2525,7 +2525,7 @@ FUNCTIONS: Mapping[str, FunctionDecl] = dict([ ), FunctionParameterDecl( name='dstsig', - type=ValueType(name='unsigned int'), + type=ValueType(name='int'), ), ), doc='Extract a subset of components from a state previously obtained via mj_getState.', # pylint: disable=line-too-long @@ -2555,7 +2555,7 @@ FUNCTIONS: Mapping[str, FunctionDecl] = dict([ ), FunctionParameterDecl( name='sig', - type=ValueType(name='unsigned int'), + type=ValueType(name='int'), ), ), doc='Set state.', @@ -2585,7 +2585,7 @@ FUNCTIONS: Mapping[str, FunctionDecl] = dict([ ), FunctionParameterDecl( name='sig', - type=ValueType(name='unsigned int'), + type=ValueType(name='int'), ), ), doc='Copy state from src to dst.', diff --git a/src/engine/engine_support.c b/src/engine/engine_support.c index 2cd58f6e..78ca676e 100644 --- a/src/engine/engine_support.c +++ b/src/engine/engine_support.c @@ -164,9 +164,15 @@ static inline const mjtNum* mj_stateElemConstPtr(const mjModel* m, const mjData* // get size of state signature -int mj_stateSize(const mjModel* m, unsigned int sig) { +int mj_stateSize(const mjModel* m, int sig) { + if (sig < 0) { + mjERROR("invalid state signature %d < 0", sig); + return 0; + } + if (sig >= (1<= 2^mjNSTATE", sig); + mjERROR("invalid state signature %d >= 2^mjNSTATE", sig); + return 0; } int size = 0; @@ -182,9 +188,15 @@ int mj_stateSize(const mjModel* m, unsigned int sig) { // get state -void mj_getState(const mjModel* m, const mjData* d, mjtNum* state, unsigned int sig) { +void mj_getState(const mjModel* m, const mjData* d, mjtNum* state, int sig) { + if (sig < 0) { + mjERROR("invalid state signature %d < 0", sig); + return; + } + if (sig >= (1<= 2^mjNSTATE", sig); + mjERROR("invalid state signature %d >= 2^mjNSTATE", sig); + return; } int adr = 0; @@ -213,8 +225,17 @@ void mj_getState(const mjModel* m, const mjData* d, mjtNum* state, unsigned int // extract a sub-state from a state -void mj_extractState(const mjModel* m, const mjtNum* src, unsigned int srcsig, - mjtNum* dst, unsigned int dstsig) { +void mj_extractState(const mjModel* m, const mjtNum* src, int srcsig, mjtNum* dst, int dstsig) { + if (srcsig < 0) { + mjERROR("invalid srcsig %d < 0", srcsig); + return; + } + + if (srcsig >= (1<= 2^mjNSTATE", srcsig); + return; + } + if ((srcsig & dstsig) != dstsig) { mjERROR("dstsig is not a subset of srcsig"); return; @@ -235,9 +256,15 @@ void mj_extractState(const mjModel* m, const mjtNum* src, unsigned int srcsig, // set state -void mj_setState(const mjModel* m, mjData* d, const mjtNum* state, unsigned int sig) { +void mj_setState(const mjModel* m, mjData* d, const mjtNum* state, int sig) { + if (sig < 0) { + mjERROR("invalid state signature %d < 0", sig); + return; + } + if (sig >= (1<= 2^mjNSTATE", sig); + mjERROR("invalid state signature %d >= 2^mjNSTATE", sig); + return; } int adr = 0; @@ -266,9 +293,15 @@ void mj_setState(const mjModel* m, mjData* d, const mjtNum* state, unsigned int // copy state from src to dst -void mj_copyState(const mjModel* m, const mjData* src, mjData* dst, unsigned int sig) { +void mj_copyState(const mjModel* m, const mjData* src, mjData* dst, int sig) { + if (sig < 0) { + mjERROR("invalid state signature %d < 0", sig); + return; + } + if (sig >= (1<= 2^mjNSTATE", sig); + mjERROR("invalid state signature %d >= 2^mjNSTATE", sig); + return; } for (int i=0; i < mjNSTATE; i++) { diff --git a/src/engine/engine_support.h b/src/engine/engine_support.h index 089b50fa..f4366b96 100644 --- a/src/engine/engine_support.h +++ b/src/engine/engine_support.h @@ -36,20 +36,20 @@ MJAPI extern const int mjCONDATA_SIZE[mjNCONDATA]; // TODO(tassa): expose in pu //-------------------------- get/set state --------------------------------------------------------- // return size of state signature -MJAPI int mj_stateSize(const mjModel* m, unsigned int sig); +MJAPI int mj_stateSize(const mjModel* m, int sig); // get state -MJAPI void mj_getState(const mjModel* m, const mjData* d, mjtNum* state, unsigned int sig); +MJAPI void mj_getState(const mjModel* m, const mjData* d, mjtNum* state, int sig); // extract a sub-state from a state -MJAPI void mj_extractState(const mjModel* m, const mjtNum* src, unsigned int srcsig, - mjtNum* dst, unsigned int dstsig); +MJAPI void mj_extractState(const mjModel* m, const mjtNum* src, int srcsig, + mjtNum* dst, int dstsig); // set state -MJAPI void mj_setState(const mjModel* m, mjData* d, const mjtNum* state, unsigned int sig); +MJAPI void mj_setState(const mjModel* m, mjData* d, const mjtNum* state, int sig); // copy state from src to dst -MJAPI void mj_copyState(const mjModel* m, const mjData* src, mjData* dst, unsigned int sig); +MJAPI void mj_copyState(const mjModel* m, const mjData* src, mjData* dst, int sig); // copy current state to the k-th model keyframe MJAPI void mj_setKeyframe(mjModel* m, const mjData* d, int k); diff --git a/test/engine/engine_support_test.cc b/test/engine/engine_support_test.cc index 036179b9..891a6f31 100644 --- a/test/engine/engine_support_test.cc +++ b/test/engine/engine_support_test.cc @@ -844,14 +844,22 @@ TEST_F(SupportTest, ExtractState) { std::strncpy(last_error_msg, msg, sizeof(last_error_msg)); ++error_count; }; + auto* old_mju_user_error = mju_user_error; mju_user_error = error_handler; + mj_extractState(model, nullptr, srcsig, nullptr, mjSTATE_QFRC_APPLIED); - mju_user_error = old_mju_user_error; EXPECT_EQ(error_count, 1); EXPECT_EQ(std::string_view(last_error_msg), "mj_extractState: dstsig is not a subset of srcsig"); + mj_extractState(model, nullptr, -1, nullptr, mjSTATE_QFRC_APPLIED); + EXPECT_EQ(error_count, 2); + EXPECT_EQ(std::string_view(last_error_msg), + "mj_extractState: invalid srcsig -1 < 0"); + + mju_user_error = old_mju_user_error; + mj_deleteData(data); mj_deleteModel(model); } diff --git a/unity/Runtime/Bindings/MjBindings.cs b/unity/Runtime/Bindings/MjBindings.cs index 0553fd8a..785d2532 100644 --- a/unity/Runtime/Bindings/MjBindings.cs +++ b/unity/Runtime/Bindings/MjBindings.cs @@ -6655,19 +6655,19 @@ public static unsafe extern void mj_referenceConstraint(mjModel_* m, mjData_* d) public static unsafe extern void mj_constraintUpdate(mjModel_* m, mjData_* d, double* jar, double* cost, int flg_coneHessian); [DllImport("mujoco", CallingConvention = CallingConvention.Cdecl)] -public static unsafe extern int mj_stateSize(mjModel_* m, uint sig); +public static unsafe extern int mj_stateSize(mjModel_* m, int sig); [DllImport("mujoco", CallingConvention = CallingConvention.Cdecl)] -public static unsafe extern void mj_getState(mjModel_* m, mjData_* d, double* state, uint sig); +public static unsafe extern void mj_getState(mjModel_* m, mjData_* d, double* state, int sig); [DllImport("mujoco", CallingConvention = CallingConvention.Cdecl)] -public static unsafe extern void mj_extractState(mjModel_* m, double* src, uint srcsig, double* dst, uint dstsig); +public static unsafe extern void mj_extractState(mjModel_* m, double* src, int srcsig, double* dst, int dstsig); [DllImport("mujoco", CallingConvention = CallingConvention.Cdecl)] -public static unsafe extern void mj_setState(mjModel_* m, mjData_* d, double* state, uint sig); +public static unsafe extern void mj_setState(mjModel_* m, mjData_* d, double* state, int sig); [DllImport("mujoco", CallingConvention = CallingConvention.Cdecl)] -public static unsafe extern void mj_copyState(mjModel_* m, mjData_* src, mjData_* dst, uint sig); +public static unsafe extern void mj_copyState(mjModel_* m, mjData_* src, mjData_* dst, int sig); [DllImport("mujoco", CallingConvention = CallingConvention.Cdecl)] public static unsafe extern void mj_setKeyframe(mjModel_* m, mjData_* d, int k); diff --git a/wasm/codegen/generated/bindings.cc b/wasm/codegen/generated/bindings.cc index 536e3d27..a0763b9e 100644 --- a/wasm/codegen/generated/bindings.cc +++ b/wasm/codegen/generated/bindings.cc @@ -7935,7 +7935,7 @@ int mj_copyBack_wrapper(MjSpec& s, const MjModel& m) { return mj_copyBack(s.get(), m.get()); } -void mj_copyState_wrapper(const MjModel& m, const MjData& src, MjData& dst, unsigned int sig) { +void mj_copyState_wrapper(const MjModel& m, const MjData& src, MjData& dst, int sig) { mj_copyState(m.get(), src.get(), dst.get(), sig); } @@ -7979,7 +7979,7 @@ void mj_energyVel_wrapper(const MjModel& m, MjData& d) { mj_energyVel(m.get(), d.get()); } -void mj_extractState_wrapper(const MjModel& m, const NumberArray& src, unsigned int srcsig, const val& dst, unsigned int dstsig) { +void mj_extractState_wrapper(const MjModel& m, const NumberArray& src, int srcsig, const val& dst, int dstsig) { UNPACK_ARRAY(mjtNum, src); UNPACK_VALUE(mjtNum, dst); mj_extractState(m.get(), src_.data(), srcsig, dst_.data(), dstsig); @@ -8039,7 +8039,7 @@ mjtNum mj_geomDistance_wrapper(const MjModel& m, const MjData& d, int geom1, int return mj_geomDistance(m.get(), d.get(), geom1, geom2, distmax, fromto_.data()); } -void mj_getState_wrapper(const MjModel& m, const MjData& d, const val& state, unsigned int sig) { +void mj_getState_wrapper(const MjModel& m, const MjData& d, const val& state, int sig) { UNPACK_VALUE(mjtNum, state); CHECK_SIZE(state, mj_stateSize(m.get(), sig)); mj_getState(m.get(), d.get(), state_.data(), sig); @@ -8363,7 +8363,7 @@ void mj_setKeyframe_wrapper(MjModel& m, const MjData& d, int k) { mj_setKeyframe(m.get(), d.get(), k); } -void mj_setState_wrapper(const MjModel& m, MjData& d, const NumberArray& state, unsigned int sig) { +void mj_setState_wrapper(const MjModel& m, MjData& d, const NumberArray& state, int sig) { UNPACK_ARRAY(mjtNum, state); CHECK_SIZE(state, mj_stateSize(m.get(), sig)); mj_setState(m.get(), d.get(), state_.data(), sig); @@ -8397,7 +8397,7 @@ void mj_solveM2_wrapper(const MjModel& m, MjData& d, const val& x, const NumberA mj_solveM2(m.get(), d.get(), x_.data(), y_.data(), sqrtInvD_.data(), n); } -int mj_stateSize_wrapper(const MjModel& m, unsigned int sig) { +int mj_stateSize_wrapper(const MjModel& m, int sig) { return mj_stateSize(m.get(), sig); }