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 00000000000..d2310ee82a4 --- /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. diff --git a/source/isaaclab_newton/isaaclab_newton/physics/_cubric.py b/source/isaaclab_newton/isaaclab_newton/physics/_cubric.py index abe09cb03bd..e009580a699 100644 --- a/source/isaaclab_newton/isaaclab_newton/physics/_cubric.py +++ b/source/isaaclab_newton/isaaclab_newton/physics/_cubric.py @@ -35,11 +35,22 @@ # 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 # --------------------------------------------------------------------------- # IAdapter struct layout (from omni/cubric/IAdapter.h) # --------------------------------------------------------------------------- +# v0.1 layout: # 0: getAttribute # 8: create(AdapterId*) # 16: refcount @@ -53,6 +64,10 @@ _IA_OFF_BIND = 40 _IA_OFF_COMPUTE = 56 +# Expected IAdapter version. +_IA_EXPECTED_MAJOR = 0 +_IA_EXPECTED_MINOR = 1 + # AdapterId sentinel _INVALID_ADAPTER_ID = ctypes.c_uint64(~0).value @@ -90,6 +105,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 +180,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 tryAcquire first (non-loading); fall back to acquire (will load the plugin if registered). @@ -175,10 +197,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. @@ -219,6 +246,78 @@ def initialize(self) -> bool: logger.info("cubric IAdapter bindings ready") return True + @staticmethod + def _verify_iadapter_version(fw_ptr: int, ia_ptr: int) -> bool: + """Verify the acquired IAdapter is compatible with this shim's vtable offsets. + + 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: + 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 + 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 + 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 + if major != _IA_EXPECTED_MAJOR or minor < _IA_EXPECTED_MINOR: + logger.warning( + "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, + _IA_EXPECTED_MAJOR, + _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( + "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