GNU bug report logs - #41520
28.0.50; Crash in character.h due to assertion error

Previous Next

Package: emacs;

Reported by: Stefan Kangas <stefan <at> marxist.se>

Date: Mon, 25 May 2020 07:06:01 UTC

Severity: normal

Found in version 28.0.50

Fixed in version 28.1

Done: Lars Ingebrigtsen <larsi <at> gnus.org>

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: Pip Cet <pipcet <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 41520 <at> debbugs.gnu.org, stefan <at> marxist.se
Subject: bug#41520: 28.0.50; Crash in character.h due to assertion error
Date: Mon, 25 May 2020 17:54:01 +0000
On Mon, May 25, 2020 at 4:09 PM Eli Zaretskii <eliz <at> gnu.org> wrote:
> > From: Pip Cet <pipcet <at> gmail.com>
> > Date: Mon, 25 May 2020 15:16:09 +0000
> > Cc: stefan <at> marxist.se, 41520 <at> debbugs.gnu.org
> >
> > > But wouldn't it be strange to see a macro that accepts a struct, but
> > > only uses one member of that struct?
> >
> > I don't think so. CHARPOS and BYTEPOS already exist, and that's
> > precisely what they do.
> >
> > What is a little strange is that the ancient convention of not
> > returning struct types is still followed in much of Emacs.
>
> It's more expensive.

Only for very large structs, or on old architectures.

> That's what I meant when I said "strange": why
> would we fill 2 fields of a struct, but use only one?

As I said, I'm not talking about cases in which one variable suffices.
It's those cases where we have:

ptrdiff_t charpos;
ptrdiff_t bytepos;

(not usually named like that, or indeed consistently).

My suggestion is we use

pos_t pos;

and then pos.charpos and pos.bytepos as appropriate.

> > > I mean we already have assertions: that's what eassume does in a debug
> > > build.
> >
> > Yes, but we could do with some stricter checking, I think.
>
> It cannot catch the cases where we put a character position into the
> byte position slot.  That's the general problem with using simple
> scalars.

Incorrect code becomes way more obvious.

bytepos = PT;

is incorrect but shorter than the correct version.

pos.bytepos = PT_POS.charpos;
pos.charpos = PT_POS.bytepos;

is much more obviously wrong, and the correct version is simply:

pos =  PT_POS;

On non-obsolescent architectures, returning a two-word struct is
cheaper than accessing two parameters through pointers, too; and
that's only relevant for those few cases in which the function isn't
inlined, anyway.

All I'm hoping for, at this point, is a "maybe, show me a patch".




This bug report was last modified 4 years and 240 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.