diff --git a/src/parsers/choice.c b/src/parsers/choice.c
index 90c3662b515babe4a69b0e24dc146ebe1d0a647d..69e4aee778977243594f0ffc124cb3931f4a8d03 100644
--- a/src/parsers/choice.c
+++ b/src/parsers/choice.c
@@ -164,5 +164,6 @@ HParser* h_choice__ma(HAllocator* mm__, void *args[]) {
ret->vtable = &choice_vt;
ret->env = (void*)s;
ret->backend = PB_MIN;
+ ret->desugared = NULL;
return ret;
}
diff --git a/src/parsers/permutation.c b/src/parsers/permutation.c
index b16758413eeafe2ce2ae91db2ebbe7593681d3cd..ec256c4af1f76292847102d0a07eca5cb19e5bae 100644
--- a/src/parsers/permutation.c
+++ b/src/parsers/permutation.c
@@ -176,5 +176,6 @@ HParser* h_permutation__ma(HAllocator* mm__, void *args[]) {
ret->vtable = &permutation_vt;
ret->env = (void*)s;
ret->backend = PB_MIN;
+ ret->desugared = NULL;
return ret;
}
diff --git a/src/parsers/sequence.c b/src/parsers/sequence.c
index 55c0c8885573ef7779714efd49eaf64cc59ac878..5ca034ad50c1b752ff48e202008b21c0d1e75375 100644
--- a/src/parsers/sequence.c
+++ b/src/parsers/sequence.c
@@ -171,5 +171,6 @@ HParser* h_sequence__ma(HAllocator* mm__, void *args[]) {
ret->vtable = &sequence_vt;
ret->env = (void*)s;
ret->backend = PB_MIN;
+ ret->desugared = NULL;
return ret;
}
diff --git a/src/system_allocator.c b/src/system_allocator.c
index 39a1a7e77040c865f2d4f99977eb264391286bb4..f6e9cdcbbe74fedea568a73ada868e93d83c0660 100644
--- a/src/system_allocator.c
+++ b/src/system_allocator.c
@@ -59,6 +59,8 @@ static void* system_realloc(HAllocator *allocator, void* uptr, size_t size) {
if (!uptr) {
return system_alloc(allocator, size);
}
+ // XXX this is incorrect if size == 0 and BLOCK_HEADER_SIZE != 0; it fails
+ // to behave like free(3)
void* block = realloc(block_for_user_ptr(uptr), block_size(size));
if (!block) {
return NULL;
@@ -66,6 +68,7 @@ static void* system_realloc(HAllocator *allocator, void* uptr, size_t size) {
uptr = user_ptr(block);
#ifdef DEBUG__MEMFILL
+ // XXX this is the wrong block; this is reading uninitialized memory
size_t old_size = ((HDebugBlockHeader*)block)->size;
if (size > old_size)
memset((char*)uptr+old_size, DEBUG__MEMFILL, size - old_size);
diff --git a/src/t_regression.c b/src/t_regression.c
index 3dfe5dce12d0d9c9306ede01d5daf3a45ebab488..4e8ad0cdfc9df5c8ba2df32e4faaead50218ee7a 100644
--- a/src/t_regression.c
+++ b/src/t_regression.c
@@ -193,6 +193,83 @@ static void test_charset_bits(void) {
g_check_cmp_uint32(test_charset_bits__buf[32], ==, 0xAB);
}
+
+// Allocator for reproducing error 19.
+
+// The bug is a result of uninitialized data being used, initially
+// assumed to be zero. Unfortunately, this assumption is often true,
+// so reproducing the bug reliably and in a minimal fashion requires
+// making it false. Fortunately, glibc malloc has an M_PERTURB option
+// for making that assumption false. Unfortunately, we want the test
+// to reproduce the bug on systems that don't use glibc. Fortunately,
+// the standard Hammer system allocator has a DEBUG__MEMFILL option to
+// fill uninitialized memory with a fill byte. Unfortunately, you
+// have to recompile Hammer with that symbol #defined in order to
+// enable it. Fortunately, hammer allows you to supply your own
+// allocator. So this is a simple non-#define-dependent allocator
+// that writes 0xbabababa†over all the memory it allocates. (But not
+// the memory it reallocs, because, as it happens, the uninitialized
+// memory in this case didn't come from a realloc.)
+//
+// Honestly I think we ought to remove the #ifdefs from
+// system_allocator and always compile both the DEBUG__MEMFILL version
+// and the non-DEBUG__MEMFILL version, merely changing which one is
+// system_allocator, which is after all a struct of three pointers
+// that can even be modified at run-time.
+//
+// †Can you hear it, Mr. Toot?
+
+static void* deadbeefing_malloc(HAllocator *allocator, size_t size) {
+ char *block = malloc(size);
+ if (block) memset(block, 0xba, size);
+ return block;
+}
+
+// Don't deadbeef on realloc because it isn't necessary to reproduce this bug.
+static void* deadbeefing_realloc(HAllocator *allocator, void *uptr, size_t size) {
+ return realloc(uptr, size);
+}
+
+static void deadbeefing_free(HAllocator *allocator, void *uptr) {
+ free(uptr);
+}
+
+static HAllocator deadbeefing_allocator = {
+ .alloc = deadbeefing_malloc,
+ .realloc = deadbeefing_realloc,
+ .free = deadbeefing_free,
+};
+
+static void test_bug_19() {
+ void *args[] = {
+ h_ch_range__m(&deadbeefing_allocator, '0', '9'),
+ h_ch_range__m(&deadbeefing_allocator, 'A', 'Z'),
+ h_ch_range__m(&deadbeefing_allocator, 'a', 'z'),
+ NULL,
+ };
+
+ HParser *parser = h_choice__ma(&deadbeefing_allocator, args);
+
+ // In bug 19 ("GLR backend reaches unreachable code"), this call
+ // would fail because h_choice__ma allocated an HParser with h_new
+ // and didn't initialize its ->desugared field; consequently in
+ // the call chain h_compile ... h_lalr_compile ... h_desugar,
+ // h_desugar would find that ->desugared was already non-NULL (set
+ // to 0xbabababa in the above deadbeefing_malloc), and just return
+ // it, leading to a crash immediately afterwards in collect_nts.
+ // We don't actually care if the compile succeeds or fails, just
+ // that it doesn't crash.
+ h_compile(parser, PB_GLR, NULL);
+
+ // The same bug happened in h_sequence__ma.
+ h_compile(h_sequence__ma(&deadbeefing_allocator, args), PB_GLR, NULL);
+
+ // It also exists in h_permutation__ma, but it doesn't happen to
+ // manifest in the same way. I don't know how to write a test for
+ // the h_permutation__ma case.
+ g_assert_true(1);
+}
+
void register_regression_tests(void) {
g_test_add_func("/core/regression/bug118", test_bug118);
g_test_add_func("/core/regression/seq_index_path", test_seq_index_path);
@@ -202,4 +279,5 @@ void register_regression_tests(void) {
g_test_add_func("/core/regression/lalr_charset_lhs", test_lalr_charset_lhs);
g_test_add_func("/core/regression/cfg_many_seq", test_cfg_many_seq);
g_test_add_func("/core/regression/charset_bits", test_charset_bits);
+ g_test_add_func("/core/regression/bug19", test_bug_19);
}