Re: Xemacs 20.4 crash (PR#1039)
I actually know this bug. A long time ago I tracked it down but
never
fixed it :-( I sent an explanation to xemacs-beta:
http://www.xemacs.org/list-archives/xemacs-beta/9905/msg00184.html
but nobody bothered to reply... and I guess I forgot about it.
What do you think?
I hate stupid assert's like that. There is no reason to crash here.
Just give an error exist value and/or signal an error.
In addition that code suffers from Macroitis.
The Lispref says:
Emacs supports two comment styles simultaneously in any one syntax
table. This is for the sake of C++. [...]
The two comment-start sequences must begin with the same
character; only the second character may differ.
but this isn't really enforced. A consequence of this is that you can
make xemacs hit an abort() in font-lock.c. An example of this occurs
in some recent version of a sql mode, which tries to support '/*' and
'--' as comment starters (see recipe below for the crash).
The Lisp ref is waaaayyy out of date.. It doesn't mention the 5678
flags at all.
The abort() is triggered in find_context(). In Fparse_partial_sexp()
we are not so aggressive and just parse '/-' and '-*' as comments --
this also happens in FSF emacs, which uses Fparse_partial_sexp in
font-lock.
I think this a bug. The infra structure seems there to do it
correctly because of the 5-8 flags.
This the code that aborts
if ((SYNTAX_COMMENT_BITS (mirrortab, c) &
SYNTAX_SECOND_CHAR_START) &&
context_cache.context == context_none &&
context_cache.ccontext == ccontext_start1 &&
SYNTAX_START_P (mirrortab, prev_c, c) /* the two chars match */
)
{
context_cache.ccontext = ccontext_start2;
context_cache.style = SYNTAX_START_STYLE (mirrortab, prev_c, c);
if (context_cache.style == comment_style_none) abort ();
}
The problem is that in this case SYNTAX_START_P (mirrortab, prev_c, c)
is defined as
#define SYNTAX_START_P(table, a, b) \
((SYNTAX_COMMENT_BITS (table, a) & SYNTAX_FIRST_CHAR_START) \
&& (SYNTAX_COMMENT_BITS (table, b) & SYNTAX_SECOND_CHAR_START))
And thus it returns true even if a is the first char of a style a
comment and b is the second char of a style b comment.
i.e. (SYNTAX_COMMENT_BITS (table, a) & SYNTAX_FIRST_CHAR_START) and
(SYNTAX_COMMENT_BITS (table, b) & SYNTAX_SECOND_CHAR_START) must have
a type in common.. I think you can write this as
/* See if a and b can be first and second char of the same comment type */
#define SYNTAX_START_P(table, a, b) \
(((SYNTAX_COMMENT_BITS (table, a) & SYNTAX_FIRST_CHAR_START) >> 2) \
& (SYNTAX_COMMENT_BITS (table, b) & SYNTAX_SECOND_CHAR_START))
and probably a similar definition of SYNTAX_END_P.
This definition could even make parse-partial-sexp do the right
thins...
Jan