On Tue, Oct 31, 2017 at 2:32 AM, Andreas Schwab wrote: > On Okt 31 2017, Eli Zaretskii wrote: > > >> I also checked the following works, and seems better to me (stop having > dangling pointers, instead of being > >> careful with them): > >> > >> diff --git a/src/alloc.c b/src/alloc.c > >> index da0c3ad4b3..44dfa95cf5 100644 > >> --- a/src/alloc.c > >> +++ b/src/alloc.c > >> @@ -7030,8 +7030,10 @@ sweep_symbols (void) > >> { > >> if (!sym->s.gcmarkbit) > >> { > >> - if (sym->s.redirect == SYMBOL_LOCALIZED) > >> + if (sym->s.redirect == SYMBOL_LOCALIZED) { > >> xfree (SYMBOL_BLV (&sym->s)); > >> + sym->s.val.blv = NULL; > >> + } > > > > That was my first attempt, but various macros like SYMBOL_BLV and > > SET_SYMBOL_BLV insist on val.blv being non-NULL. > > SET_SYMBOL_BLV doesn't. And calling SYMBOL_BLV with a freed symbol is a > bug anyway. > ​SET_SYMBOL_BLV insists that the new value is not NULL, even if it asserts nothing about the current value. We do call SYMBOL_BLV after freeing, when we re-sweep the symbol, which is fine because free does nothing when given NULL, but triggers the assertion​. I would do this, to avoid the assertion failure: diff --git a/src/alloc.c b/src/alloc.c index da0c3ad4b3..72550e812b 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -7030,8 +7030,10 @@ sweep_symbols (void) { if (!sym->s.gcmarkbit) { - if (sym->s.redirect == SYMBOL_LOCALIZED) + if (sym->s.redirect == SYMBOL_LOCALIZED && sym->s.val.blv) { xfree (SYMBOL_BLV (&sym->s)); + sym->s.val.blv = NULL; + } sym->s.next = symbol_free_list; symbol_free_list = &sym->s; symbol_free_list->function = Vdead; Or changing the redirect type: diff --git a/src/alloc.c b/src/alloc.c index da0c3ad4b3..6966d96c6d 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -7030,8 +7030,11 @@ sweep_symbols (void) { if (!sym->s.gcmarkbit) { - if (sym->s.redirect == SYMBOL_LOCALIZED) + if (sym->s.redirect == SYMBOL_LOCALIZED) { xfree (SYMBOL_BLV (&sym->s)); + sym->s.redirect = SYMBOL_PLAINVAL; + SET_SYMBOL_VAL (&sym->s, Qunbound); + } sym->s.next = symbol_free_list; symbol_free_list = &sym->s; symbol_free_list->function = Vdead;