Skip to content

Fix zero-size malloc and memcpy#3081

Open
rdeits-bd wants to merge 1 commit intogoogle-deepmind:mainfrom
rdeits-bd:rd/zero-size-memcpy-fix
Open

Fix zero-size malloc and memcpy#3081
rdeits-bd wants to merge 1 commit intogoogle-deepmind:mainfrom
rdeits-bd:rd/zero-size-memcpy-fix

Conversation

@rdeits-bd
Copy link

When running some our internal code under valgrind, I ran into warnings inside Mujoco's set0 function:

==720211== Thread 2:
==720211== Unsafe allocation with size of zero is implementation-defined
==720211==    at 0x4901C8B: memalign (vg_replace_malloc.c:2026)
==720211==    by 0x4A42AE7: mju_malloc (in /home/BOSDYN/rdeits/Projects/mujoco/build/lib/libmujoco.so.3.4.1)
==720211==    by 0x4A30CDC: set0 (in /home/BOSDYN/rdeits/Projects/mujoco/build/lib/libmujoco.so.3.4.1)
==720211==    by 0x4A336B2: mj_setConst (in /home/BOSDYN/rdeits/Projects/mujoco/build/lib/libmujoco.so.3.4.1)
==720211==    by 0x4AD6AB6: mjCModel::TryCompile(mjModel_*&, mjData_*&, mjVFS_ const*) (in /home/BOSDYN/rdeits/Projects/mujoco/build/lib/libmujoco.so.3.4.1)
==720211==    by 0x4AD2FA2: mjCModel::Compile(mjVFS_ const*, mjModel_**) (in /home/BOSDYN/rdeits/Projects/mujoco/build/lib/libmujoco.so.3.4.1)
==720211==    by 0x4B2CAEB: mj_loadXML (in /home/BOSDYN/rdeits/Projects/mujoco/build/lib/libmujoco.so.3.4.1)
==720211==    by 0x40123D8: (anonymous namespace)::LoadModel(char const*, mujoco::Simulate&) (in /home/BOSDYN/rdeits/Projects/mujoco/build/bin/simulate)
==720211==    by 0x40172EF: PhysicsThread(mujoco::Simulate*, char const*) (in /home/BOSDYN/rdeits/Projects/mujoco/build/bin/simulate)
==720211==    by 0x4DF2252: ??? (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.30)
==720211==    by 0x50DFAC2: start_thread (pthread_create.c:442)
==720211==    by 0x5170A83: clone (clone.S:100)

I can reproduce this by running the simulate binary after building from source:

~/apps/valgrind-3.26.0/build/bin/valgrind ./bin/simulate <my robot model.xml>

The issue is that m->nflex can be zero, in which case we end up calling aligned_alloc with a size of zero. I can't find clear information on whether that's allowed or not -- some sources say yes, others say no. The memcpy on the subsequent line is also suspect: If malloc returns null (which I think it's allowed to for zero size), then even a zero-size memcpy is undefined behavior with a null destination.

This change resolves the valgrind warning in set0. Note that we do still see some warnings inside libnvidia-glcore both before and after this change.

@google-cla
Copy link

google-cla bot commented Feb 10, 2026

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@kevinzakka kevinzakka requested a review from quagla February 10, 2026 22:34
Copy link
Contributor

@quagla quagla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the change! A small remark, can you please move the comments from inside the if to the start of the blocks that you changed?

@rdeits-bd
Copy link
Author

Happy to make that change. I'm currently blocked waiting for our lawyers to approve your CLA. If you'd rather just make the relevant commit yourself that's of course fine by me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants