Skip to content

Fix the front-coded dictionaries with no keys - #13

Open
ilgrad wants to merge 1 commit into
migumar2:masterfrom
ilgrad:fix/zero-keys
Open

ilgrad wants to merge 1 commit into
migumar2:masterfrom
ilgrad:fix/zero-keys

Conversation

@ilgrad

@ilgrad ilgrad commented Nov 3, 2026

Copy link
Copy Markdown

Problem

A dictionary built with no keys reads past a heap array when it is built. A query on it then crashes.

The over-read is silent in a plain run: build, save, load, extract and extractTable exit 0. It shows only under AddressSanitizer. The queries are not silent. In a plain run locate aborts, and locatePrefix and extractPrefix give SIGSEGV.

The Build tool refuses an empty key file (Error: malloc failed, from RePair/Coder/basics.cpp:38), so this is reached through the library, from an iterator with no keys.

Reproducer

master at 568bf8a, all five front-coded structures (HTFC, HHTFC, RPFC and RPHTFC build a PFC first):

uchar *text = new uchar[1]();          // the iterator takes it
StringDictionary *d = new StringDictionaryPFC(new IteratorDictStringPlain(text, 0), 8);
uchar q[] = "a";
d->locate(q, 1);           // GCC 16.2.1: terminate called after throwing 'char const*', SIGABRT
d->locatePrefix(q, 1);     // SIGSEGV

clang 22.1.8 with -fsanitize=address, at the first line:

ERROR: AddressSanitizer: heap-buffer-overflow
    #0 LogSequence::setField  utils/LogSequence.h:146
    #1 LogSequence::LogSequence
    #2 StringDictionaryPFC::StringDictionaryPFC  StringDictionaryPFC.cpp:122

Cause

Two separate causes, one input:

  1. The positional index. StringDictionaryPFC.cpp:122 builds it with bits(bytesStrings), which is bits(0) = 0. LogSequence then has fields of 0 bits and numElementsFor(0, 1) = 0 words, and setField and getField still go through array[0]: set_field and get_field in utils/LogSequence.h, called at utils/LogSequence.cpp:98 and :81.
  2. The searches. locateBucket starts with center = 0 and never enters its loop, because right = buckets = 0. It then looks at bucket center-1, which wraps to SIZE_MAX, and getField throws. locatePrefix and extractPrefix get no bucket range and read a bucket anyway.
Search PFC HTFC HHTFC RPFC RPHTFC
center = 0 in locateBucket :410 :628 :687 :505 :630
leftBucket > NORESULT in prefix search :226 :415 :468 :319 :410

Fix

  • LogSequence: a field of 0 bits holds 0, so getField returns 0 and setField writes nothing. It is fixed in LogSequence, not at the caller, so the next caller with 0 bits is safe too. The rejected alternative was max(bits(bytesStrings),1) at StringDictionaryPFC.cpp:122, StringDictionaryHTFC.cpp:300 and StringDictionaryHHTFC.cpp:352. It moves the saved bytes of a zero-key file and leaves LogSequence as it is.
  • locateBucket starts at center = 1. It changes nothing with buckets, because the loop sets center first.
  • The prefix searches also check buckets > 0.
  • StringDictionaryRPFC.cpp:159 and StringDictionaryRPHTFC.cpp:172 take beginnings[buckets] = ibytes;, which is the change in Fix write past the bucket beginnings in RPFC and RPHTFC #5, since zero keys write index 1 of a 1-entry vector there.

Validation

tests/zero_keys.cpp: all five structures at buckets 2 and 8, built, saved and loaded. It expects numElements 0, locate NORESULT, extract NULL with length 0, extractTable and locatePrefix with no next, and extractPrefix NULL. The repository has no test framework, so this is a standalone program with a tests/% rule in the Makefile.

F="-O2 -DNDEBUG -std=gnu++14 -Wno-narrowing"          # master needs these two on a current GCC
make -C libcds CPPFLAGS="$F"
make FLAGS="$F -I libcds/includes/"
make tests/zero_keys FLAGS="$F -I libcds/includes/" && tests/zero_keys
Tree GCC 16.2.1 clang 22.1.8 with -fsanitize=address
master terminate called, SIGABRT heap-buffer-overflow
utils/LogSequence.cpp back at master ok heap-buffer-overflow
any one of StringDictionary{PFC,HTFC,HHTFC,RPFC,RPHTFC}.cpp back at master terminate called, SIGABRT terminate called or heap-buffer-overflow
this branch ok ok

The LogSequence row is the plain over-read: the test passes there in a plain build, so that half fails only under ASan.

Not run: libcds/tests, Test r, GCC's ASan (its library is not on this machine), compilers older than GCC 16, and a zero-key file written by master and read by this branch. Those files are cmp identical to this branch's.

Compatibility

No saved byte moves. Zero-key files equal master's by cmp in all five structures at bucket 8. On 2,048 and 100,000 keys, all five structures at buckets 2 and 8, all 20 files equal master's. LogSequence changes only where numbits is 0.

Conflicts: git merge-tree merges this branch cleanly with each of #5 to #11 and #12. It carries the identical line of #5 in two files, and git merges that without a conflict, so either can land first. The merge of all nine passes all nine tests, plain and under ASan.

With no keys the positional index has fields of 0 bits and no word to hold them,
yet LogSequence touched word 0. RPFC and RPHTFC also wrote past their bucket
beginnings, the search for a string gave bucket center-1 from center 0, and the
search for a prefix read bucket 1. A field of no bits now reads as 0, the search
gives NORESULT, and an empty range ends the prefix search.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant