From bbc13cd754f5b489a5e84d5b3b5ba85d8d75d96b Mon Sep 17 00:00:00 2001 From: Rodrigo Batista de Moraes Date: Wed, 2 Sep 2026 19:13:13 -0300 Subject: [PATCH] Fix section lookup after a boundary flush Problem: A value lookup at a section's end time also checks the next section, which may contain a later value change at that same time. When trying to read to next section, the code was seeking to the wrong position, causing it to read the current section's header instead of the next section's header. This led to incorrect behavior when the next section was a value change section, later resulting in a heap overflow when parsing the chain table. Solution: read the next section at its actual start and use its header only when it begins at the requested time. AI-assisted --- src/fstapi.c | 34 +++++++++++++--------- tests/meson.build | 1 + tests/rvat_section_boundary.c | 54 +++++++++++++++++++++++++++++++++++ 3 files changed, 76 insertions(+), 13 deletions(-) create mode 100644 tests/rvat_section_boundary.c diff --git a/src/fstapi.c b/src/fstapi.c index 15440f0..eaa267f 100644 --- a/src/fstapi.c +++ b/src/fstapi.c @@ -6092,7 +6092,7 @@ char *fstReaderGetValueFromHandleAtTime(fstReaderContext *xc, fstHandle facidx, char *buf) { - fst_off_t blkpos = 0, prev_blkpos; + fst_off_t blkpos = 0; uint64_t beg_tim, end_tim, beg_tim2, end_tim2; int sectype; #ifdef FST_DEBUG @@ -6136,7 +6136,7 @@ char *fstReaderGetValueFromHandleAtTime(fstReaderContext *xc, xc->rvat_chain_pos_valid = 0; for (;;) { - fstReaderFseeko(xc, xc->f, (prev_blkpos = blkpos), SEEK_SET); + fstReaderFseeko(xc, xc->f, blkpos, SEEK_SET); sectype = fgetc(xc->f); seclen = fstReaderUint64(xc->f); @@ -6158,23 +6158,31 @@ char *fstReaderGetValueFromHandleAtTime(fstReaderContext *xc, if ((beg_tim <= tim) && (tim <= end_tim)) { if ((tim == end_tim) && (tim != xc->end_time)) { fst_off_t cached_pos = ftello(xc->f); - fstReaderFseeko(xc, xc->f, blkpos, SEEK_SET); + fst_off_t next_blkpos = blkpos + seclen; + int sectype2; + uint64_t seclen2; + + fstReaderFseeko(xc, xc->f, next_blkpos, SEEK_SET); - sectype = fgetc(xc->f); - seclen = fstReaderUint64(xc->f); + sectype2 = fgetc(xc->f); + seclen2 = fstReaderUint64(xc->f); beg_tim2 = fstReaderUint64(xc->f); end_tim2 = fstReaderUint64(xc->f); - if (((sectype != FST_BL_VCDATA) && (sectype != FST_BL_VCDATA_DYN_ALIAS) && - (sectype != FST_BL_VCDATA_DYN_ALIAS2)) || - (!seclen) || (beg_tim2 != tim)) { - blkpos = prev_blkpos; - break; + if (((sectype2 != FST_BL_VCDATA) && (sectype2 != FST_BL_VCDATA_DYN_ALIAS) && + (sectype2 != FST_BL_VCDATA_DYN_ALIAS2)) || + (!seclen2) || (beg_tim2 != tim)) { + /* keep this section, and the read position within it */ + fstReaderFseeko(xc, xc->f, cached_pos, SEEK_SET); + } else { + /* next section have the time, use it instead */ + sectype = sectype2; + seclen = seclen2; + beg_tim = beg_tim2; + end_tim = end_tim2; + blkpos = next_blkpos + 1; } - beg_tim = beg_tim2; - end_tim = end_tim2; - fstReaderFseeko(xc, xc->f, cached_pos, SEEK_SET); } break; } diff --git a/tests/meson.build b/tests/meson.build index a2aa892..4f919ae 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -3,6 +3,7 @@ libfst_tests = [ 'empty_file', 'mask_no_variables', 'packed_extent_wraps', + 'rvat_section_boundary', 'write_and_read', ] diff --git a/tests/rvat_section_boundary.c b/tests/rvat_section_boundary.c new file mode 100644 index 0000000..9dfa0ad --- /dev/null +++ b/tests/rvat_section_boundary.c @@ -0,0 +1,54 @@ +#include +#include + +// Random access reads at the end time of a value change section: one where +// the next section begins at that same time, and one at the end of the last +// value change section, in a file that ends later. + +int main(int argc, char **argv) +{ + (void)argc; + (void)argv; + + fstWriterContext *writer = fstWriterCreate("rvat_section_boundary.fst", 1); + g_assert_true(writer != NULL); + + fstHandle var = fstWriterCreateVar(writer, FST_VT_VCD_WIRE, FST_VD_IMPLICIT, 1, "var", 0); + + fstWriterEmitTimeChange(writer, 0); + fstWriterEmitValueChange(writer, var, "0"); + fstWriterEmitTimeChange(writer, 1); + fstWriterEmitValueChange(writer, var, "1"); + fstWriterEmitTimeChange(writer, 2); + fstWriterEmitValueChange(writer, var, "0"); + + // The section is flushed at the next time change: it ends at 2, and the + // next one begins at 2. + fstWriterFlushContext(writer); + + fstWriterEmitTimeChange(writer, 3); + fstWriterEmitValueChange(writer, var, "1"); + fstWriterEmitTimeChange(writer, 4); + fstWriterEmitValueChange(writer, var, "0"); + + // Same again, ending the second section at 4; the third one holds no value + // changes, so it is not written, but the file still ends at 5. + fstWriterFlushContext(writer); + + fstWriterEmitTimeChange(writer, 5); + + fstWriterClose(writer); + + fstReaderContext *reader = fstReaderOpen("rvat_section_boundary.fst"); + g_assert_true(reader != NULL); + g_assert_cmpint(fstReaderGetEndTime(reader), ==, 5); + + char buf[64]; + + g_assert_cmpstr(fstReaderGetValueFromHandleAtTime(reader, 2, var, buf), ==, "0"); + g_assert_cmpstr(fstReaderGetValueFromHandleAtTime(reader, 4, var, buf), ==, "0"); + + fstReaderClose(reader); + + return 0; +}