lei.chen/myxce
1
0
Fork 1
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix: sel_key_cmp_fn padding-byte mismatch in kvtab lookups

Browse source 
sel_key_cmp_fn compared keys with memcmp on the whole sel_node_key_t
struct, including the 4-byte padding between type and op1.  The stack-
allocated search key in gc_sweep_select_nodes can have different padding
bytes than the heap-allocated copy stored by key_add, causing key_del
to miss the entry and leave a dangling pointer in the CSE intern table.

Replace memcmp with field-by-field comparison on type, op1, op2.

Also fix GC sweep tests that asserted pointer inequality after re-intern:
calloc may lawfully reuse the freed address, so pointer inequality does
not reliably indicate a successful sweep.  Track the swept node's
node_id before freeing and assert the re-interned node gets a fresh id.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Rongsong Shen 2026-05-12 09:53:59 +00:00
parent 3e652d2d7e
commit 450a4d467c
2 changed files with 25 additions and 11 deletions
Download patch file Download diff file Expand all files Collapse all files

25
src/lib/testsuite/test_select.c
View file

@ -726,12 +726,12 @@ static void
test_gc_sweep_unreachable_freed(void)
{
/* After a sweep with no live jobs, every node in the table is
* unreachable. Re-interning the same arguments must return a freshly
* allocated pointer — the old node was freed and removed from the table.
* unreachable. Re-interning the same arguments must return a fresh
* node — the old node was freed and removed from the table.
*
* The pointer-inequality assertion captures the contract; it becomes
* meaningful once Phase 4f wires the mark-and-sweep to the intern table.
*/
* We track node_id rather than pointer equality: calloc may lawfully
* reuse the freed address, so pointer-inequality is not a reliable
* indicator that GC swept correctly. */
sel_node_table_init();
sel_optimize_set(1);
@ -741,16 +741,18 @@ test_gc_sweep_unreachable_freed(void)
xce_select_node_t *node1 = makeSelectNode(SEL_NUM, bits, 0);
TEST_ASSERT_NOT_NULL(node1);
uint32_t node1_id = node1->node_id;
/* No live jobs → all nodes are unreachable */
gc_clear_marks();
gc_sweep_select_nodes(NULL, 0);
/* Re-intern: node1 was freed; a new node must be allocated */
/* Re-intern: node1 was freed; a new node must be allocated.
* node_id is monotonically assigned — a fresh intern produces a
* different id even if the heap address happens to be reused. */
xce_select_node_t *node2 = makeSelectNode(SEL_NUM, bits, 0);
TEST_ASSERT_NOT_NULL(node2);
/* Phase 4f: node2 must be a distinct allocation from the freed node1 */
TEST_ASSERT_TRUE(node2 != node1);
TEST_ASSERT_NOT_EQUAL(node1_id, node2->node_id);
sel_optimize_test_reset();
}
@ -2230,6 +2232,7 @@ test_gc_clear_marks_exported(void)
xce_select_node_t* n2 = makeSelectNode(SEL_NUM, b2, 0);
TEST_ASSERT_NOT_NULL(n1);
TEST_ASSERT_NOT_NULL(n2);
uint32_t n1_id = n1->node_id;
/* manually mark n1 */
n1->gc_mark = 1;
@ -2246,10 +2249,12 @@ test_gc_clear_marks_exported(void)
gc_sweep_select_nodes(NULL, 0);
TEST_ASSERT_EQUAL_INT(1, n2->gc_mark);
/* verify n1 was swept — re-interning gives a fresh allocation */
/* n1 was swept — re-interning must produce a fresh node_id.
* Pointer inequality would be unreliable because calloc may
* lawfully reuse the freed address. */
xce_select_node_t* n1_again = makeSelectNode(SEL_NUM, b1, 0);
TEST_ASSERT_NOT_NULL(n1_again);
TEST_ASSERT_NOT_EQUAL(n1, n1_again); /* different pointer = n1 was freed */
TEST_ASSERT_NOT_EQUAL(n1_id, n1_again->node_id);
sel_node_table_fini();
sel_optimize_test_reset();

11
src/lib/xce_select.c
View file

@ -98,7 +98,16 @@ static uint32_t sel_key_hash_fn(void* k)
static int sel_key_cmp_fn(void* a, void* b)
{
return memcmp(a, b, sizeof(sel_node_key_t));
/* Field-by-field comparison: memcmp on the whole struct would
* compare padding bytes between type and op1, which can differ
* between the stack-local search key and the heap-allocated copy
* stored in the kvtab, causing false misses in key_del. */
sel_node_key_t* ka = (sel_node_key_t*)a;
sel_node_key_t* kb = (sel_node_key_t*)b;
if (ka->type != kb->type) return 1;
if (ka->op1 != kb->op1) return 1;
if (ka->op2 != kb->op2) return 1;
return 0;
}
static int sel_key_assign_fn(void** dst, void* src)