From 0fddd8bb10991b5293441e0fe95978b1918fde06 Mon Sep 17 00:00:00 2001 From: Jessica Martinez Date: Wed, 29 Apr 2026 15:31:04 -0500 Subject: [PATCH 1/6] OMPE-91419: Fix silent ABI mismatch in cubric Python shim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The _cubric.py ctypes shim was pinned to IAdapter v0.1 vtable offsets, but newer Kit builds ship v0.2 — compute calls were silently landing on unbind, disabling cubric's GPU transform hierarchy propagation. carb accepts the version mismatch with only a stderr warning. Update offsets to the v0.2 layout, request v0.2 from the framework, and add a runtime InterfaceDesc check that refuses to acquire on any unexpected version. --- .../isaaclab_newton/physics/_cubric.py | 88 ++++++++++++++++--- 1 file changed, 78 insertions(+), 10 deletions(-) diff --git a/source/isaaclab_newton/isaaclab_newton/physics/_cubric.py b/source/isaaclab_newton/isaaclab_newton/physics/_cubric.py index cc549d4b82be..0e1d531efbc8 100644 --- a/source/isaaclab_newton/isaaclab_newton/physics/_cubric.py +++ b/source/isaaclab_newton/isaaclab_newton/physics/_cubric.py @@ -31,27 +31,34 @@ # Carb Framework struct layout (CARB_ABI function-pointer offsets, x86_64) # --------------------------------------------------------------------------- # Counting only CARB_ABI fields from the top of ``struct Framework``: -# 0: loadPluginsEx -# 8: unloadAllPlugins -# 16: acquireInterfaceWithClient -# 24: tryAcquireInterfaceWithClient ← we use this one +# 24 = tryAcquireInterfaceWithClient +# 96 = getInterfacePluginDesc _FW_OFF_TRY_ACQUIRE = 24 +_FW_OFF_GET_INTERFACE_PLUGIN_DESC = 96 # --------------------------------------------------------------------------- # IAdapter struct layout (from omni/cubric/IAdapter.h) # --------------------------------------------------------------------------- +# v0.2 layout: # 0: getAttribute # 8: create(AdapterId*) # 16: refcount # 24: retain # 32: release(AdapterId) # 40: bindToStage(AdapterId, const FabricId&) -# 48: unbind -# 56: compute(AdapterId, options, dirtyMode, outFlags*) +# 48: bindToStageWithListener +# 56: unbind +# 64: compute(AdapterId, options, dirtyMode, outFlags*) _IA_OFF_CREATE = 8 _IA_OFF_RELEASE = 32 _IA_OFF_BIND = 40 -_IA_OFF_COMPUTE = 56 +_IA_OFF_COMPUTE = 64 + +# Pinned IAdapter version. ``_verify_iadapter_version`` enforces an exact +# match — carb's 0.x version negotiation is permissive (minor mismatches +# yield only a stderr warning), so a silent miscall is otherwise possible. +_IA_EXPECTED_MAJOR = 0 +_IA_EXPECTED_MINOR = 2 # AdapterId sentinel _INVALID_ADAPTER_ID = ctypes.c_uint64(~0).value @@ -90,6 +97,13 @@ class _InterfaceDesc(ctypes.Structure): ] +# carb::PluginDesc offsets. PluginImplDesc occupies the first 40 bytes +# (3 char* + 4-byte hotReload + 4-byte pad + char*). +_PD_OFF_INTERFACES = 40 +_PD_OFF_INTERFACE_COUNT = 48 +_INTERFACE_DESC_STRIDE = 16 # char* + Version + + def _read_u64(addr: int) -> int: return ctypes.c_uint64.from_address(addr).value @@ -158,7 +172,7 @@ def initialize(self) -> bool: desc = _InterfaceDesc( name=b"omni::cubric::IAdapter", - version=_Version(0, 1), + version=_Version(_IA_EXPECTED_MAJOR, _IA_EXPECTED_MINOR), ) # Try several acquisition strategies — the required client name @@ -178,10 +192,15 @@ def initialize(self) -> bool: ia_ptr = acquire_fn(b"isaaclab.cubric", desc, None) if not ia_ptr: logger.warning( - "Could not acquire omni::cubric::IAdapter — " - "cubric plugin may not be registered or interface version mismatch" + "Could not acquire omni::cubric::IAdapter v%d.%d — plugin may not be " + "registered or its version is older. Falling back to update_world_xforms().", + _IA_EXPECTED_MAJOR, + _IA_EXPECTED_MINOR, ) return False + + if not self._verify_iadapter_version(fw_ptr, ia_ptr): + return False self._ia_ptr = ia_ptr # Wrap the four IAdapter function pointers we need. @@ -222,6 +241,55 @@ def initialize(self) -> bool: logger.info("cubric IAdapter bindings ready") return True + @staticmethod + def _verify_iadapter_version(fw_ptr: int, ia_ptr: int) -> bool: + """Confirm the acquired IAdapter advertises the expected version.""" + get_desc_addr = _read_u64(fw_ptr + _FW_OFF_GET_INTERFACE_PLUGIN_DESC) + if get_desc_addr == 0: + logger.warning("getInterfacePluginDesc is null in Framework") + return False + + get_desc_fn = ctypes.CFUNCTYPE(ctypes.c_void_p, ctypes.c_void_p)(get_desc_addr) + plugin_desc_ptr = get_desc_fn(ia_ptr) + if not plugin_desc_ptr: + logger.warning("getInterfacePluginDesc returned null for IAdapter") + return False + + interfaces_ptr = _read_u64(plugin_desc_ptr + _PD_OFF_INTERFACES) + interface_count = _read_u64(plugin_desc_ptr + _PD_OFF_INTERFACE_COUNT) + if interfaces_ptr == 0 or interface_count == 0: + logger.warning("PluginDesc reports zero interfaces for cubric plugin") + return False + + for i in range(interface_count): + entry_addr = interfaces_ptr + i * _INTERFACE_DESC_STRIDE + name_addr = _read_u64(entry_addr) + if name_addr == 0: + continue + if ctypes.string_at(name_addr) != b"omni::cubric::IAdapter": + continue + major = ctypes.c_uint32.from_address(entry_addr + 8).value + minor = ctypes.c_uint32.from_address(entry_addr + 12).value + if (major, minor) != (_IA_EXPECTED_MAJOR, _IA_EXPECTED_MINOR): + logger.warning( + "cubric IAdapter version mismatch: plugin reports v%d.%d, " + "shim is pinned to v%d.%d. The vtable layout may have " + "changed; see omni/cubric/IAdapter.h. Falling back to " + "update_world_xforms().", + major, + minor, + _IA_EXPECTED_MAJOR, + _IA_EXPECTED_MINOR, + ) + return False + return True + + logger.warning( + "cubric plugin does not advertise omni::cubric::IAdapter — unexpected. " + "Falling back to update_world_xforms()." + ) + return False + @property def available(self) -> bool: return self._ia_ptr != 0 From 4fad5e10eaa3b8a5784121adeb21d62fa1bab010 Mon Sep 17 00:00:00 2001 From: Jessica Martinez Date: Thu, 30 Apr 2026 13:05:49 -0500 Subject: [PATCH 2/6] OMPE-91419: Review feedback - Capped interface_count at 64 as a sanity check. - Bound string_at read length to expected value. - Clarified some comments. --- .../isaaclab_newton/physics/_cubric.py | 35 +++++++++++++++---- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/source/isaaclab_newton/isaaclab_newton/physics/_cubric.py b/source/isaaclab_newton/isaaclab_newton/physics/_cubric.py index 0e1d531efbc8..004b8056e868 100644 --- a/source/isaaclab_newton/isaaclab_newton/physics/_cubric.py +++ b/source/isaaclab_newton/isaaclab_newton/physics/_cubric.py @@ -31,8 +31,19 @@ # Carb Framework struct layout (CARB_ABI function-pointer offsets, x86_64) # --------------------------------------------------------------------------- # Counting only CARB_ABI fields from the top of ``struct Framework``: -# 24 = tryAcquireInterfaceWithClient -# 96 = getInterfacePluginDesc +# 0: loadPluginsEx +# 8: unloadAllPlugins +# 16: acquireInterfaceWithClient +# 24: tryAcquireInterfaceWithClient ← we use this one +# 32: acquireInterfaceFromInterfaceWithClient +# 40: tryAcquireInterfaceFromInterfaceWithClient +# 48: acquireInterfaceFromLibraryWithClient +# 56: tryAcquireInterfaceFromLibraryWithClient +# 64: getInterfacesCountEx +# 72: acquireInterfacesWithClient +# 80: releaseInterfaceWithClient +# 88: getPluginDesc +# 96: getInterfacePluginDesc ← we use this one _FW_OFF_TRY_ACQUIRE = 24 _FW_OFF_GET_INTERFACE_PLUGIN_DESC = 96 @@ -54,9 +65,7 @@ _IA_OFF_BIND = 40 _IA_OFF_COMPUTE = 64 -# Pinned IAdapter version. ``_verify_iadapter_version`` enforces an exact -# match — carb's 0.x version negotiation is permissive (minor mismatches -# yield only a stderr warning), so a silent miscall is otherwise possible. +# Expected IAdapter version. _IA_EXPECTED_MAJOR = 0 _IA_EXPECTED_MINOR = 2 @@ -243,7 +252,12 @@ def initialize(self) -> bool: @staticmethod def _verify_iadapter_version(fw_ptr: int, ia_ptr: int) -> bool: - """Confirm the acquired IAdapter advertises the expected version.""" + """Confirm the acquired IAdapter advertises the expected version. + + Carb's 0.x version negotiation is permissive — minor mismatches yield + only a stderr warning — so an exact-version match must be enforced + explicitly or a silent vtable miscall is possible. + """ get_desc_addr = _read_u64(fw_ptr + _FW_OFF_GET_INTERFACE_PLUGIN_DESC) if get_desc_addr == 0: logger.warning("getInterfacePluginDesc is null in Framework") @@ -260,13 +274,20 @@ def _verify_iadapter_version(fw_ptr: int, ia_ptr: int) -> bool: if interfaces_ptr == 0 or interface_count == 0: logger.warning("PluginDesc reports zero interfaces for cubric plugin") return False + if interface_count > 64: + logger.warning( + "PluginDesc interfaceCount suspiciously large (%d); struct layout mismatch?", + interface_count, + ) + return False for i in range(interface_count): entry_addr = interfaces_ptr + i * _INTERFACE_DESC_STRIDE name_addr = _read_u64(entry_addr) if name_addr == 0: continue - if ctypes.string_at(name_addr) != b"omni::cubric::IAdapter": + target_name = b"omni::cubric::IAdapter\x00" + if ctypes.string_at(name_addr, len(target_name)) != target_name: continue major = ctypes.c_uint32.from_address(entry_addr + 8).value minor = ctypes.c_uint32.from_address(entry_addr + 12).value From 778669386af8469b16029bd3912a0be2316f7930 Mon Sep 17 00:00:00 2001 From: Jessica Martinez Date: Thu, 30 Apr 2026 15:24:49 -0500 Subject: [PATCH 3/6] OMPE-91419: Review feedback - trust minor semver bumps, warn loudly on higher minor version mismatch, fall back to CPU (non-cubric) path on lower minor or major version mismatch. --- .../isaaclab_newton/physics/_cubric.py | 28 +++++++++++++------ 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/source/isaaclab_newton/isaaclab_newton/physics/_cubric.py b/source/isaaclab_newton/isaaclab_newton/physics/_cubric.py index 004b8056e868..3215bee67db1 100644 --- a/source/isaaclab_newton/isaaclab_newton/physics/_cubric.py +++ b/source/isaaclab_newton/isaaclab_newton/physics/_cubric.py @@ -252,11 +252,12 @@ def initialize(self) -> bool: @staticmethod def _verify_iadapter_version(fw_ptr: int, ia_ptr: int) -> bool: - """Confirm the acquired IAdapter advertises the expected version. + """Verify the acquired IAdapter is compatible with this shim's vtable offsets. - Carb's 0.x version negotiation is permissive — minor mismatches yield - only a stderr warning — so an exact-version match must be enforced - explicitly or a silent vtable miscall is possible. + Major mismatches and older minors return False (CPU fallback). Higher + minors are accepted under the semver compatibility contract but emit a + loud warning, so any silent ABI break — the failure mode that motivated + this verification — gets flagged early rather than miscalled. """ get_desc_addr = _read_u64(fw_ptr + _FW_OFF_GET_INTERFACE_PLUGIN_DESC) if get_desc_addr == 0: @@ -291,11 +292,10 @@ def _verify_iadapter_version(fw_ptr: int, ia_ptr: int) -> bool: continue major = ctypes.c_uint32.from_address(entry_addr + 8).value minor = ctypes.c_uint32.from_address(entry_addr + 12).value - if (major, minor) != (_IA_EXPECTED_MAJOR, _IA_EXPECTED_MINOR): + if major != _IA_EXPECTED_MAJOR or minor < _IA_EXPECTED_MINOR: logger.warning( - "cubric IAdapter version mismatch: plugin reports v%d.%d, " - "shim is pinned to v%d.%d. The vtable layout may have " - "changed; see omni/cubric/IAdapter.h. Falling back to " + "cubric IAdapter version incompatible with this shim: plugin " + "reports v%d.%d, shim is pinned to v%d.%d. Falling back to " "update_world_xforms().", major, minor, @@ -303,6 +303,18 @@ def _verify_iadapter_version(fw_ptr: int, ia_ptr: int) -> bool: _IA_EXPECTED_MINOR, ) return False + if minor > _IA_EXPECTED_MINOR: + logger.warning( + "cubric IAdapter minor version newer than this shim was " + "validated against: plugin reports v%d.%d, shim is pinned to " + "v%d.%d. Proceeding under semver minor-compatibility — if " + "transforms misbehave, verify the vtable layout against " + "omni/cubric/IAdapter.h.", + major, + minor, + _IA_EXPECTED_MAJOR, + _IA_EXPECTED_MINOR, + ) return True logger.warning( From e4573b401ee3f86178bda3b152c52804aec8d113 Mon Sep 17 00:00:00 2001 From: Jessica Martinez Date: Fri, 1 May 2026 16:30:58 -0500 Subject: [PATCH 4/6] OMPE-91419: Emit loud stderr warning on ABI minor version mismatch instead of standard IsaacLab logger --- .../isaaclab_newton/physics/_cubric.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/source/isaaclab_newton/isaaclab_newton/physics/_cubric.py b/source/isaaclab_newton/isaaclab_newton/physics/_cubric.py index 3215bee67db1..8c0ca8da95b9 100644 --- a/source/isaaclab_newton/isaaclab_newton/physics/_cubric.py +++ b/source/isaaclab_newton/isaaclab_newton/physics/_cubric.py @@ -24,6 +24,7 @@ import ctypes import logging +import warnings logger = logging.getLogger(__name__) @@ -304,16 +305,14 @@ def _verify_iadapter_version(fw_ptr: int, ia_ptr: int) -> bool: ) return False if minor > _IA_EXPECTED_MINOR: - logger.warning( - "cubric IAdapter minor version newer than this shim was " - "validated against: plugin reports v%d.%d, shim is pinned to " - "v%d.%d. Proceeding under semver minor-compatibility — if " - "transforms misbehave, verify the vtable layout against " - "omni/cubric/IAdapter.h.", - major, - minor, - _IA_EXPECTED_MAJOR, - _IA_EXPECTED_MINOR, + warnings.warn( + f"cubric IAdapter minor version newer than this shim was validated " + f"against: plugin reports v{major}.{minor}, shim is pinned to " + f"v{_IA_EXPECTED_MAJOR}.{_IA_EXPECTED_MINOR}. Proceeding under " + f"semver minor-compatibility — if transforms misbehave, verify the " + f"vtable layout against omni/cubric/IAdapter.h.", + RuntimeWarning, + stacklevel=2, ) return True From 6272ce2aeaa12b094c2143872813b4ae90997fa6 Mon Sep 17 00:00:00 2001 From: Jessica Martinez Date: Fri, 1 May 2026 16:59:55 -0500 Subject: [PATCH 5/6] OMPE-91419: Revert cubric ABI version to v0.1 and relax warning back to logger - kit is un-breaking the semver compatibility violation upstream. --- .../isaaclab_newton/physics/_cubric.py | 29 +++++++++---------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/source/isaaclab_newton/isaaclab_newton/physics/_cubric.py b/source/isaaclab_newton/isaaclab_newton/physics/_cubric.py index 8c0ca8da95b9..1b223bdc96a8 100644 --- a/source/isaaclab_newton/isaaclab_newton/physics/_cubric.py +++ b/source/isaaclab_newton/isaaclab_newton/physics/_cubric.py @@ -24,7 +24,6 @@ import ctypes import logging -import warnings logger = logging.getLogger(__name__) @@ -51,24 +50,23 @@ # --------------------------------------------------------------------------- # IAdapter struct layout (from omni/cubric/IAdapter.h) # --------------------------------------------------------------------------- -# v0.2 layout: +# v0.1 layout: # 0: getAttribute # 8: create(AdapterId*) # 16: refcount # 24: retain # 32: release(AdapterId) # 40: bindToStage(AdapterId, const FabricId&) -# 48: bindToStageWithListener -# 56: unbind -# 64: compute(AdapterId, options, dirtyMode, outFlags*) +# 48: unbind +# 56: compute(AdapterId, options, dirtyMode, outFlags*) _IA_OFF_CREATE = 8 _IA_OFF_RELEASE = 32 _IA_OFF_BIND = 40 -_IA_OFF_COMPUTE = 64 +_IA_OFF_COMPUTE = 56 # Expected IAdapter version. _IA_EXPECTED_MAJOR = 0 -_IA_EXPECTED_MINOR = 2 +_IA_EXPECTED_MINOR = 1 # AdapterId sentinel _INVALID_ADAPTER_ID = ctypes.c_uint64(~0).value @@ -305,14 +303,15 @@ def _verify_iadapter_version(fw_ptr: int, ia_ptr: int) -> bool: ) return False if minor > _IA_EXPECTED_MINOR: - warnings.warn( - f"cubric IAdapter minor version newer than this shim was validated " - f"against: plugin reports v{major}.{minor}, shim is pinned to " - f"v{_IA_EXPECTED_MAJOR}.{_IA_EXPECTED_MINOR}. Proceeding under " - f"semver minor-compatibility — if transforms misbehave, verify the " - f"vtable layout against omni/cubric/IAdapter.h.", - RuntimeWarning, - stacklevel=2, + logger.warning( + "cubric IAdapter minor version newer than this shim was validated " + "against: plugin reports v%d.%d, shim is pinned to v%d.%d. Proceeding " + "under semver minor-compatibility — if transforms misbehave, verify " + "the vtable layout against omni/cubric/IAdapter.h.", + major, + minor, + _IA_EXPECTED_MAJOR, + _IA_EXPECTED_MINOR, ) return True From 0e5ef2182e3708f26e317c3d8e7493c4932e9dbe Mon Sep 17 00:00:00 2001 From: Jessica Martinez Date: Wed, 6 May 2026 16:02:28 -0500 Subject: [PATCH 6/6] OMPE-91419: Added changelog fragment --- source/isaaclab_newton/changelog.d/jmart-cubric-abi.rst | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 source/isaaclab_newton/changelog.d/jmart-cubric-abi.rst diff --git a/source/isaaclab_newton/changelog.d/jmart-cubric-abi.rst b/source/isaaclab_newton/changelog.d/jmart-cubric-abi.rst new file mode 100644 index 000000000000..d2310ee82a48 --- /dev/null +++ b/source/isaaclab_newton/changelog.d/jmart-cubric-abi.rst @@ -0,0 +1,7 @@ +Added +^^^^^ + +* Added runtime verification of the ``omni::cubric::IAdapter`` interface + version in :mod:`~isaaclab_newton.physics._cubric` as defense-in-depth + against future ABI shifts. The shim falls back to the CPU path on + major-version mismatch or older-minor.