Skip to content

Record NUMA nodes >= 32 in the partition node bitmap - #542

Draft
rootkiller6788 wants to merge 1 commit into
google:masterfrom
rootkiller6788:fix-numa-node-bitmap-shift
Draft

Record NUMA nodes >= 32 in the partition node bitmap#542
rootkiller6788 wants to merge 1 commit into
google:masterfrom
rootkiller6788:fix-numa-node-bitmap-shift

Conversation

@rootkiller6788

Copy link
Copy Markdown

Summary

InitNumaTopology (tcmalloc/internal/numa.cc) records each detected NUMA node in the per-partition partition_to_nodes bitmap with:

partition_to_nodes[partition] |= 1 << node;

node is a size_t, but the literal 1 is a 32-bit signed int, so for node >= 31 the shift is undefined behavior. On x86 (and every supported LP64 Linux ABI, where int is 32-bit) the practical result is a corrupted bitmap:

  • node == 31: 1 << 31 produces a negative int that sign-extends to 0xffffffff80000000, spuriously setting bits 31..63 of the node bitmap.
  • node >= 32: the shift count is masked modulo 32 (e.g. 1 << 32 == 1 << 0), setting a lower node's bit instead of the intended one.

This bitmap is consumed by SystemAllocator::BindMemory (tcmalloc/internal/system_allocator.h) and passed to mbind(2) as the nodemask for NUMA memory binding. On machines with 32 or more NUMA nodes (8-socket EPYC, IBM POWER, etc.) memory can therefore be bound to the wrong nodes — or mbind fails with EINVAL, which is fatal under TCMALLOC_NUMA_AWARE=strict-binding.

Fix

Shift in a 64-bit type so the intended bit is set:

partition_to_nodes[partition] |= uint64_t{1} << node;

Behavior is unchanged for node < 31.

Test

Added NumaTopologyTest.HighNodeIndex, which initializes a topology with 33 synthetic NUMA nodes (node indices 0..32) and asserts that node 32 is recorded in the partition 0 bitmap. Before this change the test fails: 1 << 32 evaluates to 1 on x86, so bit 32 is never set; after the change it passes.

Verification

  • Reproduced the corruption with a standalone C++ simulation on x86: for node in {31, 32, 33, 63}, the old expression set 0xffffffff80000000, 0x1, 0x2, and 0xffffffff80000000 respectively, while the fixed expression sets exactly the intended single bit.
  • partition_to_nodes is declared as uint64_t partition_to_nodes_[kNumInternalPartitions] in numa.h; uint64_t{1} matches the shift style already used elsewhere in the tree (e.g. tcmalloc/internal/pageflags.cc).

InitNumaTopology builds partition_to_nodes by OR-ing in 1 << node, where
node is a size_t.  The literal 1 is a 32-bit signed int, so for node >= 31
the shift is undefined behavior and, on x86, corrupts the uint64_t bitmap:
node 31 sign-extends to set bits 31..63, and node 32+ has its shift count
masked modulo 32, setting a bit belonging to a lower node instead.  The
resulting nodemask is passed to mbind() for memory binding, so on machines
with 32 or more NUMA nodes memory could be bound to the wrong nodes or
mbind could fail (fatal under strict binding).  Shift in a 64-bit type
instead and add a regression test covering a node index of 32.
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.

1 participant