diff --git a/src/scheduler/src/repl_break.c b/src/scheduler/src/repl_break.c index e86282be..e17cb7f3 100644 --- a/src/scheduler/src/repl_break.c +++ b/src/scheduler/src/repl_break.c @@ -205,14 +205,15 @@ static bool repl_job_state_matches(uint32_t state, const char* name) return false; } -/* repl_u64_str_eq - compares a uint64_t field against a filter's string - * value (every jobid= filter goes through this). */ -static bool repl_u64_str_eq(uint64_t v, const char* value) +/* repl_filter_jobid_matches - a `jobid=` filter against one jobid. The + * value was parsed once, when the breakpoint was set (repl_cmd_break()), + * so every form the REPL prints or `info trace`/`trace on job` accept + * matches here (bug-1926; repl_jobsel_matches() has the semantics). Every + * jobid= filter, event or decision, goes through this. */ +static bool repl_filter_jobid_matches(const xce_repl_filter_t* f, + uint64_t jobid) { - char buf[32]; - - snprintf(buf, sizeof(buf), "%llu", (unsigned long long)v); - return strcmp(buf, value) == 0; + return repl_jobsel_matches(&(f->jobsel), f->jobsel_raw, jobid); } /* @@ -225,13 +226,16 @@ static bool repl_u64_str_eq(uint64_t v, const char* value) * user typo this function has no channel to report from (called deep * inside the driver loop, not at command-entry time). */ -static bool repl_event_matches_filter(const xce_core_event_t* ev, - const char* key, const char* value) +static bool repl_event_matches_filter(const xce_core_event_t* ev, + const xce_repl_filter_t* f) { + const char* key = f->key; + const char* value = f->value; + switch (ev->type) { case XCE_CORE_EVENT_JOB_STATE_CHANGE: if (strcmp(key, "jobid") == 0) { - return repl_u64_str_eq(ev->u.job_state_change.jobid, value); + return repl_filter_jobid_matches(f, ev->u.job_state_change.jobid); } if (strcmp(key, "state") == 0) { return repl_job_state_matches(ev->u.job_state_change.new_state, @@ -240,12 +244,12 @@ static bool repl_event_matches_filter(const xce_core_event_t* ev, return false; case XCE_CORE_EVENT_JOB_UPDATE: if (strcmp(key, "jobid") == 0) { - return repl_u64_str_eq(ev->u.job_update.jobid, value); + return repl_filter_jobid_matches(f, ev->u.job_update.jobid); } return false; case XCE_CORE_EVENT_JOB_RUSAGE: if (strcmp(key, "jobid") == 0) { - return repl_u64_str_eq(ev->u.job_rusage.jobid, value); + return repl_filter_jobid_matches(f, ev->u.job_rusage.jobid); } return false; case XCE_CORE_EVENT_JOBS_PUSH: @@ -255,7 +259,7 @@ static bool repl_event_matches_filter(const xce_core_event_t* ev, for (i = 0; i < ev->u.jobs_push.njobs; i++) { xce_job_t* j = ev->u.jobs_push.jobs[i]; - if ((j != NULL) && repl_u64_str_eq(j->jobid, value)) { + if ((j != NULL) && repl_filter_jobid_matches(f, j->jobid)) { return true; } } @@ -284,8 +288,7 @@ static bool repl_breakpoint_matches_event(const xce_repl_breakpoint_t* bp, return false; } for (i = 0; i < bp->nfilters; i++) { - if (!repl_event_matches_filter(ev, bp->filters[i].key, - bp->filters[i].value)) { + if (!repl_event_matches_filter(ev, &(bp->filters[i]))) { return false; } } @@ -335,15 +338,18 @@ static uint64_t decision_jobid(const xce_decision_t* d) * core->jobs (freed after completion) never matches a queue= filter, a * documented, narrow gap rather than a crash. */ -static bool repl_decision_matches_filter(xce_sched_core_t* core, - const xce_decision_t* d, - const char* key, const char* value) +static bool repl_decision_matches_filter(xce_sched_core_t* core, + const xce_decision_t* d, + const xce_repl_filter_t* f) { + const char* key = f->key; + const char* value = f->value; + if (strcmp(key, "type") == 0) { return strcasecmp(decision_op_name(d->op), value) == 0; } if (strcmp(key, "jobid") == 0) { - return repl_u64_str_eq(decision_jobid(d), value); + return repl_filter_jobid_matches(f, decision_jobid(d)); } if (strcmp(key, "queue") == 0) { xce_sched_job_t* job = sched_get_job(core, decision_jobid(d)); @@ -365,8 +371,7 @@ static bool repl_breakpoint_matches_decision(xce_sched_core_t* core, return false; } for (i = 0; i < bp->nfilters; i++) { - if (!repl_decision_matches_filter(core, d, bp->filters[i].key, - bp->filters[i].value)) { + if (!repl_decision_matches_filter(core, d, &(bp->filters[i]))) { return false; } } @@ -522,6 +527,13 @@ static bool repl_breakpoint_remove(xce_sched_repl_state_t* rs, int id) * filter aborts the WHOLE command (no partially-applied breakpoint), same * "fail loud, no silent partial state" discipline repl.c's own * repl_cmd_step() doc comment establishes for a bad `step` argument. + * + * bug-1926: a `jobid=` value is parsed here, once, with + * repl_parse_jobsel_arg() (the dashed form the REPL prints, `X[k]`, + * `X[a-b]` or a raw packed decimal id) and the result stored in the + * filter; a value it rejects aborts the command the same way, naming the + * value. The filter's `value` keeps the text as typed, so the "set" line + * and `info break` echo what the operator wrote. */ bool repl_cmd_break(xce_sched_sim_driver_t* sim, const char* line) { @@ -530,6 +542,9 @@ bool repl_cmd_break(xce_sched_sim_driver_t* sim, const char* line) XCE_REPL_MAX_TOKENS); xce_repl_bp_kind_t kind; xce_repl_breakpoint_t* bp; + jobsel_t sel; + jobsel_err_t err; + bool raw; int i; if (ntok < 2) { @@ -566,6 +581,21 @@ bool repl_cmd_break(xce_sched_sim_driver_t* sim, const char* line) } *eq = '\0'; + /* bug-1926: parse a jobid= value now, so a value that can never + * name a job is refused here instead of silently never matching */ + if (strcmp(tokens[i], "jobid") == 0) { + err = repl_parse_jobsel_arg(eq + 1, &sel, &raw); + if (err != JOBSEL_OK) { + fprintf(stdout, "[sim] break: invalid jobid '%s': %s\n", + eq + 1, jobsel_strerror(err)); + repl_breakpoint_free(bp); + return false; + } + } else { + memset(&sel, 0, sizeof(sel)); + raw = false; + } + grown = (xce_repl_filter_t*)realloc( bp->filters, (size_t)(bp->nfilters + 1) * sizeof(*(bp->filters))); if (grown == NULL) { @@ -576,6 +606,8 @@ bool repl_cmd_break(xce_sched_sim_driver_t* sim, const char* line) bp->filters = grown; bp->filters[bp->nfilters].key = makeString(tokens[i]); bp->filters[bp->nfilters].value = makeString(eq + 1); + bp->filters[bp->nfilters].jobsel = sel; + bp->filters[bp->nfilters].jobsel_raw = raw; bp->nfilters++; } diff --git a/src/scheduler/src/repl_fmt.c b/src/scheduler/src/repl_fmt.c index a4442f12..64ed8880 100644 --- a/src/scheduler/src/repl_fmt.c +++ b/src/scheduler/src/repl_fmt.c @@ -1,3 +1,4 @@ +#include #include #include #include @@ -39,6 +40,12 @@ * repl_format_consumable_value_and_used() and repl_current_units_mode(). * The last reads sim->repl_state->units_mode -- the one helper here * that looks at REPL session state, read-only. + * - Job selector arguments (bug-1930, moved here from repl_trace_store.c + * by bug-1926): repl_parse_jobsel_arg(), the one parser for every REPL + * argument that names jobs (`info trace`, `trace on job`, a `break` + * `jobid=` filter), and repl_jobsel_matches(), the `jobid=` filter's + * match against one jobid. They live here, not in the trace store, so + * breakpoints (repl_break.c) need not depend on the trace store. * * Every one of these functions is already extern-declared in * repl_internal.h -- this move changes only which .c file DEFINES them, @@ -50,7 +57,7 @@ * * Boundary (Req 5 AC 17, "Simulation isolation", same as repl.c/repl_break.c/ * repl_info.c/repl_params.c): this file touches only stdio/stdlib/string/ - * stdarg/time plus repl.h/repl_internal.h/utils.h -- no xcecli_ctx_t, no + * stdarg/time/errno plus repl.h/repl_internal.h/utils.h -- no xcecli_ctx_t, no * xce_cluster_config_t, no capnp socket, no controller RPC. * test_sim_isolation.c (task 12.1, extended by this remediation) scans this * file too. @@ -489,3 +496,74 @@ repl_current_units_mode(xce_sched_sim_driver_t* sim) return (sim->repl_state != NULL) ? sim->repl_state->units_mode : XCE_REPL_UNITS_BOTH; } + +/* ---- job selector arguments (bug-1930, bug-1926) ------------------------ */ + +static bool all_digits(const char* s) +{ + if (*s == '\0') { + return false; + } + for (; *s != '\0'; s++) { + if ((*s < '0') || (*s > '9')) { + return false; + } + } + return true; +} + +jobsel_err_t repl_parse_jobsel_arg(const char* arg, jobsel_t* out, bool* raw) +{ + jobsel_err_t err = parse_jobsel(arg, out); + unsigned long long v; + + if (raw != NULL) { + *raw = false; + } + if ((err == JOBSEL_OK) || (arg == NULL) || !all_digits(arg)) { + return err; + } + errno = 0; + v = strtoull(arg, NULL, 10); + if (errno == ERANGE) { + return JOBSEL_EWIDTH; + } + memset(out, 0, sizeof(*out)); + out->form = JOBSEL_BARE; + out->key = (uint64_t)v; + if (raw != NULL) { + *raw = true; + } + return JOBSEL_OK; +} + +bool repl_jobsel_matches(const jobsel_t* sel, bool raw, uint64_t jobid) +{ + uint32_t idx; + + if (sel == NULL) { + return false; + } + if (raw) { /* a raw packed id names exactly that id */ + return jobid == sel->key; + } + + switch (sel->form) { + case JOBSEL_SINGLE: + case JOBSEL_RANGE: + idx = (uint32_t)EXTRACT_ARRAY_IDX(jobid); + return is_array_tag(jobid) && (idx >= sel->lo) && (idx <= sel->hi) + && (jobsel_element_id(sel, idx) == jobid); + case JOBSEL_BARE: + break; + } + + /* the plain job, or (only for an array-aligned id) any element of the + * array with that id: what `trace on job` selects + * (scheduler-sim-decision-trace Requirements 1.2, 1.3) */ + if (jobid == sel->key) { + return true; + } + return (EXTRACT_ARRAY_IDX(sel->key) == 0) && is_array_tag(jobid) + && (array_jobid(jobid) == sel->key); +} diff --git a/src/scheduler/src/repl_internal.h b/src/scheduler/src/repl_internal.h index 099e0f7b..374bd316 100644 --- a/src/scheduler/src/repl_internal.h +++ b/src/scheduler/src/repl_internal.h @@ -117,6 +117,12 @@ typedef enum { typedef struct { char* key; char* value; + /* bug-1926: a `jobid=` value, parsed once by repl_cmd_break() with + * repl_parse_jobsel_arg() and matched with repl_jobsel_matches(); + * `value` keeps the text as typed for the listings. Unused (zero) for + * every other key. */ + jobsel_t jobsel; + bool jobsel_raw; } xce_repl_filter_t; /* xce_repl_breakpoint_t (task 12.2) - one entry set by `break`, listed by @@ -647,15 +653,26 @@ extern bool repl_cmd_info_trace(xce_sched_sim_driver_t* sim, char tokens[][XCE_REPL_TOKEN_LEN], int ntok); /* - * repl_parse_jobsel_arg (repl_trace_store.c, bug-1930) - the job selector - * forms `info trace` and `trace on job` both take: parse_jobsel()'s dashed - * id, `X[k]` and `X[a-b]`, plus (like `info job`) a raw packed decimal id, - * returned as JOBSEL_BARE with `key` = the number and `*raw` = true (a - * decimal too wide for 64 bits is JOBSEL_EWIDTH). `*raw` is false for - * every other form; `raw` may be NULL. + * repl_parse_jobsel_arg (repl_fmt.c, bug-1930; moved from + * repl_trace_store.c by bug-1926) - the job selector forms `info trace`, + * `trace on job` and a `break ... jobid=` filter all take: parse_jobsel()'s + * dashed id, `X[k]` and `X[a-b]`, plus (like `info job`) a raw packed + * decimal id, returned as JOBSEL_BARE with `key` = the number and `*raw` = + * true (a decimal too wide for 64 bits is JOBSEL_EWIDTH). `*raw` is false + * for every other form; `raw` may be NULL. + * + * repl_jobsel_matches (repl_fmt.c, bug-1926) - whether `jobid` is one of + * the jobs a selector parsed by repl_parse_jobsel_arg() names: with `raw`, + * exactly the packed id `sel->key`; `X[k]`/`X[a-b]`, the elements k or + * a..b of array X; a bare id, the plain job with that id or, when the id is + * array-aligned, any element of the array with that id (the same set + * `trace on job` selects, scheduler-sim-decision-trace Requirements 1.2, + * 1.3). NULL `sel`: false. */ extern jobsel_err_t repl_parse_jobsel_arg(const char* arg, jobsel_t* out, bool* raw); +extern bool repl_jobsel_matches(const jobsel_t* sel, bool raw, + uint64_t jobid); /* * ReplTraceControl (repl_trace.c, scheduler-sim-decision-trace spec, task diff --git a/src/scheduler/src/repl_trace_store.c b/src/scheduler/src/repl_trace_store.c index e9c9de65..fefdc461 100644 --- a/src/scheduler/src/repl_trace_store.c +++ b/src/scheduler/src/repl_trace_store.c @@ -30,7 +30,6 @@ * here: `info trace` writes to the console only, and inline emission to a * trace file (task 4.6) uses the settings the session was captured under. */ -#include #include #include #include @@ -430,44 +429,6 @@ static bool info_trace_array(xce_sched_sim_driver_t* sim, return true; } -static bool all_digits(const char* s) -{ - if (*s == '\0') { - return false; - } - for (; *s != '\0'; s++) { - if ((*s < '0') || (*s > '9')) { - return false; - } - } - return true; -} - -jobsel_err_t repl_parse_jobsel_arg(const char* arg, jobsel_t* out, bool* raw) -{ - jobsel_err_t err = parse_jobsel(arg, out); - unsigned long long v; - - if (raw != NULL) { - *raw = false; - } - if ((err == JOBSEL_OK) || (arg == NULL) || !all_digits(arg)) { - return err; - } - errno = 0; - v = strtoull(arg, NULL, 10); - if (errno == ERANGE) { - return JOBSEL_EWIDTH; - } - memset(out, 0, sizeof(*out)); - out->form = JOBSEL_BARE; - out->key = (uint64_t)v; - if (raw != NULL) { - *raw = true; - } - return JOBSEL_OK; -} - /* One `info trace` argument: a job, `array[k]`, `array[a-b]`, an array id, * or (like `info job`) a raw packed jobid. */ static void info_trace_sel(xce_sched_sim_driver_t* sim, diff --git a/src/scheduler/testsuite/test_repl_breakpoints_inspection.c b/src/scheduler/testsuite/test_repl_breakpoints_inspection.c index 4a89455c..80a53e0c 100644 --- a/src/scheduler/testsuite/test_repl_breakpoints_inspection.c +++ b/src/scheduler/testsuite/test_repl_breakpoints_inspection.c @@ -906,6 +906,296 @@ static void test_decision_breakpoint_matches_type_filter(void) printf("test_decision_breakpoint_matches_type_filter: PASS\n"); } +/* ---------------------------------------------------------------------- */ +/* bug-1926: a `jobid=` filter takes the jobid forms the REPL prints and */ +/* `info job`/`info trace`/`trace on job` accept (the dashed id, X[k], */ +/* X[a-b]) as well as the raw packed decimal id, and an unparseable value */ +/* is rejected when the breakpoint is set. */ +/* ---------------------------------------------------------------------- */ + +/* The array 0-0-0-32768 (sequence 1 << 15, so array-aligned) and its + * element k, the packed ids parse_jobsel()'s `X[k]` names. */ +#define B1926_ARRAY_SEQ (1U << N_ARRAY_IDX_BITS) +static uint64_t b1926_elem(uint32_t seq, uint32_t k) +{ + return MAKE_JOBID(0, 0, 0, TAG_JOB_ARRAY, seq | k); +} + +static void b1926_sim_init(xce_sched_sim_driver_t* sim) +{ + memset(sim, 0, sizeof(*sim)); + /* makeString() needs xce_sched_core_init()'s xstring table */ + TEST_ASSERT_EQUAL_INT(0, xce_sched_core_init(&(sim->core))); + sim->repl_state = repl_state_ensure(sim); + TEST_ASSERT_NOT_NULL(sim->repl_state); +} + +/* true iff a PEND decision for `jobid` hits a breakpoint */ +static bool b1926_pend_hits(xce_sched_sim_driver_t* sim, uint64_t jobid) +{ + xce_pendreason_t reason; + xce_decision_t d; + + memset(&reason, 0, sizeof(reason)); + reason.jobid = jobid; + memset(&d, 0, sizeof(d)); + d.op = XCE_DECISION_PEND; + d.u.reason = &reason; + + return repl_find_decision_breakpoint(&(sim->core), sim->repl_state, &d) + != NULL; +} + +static void test_decision_breakpoint_jobid_dashed_form_matches(void) +{ + xce_sched_sim_driver_t sim; + char printed[64]; + char line[128]; + + b1926_sim_init(&sim); + /* the form the hit line prints: "decision: PEND jobid=0-0-0-104" */ + fmt_jobid(printed, sizeof(printed), 104ULL); + TEST_ASSERT_EQUAL_STRING("0-0-0-104", printed); + snprintf(line, sizeof(line), "break DECISION jobid=%s", printed); + TEST_ASSERT_FALSE(repl_dispatch(&sim, line)); + TEST_ASSERT_EQUAL_INT(1, sim.repl_state->nbreakpoints); + + TEST_ASSERT_TRUE_MESSAGE(b1926_pend_hits(&sim, 104ULL), + "jobid=0-0-0-104 must match job 104"); + TEST_ASSERT_FALSE_MESSAGE(b1926_pend_hits(&sim, 105ULL), + "jobid=0-0-0-104 must NOT match job 105"); + TEST_ASSERT_FALSE_MESSAGE(b1926_pend_hits(&sim, 103ULL), + "jobid=0-0-0-104 must NOT match job 103"); + + printf("test_decision_breakpoint_jobid_dashed_form_matches: PASS\n"); +} + +static void test_decision_breakpoint_jobid_element_and_range_match(void) +{ + xce_sched_sim_driver_t sim; + char printed[64]; + char line[128]; + + /* X[k], copied from the REPL's own rendering of element 2 */ + b1926_sim_init(&sim); + fmt_jobid(printed, sizeof(printed), b1926_elem(B1926_ARRAY_SEQ, 2)); + TEST_ASSERT_EQUAL_STRING("0-0-0-32768[2]", printed); + snprintf(line, sizeof(line), "break DECISION jobid=%s", printed); + TEST_ASSERT_FALSE(repl_dispatch(&sim, line)); + TEST_ASSERT_EQUAL_INT(1, sim.repl_state->nbreakpoints); + TEST_ASSERT_TRUE_MESSAGE(b1926_pend_hits(&sim, + b1926_elem(B1926_ARRAY_SEQ, 2)), + "X[2] must match element 2"); + TEST_ASSERT_FALSE_MESSAGE(b1926_pend_hits(&sim, + b1926_elem(B1926_ARRAY_SEQ, 1)), + "X[2] must NOT match element 1"); + TEST_ASSERT_FALSE_MESSAGE(b1926_pend_hits(&sim, + b1926_elem(B1926_ARRAY_SEQ, 3)), + "X[2] must NOT match element 3"); + TEST_ASSERT_FALSE_MESSAGE(b1926_pend_hits(&sim, B1926_ARRAY_SEQ + 2), + "X[2] must NOT match the plain job whose " + "sequence equals element 2's"); + TEST_ASSERT_FALSE_MESSAGE( + b1926_pend_hits(&sim, b1926_elem(2 * B1926_ARRAY_SEQ, 2)), + "X[2] must NOT match element 2 of another array"); + + /* X[a-b] */ + b1926_sim_init(&sim); + TEST_ASSERT_FALSE(repl_dispatch(&sim, "break DECISION jobid=0-0-0-32768[1-3]")); + TEST_ASSERT_EQUAL_INT(1, sim.repl_state->nbreakpoints); + TEST_ASSERT_FALSE(b1926_pend_hits(&sim, b1926_elem(B1926_ARRAY_SEQ, 0))); + TEST_ASSERT_TRUE(b1926_pend_hits(&sim, b1926_elem(B1926_ARRAY_SEQ, 1))); + TEST_ASSERT_TRUE(b1926_pend_hits(&sim, b1926_elem(B1926_ARRAY_SEQ, 2))); + TEST_ASSERT_TRUE(b1926_pend_hits(&sim, b1926_elem(B1926_ARRAY_SEQ, 3))); + TEST_ASSERT_FALSE(b1926_pend_hits(&sim, b1926_elem(B1926_ARRAY_SEQ, 4))); + + printf("test_decision_breakpoint_jobid_element_and_range_match: PASS\n"); +} + +/* A bare array-aligned dashed id selects what `trace on job` selects: the + * plain job with that id and every element of the array with that id. A + * raw packed decimal id keeps its pre-bug-1926 meaning: that exact id. */ +static void test_decision_breakpoint_jobid_bare_array_id_and_raw(void) +{ + xce_sched_sim_driver_t sim; + char line[128]; + + b1926_sim_init(&sim); + TEST_ASSERT_FALSE(repl_dispatch(&sim, "break DECISION jobid=0-0-0-32768")); + TEST_ASSERT_TRUE_MESSAGE(b1926_pend_hits(&sim, B1926_ARRAY_SEQ), + "a bare id matches the plain job"); + TEST_ASSERT_TRUE_MESSAGE(b1926_pend_hits(&sim, + b1926_elem(B1926_ARRAY_SEQ, 0)), + "a bare array id matches element 0"); + TEST_ASSERT_TRUE_MESSAGE(b1926_pend_hits(&sim, + b1926_elem(B1926_ARRAY_SEQ, 7)), + "a bare array id matches element 7"); + TEST_ASSERT_FALSE(b1926_pend_hits(&sim, B1926_ARRAY_SEQ + 1)); + TEST_ASSERT_FALSE( + b1926_pend_hits(&sim, b1926_elem(2 * B1926_ARRAY_SEQ, 0))); + + /* an unaligned bare id is only a plain job (an element's packed id + * carries the array tag, so it never equals a plain id) */ + b1926_sim_init(&sim); + TEST_ASSERT_FALSE(repl_dispatch(&sim, "break DECISION jobid=0-0-0-104")); + TEST_ASSERT_TRUE(b1926_pend_hits(&sim, 104ULL)); + TEST_ASSERT_FALSE(b1926_pend_hits(&sim, b1926_elem(B1926_ARRAY_SEQ, 0))); + + /* raw decimal: exactly that packed id, as before */ + b1926_sim_init(&sim); + snprintf(line, sizeof(line), "break DECISION jobid=%llu", + (unsigned long long)B1926_ARRAY_SEQ); + TEST_ASSERT_FALSE(repl_dispatch(&sim, line)); + TEST_ASSERT_EQUAL_INT(1, sim.repl_state->nbreakpoints); + TEST_ASSERT_TRUE(b1926_pend_hits(&sim, B1926_ARRAY_SEQ)); + TEST_ASSERT_FALSE_MESSAGE(b1926_pend_hits(&sim, + b1926_elem(B1926_ARRAY_SEQ, 0)), + "a raw packed id names one exact job"); + TEST_ASSERT_FALSE(b1926_pend_hits(&sim, B1926_ARRAY_SEQ + 1)); + + /* raw decimal of an element's packed id: that element */ + b1926_sim_init(&sim); + snprintf(line, sizeof(line), "break DECISION jobid=%llu", + (unsigned long long)b1926_elem(B1926_ARRAY_SEQ, 5)); + TEST_ASSERT_FALSE(repl_dispatch(&sim, line)); + TEST_ASSERT_TRUE(b1926_pend_hits(&sim, b1926_elem(B1926_ARRAY_SEQ, 5))); + TEST_ASSERT_FALSE(b1926_pend_hits(&sim, b1926_elem(B1926_ARRAY_SEQ, 4))); + + printf("test_decision_breakpoint_jobid_bare_array_id_and_raw: PASS\n"); +} + +static void test_break_invalid_jobid_rejected(void) +{ + static const char* bad[] = { + "bogus", "0-0-0", "0-0-0-104[", "0-0-0-104x", "-5", + "0-0-0-32768[3-1]", /* reversed range */ + "0-0-0-104[1]", /* element of an unaligned id */ + "99999999999999999999999", /* wider than 64 bits */ + "", /* jobid= with no value */ + }; + size_t i; + + for (i = 0; i < sizeof(bad) / sizeof(bad[0]); i++) { + xce_sched_sim_driver_t sim; + char line[128]; + char want[96]; + char* captured; + + b1926_sim_init(&sim); + snprintf(line, sizeof(line), "break DECISION type=PEND jobid=%s", + bad[i]); + captured = capture_dispatch(&sim, line); + TEST_ASSERT_EQUAL_INT_MESSAGE(0, sim.repl_state->nbreakpoints, line); + snprintf(want, sizeof(want), "invalid jobid '%s'", bad[i]); + TEST_ASSERT_NOT_NULL_MESSAGE(strstr(captured, want), captured); + TEST_ASSERT_NULL_MESSAGE(strstr(captured, "breakpoint 1 set"), + captured); + free(captured); + } + + /* event kinds validate the value the same way */ + { + xce_sched_sim_driver_t sim; + char* captured; + + b1926_sim_init(&sim); + captured = capture_dispatch(&sim, "break JOB_STATE_CHANGE jobid=abc"); + TEST_ASSERT_EQUAL_INT(0, sim.repl_state->nbreakpoints); + TEST_ASSERT_NOT_NULL(strstr(captured, "invalid jobid 'abc'")); + free(captured); + } + + printf("test_break_invalid_jobid_rejected: PASS\n"); +} + +static void test_event_breakpoint_jobid_dashed_form_matches(void) +{ + xce_sched_sim_driver_t sim; + xce_core_event_t ev; + xce_job_t job_a; + xce_job_t job_b; + xce_job_t* jobs[2]; + + /* JOB_STATE_CHANGE */ + b1926_sim_init(&sim); + TEST_ASSERT_FALSE( + repl_dispatch(&sim, "break JOB_STATE_CHANGE jobid=0-0-0-42")); + memset(&ev, 0, sizeof(ev)); + ev.type = XCE_CORE_EVENT_JOB_STATE_CHANGE; + ev.u.job_state_change.jobid = 42; + ev.u.job_state_change.new_state = JOB_STAT_RUN; + TEST_ASSERT_NOT_NULL(repl_find_event_breakpoint(sim.repl_state, &ev)); + ev.u.job_state_change.jobid = 43; + TEST_ASSERT_NULL(repl_find_event_breakpoint(sim.repl_state, &ev)); + + /* JOB_UPDATE, an element */ + b1926_sim_init(&sim); + TEST_ASSERT_FALSE( + repl_dispatch(&sim, "break JOB_UPDATE jobid=0-0-0-32768[4]")); + memset(&ev, 0, sizeof(ev)); + ev.type = XCE_CORE_EVENT_JOB_UPDATE; + ev.u.job_update.jobid = b1926_elem(B1926_ARRAY_SEQ, 4); + TEST_ASSERT_NOT_NULL(repl_find_event_breakpoint(sim.repl_state, &ev)); + ev.u.job_update.jobid = b1926_elem(B1926_ARRAY_SEQ, 5); + TEST_ASSERT_NULL(repl_find_event_breakpoint(sim.repl_state, &ev)); + + /* JOB_RUSAGE, a range */ + b1926_sim_init(&sim); + TEST_ASSERT_FALSE( + repl_dispatch(&sim, "break JOB_RUSAGE jobid=0-0-0-32768[0-1]")); + memset(&ev, 0, sizeof(ev)); + ev.type = XCE_CORE_EVENT_JOB_RUSAGE; + ev.u.job_rusage.jobid = b1926_elem(B1926_ARRAY_SEQ, 1); + TEST_ASSERT_NOT_NULL(repl_find_event_breakpoint(sim.repl_state, &ev)); + ev.u.job_rusage.jobid = b1926_elem(B1926_ARRAY_SEQ, 2); + TEST_ASSERT_NULL(repl_find_event_breakpoint(sim.repl_state, &ev)); + + /* JOB_PUSH, any pushed job */ + b1926_sim_init(&sim); + TEST_ASSERT_FALSE(repl_dispatch(&sim, "break JOB_PUSH jobid=0-0-0-4242")); + memset(&job_a, 0, sizeof(job_a)); + memset(&job_b, 0, sizeof(job_b)); + job_a.jobid = 7; + job_b.jobid = 4242; + jobs[0] = &job_a; + jobs[1] = &job_b; + memset(&ev, 0, sizeof(ev)); + ev.type = XCE_CORE_EVENT_JOBS_PUSH; + ev.u.jobs_push.jobs = jobs; + ev.u.jobs_push.njobs = 2; + TEST_ASSERT_NOT_NULL(repl_find_event_breakpoint(sim.repl_state, &ev)); + job_b.jobid = 4243; + TEST_ASSERT_NULL(repl_find_event_breakpoint(sim.repl_state, &ev)); + + printf("test_event_breakpoint_jobid_dashed_form_matches: PASS\n"); +} + +/* `info break` and the "set" line echo the filter as typed, not a + * re-rendering of the parsed selector. */ +static void test_info_break_echoes_jobid_filter_as_typed(void) +{ + xce_sched_sim_driver_t sim; + char* captured; + + b1926_sim_init(&sim); + captured = capture_dispatch(&sim, + "break DECISION jobid=0-0-0-32768[1-3]"); + TEST_ASSERT_NOT_NULL_MESSAGE( + strstr(captured, + "[sim] breakpoint 1 set: DECISION jobid=0-0-0-32768[1-3]\n"), + captured); + free(captured); + TEST_ASSERT_FALSE(repl_dispatch(&sim, "break DECISION jobid=104")); + + captured = capture_dispatch(&sim, "info break"); + TEST_ASSERT_NOT_NULL_MESSAGE(strstr(captured, "jobid=0-0-0-32768[1-3]"), + captured); + TEST_ASSERT_NOT_NULL_MESSAGE(strstr(captured, "jobid=104"), captured); + free(captured); + + printf("test_info_break_echoes_jobid_filter_as_typed: PASS\n"); +} + static void test_begin_round_breakpoint_bare_matches_filtered_does_not(void) { xce_sched_sim_driver_t sim; @@ -2306,6 +2596,53 @@ static void test_decision_breakpoint_pauses_after_decision_output(void) printf("test_decision_breakpoint_pauses_after_decision_output: PASS\n"); } +/* + * bug-1926: a decision breakpoint set with the jobid exactly as the REPL + * prints it (fmt_jobid(), the dashed form) fires in a real + * sched_sim_driver_run(); before the fix only the raw decimal id matched. + * Same fixture and `step 1` script shape as the test above. + */ +static void test_decision_breakpoint_dashed_jobid_pauses_run(void) +{ + char replay_path[PATH_MAX]; + char aux_dir[PATH_MAX]; + char script_path[PATH_MAX]; + char capture_path[PATH_MAX]; + char script[160]; + char bp_marker[112]; + char jobid_buf[64]; + FILE* f; + char* captured; + + f = write_header_and_snapshot(replay_path, sizeof(replay_path), + "_decdash"); + TEST_ASSERT_NOT_NULL(f); + + append_queue_update_event(f, 1000ULL, "normal"); + append_jobs_push_event(f, 1500ULL, 555ULL, "normal"); + append_replay_session(f, 2000ULL); + append_end_of_stream_marker(f); + fclose(f); + + make_tmp_dir(aux_dir, sizeof(aux_dir), "_decdashaux"); + snprintf(script_path, sizeof(script_path), "%s/script.txt", aux_dir); + snprintf(capture_path, sizeof(capture_path), "%s/capture.out", aux_dir); + fmt_jobid(jobid_buf, sizeof(jobid_buf), 555ULL); + snprintf(script, sizeof(script), "break DECISION jobid=%s\nstep 1\nquit\n", + jobid_buf); + write_text_file(script_path, script); + + captured = run_group_d_and_capture(replay_path, script_path, capture_path); + + snprintf(bp_marker, sizeof(bp_marker), + "hit breakpoint 1 @ decision: PEND jobid=%s", jobid_buf); + TEST_ASSERT_NOT_NULL_MESSAGE(strstr(captured, bp_marker), captured); + + free(captured); + + printf("test_decision_breakpoint_dashed_jobid_pauses_run: PASS\n"); +} + /* * bug-1595 (task 12.5): `step`/`continue` must echo each input event, in * the order it is decoded, BEFORE the session's own resulting decision -- @@ -2465,6 +2802,12 @@ int main(void) RUN_TEST(test_event_breakpoint_matches_state_filter); RUN_TEST(test_decision_breakpoint_matches_type_filter); RUN_TEST(test_begin_round_breakpoint_bare_matches_filtered_does_not); + RUN_TEST(test_decision_breakpoint_jobid_dashed_form_matches); + RUN_TEST(test_decision_breakpoint_jobid_element_and_range_match); + RUN_TEST(test_decision_breakpoint_jobid_bare_array_id_and_raw); + RUN_TEST(test_break_invalid_jobid_rejected); + RUN_TEST(test_event_breakpoint_jobid_dashed_form_matches); + RUN_TEST(test_info_break_echoes_jobid_filter_as_typed); RUN_TEST(test_info_job_reports_state_queue_priority); RUN_TEST(test_info_job_unknown_reports_not_found); @@ -2486,6 +2829,7 @@ int main(void) RUN_TEST(test_event_breakpoint_pauses_repl_at_matching_event); RUN_TEST(test_decision_breakpoint_pauses_after_decision_output); + RUN_TEST(test_decision_breakpoint_dashed_jobid_pauses_run); RUN_TEST(test_step_echoes_events_before_session_decisions); RUN_TEST(test_replay_load_reports_total_session_count); diff --git a/src/scheduler/testsuite/test_repl_trace_e2e.c b/src/scheduler/testsuite/test_repl_trace_e2e.c index f804a1a4..3a404e6d 100644 --- a/src/scheduler/testsuite/test_repl_trace_e2e.c +++ b/src/scheduler/testsuite/test_repl_trace_e2e.c @@ -26,8 +26,8 @@ * (0-0-0-32768[1] and [2]) and a host-selection mismatch (203 selects * mem>20000, which only w02 has). * - * Breakpoint filters compare the raw decimal jobid (repl_break.c - * repl_u64_str_eq), so `break DECISION jobid=105` names 0-0-0-105. + * `break DECISION jobid=105` (a raw packed decimal id) names 0-0-0-105; + * since bug-1926 the dashed form 0-0-0-105 names it too. * * Scenarios: * 1. step mode, all jobs, level full, file copy (baseline replay, five