From a6cbb323b0ba56f63e552c895494792515ce0347 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Mon, 28 Sep 2026 09:35:08 +0000 Subject: [PATCH] feat(libxce-alloc-preason-equality): public alloc_eq/host_alloc_eq/job_alloc_eq with first-difference description (task 2.1) Co-Authored-By: Claude Opus 5.5 (1M context) --- .../libxce-alloc-preason-equality/tasks.md | 3 +- src/include/xcealloc.h | 51 ++ src/lib/meson.build | 7 +- src/lib/testsuite/test_xcealloc.c | 544 ++++++++++++++++++ src/lib/testsuite/test_xcealloc_oom.c | 126 +++- src/lib/xcealloc.c | 226 ++++++++ 6 files changed, 952 insertions(+), 5 deletions(-) diff --git a/.kiro/specs/libxce-alloc-preason-equality/tasks.md b/.kiro/specs/libxce-alloc-preason-equality/tasks.md index 99fda432..383af532 100644 --- a/.kiro/specs/libxce-alloc-preason-equality/tasks.md +++ b/.kiro/specs/libxce-alloc-preason-equality/tasks.md @@ -10,7 +10,7 @@ Test-first throughout (red → green → refactor). Every sub-task leaves the bu - _Requirements: 1.5, 2.3_ - [ ] 2. Core: public equality API in libxce -- [ ] 2.1 Allocation equality for single records, host allocations and job allocations, with an optional first-difference description +- [x] 2.1 Allocation equality for single records, host allocations and job allocations, with an optional first-difference description - Write the tests first: a match; a mismatch on each compared field (resource name, reserve, value, duration, reservation start time) with the expected description; an allocation-instance-id difference still compares equal; host name, entry order and list length matter; job id, ordered hosts, ordered shared resources and list-length mismatches; NULL handling at the top level, in list entries and in string fields; no description requested; no description returned when equal. - Add a wrapped description-allocation failure to the existing allocation OOM test binary (bug-1882) and test that a failed description leaves no description and the same result. - Implement the three comparisons in the allocation module with the field order, exact value comparison, description path format and ownership contract of design.md (Data Models, Components: Allocation equality, Decision 9), enabling the GNU extension the description formatter needs. @@ -87,3 +87,4 @@ Test-first throughout (red → green → refactor). Every sub-task leaves the bu ## Implementation Notes - `xce-build.sh` resolves a relative build dir under `src/`: `bash src/tools/xce-build.sh lib asan xce.build.alloceq` builds into `src/xce.build.alloceq`, so iterate with `meson test -C src/xce.build.alloceq`. +- `xce-build.sh lib` runs only the `libxce test` binary; run `meson test -C src/xce.build.alloceq "libxce alloc OOM test"` (or plain `meson test`) for the allocation OOM binary, which also wraps `vasprintf`/`__vasprintf_chk` since 2.1. diff --git a/src/include/xcealloc.h b/src/include/xcealloc.h index be370b2f..f14d8b37 100644 --- a/src/include/xcealloc.h +++ b/src/include/xcealloc.h @@ -16,4 +16,55 @@ extern xce_job_account_t* make_job_account(xce_job_alloc_t*); extern xce_job_alloc_t* dup_job_alloc(const xce_job_alloc_t*); extern int merge_job_alloc(xce_job_alloc_t*, const xce_job_alloc_t*); +/* + * Allocation equality: "do these two records describe the same allocation + * decision". + * + * alloc_eq() compares, in this order: resname, reserve, value, duration, + * from. `value` is compared exactly (`==`, no floating-point tolerance): both + * sides are exact copies of the same double, so any difference is real. + * `from` (the reservation start time) is compared: the scheduler stamps it + * with the deciding session's start, which a replay reproduces. `instid` is + * NOT compared: it comes from a per-process counter (sched_get_id()) whose + * value depends on how many allocations that process built before, so a + * recording and its replay give the same allocation different instids + * (bug-1844). + * + * This is not merge_job_alloc()'s slot matching. merge_job_alloc() asks + * "are these the same reservation slot to add into" (resname, instid, + * reserve; value ignored because merging adds values), where instid IS + * significant. Neither predicate can stand in for the other. + * + * host_alloc_eq() compares host, then nallocs, then allocs[i] in order. + * job_alloc_eq() compares jobid, nhosts, nres_shared, then allocs[i] in + * order, then allocs_shared[i] in order. Lists are order-sensitive: the same + * entries in a different order are not equal. + * + * NULL handling: identical pointers (including two NULLs) are equal; exactly + * one NULL argument, list entry or string field is not equal. Precondition: + * a non-zero count with a NULL entry array is a malformed record and is not + * handled. Neither argument is modified. + * + * `why` is optional. When NULL, nothing is allocated. Otherwise *why is set + * to NULL when the arguments are equal, or to a newly allocated, + * NUL-terminated description of the first difference found, in comparison + * order, e.g. + * allocs[1](host=h1).allocs[0](resname=mem).value: 2 != 3 + * allocs[0](host=h0).host: "h0" != "h2" + * allocs_shared[0]: NULL != non-NULL + * nhosts: 1 != 2 + * job_alloc: non-NULL != NULL (a NULL top-level argument) + * Path labels come from `a`, from `b` when a's entry/string is NULL, or are + * `?`. The caller owns the description and releases it with free(). The value + * *why holds on entry is never read or freed. If allocating the description + * fails, *why is NULL and the result is unchanged. Nothing is logged. + * + * Returns non-zero when equal, 0 when not. + */ +extern int alloc_eq(const xce_alloc_t* a, const xce_alloc_t* b, char** why); +extern int host_alloc_eq(const xce_host_alloc_t* a, const xce_host_alloc_t* b, + char** why); +extern int job_alloc_eq(const xce_job_alloc_t* a, const xce_job_alloc_t* b, + char** why); + #endif diff --git a/src/lib/meson.build b/src/lib/meson.build index d32f3070..6781f49c 100644 --- a/src/lib/meson.build +++ b/src/lib/meson.build @@ -246,12 +246,17 @@ test('libxce test', libxce_test, timeout: 120) # idiom as bug-1843's sched_record_lifecycle_test `--wrap=realloc`) so the # tests can make a chosen strdup() inside make_alloc()/make_host_alloc() # fail. Its own binary, so xcelib_test's other tests never see the wrap. +# libxce-alloc-preason-equality: it also wraps vasprintf (and the +# _FORTIFY_SOURCE __vasprintf_chk variant) to fail the equality functions' +# description allocation. libxce_alloc_oom_test = executable('xcealloc_oom_test', ['testsuite/test_xcealloc_oom.c'], include_directories: [incdir, build_inc], dependencies: libxce_test_deps, c_args: ['-DXCE_TESTING'], - link_args: ['-Wl,--wrap=strdup'], + link_args: ['-Wl,--wrap=strdup', + '-Wl,--wrap=vasprintf', + '-Wl,--wrap=__vasprintf_chk'], install: false) test('libxce alloc OOM test', libxce_alloc_oom_test, timeout: 60) diff --git a/src/lib/testsuite/test_xcealloc.c b/src/lib/testsuite/test_xcealloc.c index 975d3756..e274e15c 100644 --- a/src/lib/testsuite/test_xcealloc.c +++ b/src/lib/testsuite/test_xcealloc.c @@ -5,6 +5,7 @@ #include "misc.h" #include "unity.h" +#include "utils.h" #include "xce/types.h" #include "xcealloc.h" @@ -455,6 +456,533 @@ static void test_dup_job_alloc_from_duration_fidelity(void) free_JobAlloc_ptr(&dup); } +/* ------------------------------------------------------------------------ + * alloc_eq / host_alloc_eq / job_alloc_eq (libxce-alloc-preason-equality + * task 2.1; Req 1.1-1.7, 3.1-3.5, 6.4). Every test that asks for a + * description frees it, so ASAN/LeakSanitizer proves the caller-owns + * contract (Req 3.4). + * ---------------------------------------------------------------------- */ + +/* One record with every compared field set explicitly (make_alloc() stamps + * from = time(NULL); the tests pin it). */ +static xce_alloc_t* eq_alloc(const char* resname, const char* reserve, + uint64_t instid, double value, int64_t duration, + uint64_t from) +{ + xce_alloc_t* a = make_alloc((char*)resname, (char*)reserve, instid, + (float)value, 0); + + TEST_ASSERT_NOT_NULL(a); + a->value = value; + a->duration = duration; + a->from = from; + + return a; +} + +/* h0: mem=2, cpu=4; h1: mem=2; shared: lic=1. */ +static xce_job_alloc_t* eq_job(void) +{ + xce_host_alloc_t** hallocs; + xce_job_alloc_t* j; + + hallocs = make_host_allocs( + 2, + make_host_alloc((char*)"h0", 2, + make_allocs(2, eq_alloc("mem", "", 1, 2, 10, 1000), + eq_alloc("cpu", "", 2, 4, 10, 1000))), + make_host_alloc((char*)"h1", 1, + make_allocs(1, eq_alloc("mem", "", 3, 2, 10, 1000)))); + j = make_job_alloc(4242, 2, hallocs); + TEST_ASSERT_NOT_NULL(j); + + j->nres_shared = 1; + j->allocs_shared = make_allocs(1, eq_alloc("lic", "r1", 4, 1, 10, 1000)); + + return j; +} + +/* Replace a string field (the constructors never leave one NULL; a decoded + * or hand-built record can). */ +static void eq_set_str(char** field, const char* v) +{ + free(*field); + *field = (v != NULL) ? strdup(v) : NULL; +} + +/* Assert not-equal with exactly the expected description, then free it. */ +#define ASSERT_ALLOC_NE(expect, a, b) \ + do { \ + char* why_ = NULL; \ + TEST_ASSERT_FALSE(alloc_eq((a), (b), &why_)); \ + TEST_ASSERT_NOT_NULL(why_); \ + TEST_ASSERT_EQUAL_STRING((expect), why_); \ + free(why_); \ + TEST_ASSERT_FALSE(alloc_eq((a), (b), NULL)); \ + } while (0) + +#define ASSERT_HOST_NE(expect, a, b) \ + do { \ + char* why_ = NULL; \ + TEST_ASSERT_FALSE(host_alloc_eq((a), (b), &why_)); \ + TEST_ASSERT_NOT_NULL(why_); \ + TEST_ASSERT_EQUAL_STRING((expect), why_); \ + free(why_); \ + TEST_ASSERT_FALSE(host_alloc_eq((a), (b), NULL)); \ + } while (0) + +#define ASSERT_JOB_NE(expect, a, b) \ + do { \ + char* why_ = NULL; \ + TEST_ASSERT_FALSE(job_alloc_eq((a), (b), &why_)); \ + TEST_ASSERT_NOT_NULL(why_); \ + TEST_ASSERT_EQUAL_STRING((expect), why_); \ + free(why_); \ + TEST_ASSERT_FALSE(job_alloc_eq((a), (b), NULL)); \ + } while (0) + +static void test_alloc_eq_match_sets_why_null(void) +{ + xce_alloc_t* a = eq_alloc("mem", "r1", 1, 2.5, 10, 1000); + xce_alloc_t* b = eq_alloc("mem", "r1", 1, 2.5, 10, 1000); + char dummy; + char* why = &dummy; /* never read or freed on entry (Req 3.4) */ + + TEST_ASSERT_TRUE(alloc_eq(a, b, &why)); + TEST_ASSERT_NULL(why); + TEST_ASSERT_TRUE(alloc_eq(a, b, NULL)); + TEST_ASSERT_TRUE(alloc_eq(a, a, NULL)); + + free_ResAlloc_ptr(&a); + free_ResAlloc_ptr(&b); +} + +static void test_alloc_eq_each_field_mismatch_described(void) +{ + xce_alloc_t* a = eq_alloc("mem", "r1", 1, 2, 10, 1000); + xce_alloc_t* b = eq_alloc("mem", "r1", 1, 2, 10, 1000); + + eq_set_str(&b->resname, "cpu"); + ASSERT_ALLOC_NE("resname: \"mem\" != \"cpu\"", a, b); + eq_set_str(&b->resname, "mem"); + + eq_set_str(&b->reserve, "r2"); + ASSERT_ALLOC_NE("reserve: \"r1\" != \"r2\"", a, b); + eq_set_str(&b->reserve, "r1"); + + b->value = 3; + ASSERT_ALLOC_NE("value: 2 != 3", a, b); + /* exact compare: a difference far below any tolerance still counts, and + * %.17g makes it visible */ + b->value = 2.0000000000000004; + ASSERT_ALLOC_NE("value: 2 != 2.0000000000000004", a, b); + b->value = 2; + + b->duration = -1; + ASSERT_ALLOC_NE("duration: 10 != -1", a, b); + b->duration = 10; + + b->from = 1001; + ASSERT_ALLOC_NE("from: 1000 != 1001", a, b); + b->from = 1000; + + TEST_ASSERT_TRUE(alloc_eq(a, b, NULL)); + + free_ResAlloc_ptr(&a); + free_ResAlloc_ptr(&b); +} + +/* The FIRST difference in comparison order is reported: resname, reserve, + * value, duration, from. */ +static void test_alloc_eq_reports_first_difference_in_order(void) +{ + xce_alloc_t* a = eq_alloc("mem", "r1", 1, 2, 10, 1000); + xce_alloc_t* b = eq_alloc("mem", "r2", 1, 3, 20, 1001); + + ASSERT_ALLOC_NE("reserve: \"r1\" != \"r2\"", a, b); + eq_set_str(&b->reserve, "r1"); + ASSERT_ALLOC_NE("value: 2 != 3", a, b); + b->value = 2; + ASSERT_ALLOC_NE("duration: 10 != 20", a, b); + + free_ResAlloc_ptr(&a); + free_ResAlloc_ptr(&b); +} + +/* instid is a per-process counter (bug-1844): excluded from equality. */ +static void test_alloc_eq_ignores_instid(void) +{ + xce_alloc_t* a = eq_alloc("mem", "", 826, 2, 10, 1000); + xce_alloc_t* b = eq_alloc("mem", "", 1, 2, 10, 1000); + char* why = NULL; + + TEST_ASSERT_TRUE(alloc_eq(a, b, &why)); + TEST_ASSERT_NULL(why); + + free_ResAlloc_ptr(&a); + free_ResAlloc_ptr(&b); +} + +static void test_alloc_eq_null_handling(void) +{ + xce_alloc_t* a = eq_alloc("mem", "r1", 1, 2, 10, 1000); + xce_alloc_t* b = eq_alloc("mem", "r1", 1, 2, 10, 1000); + char* why = NULL; + + TEST_ASSERT_TRUE(alloc_eq(NULL, NULL, &why)); + TEST_ASSERT_NULL(why); + ASSERT_ALLOC_NE("alloc: NULL != non-NULL", NULL, b); + ASSERT_ALLOC_NE("alloc: non-NULL != NULL", a, NULL); + + /* NULL string fields */ + eq_set_str(&a->resname, NULL); + ASSERT_ALLOC_NE("resname: NULL != \"mem\"", a, b); + eq_set_str(&b->resname, NULL); + TEST_ASSERT_TRUE(alloc_eq(a, b, NULL)); /* NULL == NULL */ + eq_set_str(&a->reserve, NULL); + ASSERT_ALLOC_NE("reserve: NULL != \"r1\"", a, b); + ASSERT_ALLOC_NE("reserve: \"r1\" != NULL", b, a); + + free_ResAlloc_ptr(&a); + free_ResAlloc_ptr(&b); +} + +static void test_host_alloc_eq_match_and_mismatch(void) +{ + xce_job_alloc_t* ja = eq_job(); + xce_job_alloc_t* jb = eq_job(); + xce_host_alloc_t* a = ja->allocs[0]; + xce_host_alloc_t* b = jb->allocs[0]; + char* why = NULL; + + TEST_ASSERT_TRUE(host_alloc_eq(a, b, &why)); + TEST_ASSERT_NULL(why); + TEST_ASSERT_TRUE(host_alloc_eq(a, b, NULL)); + + /* host name matters */ + eq_set_str(&b->host, "h2"); + ASSERT_HOST_NE("host: \"h0\" != \"h2\"", a, b); + eq_set_str(&b->host, "h0"); + + /* a nested field difference names the entry path */ + b->allocs[1]->value = 5; + ASSERT_HOST_NE("allocs[1](resname=cpu).value: 4 != 5", a, b); + b->allocs[1]->value = 4; + + /* instid differences in the entries do not matter */ + b->allocs[0]->instid = 99; + TEST_ASSERT_TRUE(host_alloc_eq(a, b, NULL)); + + free_JobAlloc_ptr(&ja); + free_JobAlloc_ptr(&jb); +} + +static void test_host_alloc_eq_order_and_length_matter(void) +{ + xce_job_alloc_t* ja = eq_job(); + xce_job_alloc_t* jb = eq_job(); + xce_host_alloc_t* a = ja->allocs[0]; + xce_host_alloc_t* b = jb->allocs[0]; + xce_alloc_t* tmp; + + /* same entries, different order */ + tmp = b->allocs[0]; + b->allocs[0] = b->allocs[1]; + b->allocs[1] = tmp; + ASSERT_HOST_NE("allocs[0](resname=mem).resname: \"mem\" != \"cpu\"", a, b); + tmp = b->allocs[0]; + b->allocs[0] = b->allocs[1]; + b->allocs[1] = tmp; + + /* list length (the trailing entry is still owned via allocs[]) */ + b->nallocs = 1; + ASSERT_HOST_NE("nallocs: 2 != 1", a, b); + b->nallocs = 2; + + free_JobAlloc_ptr(&ja); + free_JobAlloc_ptr(&jb); +} + +static void test_host_alloc_eq_null_handling(void) +{ + xce_job_alloc_t* ja = eq_job(); + xce_job_alloc_t* jb = eq_job(); + xce_host_alloc_t* a = ja->allocs[0]; + xce_host_alloc_t* b = jb->allocs[0]; + xce_alloc_t* saved_a; + xce_alloc_t* saved_b; + char* why = NULL; + + TEST_ASSERT_TRUE(host_alloc_eq(NULL, NULL, &why)); + TEST_ASSERT_NULL(why); + ASSERT_HOST_NE("host_alloc: NULL != non-NULL", NULL, b); + ASSERT_HOST_NE("host_alloc: non-NULL != NULL", a, NULL); + + /* NULL list entry */ + saved_a = a->allocs[1]; + a->allocs[1] = NULL; + ASSERT_HOST_NE("allocs[1]: NULL != non-NULL", a, b); + ASSERT_HOST_NE("allocs[1]: non-NULL != NULL", b, a); + saved_b = b->allocs[1]; + b->allocs[1] = NULL; + TEST_ASSERT_TRUE(host_alloc_eq(a, b, NULL)); /* NULL entry == NULL */ + a->allocs[1] = saved_a; + b->allocs[1] = saved_b; + + /* NULL host string */ + eq_set_str(&a->host, NULL); + ASSERT_HOST_NE("host: NULL != \"h0\"", a, b); + + /* label falls back to b's string, or '?' when both are NULL */ + eq_set_str(&a->allocs[0]->resname, NULL); + eq_set_str(&b->host, NULL); + ASSERT_HOST_NE("allocs[0](resname=mem).resname: NULL != \"mem\"", a, b); + eq_set_str(&b->allocs[0]->resname, NULL); + b->allocs[0]->value = 7; + ASSERT_HOST_NE("allocs[0](resname=?).value: 2 != 7", a, b); + + free_JobAlloc_ptr(&ja); + free_JobAlloc_ptr(&jb); +} + +static void test_job_alloc_eq_match(void) +{ + xce_job_alloc_t* a = eq_job(); + xce_job_alloc_t* b = dup_job_alloc(a); + char dummy; + char* why = &dummy; + + TEST_ASSERT_NOT_NULL(b); + TEST_ASSERT_TRUE(job_alloc_eq(a, b, &why)); + TEST_ASSERT_NULL(why); + TEST_ASSERT_TRUE(job_alloc_eq(a, b, NULL)); + TEST_ASSERT_TRUE(job_alloc_eq(a, a, NULL)); + + /* instid anywhere in the tree does not matter */ + b->allocs[1]->allocs[0]->instid = 12345; + b->allocs_shared[0]->instid = 67890; + TEST_ASSERT_TRUE(job_alloc_eq(a, b, NULL)); + + free_JobAlloc_ptr(&a); + free_JobAlloc_ptr(&b); +} + +static void test_job_alloc_eq_jobid_and_lengths(void) +{ + xce_job_alloc_t* a = eq_job(); + xce_job_alloc_t* b = eq_job(); + char ja[128]; + char jb[128]; + char expect[300]; + + b->jobid = 4243; + TEST_ASSERT_TRUE(fmt_jobid(ja, sizeof(ja), a->jobid) >= 0); + TEST_ASSERT_TRUE(fmt_jobid(jb, sizeof(jb), b->jobid) >= 0); + snprintf(expect, sizeof(expect), "jobid: %s != %s", ja, jb); + ASSERT_JOB_NE(expect, a, b); + b->jobid = a->jobid; + + b->nhosts = 1; /* allocs[1] stays owned; restored before free */ + ASSERT_JOB_NE("nhosts: 2 != 1", a, b); + b->nhosts = 2; + + b->nres_shared = 0; + ASSERT_JOB_NE("nres_shared: 1 != 0", a, b); + b->nres_shared = 1; + + TEST_ASSERT_TRUE(job_alloc_eq(a, b, NULL)); + + free_JobAlloc_ptr(&a); + free_JobAlloc_ptr(&b); +} + +static void test_job_alloc_eq_nested_differences(void) +{ + xce_job_alloc_t* a = eq_job(); + xce_job_alloc_t* b = eq_job(); + + /* the design.md Data Models examples */ + b->allocs[1]->allocs[0]->value = 3; + ASSERT_JOB_NE("allocs[1](host=h1).allocs[0](resname=mem).value: 2 != 3", + a, b); + b->allocs[1]->allocs[0]->value = 2; + + eq_set_str(&b->allocs[0]->host, "h2"); + ASSERT_JOB_NE("allocs[0](host=h0).host: \"h0\" != \"h2\"", a, b); + eq_set_str(&b->allocs[0]->host, "h0"); + + /* from is part of the decision */ + b->allocs[0]->allocs[1]->from = 999; + ASSERT_JOB_NE("allocs[0](host=h0).allocs[1](resname=cpu).from: 1000 != 999", + a, b); + b->allocs[0]->allocs[1]->from = 1000; + + b->allocs[0]->nallocs = 1; + ASSERT_JOB_NE("allocs[0](host=h0).nallocs: 2 != 1", a, b); + b->allocs[0]->nallocs = 2; + + /* shared resources */ + b->allocs_shared[0]->duration = 11; + ASSERT_JOB_NE("allocs_shared[0](resname=lic).duration: 10 != 11", a, b); + b->allocs_shared[0]->duration = 10; + eq_set_str(&b->allocs_shared[0]->reserve, "r9"); + ASSERT_JOB_NE("allocs_shared[0](resname=lic).reserve: \"r1\" != \"r9\"", a, + b); + eq_set_str(&b->allocs_shared[0]->reserve, "r1"); + + TEST_ASSERT_TRUE(job_alloc_eq(a, b, NULL)); + + free_JobAlloc_ptr(&a); + free_JobAlloc_ptr(&b); +} + +/* Hosts are compared in order: the same hosts swapped are not equal. Hosts + * are compared before shared resources. */ +static void test_job_alloc_eq_order_matters(void) +{ + xce_job_alloc_t* a = eq_job(); + xce_job_alloc_t* b = eq_job(); + xce_host_alloc_t* tmp; + + tmp = b->allocs[0]; + b->allocs[0] = b->allocs[1]; + b->allocs[1] = tmp; + b->allocs_shared[0]->value = 8; /* later difference is not reported */ + ASSERT_JOB_NE("allocs[0](host=h0).host: \"h0\" != \"h1\"", a, b); + + free_JobAlloc_ptr(&a); + free_JobAlloc_ptr(&b); +} + +static void test_job_alloc_eq_shared_order_matters(void) +{ + xce_job_alloc_t* a = eq_job(); + xce_job_alloc_t* b = eq_job(); + int i; + + for (i = 0; i < 2; i++) { + xce_job_alloc_t* j = (i == 0) ? a : b; + xce_alloc_t** grown; + + grown = (xce_alloc_t**)realloc(j->allocs_shared, 2 * sizeof(*grown)); + TEST_ASSERT_NOT_NULL(grown); + j->allocs_shared = grown; + j->allocs_shared[1] = eq_alloc("gpu", "", 5, 1, 10, 1000); + j->nres_shared = 2; + } + + TEST_ASSERT_TRUE(job_alloc_eq(a, b, NULL)); + { + xce_alloc_t* tmp = b->allocs_shared[0]; + b->allocs_shared[0] = b->allocs_shared[1]; + b->allocs_shared[1] = tmp; + } + ASSERT_JOB_NE("allocs_shared[0](resname=lic).resname: \"lic\" != \"gpu\"", + a, b); + + free_JobAlloc_ptr(&a); + free_JobAlloc_ptr(&b); +} + +static void test_job_alloc_eq_null_handling(void) +{ + xce_job_alloc_t* a = eq_job(); + xce_job_alloc_t* b = eq_job(); + xce_host_alloc_t* saved_h; + xce_alloc_t* saved_s; + char* why = NULL; + + TEST_ASSERT_TRUE(job_alloc_eq(NULL, NULL, &why)); + TEST_ASSERT_NULL(why); + ASSERT_JOB_NE("job_alloc: NULL != non-NULL", NULL, b); + ASSERT_JOB_NE("job_alloc: non-NULL != NULL", a, NULL); + + /* NULL host entry */ + saved_h = a->allocs[1]; + a->allocs[1] = NULL; + ASSERT_JOB_NE("allocs[1]: NULL != non-NULL", a, b); + a->allocs[1] = saved_h; + + /* NULL nested resource entry */ + saved_s = b->allocs[0]->allocs[0]; + b->allocs[0]->allocs[0] = NULL; + ASSERT_JOB_NE("allocs[0](host=h0).allocs[0]: non-NULL != NULL", a, b); + b->allocs[0]->allocs[0] = saved_s; + + /* NULL shared entry (the design.md example) */ + saved_s = a->allocs_shared[0]; + a->allocs_shared[0] = NULL; + ASSERT_JOB_NE("allocs_shared[0]: NULL != non-NULL", a, b); + a->allocs_shared[0] = saved_s; + + /* NULL host string: the path label comes from b */ + eq_set_str(&a->allocs[1]->host, NULL); + ASSERT_JOB_NE("allocs[1](host=h1).host: NULL != \"h1\"", a, b); + eq_set_str(&b->allocs[1]->host, NULL); + TEST_ASSERT_TRUE(job_alloc_eq(a, b, NULL)); + b->allocs[1]->allocs[0]->value = 6; + ASSERT_JOB_NE("allocs[1](host=?).allocs[0](resname=mem).value: 2 != 6", a, + b); + + free_JobAlloc_ptr(&a); + free_JobAlloc_ptr(&b); +} + +/* The function sizes the description itself: a long path is never + * truncated (Req 3.2). */ +static void test_job_alloc_eq_long_description_not_truncated(void) +{ + xce_job_alloc_t* a = eq_job(); + xce_job_alloc_t* b = eq_job(); + char host[1001]; + char expect[2200]; + char* why = NULL; + + memset(host, 'x', sizeof(host) - 1); + host[sizeof(host) - 1] = '\0'; + eq_set_str(&a->allocs[0]->host, host); + eq_set_str(&b->allocs[0]->host, host); + b->allocs[0]->allocs[0]->value = 9; + snprintf(expect, sizeof(expect), + "allocs[0](host=%s).allocs[0](resname=mem).value: 2 != 9", host); + + TEST_ASSERT_FALSE(job_alloc_eq(a, b, &why)); + TEST_ASSERT_NOT_NULL(why); + TEST_ASSERT_EQUAL_STRING(expect, why); + free(why); + + free_JobAlloc_ptr(&a); + free_JobAlloc_ptr(&b); +} + +/* Read-only (Req 1.7): comparing leaves both sides as they were. */ +static void test_job_alloc_eq_does_not_modify_arguments(void) +{ + xce_job_alloc_t* a = eq_job(); + xce_job_alloc_t* b = eq_job(); + xce_job_alloc_t* ca = dup_job_alloc(a); + xce_job_alloc_t* cb; + char* why = NULL; + + b->allocs[1]->allocs[0]->value = 3; + cb = dup_job_alloc(b); + TEST_ASSERT_NOT_NULL(ca); + TEST_ASSERT_NOT_NULL(cb); + + TEST_ASSERT_FALSE(job_alloc_eq(a, b, &why)); + free(why); + + TEST_ASSERT_TRUE(job_alloc_eq(a, ca, NULL)); + TEST_ASSERT_TRUE(job_alloc_eq(b, cb, NULL)); + TEST_ASSERT_EQUAL_UINT64(a->allocs[0]->allocs[0]->instid, + ca->allocs[0]->allocs[0]->instid); + + free_JobAlloc_ptr(&a); + free_JobAlloc_ptr(&b); + free_JobAlloc_ptr(&ca); + free_JobAlloc_ptr(&cb); +} + void test_xcealloc(void) { printf("\n>> xcealloc test\n\n"); @@ -467,4 +995,20 @@ void test_xcealloc(void) RUN_TEST(test_dup_job_alloc_shared_resource_pool_full_equivalence); RUN_TEST(test_dup_job_alloc_independence_after_mutate_and_free); RUN_TEST(test_dup_job_alloc_from_duration_fidelity); + RUN_TEST(test_alloc_eq_match_sets_why_null); + RUN_TEST(test_alloc_eq_each_field_mismatch_described); + RUN_TEST(test_alloc_eq_reports_first_difference_in_order); + RUN_TEST(test_alloc_eq_ignores_instid); + RUN_TEST(test_alloc_eq_null_handling); + RUN_TEST(test_host_alloc_eq_match_and_mismatch); + RUN_TEST(test_host_alloc_eq_order_and_length_matter); + RUN_TEST(test_host_alloc_eq_null_handling); + RUN_TEST(test_job_alloc_eq_match); + RUN_TEST(test_job_alloc_eq_jobid_and_lengths); + RUN_TEST(test_job_alloc_eq_nested_differences); + RUN_TEST(test_job_alloc_eq_order_matters); + RUN_TEST(test_job_alloc_eq_shared_order_matters); + RUN_TEST(test_job_alloc_eq_null_handling); + RUN_TEST(test_job_alloc_eq_long_description_not_truncated); + RUN_TEST(test_job_alloc_eq_does_not_modify_arguments); } diff --git a/src/lib/testsuite/test_xcealloc_oom.c b/src/lib/testsuite/test_xcealloc_oom.c index 24e053eb..e7ccabdb 100644 --- a/src/lib/testsuite/test_xcealloc_oom.c +++ b/src/lib/testsuite/test_xcealloc_oom.c @@ -11,8 +11,14 @@ * wrap reaches the calls inside xcealloc.c too. __wrap_strdup() passes * through to __real_strdup() unless a test arms it; this is a separate * binary so xcelib_test's other tests are untouched. + * + * libxce-alloc-preason-equality (task 2.1, Req 3.5): the same binary also + * wraps vasprintf() (and __vasprintf_chk(), which a _FORTIFY_SOURCE build + * calls instead) so a test can make the equality functions' description + * allocation fail: the result must be unchanged and *why left NULL. */ #include +#include #include #include #include @@ -41,6 +47,50 @@ char* __wrap_strdup(const char* s) return __real_strdup(s); } +extern int __real_vasprintf(char** strp, const char* fmt, va_list ap); +extern int __real___vasprintf_chk(char** strp, int flag, const char* fmt, + va_list ap); + +static int g_vasprintf_fail_at = -1; /* -1 == disabled */ +static int g_vasprintf_calls = 0; +static char g_garbage; /* vasprintf leaves *strp undefined on failure */ + +static int vasprintf_should_fail(char** strp) +{ + if (g_vasprintf_fail_at >= 0) { + int this_call = g_vasprintf_calls++; + + if (this_call == g_vasprintf_fail_at) { + *strp = &g_garbage; + return 1; + } + } + return 0; +} + +int __wrap_vasprintf(char** strp, const char* fmt, va_list ap) +{ + if (vasprintf_should_fail(strp)) { + return -1; + } + return __real_vasprintf(strp, fmt, ap); +} + +int __wrap___vasprintf_chk(char** strp, int flag, const char* fmt, va_list ap) +{ + if (vasprintf_should_fail(strp)) { + return -1; + } + return __real___vasprintf_chk(strp, flag, fmt, ap); +} + +/* Make the vasprintf() call with 0-based index `n` (counted from now) fail. */ +static void fail_vasprintf_at(int n) +{ + g_vasprintf_calls = 0; + g_vasprintf_fail_at = n; +} + /* Make the strdup() call with 0-based index `n` (counted from now) fail. */ static void fail_strdup_at(int n) { @@ -50,13 +100,16 @@ static void fail_strdup_at(int n) void setUp(void) { - g_strdup_fail_at = -1; - g_strdup_calls = 0; + g_strdup_fail_at = -1; + g_strdup_calls = 0; + g_vasprintf_fail_at = -1; + g_vasprintf_calls = 0; } void tearDown(void) { - g_strdup_fail_at = -1; + g_strdup_fail_at = -1; + g_vasprintf_fail_at = -1; } static void test_make_alloc_null_resname_returns_null(void) @@ -191,6 +244,71 @@ static void test_merge_job_alloc_strdup_failure_reports_enomem(void) } } +/* Req 3.5: a failed description allocation leaves *why NULL and the + * comparison result unchanged. */ +static void test_alloc_eq_description_oom_leaves_why_null(void) +{ + xce_alloc_t* a = make_alloc((char*)"mem", NULL, 1, 1.0f, 0); + xce_alloc_t* b = make_alloc((char*)"mem", NULL, 2, 2.0f, 0); + char* why = NULL; + + TEST_ASSERT_NOT_NULL(a); + TEST_ASSERT_NOT_NULL(b); + b->from = a->from; + + fail_vasprintf_at(0); + TEST_ASSERT_FALSE(alloc_eq(a, b, &why)); + TEST_ASSERT_NULL(why); + TEST_ASSERT_EQUAL_INT(1, g_vasprintf_calls); /* the wrap was reached */ + + /* equal: no description, so nothing to fail */ + fail_vasprintf_at(0); + b->value = a->value; + why = (char*)&g_garbage; + TEST_ASSERT_TRUE(alloc_eq(a, b, &why)); + TEST_ASSERT_NULL(why); + + free_ResAlloc_ptr(&a); + free_ResAlloc_ptr(&b); +} + +/* A nested difference is described by the innermost call and re-wrapped by + * each parent: failing any one of those formatter calls leaves *why NULL + * (and nothing leaked, per ASAN) with the same result. */ +static void test_job_alloc_eq_description_oom_at_every_level(void) +{ + xce_job_alloc_t* a = one_host_job("h0"); + xce_job_alloc_t* b = one_host_job("h0"); + int n; + + TEST_ASSERT_NOT_NULL(a); + TEST_ASSERT_NOT_NULL(b); + b->allocs[0]->allocs[0]->from = a->allocs[0]->allocs[0]->from; + b->allocs[0]->allocs[0]->value = 5; + + /* alloc (0), host wrap (1), job wrap (2) */ + for (n = 0; n < 3; n++) { + char* why = NULL; + + fail_vasprintf_at(n); + TEST_ASSERT_FALSE(job_alloc_eq(a, b, &why)); + TEST_ASSERT_NULL(why); + } + + /* unarmed: the description is produced as usual */ + { + char* why = NULL; + + g_vasprintf_fail_at = -1; + TEST_ASSERT_FALSE(job_alloc_eq(a, b, &why)); + TEST_ASSERT_NOT_NULL(why); + free(why); + } + + free_JobAlloc_ptr(&a); + free_JobAlloc_ptr(&b); +} + int main(void) { UNITY_BEGIN(); @@ -205,6 +323,8 @@ int main(void) RUN_TEST(test_make_host_alloc_null_host_returns_null); RUN_TEST(test_make_host_alloc_strdup_failure_leaves_allocs_to_caller); RUN_TEST(test_merge_job_alloc_strdup_failure_reports_enomem); + RUN_TEST(test_alloc_eq_description_oom_leaves_why_null); + RUN_TEST(test_job_alloc_eq_description_oom_at_every_level); return UNITY_END(); } diff --git a/src/lib/xcealloc.c b/src/lib/xcealloc.c index 4a4724be..182eac54 100644 --- a/src/lib/xcealloc.c +++ b/src/lib/xcealloc.c @@ -1,4 +1,8 @@ +/* vasprintf() (GNU extension) builds the equality functions' difference + * descriptions. */ +#define _GNU_SOURCE #include +#include #include #include #include @@ -454,3 +458,225 @@ int merge_job_alloc(xce_job_alloc_t* dst, const xce_job_alloc_t* src) return merge_allocs(&(dst->nres_shared), &(dst->allocs_shared), src->nres_shared, src->allocs_shared); } + +/* ------------------------------------------------------------------------ + * Allocation equality (libxce-alloc-preason-equality). Contract in + * xcealloc.h. The description, when requested, is built bottom-up: the + * innermost comparison formats the differing field, and each enclosing list + * prefixes its own path segment and frees the inner string. + * ---------------------------------------------------------------------- */ + +/* *why = formatted string, or NULL if the allocation fails (vasprintf leaves + * its output undefined then). No-op when why is NULL. */ +static void eq_why(char** why, const char* fmt, ...) + __attribute__((format(printf, 2, 3))); + +static void eq_why(char** why, const char* fmt, ...) +{ + va_list ap; + char* s = NULL; + + if (why == NULL) { + return; + } + + va_start(ap, fmt); + if (vasprintf(&s, fmt, ap) < 0) { + s = NULL; + } + va_end(ap); + + *why = s; +} + +/* Prefix an inner description with the list entry's path segment, + * `list[i](key=label).`; a failed inner description stays NULL. */ +static void eq_why_wrap(char** why, const char* list, int i, const char* key, + const char* label) +{ + char* inner; + + if ((why == NULL) || (*why == NULL)) { + return; + } + + inner = *why; + eq_why(why, "%s[%d](%s=%s).%s", list, i, key, label, inner); + free(inner); +} + +static const char* eq_label(const char* a, const char* b) +{ + if (a != NULL) { + return a; + } + return (b != NULL) ? b : "?"; +} + +static const char* eq_nullness(const void* p) +{ + return (p != NULL) ? "non-NULL" : "NULL"; +} + +static int eq_str_field(const char* name, const char* a, const char* b, + char** why) +{ + if (str_eq(a, b)) { + return 1; + } + + eq_why(why, "%s: %s%s%s != %s%s%s", name, (a != NULL) ? "\"" : "", + (a != NULL) ? a : "NULL", (a != NULL) ? "\"" : "", + (b != NULL) ? "\"" : "", (b != NULL) ? b : "NULL", + (b != NULL) ? "\"" : ""); + return 0; +} + +/* In-order comparison of two `n`-entry xce_alloc_t lists named `list`. */ +static int eq_alloc_list(const char* list, xce_alloc_t* const* a, + xce_alloc_t* const* b, int n, char** why) +{ + int i; + + for (i = 0; i < n; i++) { + if (a[i] == b[i]) { + continue; + } + if ((a[i] == NULL) || (b[i] == NULL)) { + eq_why(why, "%s[%d]: %s != %s", list, i, eq_nullness(a[i]), + eq_nullness(b[i])); + return 0; + } + if (!alloc_eq(a[i], b[i], why)) { + eq_why_wrap(why, list, i, "resname", + eq_label(a[i]->resname, b[i]->resname)); + return 0; + } + } + + return 1; +} + +int alloc_eq(const xce_alloc_t* a, const xce_alloc_t* b, char** why) +{ + if (why != NULL) { + *why = NULL; + } + if (a == b) { + return 1; + } + if ((a == NULL) || (b == NULL)) { + eq_why(why, "alloc: %s != %s", eq_nullness(a), eq_nullness(b)); + return 0; + } + + if (!eq_str_field("resname", a->resname, b->resname, why) + || !eq_str_field("reserve", a->reserve, b->reserve, why)) { + return 0; + } + if (a->value != b->value) { + eq_why(why, "value: %.17g != %.17g", a->value, b->value); + return 0; + } + if (a->duration != b->duration) { + eq_why(why, "duration: %" PRId64 " != %" PRId64, a->duration, + b->duration); + return 0; + } + if (a->from != b->from) { + eq_why(why, "from: %" PRIu64 " != %" PRIu64, a->from, b->from); + return 0; + } + + return 1; +} + +int host_alloc_eq(const xce_host_alloc_t* a, const xce_host_alloc_t* b, + char** why) +{ + if (why != NULL) { + *why = NULL; + } + if (a == b) { + return 1; + } + if ((a == NULL) || (b == NULL)) { + eq_why(why, "host_alloc: %s != %s", eq_nullness(a), eq_nullness(b)); + return 0; + } + + if (!eq_str_field("host", a->host, b->host, why)) { + return 0; + } + if (a->nallocs != b->nallocs) { + eq_why(why, "nallocs: %d != %d", a->nallocs, b->nallocs); + return 0; + } + + return eq_alloc_list("allocs", a->allocs, b->allocs, a->nallocs, why); +} + +static void eq_fmt_jobid(char* buf, size_t sz, uint64_t jobid) +{ + if (fmt_jobid(buf, sz, jobid) < 0) { + snprintf(buf, sz, "%" PRIu64, jobid); + } +} + +int job_alloc_eq(const xce_job_alloc_t* a, const xce_job_alloc_t* b, + char** why) +{ + int i; + + if (why != NULL) { + *why = NULL; + } + if (a == b) { + return 1; + } + if ((a == NULL) || (b == NULL)) { + eq_why(why, "job_alloc: %s != %s", eq_nullness(a), eq_nullness(b)); + return 0; + } + + if (a->jobid != b->jobid) { + if (why != NULL) { + char ja[128]; + char jb[128]; + + eq_fmt_jobid(ja, sizeof(ja), a->jobid); + eq_fmt_jobid(jb, sizeof(jb), b->jobid); + eq_why(why, "jobid: %s != %s", ja, jb); + } + return 0; + } + if (a->nhosts != b->nhosts) { + eq_why(why, "nhosts: %d != %d", a->nhosts, b->nhosts); + return 0; + } + if (a->nres_shared != b->nres_shared) { + eq_why(why, "nres_shared: %d != %d", a->nres_shared, b->nres_shared); + return 0; + } + + for (i = 0; i < a->nhosts; i++) { + const xce_host_alloc_t* ha = a->allocs[i]; + const xce_host_alloc_t* hb = b->allocs[i]; + + if (ha == hb) { + continue; + } + if ((ha == NULL) || (hb == NULL)) { + eq_why(why, "allocs[%d]: %s != %s", i, eq_nullness(ha), + eq_nullness(hb)); + return 0; + } + if (!host_alloc_eq(ha, hb, why)) { + eq_why_wrap(why, "allocs", i, "host", eq_label(ha->host, hb->host)); + return 0; + } + } + + return eq_alloc_list("allocs_shared", a->allocs_shared, b->allocs_shared, + a->nres_shared, why); +}