Fix crashes and memory leaks reachable from documented commands

[1a1a11a]

Part 3 of 5, split out of #324. Base of the code stack — #329 builds on this, and #330 on that. Independent of #326 and #327.

Every bug here is reachable from a command in the quickstart guides. The regression tests that pin them are in #330, split out so this PR stays a readable set of fixes.

Crashes

Command	Cause
cachesim … slru/qdlp/s3fifod/s3fifov0/flashProb … -e print	the reporting path dereferences state that parse_params builds after it
traceAnalyzer -o my-output …	-o declared OPTION_ARG_OPTIONAL, so argp passes arg == NULL for the space-separated form and strncpy dereferences it
mrcProfiler -o out …	same, plus --algo, --size, --profiler, --profiler-params
traceAnalyzer --verbose …	is_true(NULL) → strcasecmp(NULL, …)
cachesim … slru … -e n-seg=0	divide by zero in SLRU_init
cachesim … wtinyLFU … --consider-obj-metadata=true	WTinyLFU_init read params->main_cache->obj_md_size from a malloc'd, un-memset struct before main_cache was assigned

SLRU also wrote past seg_size_array[SLRU_MAX_N_SEG] when given more than 16 colon-separated sizes, before the validity check ran. n-seg and seg-size are now validated at parse time, so invalid input says what is wrong instead of aborting later with a figure like -9223372036854775808 bytes.

Memory leaks

The ubuntu job builds with LeakSanitizer, but nothing ran the binaries to completion, which is where these live:

• The -e print early exit leaked the strdup'd parameter string in 29 files — exit(0) comes before the free at the end of the parser. GLCache, Mithril and PG never kept the original pointer at all, so they leaked on every call, not just the print path.
• Missing frees, visible only on teardown: Size_free, RandomLRU_free and SLRUv0_free never freed their params struct; S3FIFOd_free freed three of its five sub-caches, leaving 73 KB; SLRUv0_cool allocated a request and early-returned on i == 0 without freeing it, once per eviction from the bottom segment; WTinyLFU_free never freed its params.

Consistency

-e print is documented for every algorithm, but Clock2QPlus and pluginCache rejected the bare print key before reaching their own print branch, and WTinyLFU had no print branch at all despite taking parameters. All three now behave like the rest.

One deliberate behavior change

With the uninitialized read fixed, WTinyLFU_can_insert() charges the window its own per-object overhead rather than the main cache's. The two genuinely differ — LRU and SLRU reserve 16 bytes, FIFO and Clock none — so main-cache=FIFO was charging the window nothing for an overhead it does reserve.

This moves results: main-cache=FIFO with metadata on goes 0.8619 → 0.8070, and clock 0.7732 → 0.7733. It is only reachable at all because the configuration used to segfault, so there is no working behavior on either side being changed. With metadata off nothing changes, and LRU, SLRU, sieve and ARC are unchanged either way.

Testing

• builds clean on this branch alone with -Wall -Wextra -Werror
• ctest --output-on-failure — 9/9 (the tenth target, testCLI, arrives in Add a CLI regression test target #330)
• 28 deterministic algorithms produce miss ratios identical to develop; lecarv0, cacheus, fifo-merge and RandomLRU vary run to run on their own (FIFO_Merge has an explicit next_rand() tiebreaker) and were compared against themselves

---

Generated by Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 256a188ee6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Use the main cache overhead when checking promotions

When metadata accounting is enabled with a lower-overhead main policy such as FIFO or Clock, assigning the parent the maximum overhead makes WTinyLFU_evict() use the window's 16-byte overhead in its main-cache capacity check (params->req_local->obj_size + cache->obj_md_size). This incorrectly treats the main cache as full, causing unnecessary evictions and altered simulation results; if the main cache is empty and the promoted object fits exactly under its own zero-byte overhead, the code enters the full-cache branch and dereferences the null result of main_cache->to_evict(). The promotion check needs to use main_cache->obj_md_size rather than this parent-level value.

Useful? React with 👍 / 👎.

[1a1a11a]

Confirmed, and this one was mine — fixed.

cache->obj_md_size is the MAX of the two sub-caches so that a caller asking the composite what it reserves is not told less than it really does. Using that same value for the main cache's capacity check was wrong, exactly as you describe: against a FIFO or Clock main cache, which reserve nothing, it billed the window's 16 bytes and called the main cache full early. The check now uses main_cache->obj_md_size:

if (main_cache->get_occupied_byte(main_cache) +
        params->req_local->obj_size + main_cache->obj_md_size <=
    main_cache->cache_size) {

Worth being precise about what changed and when. Before this PR the same line read cache->obj_md_size too, but that field was copied from a malloc'd, un-memset struct before main_cache was assigned, so the whole configuration segfaulted and no value was ever correct. Fixing the uninitialized read is what made this line reachable, and my MAX is what made it wrong in a new way. So: not a pre-existing bug I inherited, a regression I introduced while fixing the crash.

Results move again as a result — main-cache=FIFO with metadata on is now 0.7836, where my previous number in this PR's description was 0.8070:

main-cache	miss ratio
FIFO	0.7836
clock	0.7679
LRU	0.7766
SLRU	0.8040

The null-dereference half is real but latent rather than observed: DEBUG_ASSERT compiles out in Release, so an empty main cache reaching the else branch would deref main_cache->to_evict()'s NULL. I did not manage to trigger it on the sample trace, so I am reporting it as reasoning rather than a reproduction — the arithmetic fix removes the path either way.

ctest 10/10.

---

Generated by Claude Code

[Copilot]

Pull request overview

This PR hardens libCacheSim’s CLI-reachable code paths by fixing crashes and teardown-visible leaks across multiple cache/prefetch implementations and the traceAnalyzer / mrcProfiler command-line parsers. It primarily targets early-exit parameter reporting (-e print), option parsing edge cases, and missing frees in cache destructors.

Changes:

• Fix -e print / parameter parsing early-exit leaks by tracking and freeing the original strdup() buffer across many algorithms.
• Prevent crashes from NULL option arguments (argp optional args) and make output-path options require an argument in traceAnalyzer / mrcProfiler.
• Address correctness/leak issues in specific algorithms (e.g., WTinyLFU init ordering/metadata sizing, SLRU param reporting before segment allocation, missing frees in several cache free functions, hashpower clamping).

Reviewed changes

Copilot reviewed 35 out of 38 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
libCacheSim/cache/prefetch/PG.c	Free duplicated param string on print/default and normal completion.
libCacheSim/cache/prefetch/Mithril.c	Free duplicated param string on print/default and normal completion.
libCacheSim/cache/eviction/WTinyLFU.c	Zero-init params, defer metadata sizing until subcaches exist, add print support, free params on teardown.
libCacheSim/cache/eviction/TwoQ.c	Free param buffer on print early exit.
libCacheSim/cache/eviction/SLRUv0.c	Zero-init params, clamp hashpower, free params on teardown, avoid leaking request on early return in cool path.
libCacheSim/cache/eviction/SLRU.c	Make current_params safe before segment allocation; validate n-seg / seg-size; free param buffer on print exit.
libCacheSim/cache/eviction/Size.c	Free params struct in Size_free().
libCacheSim/cache/eviction/S3FIFOv0.c	Avoid deref of NULL main cache during -e print; free param buffer on print exit.
libCacheSim/cache/eviction/S3FIFOd.c	Clamp hashpower; free all subcaches (including eviction trackers); safe param reporting before build; free param buffer on print exit.
libCacheSim/cache/eviction/S3FIFO.c	Free param buffer on print exit.
libCacheSim/cache/eviction/RandomLRU.c	Free params struct on teardown; free param buffer on print exit.
libCacheSim/cache/eviction/QDLP.c	Avoid deref of NULL main cache during -e print; free param buffer on print exit.
libCacheSim/cache/eviction/plugin_cache.c	Allow bare print flag; free param buffer on print exit.
libCacheSim/cache/eviction/other/S3LRU.c	Free param buffer on print exit.
libCacheSim/cache/eviction/other/flashProb.c	Avoid deref of NULL RAM cache during -e print; free param buffer on print exit.
libCacheSim/cache/eviction/LRUProb.c	Free param buffer on print exit.
libCacheSim/cache/eviction/LeCaR.c	Free param buffer on print exit.
libCacheSim/cache/eviction/Hyperbolic.c	Free param buffer on print exit.
libCacheSim/cache/eviction/GLCache/GLCache.c	Free duplicated init-param string on print/default and normal completion.
libCacheSim/cache/eviction/fifo/SFIFOv0.c	Guard against divide-by-zero in hashpower divisor; free param buffer on print exit.
libCacheSim/cache/eviction/fifo/SFIFO.c	Free param buffer on print exit.
libCacheSim/cache/eviction/fifo/LP_TwoQ.c	Free param buffer on print exit.
libCacheSim/cache/eviction/fifo/LP_SFIFO.c	Clamp hashpower; free param buffer on print exit.
libCacheSim/cache/eviction/fifo/LP_ARC.c	Free param buffer on print exit.
libCacheSim/cache/eviction/FIFO_Reinsertion.c	Free param buffer on print exit.
libCacheSim/cache/eviction/FIFO_Merge.c	Free param buffer on print exit.
libCacheSim/cache/eviction/cpp/LRU_K.cpp	Free duplicated param buffer on print exit.
libCacheSim/cache/eviction/ClockPro.c	Free param buffer on print exit.
libCacheSim/cache/eviction/Clock2QPlus.c	Allow bare print flag; free param buffer on print exit.
libCacheSim/cache/eviction/Clock.c	Free param buffer on print exit.
libCacheSim/cache/eviction/CAR.c	Free param buffer on print exit.
libCacheSim/cache/eviction/Cacheus.c	Clamp hashpower reduction.
libCacheSim/cache/eviction/BeladySize.c	Free param buffer on print exit.
libCacheSim/cache/eviction/ARCv0.c	Free param buffer on print exit.
libCacheSim/cache/eviction/ARC.c	Free param buffer on print exit.
libCacheSim/bin/traceAnalyzer/cli_parser.cpp	Make --output/-o require an argument; update example path.
libCacheSim/bin/mrcProfiler/cli_parser.cpp	Make multiple options require arguments (incl. --output); avoid NULL arg derefs.
libCacheSim/bin/cli_reader_utils.c	Treat presence of optional argp flags (NULL arg) as true in is_true().

Suppressed comments (2)

libCacheSim/cache/eviction/SLRU.c:494

• In the parse loop, the unknown-parameter branch exits without freeing the strdup'd parameter buffer (old_params_str). This still leaks when the process exits normally (e.g., in builds where ERROR() doesn't abort), even though the print path was fixed.

      free(old_params_str);
      exit(0);
    } else {
      ERROR("%s does not have parameter %s\n", cache->cache_name, key);
      exit(1);

libCacheSim/cache/eviction/TwoQ.c:359

• TwoQ_parse_params() frees old_params_str for the "print" early-exit, but the unknown-parameter branch still exits without freeing the strdup'd buffer. That leaves a leak on exit(1) in builds where ERROR() doesn't abort, and is inconsistent with the other parsers in this PR.

      free(old_params_str);
      exit(0);
    } else {
      ERROR("%s does not have parameter %s\n", cache->cache_name, key);
      exit(1);

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d3dda43c07

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Charge window insertions their actual metadata

With --consider-obj-metadata=true and a main policy whose overhead exceeds the window LRU's 16 bytes—ARC, LeCaR, and Cacheus each charge 40—this maximum is also used by the generic cache_get_base() capacity loop. Every incoming miss is inserted into the window, so reserving 40 bytes there instead of 16 can invoke WTinyLFU_evict() even when the object's actual window footprint fits; that causes an unnecessary admission comparison or eviction and changes simulation results. Preserve the composite's advertised maximum separately or override the parent capacity calculation to charge the window's overhead.

Useful? React with 👍 / 👎.

[1a1a11a]

Consolidated response to the review comments on this PR.

Window metadata, second finding (P1) — fixed. WTinyLFU_get calls cache_get_base, whose eviction loop at cache.c:252 charges an incoming object cache->obj_md_size, and every incoming object enters the window. Setting that field to the maximum of the two sub-caches made the loop reserve up to 40 bytes for a 16-byte window insertion against an ARC, LeCaR or Cacheus main cache. It is the window's overhead now, so all three sites charge the cache the object is actually entering — this one and can_insert the window, evict the main cache on promotion.

Measured at 100 MB with metadata on: LeCaR moves 0.7894 → 0.7895 and ARC, LRU, SLRU and FIFO do not move, because objects are large relative to a 24-byte difference. Correct rather than consequential, but it removes the last place the composite billed an overhead belonging to a cache the object was not entering.

WTinyLFU_parse_params and the other exit(1) paths — not changed, and here is the check. The suggestion was that the out-of-range and unknown-parameter branches leak old_params_str. They sit behind ERROR(), which calls abort(), and abort() bypasses LeakSanitizer's atexit handler, so nothing is reported and exit(1) is never reached:

$ cachesim ... slru 100mb -e bogus=1        # LSan build
[ERROR] SLRU.c:493 SLRU does not have parameter bogus
rc=134                                       # SIGABRT, no leak report

27 files share that shape. The frees I did add are on the -e print path, where exit(0) genuinely runs and LSan genuinely reported. Adding 27 more frees to satisfy a path that only executes if ERROR() is compiled out would be churn — and in that configuration the error message is compiled out too, which is the larger problem.

Everything else raised here was already covered: the -e print early exits, the missing frees on teardown, the OPTION_ARG_OPTIONAL crashes, and the SLRU validation.

---

Generated by Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a65dc8af41

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Preserve the zero hashpower sentinel

When an API caller uses a designated common_cache_params_t initializer without setting hashpower (as several repository tests do), its value is zero, which cache_struct_init() deliberately interprets as HASH_POWER_DEFAULT (23). This clamp converts that sentinel to 4, so every SLRUv0 segment starts with only 16 buckets and must repeatedly reallocate and rehash while processing a large trace, creating a substantial initialization/runtime regression. Keep nonpositive values unchanged and apply the reduction only to an explicitly supplied positive hashpower.

Useful? React with 👍 / 👎.