buffer.c/buffer.h: How to add new buffer-local variables?

buffer.c/buffer.h: How to add new buffer-local variables?

[Keith David Bershatsky]

[-- Attachment #1: Type: text/plain, Size: 1559 bytes --]

I am working on feature requests #22873 (multiple fake cursors) and #17684 (crosshairs that track the cursor position).

In an earlier version of the Emacs master branch from a few months ago, I was able to increase the number of buffer-local variables in buffer.c/buffer.h by changing the value of MAX_PER_BUFFER_VARS from 50 to 60.  That approach does not permit me to build a current version of the Emacs master branch, which fails with the following error:

    Dumping under the name bootstrap-emacs.pdmp
    dumping fingerprint: 50894efcc2cdb17747a4536c9f53c4d093895712c3604daa88d068c383ea4780

    pdumper.c:1786: Emacs fatal error: assertion failed: relpos < 1024
    Fatal error 6: Abort trapmake[1]: *** [bootstrap-emacs.pdmp] Abort trap
    make[1]: *** Deleting file `bootstrap-emacs.pdmp'
    make: *** [src] Error 2

    Compilation exited abnormally with code 2 at Sat Mar 30 15:53:18

I have narrowed the issue down to adding just one (1) more buffer local variable, which breaks the camel's back.  Attached are git diffs for a working build and a broken build.  I am using Emacs master branch from 03/28/2019:  2da9f8bf4222fda504f43b4757e154999cdbbf2c

My config options are as follows:

CFLAGS='-Wall -O0 -g3' ./configure \
--with-ns \
--enable-checking='yes,glyphs' \
--enable-check-lisp-object-type \
--without-compress-install \
--without-makeinfo \
--with-gnutls=no \
--with-mailutils \
--without-makeinfo

How can I increase the number of buffer local variables in buffer.c/buffer.h without breaking the ability to build Emacs?

Keith


[-- Attachment #2: patch_working.diff --]
[-- Type: application/diff, Size: 10583 bytes --]

[-- Attachment #3: patch_broken.diff --]
[-- Type: application/diff, Size: 10517 bytes --]

Re: buffer.c/buffer.h: How to add new buffer-local variables?

[Eli Zaretskii]

> Date: Sat, 30 Mar 2019 16:19:50 -0700
> From: Keith David Bershatsky <esq@lawlist.com>
> 
> In an earlier version of the Emacs master branch from a few months ago, I was able to increase the number of buffer-local variables in buffer.c/buffer.h by changing the value of MAX_PER_BUFFER_VARS from 50 to 60.  That approach does not permit me to build a current version of the Emacs master branch, which fails with the following error:
> 
>     Dumping under the name bootstrap-emacs.pdmp
>     dumping fingerprint: 50894efcc2cdb17747a4536c9f53c4d093895712c3604daa88d068c383ea4780
> 
>     pdumper.c:1786: Emacs fatal error: assertion failed: relpos < 1024
>     Fatal error 6: Abort trapmake[1]: *** [bootstrap-emacs.pdmp] Abort trap
>     make[1]: *** Deleting file `bootstrap-emacs.pdmp'
>     make: *** [src] Error 2
> 
>     Compilation exited abnormally with code 2 at Sat Mar 30 15:53:18

You need to update the dumping code to the new variables you added.
See the comments in pdumper.c.

Re: buffer.c/buffer.h: How to add new buffer-local variables?

[Eli Zaretskii]

On March 31, 2019 5:37:14 AM GMT+03:00, Eli Zaretskii <eliz@gnu.org> wrote:
> > Date: Sat, 30 Mar 2019 16:19:50 -0700
> > From: Keith David Bershatsky <esq@lawlist.com>
> > 
> > In an earlier version of the Emacs master branch from a few months
> ago, I was able to increase the number of buffer-local variables in
> buffer.c/buffer.h by changing the value of MAX_PER_BUFFER_VARS from 50
> to 60.  That approach does not permit me to build a current version of
> the Emacs master branch, which fails with the following error:
> > 
> >     Dumping under the name bootstrap-emacs.pdmp
> >     dumping fingerprint:
> 50894efcc2cdb17747a4536c9f53c4d093895712c3604daa88d068c383ea4780
> > 
> >     pdumper.c:1786: Emacs fatal error: assertion failed: relpos <
> 1024
> >     Fatal error 6: Abort trapmake[1]: *** [bootstrap-emacs.pdmp]
> Abort trap
> >     make[1]: *** Deleting file `bootstrap-emacs.pdmp'
> >     make: *** [src] Error 2
> > 
> >     Compilation exited abnormally with code 2 at Sat Mar 30 15:53:18
> 
> You need to update the dumping code to the new variables you added.
> See the comments in pdumper.c.

Could it be you are overflowing MAX_PER_BUFFER_VARS?

Re: buffer.c/buffer.h: How to add new buffer-local variables?

[Keith David Bershatsky]

Thank you, Eli, for reading and responding to this particular thread.

I have tried increasing MAX_PER_BUFFER_VARS from 50 to 60 and even 75, but that no longer works.  That approach worked back in early/mid-December 2018, but does not work with a current version of Emacs master branch.

I searched pdumper.c for a few popular buffer-local variables such as mode_line_format and cache_long_scans, but did not find them.  As such, I have no idea where else to add the new buffer local variables other than the regular locations in buffer.c and buffer.h as demonstrated in the working/broken patches attached to the initial post of this particular thread.

The build messages gives little clues for us to investigate this issue.  I have learned how to extract the new hash values from dmpstruct.h and insert them at the appropriate location of pdumper.c, but that is not what causes the build to fail in this particular issue.

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

> Date: [03-30-2019 20:49:57] <31 Mar 2019 06:49:57 +0300>
> From: Eli Zaretskii <eliz@gnu.org>
> To: emacs-devel@gnu.org,Keith David Bershatsky <esq@lawlist.com>
> Subject: Re: buffer.c/buffer.h:  How to add new buffer-local variables?
> 
> * * *
> 
> Could it be you are overflowing MAX_PER_BUFFER_VARS?

Re: buffer.c/buffer.h: How to add new buffer-local variables?

[Andreas Schwab]

On Mär 30 2019, Keith David Bershatsky <esq@lawlist.com> wrote:

>     pdumper.c:1786: Emacs fatal error: assertion failed: relpos < 1024

The size of struct buffer is already close to that limit now.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: buffer.c/buffer.h: How to add new buffer-local variables?

[Eli Zaretskii]

On March 31, 2019 12:42:27 PM GMT+03:00, Andreas Schwab <schwab@linux-m68k.org> wrote:
> On Mär 30 2019, Keith David Bershatsky <esq@lawlist.com> wrote:
> 
> >     pdumper.c:1786: Emacs fatal error: assertion failed: relpos <
> 1024
> 
> The size of struct buffer is already close to that limit now.
> 
> Andreas.

So you are saying that the 1024 value is arbitrary and should be enlarged when more local vars are added?

Re: buffer.c/buffer.h: How to add new buffer-local variables?

[Andreas Schwab]

On Mär 31 2019, Eli Zaretskii <eliz@gnu.org> wrote:

> So you are saying that the 1024 value is arbitrary and should be enlarged when more local vars are added?

No.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: buffer.c/buffer.h: How to add new buffer-local variables?

[Alan Mackenzie]

Hello, Keith.

On Sat, Mar 30, 2019 at 16:19:50 -0700, Keith David Bershatsky wrote:
> I am working on feature requests #22873 (multiple fake cursors) and
> #17684 (crosshairs that track the cursor position).

> In an earlier version of the Emacs master branch from a few months ago,
> I was able to increase the number of buffer-local variables in
> buffer.c/buffer.h by changing the value of MAX_PER_BUFFER_VARS from 50
> to 60.  That approach does not permit me to build a current version of
> the Emacs master branch, which fails with the following error:

>     Dumping under the name bootstrap-emacs.pdmp
>     dumping fingerprint: 50894efcc2cdb17747a4536c9f53c4d093895712c3604daa88d068c383ea4780

>     pdumper.c:1786: Emacs fatal error: assertion failed: relpos < 1024
>     Fatal error 6: Abort trapmake[1]: *** [bootstrap-emacs.pdmp] Abort trap
>     make[1]: *** Deleting file `bootstrap-emacs.pdmp'
>     make: *** [src] Error 2

>     Compilation exited abnormally with code 2 at Sat Mar 30 15:53:18

> I have narrowed the issue down to adding just one (1) more buffer local
> variable, which breaks the camel's back.  Attached are git diffs for a
> working build and a broken build.  I am using Emacs master branch from
> 03/28/2019:  2da9f8bf4222fda504f43b4757e154999cdbbf2c

> My config options are as follows:

[ .... ]

> How can I increase the number of buffer local variables in
> buffer.c/buffer.h without breaking the ability to build Emacs?

You're surely aware that you can add a new local variable in C source by
declaring it with DEFVAR, then calling Fmake_variable_buffer_local in
syms_of_foo?  Or something like that (I've forgotten the exact details).

Is there some pressing reason why the new variable has to be part of
struct buffer rather than an "ordinary" buffer local?

It's worth noting that there's nothing in the elisp manual about how to
create buffer local variables in the C source.

It may be that there're BL variables in struct buffer that don't really
need to be there, and could be converted to "ordinary" variables.  I
think I might have added one or two of these myself, but don't remember.

> Keith

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: buffer.c/buffer.h: How to add new buffer-local variables?

[Eli Zaretskii]

> From: Andreas Schwab <schwab@linux-m68k.org>
> Date: Sun, 31 Mar 2019 13:41:51 +0200
> Cc: Keith David Bershatsky <esq@lawlist.com>, emacs-devel@gnu.org
> 
> On Mär 31 2019, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> > So you are saying that the 1024 value is arbitrary and should be enlarged when more local vars are added?
> 
> No.

"No" to which part?

Re: buffer.c/buffer.h: How to add new buffer-local variables?

[Keith David Bershatsky]

Thank you, Alan.  I was unaware that there are other means of declaring buffer-local variables in C.

In a few areas of the code for the features that I am working on, I have statements such as:

if (!NILP (BVAR (XBUFFER (w->contents), crosshairs))) ...

I have been using the existing method in buffer.c/h to define buffer local variables for the past three years (while chiseling away at the multiple fake cursors feature).  The last proof concept patch applied to an Emacs version from 12/13/2018, and I am just about ready to submit a new proof concept patch for a current version of Emacs.

If there is a way to increase the maximum number of buffer local variables to continue using the same method as is done in buffer.c/h, then I would prefer to do that.  If not, then the method you suggested below could be fallback plan.  Other than personal preference and my existing code that has worked well up until recently, there is no pressing reason that it has to be done a certain way.

Keith

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

> Date: [03-31-2019 05:22:38] <31 Mar 2019 12:22:38 +0000>
> From: Alan Mackenzie <acm@muc.de>
> 
> You're surely aware that you can add a new local variable in C source by
> declaring it with DEFVAR, then calling Fmake_variable_buffer_local in
> syms_of_foo?  Or something like that (I've forgotten the exact details).
> 
> Is there some pressing reason why the new variable has to be part of
> struct buffer rather than an "ordinary" buffer local?
> 
> It's worth noting that there's nothing in the elisp manual about how to
> create buffer local variables in the C source.
> 
> It may be that there're BL variables in struct buffer that don't really
> need to be there, and could be converted to "ordinary" variables.  I
> think I might have added one or two of these myself, but don't remember.

Re: buffer.c/buffer.h: How to add new buffer-local variables?

[Stefan Monnier]

> if (!NILP (BVAR (XBUFFER (w->contents), crosshairs))) ...

Buffer-local variables are appropriate in the `struct buffer` when it's
likely that most buffers will have their own setting for it.

For variables that are likely to be used only in some particular
buffers, or only under some particular circumstances, it can make a lot
more sense to declare them as global variables and then rely on the
Elisp side using `make-local-variable` or `make-variable-buffer-local`,
so you don't pay for that slot in every buffer even tho it's almost
never used.


        Stefan

Re: buffer.c/buffer.h: How to add new buffer-local variables?

[Keith David Bershatsky]

Thank you, Stefan, for the insight regarding how to handle this particular issue.  Greatly appreciated!

Keith

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

> From: 	Stefan Monnier
> Subject: 	Re: buffer.c/buffer.h: How to add new buffer-local variables?
> Date: 	Sun, 31 Mar 2019 16:02:49 -0400
> 
>> if (!NILP (BVAR (XBUFFER (w->contents), crosshairs))) ...
> 
> Buffer-local variables are appropriate in the `struct buffer` when it's
> likely that most buffers will have their own setting for it.
> 
> For variables that are likely to be used only in some particular
> buffers, or only under some particular circumstances, it can make a lot
> more sense to declare them as global variables and then rely on the
> Elisp side using `make-local-variable` or `make-variable-buffer-local`,
> so you don't pay for that slot in every buffer even tho it's almost
> never used.
> 
>         Stefan

Re: buffer.c/buffer.h: How to add new buffer-local variables?

[Eli Zaretskii]

> From: Andreas Schwab <schwab@linux-m68k.org>
> Date: Sun, 31 Mar 2019 13:41:51 +0200
> Cc: Keith David Bershatsky <esq@lawlist.com>, emacs-devel@gnu.org
> 
> On Mär 31 2019, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> > So you are saying that the 1024 value is arbitrary and should be enlarged when more local vars are added?
> 
> No.

Daniel, could you please help me understand why the value 1024 was
used here:

  static dump_off
  field_relpos (const void *in_start, const void *in_field)
  {
    ptrdiff_t in_start_val = (ptrdiff_t) in_start;
    ptrdiff_t in_field_val = (ptrdiff_t) in_field;
    eassert (in_start_val <= in_field_val);
    ptrdiff_t relpos = in_field_val - in_start_val;
    eassert (relpos < 1024); /* Sanity check.  */  <<<<<<<<<<<<
    return (dump_off) relpos;
  }

The comment says "sanity check", but I would like to understand what
kind of sanity is being checked here, and what should be done when
some structure we dump becomes larger than this value.  E.g., is there
some other limit that requires that offsets of dumped fields never
exceed 1024 here?  I'd like to document in comments what to do when
the assertion is violated.

TIA

Re: buffer.c/buffer.h: How to add new buffer-local variables?

[Daniel Colascione]

>> From: Andreas Schwab <schwab@linux-m68k.org>
>> Date: Sun, 31 Mar 2019 13:41:51 +0200
>> Cc: Keith David Bershatsky <esq@lawlist.com>, emacs-devel@gnu.org
>>
>> On Mär 31 2019, Eli Zaretskii <eliz@gnu.org> wrote:
>>
>> > So you are saying that the 1024 value is arbitrary and should be
>> enlarged when more local vars are added?
>>
>> No.
>
> Daniel, could you please help me understand why the value 1024 was
> used here:
>
>   static dump_off
>   field_relpos (const void *in_start, const void *in_field)
>   {
>     ptrdiff_t in_start_val = (ptrdiff_t) in_start;
>     ptrdiff_t in_field_val = (ptrdiff_t) in_field;
>     eassert (in_start_val <= in_field_val);
>     ptrdiff_t relpos = in_field_val - in_start_val;
>     eassert (relpos < 1024); /* Sanity check.  */  <<<<<<<<<<<<
>     return (dump_off) relpos;
>   }
>
> The comment says "sanity check", but I would like to understand what
> kind of sanity is being checked here, and what should be done when
> some structure we dump becomes larger than this value.  E.g., is there
> some other limit that requires that offsets of dumped fields never
> exceed 1024 here?  I'd like to document in comments what to do when
> the assertion is violated.

Indeed. That comment could have been a lot better. The general idea here
is this:

When we enter field_relpos, we're in the middle of some code that's
dumping some data structure field-by-field. The object that we're dumping
begins at in_start; and in_field is an interior pointer into that object.
We can't actually check that the two pointers refer to the same object: C
doesn't give us that level of introspection. But if the two pointers point
to addresses that differ by a lot, then the two pointers probably don't
refer to the same object, and in this case, we can fail an assertion. 1024
is probably too conservative here. We probably want to greatly increase
this number (say, to 32k) and also to give it a named constantly, maybe
something like PDUMPER_MAXIMUM_STRUCT_SIZE.

Note that this limit doesn't apply to big variable-length structures like
vectors: we dump these element-by-element instead of treating the whole
thing as one big "object" with a large and variable number of fields.

Re: buffer.c/buffer.h: How to add new buffer-local variables?

[Eli Zaretskii]

> Date: Tue, 2 Apr 2019 11:46:22 -0700
> From: "Daniel Colascione" <dancol@dancol.org>
> Cc: "Daniel Colascione" <dancol@dancol.org>,
>  emacs-devel@gnu.org
> 
> > The comment says "sanity check", but I would like to understand what
> > kind of sanity is being checked here, and what should be done when
> > some structure we dump becomes larger than this value.  E.g., is there
> > some other limit that requires that offsets of dumped fields never
> > exceed 1024 here?  I'd like to document in comments what to do when
> > the assertion is violated.
> 
> Indeed. That comment could have been a lot better. The general idea here
> is this:
> 
> When we enter field_relpos, we're in the middle of some code that's
> dumping some data structure field-by-field. The object that we're dumping
> begins at in_start; and in_field is an interior pointer into that object.
> We can't actually check that the two pointers refer to the same object: C
> doesn't give us that level of introspection. But if the two pointers point
> to addresses that differ by a lot, then the two pointers probably don't
> refer to the same object, and in this case, we can fail an assertion. 1024
> is probably too conservative here. We probably want to greatly increase
> this number (say, to 32k) and also to give it a named constantly, maybe
> something like PDUMPER_MAXIMUM_STRUCT_SIZE.

Thanks, I went with a smaller value, 2KB.  It should be good enough
for now, as the largest object is slightly below 1KB.

> Note that this limit doesn't apply to big variable-length structures like
> vectors: we dump these element-by-element instead of treating the whole
> thing as one big "object" with a large and variable number of fields.

Right.

Re: buffer.c/buffer.h: How to add new buffer-local variables?

[Keith David Bershatsky]

[-- Attachment #1: Type: text/plain, Size: 1143 bytes --]

Increasing the eassert value in field_relpos, and increasing the value of MAX_PER_BUFFER_VARS, are sufficient to build and use Emacs for the NS and X11 ports.

The w32 port of Emacs, however, crashes when adding one too many local-variables in buffer.c/h that breaks the camel's back:

Program received signal SIGSEGV, Segmentation fault.
0x01189584 in vector_marked_p (v=0xbaadf008) at alloc.c:3980
3980      return XVECTOR_MARKED_P (v);
(gdb) bt
#0  0x01189584 in vector_marked_p (v=0xbaadf008) at alloc.c:3980
#1  0x0118dc21 in mark_object (arg=...) at alloc.c:6684
#2  0x0118d2d4 in mark_vectorlike (header=0x5ab4b98) at alloc.c:6361
#3  0x0118d510 in mark_buffer (buffer=0x5ab4b98) at alloc.c:6428
#4  0x0118dd1f in mark_object (arg=...) at alloc.c:6717
#5  0x0118d69d in mark_localized_symbol (ptr=0x4a86c40) at alloc.c:6481

Attached is a gdb backtrace and a minimal diff working example that causes the w32 port to crash during garbage collection.  To reproduce the crash on the w32 port of Emacs, launch the GUI Emacs and at the *GNU Emacs* welcome screen, and hold down the right arrow key until a garbage collection occurs.

Keith


[-- Attachment #2: 001.diff --]
[-- Type: application/diff, Size: 12095 bytes --]

[-- Attachment #3: gdb_001.txt --]
[-- Type: application/txt, Size: 66888 bytes --]

Re: buffer.c/buffer.h: How to add new buffer-local variables?

[Eli Zaretskii]

> Date:  Thu, 04 Apr 2019 11:57:02 -0700
> From:  Keith David Bershatsky <esq@lawlist.com>
> Cc:  emacs-devel@gnu.org,Stefan Monnier <monnier@iro.umontreal.ca>,Andreas Schwab <schwab@linux-m68k.org>,Alan Mackenzie <acm@muc.de>,Daniel Colascione <dancol@dancol.org>
> 
> Increasing the eassert value in field_relpos, and increasing the value of MAX_PER_BUFFER_VARS, are sufficient to build and use Emacs for the NS and X11 ports.
> 
> The w32 port of Emacs, however, crashes when adding one too many local-variables in buffer.c/h that breaks the camel's back:
> 
> Program received signal SIGSEGV, Segmentation fault.
> 0x01189584 in vector_marked_p (v=0xbaadf008) at alloc.c:3980
                                 ^^^^^^^^^^^^
This 0xbaadf008 is a clear sign of using uninitialized memory.

Re: buffer.c/buffer.h: How to add new buffer-local variables?

[Keith David Bershatsky]

[-- Attachment #1: Type: text/plain, Size: 1698 bytes --]

Using an unsophisticated method of going back in time and building various versions of Emacs on the w32 platform, I tracked down the inability to add a few new buffer-local variables in buffer.c/buffer.h to a commit that occurred on 01/31/2019:  05d2fc7170fb66a87601b1c76ddae2c1b7b4b934.

Steps to reproduce the issue:

1.  git reset --hard 05d2fc7170fb66a87601b1c76ddae2c1b7b4b934

2.  Apply the attached patch that adds a few new buffer local variables.

3.  Build a w32 version of Emacs [I use MinGW_32 and ezwinports downloaded a couple of years ago, on a Windows XP SP-3 box]:

CFLAGS='-O0 -g3' ./configure \
--prefix=/c/docume~1/admini~1/desktop/trunk \
--enable-checking='yes,glyphs' \
--enable-check-lisp-object-type \
--without-compress-install \
--without-makeinfo \
--with-gnutls=no \
--with-mailutils \
--without-makeinfo

4.  The build terminates abnormally at the following location:

This application has requested the Runtime to terminate it in an unusual way.
Please contact the application's support team for more information.
make[2]: *** [emacs-lisp/macroexp.elc] Error 3
make[2]: Leaving directory `/c/docume~1/admini~1/desktop/.0.2019_01_31__05d2fc7170fb66a87601b1c76ddae2c1b7b4b934/lisp'
make[1]: *** [bootstrap-emacs.pdmp] Error 2
make[1]: Leaving directory `/c/docume~1/admini~1/desktop/.0.2019_01_31__05d2fc7170fb66a87601b1c76ddae2c1b7b4b934/src'
make: *** [src] Error 2

Beyond pinpointing the exact commit where the Emacs build goes awry, I would be pleased to help further towards resolving this issue -- however, some pointers would be greatly appreciated.  Perhaps there is something that may stand out (to a trained programmer) in the 01/31/2019 commit ....

Keith


[-- Attachment #2: 2019_04_06__19_33_22_280.diff --]
[-- Type: application/diff, Size: 11209 bytes --]

Re: buffer.c/buffer.h: How to add new buffer-local variables?

[Eli Zaretskii]

> Date:  Sat, 06 Apr 2019 19:50:14 -0700
> From:  Keith David Bershatsky <esq@lawlist.com>
> Cc:  emacs-devel@gnu.org,Stefan Monnier <monnier@iro.umontreal.ca>,Andreas Schwab <schwab@linux-m68k.org>,Alan Mackenzie <acm@muc.de>,Daniel Colascione <dancol@dancol.org>,Paul Eggert <eggert@cs.ucla.edu>
> 
> Using an unsophisticated method of going back in time and building various versions of Emacs on the w32 platform, I tracked down the inability to add a few new buffer-local variables in buffer.c/buffer.h to a commit that occurred on 01/31/2019:  05d2fc7170fb66a87601b1c76ddae2c1b7b4b934.

Is your GDB version 8.1 or later?  If so, please do this:

  $ cd /path/to/emacs/src
  $ gdb ./emacs.exe
  ...
  (gdb) start -Q
  ...
  (gdb) ptype /o current_buffer

and post here the result.  Please do this both in the build before and
after the above commit, and let us see both results.

Re: buffer.c/buffer.h: How to add new buffer-local variables?

[Keith David Bershatsky]

[-- Attachment #1: Type: text/plain, Size: 17848 bytes --]

Thank you, Eli, for providing the steps to facilitate troubleshooting this issue.  The attached files may make it easier to do a diff between the gdb output for both commits.  I performed the gdb tests on the stock Emacs versions on the w32 platform without making any edits by me to the underlying source code; i.e., I did not add any buffer-local variables in these tests.

a68eee50eb515b28b448894299334afced26ef78

and

05d2fc7170fb66a87601b1c76ddae2c1b7b4b934

I am also posing the results in text form below just in case you prefer that instead of text files:

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

a68eee50eb515b28b448894299334afced26ef78

$ gdb ./emacs.exe

...

(gdb) start -Q

...

(gdb) ptype /o current_buffer

type = struct buffer {
/*    0      |     4 */    union vectorlike_header {
/*                 4 */        ptrdiff_t size;

                               /* total size (bytes):    4 */
                           } header;
/*    4      |     4 */    Lisp_Object name_;
/*    8      |     4 */    Lisp_Object filename_;
/*   12      |     4 */    Lisp_Object directory_;
/*   16      |     4 */    Lisp_Object backed_up_;
/*   20      |     4 */    Lisp_Object save_length_;
/*   24      |     4 */    Lisp_Object auto_save_file_name_;
/*   28      |     4 */    Lisp_Object read_only_;
/*   32      |     4 */    Lisp_Object mark_;
/*   36      |     4 */    Lisp_Object local_var_alist_;
/*   40      |     4 */    Lisp_Object major_mode_;
/*   44      |     4 */    Lisp_Object mode_name_;
/*   48      |     4 */    Lisp_Object mode_line_format_;
/*   52      |     4 */    Lisp_Object header_line_format_;
/*   56      |     4 */    Lisp_Object keymap_;
/*   60      |     4 */    Lisp_Object abbrev_table_;
/*   64      |     4 */    Lisp_Object syntax_table_;
/*   68      |     4 */    Lisp_Object category_table_;
/*   72      |     4 */    Lisp_Object case_fold_search_;
/*   76      |     4 */    Lisp_Object tab_width_;
/*   80      |     4 */    Lisp_Object fill_column_;
/*   84      |     4 */    Lisp_Object left_margin_;
/*   88      |     4 */    Lisp_Object auto_fill_function_;
/*   92      |     4 */    Lisp_Object downcase_table_;
/*   96      |     4 */    Lisp_Object upcase_table_;
/*  100      |     4 */    Lisp_Object case_canon_table_;
/*  104      |     4 */    Lisp_Object case_eqv_table_;
/*  108      |     4 */    Lisp_Object truncate_lines_;
/*  112      |     4 */    Lisp_Object word_wrap_;
/*  116      |     4 */    Lisp_Object ctl_arrow_;
/*  120      |     4 */    Lisp_Object bidi_display_reordering_;
/*  124      |     4 */    Lisp_Object bidi_paragraph_direction_;
/*  128      |     4 */    Lisp_Object bidi_paragraph_separate_re_;
/*  132      |     4 */    Lisp_Object bidi_paragraph_start_re_;
/*  136      |     4 */    Lisp_Object selective_display_;
/*  140      |     4 */    Lisp_Object selective_display_ellipses_;
/*  144      |     4 */    Lisp_Object minor_modes_;
/*  148      |     4 */    Lisp_Object overwrite_mode_;
/*  152      |     4 */    Lisp_Object abbrev_mode_;
/*  156      |     4 */    Lisp_Object display_table_;
/*  160      |     4 */    Lisp_Object mark_active_;
/*  164      |     4 */    Lisp_Object enable_multibyte_characters_;
/*  168      |     4 */    Lisp_Object buffer_file_coding_system_;
/*  172      |     4 */    Lisp_Object file_format_;
/*  176      |     4 */    Lisp_Object auto_save_file_format_;
/*  180      |     4 */    Lisp_Object cache_long_scans_;
/*  184      |     4 */    Lisp_Object width_table_;
/*  188      |     4 */    Lisp_Object pt_marker_;
/*  192      |     4 */    Lisp_Object begv_marker_;
/*  196      |     4 */    Lisp_Object zv_marker_;
/*  200      |     4 */    Lisp_Object point_before_scroll_;
/*  204      |     4 */    Lisp_Object file_truename_;
/*  208      |     4 */    Lisp_Object invisibility_spec_;
/*  212      |     4 */    Lisp_Object last_selected_window_;
/*  216      |     4 */    Lisp_Object display_count_;
/*  220      |     4 */    Lisp_Object left_margin_cols_;
/*  224      |     4 */    Lisp_Object right_margin_cols_;
/*  228      |     4 */    Lisp_Object left_fringe_width_;
/*  232      |     4 */    Lisp_Object right_fringe_width_;
/*  236      |     4 */    Lisp_Object fringes_outside_margins_;
/*  240      |     4 */    Lisp_Object scroll_bar_width_;
/*  244      |     4 */    Lisp_Object scroll_bar_height_;
/*  248      |     4 */    Lisp_Object vertical_scroll_bar_type_;
/*  252      |     4 */    Lisp_Object horizontal_scroll_bar_type_;
/*  256      |     4 */    Lisp_Object indicate_empty_lines_;
/*  260      |     4 */    Lisp_Object indicate_buffer_boundaries_;
/*  264      |     4 */    Lisp_Object fringe_indicator_alist_;
---Type <return> to continue, or q <return> to quit---
/*  268      |     4 */    Lisp_Object fringe_cursor_alist_;
/*  272      |     4 */    Lisp_Object display_time_;
/*  276      |     4 */    Lisp_Object scroll_up_aggressively_;
/*  280      |     4 */    Lisp_Object scroll_down_aggressively_;
/*  284      |     4 */    Lisp_Object cursor_type_;
/*  288      |     4 */    Lisp_Object extra_line_spacing_;
/*  292      |     4 */    Lisp_Object cursor_in_non_selected_windows_;
/*  296      |    72 */    struct buffer_text {
/*  296      |     4 */        unsigned char *beg;
/*  300      |     4 */        ptrdiff_t gpt;
/*  304      |     4 */        ptrdiff_t z;
/*  308      |     4 */        ptrdiff_t gpt_byte;
/*  312      |     4 */        ptrdiff_t z_byte;
/*  316      |     4 */        ptrdiff_t gap_size;
/*  320      |     4 */        EMACS_INT modiff;
/*  324      |     4 */        EMACS_INT chars_modiff;
/*  328      |     4 */        EMACS_INT save_modiff;
/*  332      |     4 */        EMACS_INT overlay_modiff;
/*  336      |     4 */        EMACS_INT compact;
/*  340      |     4 */        ptrdiff_t beg_unchanged;
/*  344      |     4 */        ptrdiff_t end_unchanged;
/*  348      |     4 */        EMACS_INT unchanged_modified;
/*  352      |     4 */        EMACS_INT overlay_unchanged_modified;
/*  356      |     4 */        INTERVAL intervals;
/*  360      |     4 */        struct Lisp_Marker *markers;
/*  364:31   |     4 */        bool_bf inhibit_shrinking : 1;
/*  364:30   |     4 */        bool_bf redisplay : 1;

                               /* total size (bytes):   72 */
                           } own_text;
/*  368      |     4 */    struct buffer_text *text;
/*  372      |     4 */    struct buffer *next;
/*  376      |     4 */    ptrdiff_t pt;
/*  380      |     4 */    ptrdiff_t pt_byte;
/*  384      |     4 */    ptrdiff_t begv;
/*  388      |     4 */    ptrdiff_t begv_byte;
/*  392      |     4 */    ptrdiff_t zv;
/*  396      |     4 */    ptrdiff_t zv_byte;
/*  400      |     4 */    struct buffer *base_buffer;
/*  404      |     4 */    int indirections;
/*  408      |     4 */    int window_count;
/*  412      |    50 */    char local_flags[50];
/* XXX  2-byte hole  */
/*  464      |    16 */    struct timespec {
/*  464      |     8 */        union {
/*                 4 */            time_t tv_sec;
/*                 4 */            __time32_t __tv32_sec;
/*                 8 */            __time64_t __tv64_sec;

                                   /* total size (bytes):    8 */
                               };
/*  472      |     4 */        long tv_nsec;

                               /* total size (bytes):   16 */
                           } modtime;
/*  480      |     4 */    off_t modtime_size;
/*  484      |     4 */    EMACS_INT auto_save_modified;
/*  488      |     4 */    EMACS_INT display_error_modiff;
/*  492      |     4 */    time_t auto_save_failure_time;
/*  496      |     4 */    ptrdiff_t last_window_start;
/*  500      |     4 */    struct region_cache *newline_cache;
/*  504      |     4 */    struct region_cache *width_run_cache;
/*  508      |     4 */    struct region_cache *bidi_paragraph_cache;
/*  512:31   |     4 */    bool_bf prevent_redisplay_optimizations_p : 1;
/*  512:30   |     4 */    bool_bf clip_changed : 1;
/* XXX  6-bit hole   */
/* XXX  3-byte hole  */
/*  516      |     4 */    struct Lisp_Overlay *overlays_before;
/*  520      |     4 */    struct Lisp_Overlay *overlays_after;
/*  524      |     4 */    ptrdiff_t overlay_center;
/*  528      |     4 */    Lisp_Object undo_list_;

---Type <return> to continue, or q <return> to quit---
                           /* total size (bytes):  536 */
                         } *
(gdb)

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

05d2fc7170fb66a87601b1c76ddae2c1b7b4b934

$ gdb ./emacs.exe

...

(gdb) start -Q

...

(gdb) ptype /o current_buffer

type = struct buffer {
/*    0      |     4 */    union vectorlike_header {
/*                 4 */        ptrdiff_t size;

                               /* total size (bytes):    4 */
                           } header;
/*    4      |     4 */    Lisp_Object name_;
/*    8      |     4 */    Lisp_Object filename_;
/*   12      |     4 */    Lisp_Object directory_;
/*   16      |     4 */    Lisp_Object backed_up_;
/*   20      |     4 */    Lisp_Object save_length_;
/*   24      |     4 */    Lisp_Object auto_save_file_name_;
/*   28      |     4 */    Lisp_Object read_only_;
/*   32      |     4 */    Lisp_Object mark_;
/*   36      |     4 */    Lisp_Object local_var_alist_;
/*   40      |     4 */    Lisp_Object major_mode_;
/*   44      |     4 */    Lisp_Object mode_name_;
/*   48      |     4 */    Lisp_Object mode_line_format_;
/*   52      |     4 */    Lisp_Object header_line_format_;
/*   56      |     4 */    Lisp_Object keymap_;
/*   60      |     4 */    Lisp_Object abbrev_table_;
/*   64      |     4 */    Lisp_Object syntax_table_;
/*   68      |     4 */    Lisp_Object category_table_;
/*   72      |     4 */    Lisp_Object case_fold_search_;
/*   76      |     4 */    Lisp_Object tab_width_;
/*   80      |     4 */    Lisp_Object fill_column_;
/*   84      |     4 */    Lisp_Object left_margin_;
/*   88      |     4 */    Lisp_Object auto_fill_function_;
/*   92      |     4 */    Lisp_Object downcase_table_;
/*   96      |     4 */    Lisp_Object upcase_table_;
/*  100      |     4 */    Lisp_Object case_canon_table_;
/*  104      |     4 */    Lisp_Object case_eqv_table_;
/*  108      |     4 */    Lisp_Object truncate_lines_;
/*  112      |     4 */    Lisp_Object word_wrap_;
/*  116      |     4 */    Lisp_Object ctl_arrow_;
/*  120      |     4 */    Lisp_Object bidi_display_reordering_;
/*  124      |     4 */    Lisp_Object bidi_paragraph_direction_;
/*  128      |     4 */    Lisp_Object bidi_paragraph_separate_re_;
/*  132      |     4 */    Lisp_Object bidi_paragraph_start_re_;
/*  136      |     4 */    Lisp_Object selective_display_;
/*  140      |     4 */    Lisp_Object selective_display_ellipses_;
/*  144      |     4 */    Lisp_Object minor_modes_;
/*  148      |     4 */    Lisp_Object overwrite_mode_;
/*  152      |     4 */    Lisp_Object abbrev_mode_;
/*  156      |     4 */    Lisp_Object display_table_;
/*  160      |     4 */    Lisp_Object mark_active_;
/*  164      |     4 */    Lisp_Object enable_multibyte_characters_;
/*  168      |     4 */    Lisp_Object buffer_file_coding_system_;
/*  172      |     4 */    Lisp_Object file_format_;
/*  176      |     4 */    Lisp_Object auto_save_file_format_;
/*  180      |     4 */    Lisp_Object cache_long_scans_;
/*  184      |     4 */    Lisp_Object width_table_;
/*  188      |     4 */    Lisp_Object pt_marker_;
/*  192      |     4 */    Lisp_Object begv_marker_;
/*  196      |     4 */    Lisp_Object zv_marker_;
/*  200      |     4 */    Lisp_Object point_before_scroll_;
/*  204      |     4 */    Lisp_Object file_truename_;
/*  208      |     4 */    Lisp_Object invisibility_spec_;
/*  212      |     4 */    Lisp_Object last_selected_window_;
/*  216      |     4 */    Lisp_Object display_count_;
/*  220      |     4 */    Lisp_Object left_margin_cols_;
/*  224      |     4 */    Lisp_Object right_margin_cols_;
/*  228      |     4 */    Lisp_Object left_fringe_width_;
/*  232      |     4 */    Lisp_Object right_fringe_width_;
/*  236      |     4 */    Lisp_Object fringes_outside_margins_;
/*  240      |     4 */    Lisp_Object scroll_bar_width_;
/*  244      |     4 */    Lisp_Object scroll_bar_height_;
/*  248      |     4 */    Lisp_Object vertical_scroll_bar_type_;
/*  252      |     4 */    Lisp_Object horizontal_scroll_bar_type_;
/*  256      |     4 */    Lisp_Object indicate_empty_lines_;
/*  260      |     4 */    Lisp_Object indicate_buffer_boundaries_;
/*  264      |     4 */    Lisp_Object fringe_indicator_alist_;
---Type <return> to continue, or q <return> to quit---
/*  268      |     4 */    Lisp_Object fringe_cursor_alist_;
/*  272      |     4 */    Lisp_Object display_time_;
/*  276      |     4 */    Lisp_Object scroll_up_aggressively_;
/*  280      |     4 */    Lisp_Object scroll_down_aggressively_;
/*  284      |     4 */    Lisp_Object cursor_type_;
/*  288      |     4 */    Lisp_Object extra_line_spacing_;
/*  292      |     4 */    Lisp_Object cursor_in_non_selected_windows_;
/*  296      |   104 */    struct buffer_text {
/*  296      |     4 */        unsigned char *beg;
/*  300      |     4 */        ptrdiff_t gpt;
/*  304      |     4 */        ptrdiff_t z;
/*  308      |     4 */        ptrdiff_t gpt_byte;
/*  312      |     4 */        ptrdiff_t z_byte;
/*  316      |     4 */        ptrdiff_t gap_size;
/*  320      |     8 */        modiff_count modiff;
/*  328      |     8 */        modiff_count chars_modiff;
/*  336      |     8 */        modiff_count save_modiff;
/*  344      |     8 */        modiff_count overlay_modiff;
/*  352      |     8 */        modiff_count compact;
/*  360      |     4 */        ptrdiff_t beg_unchanged;
/*  364      |     4 */        ptrdiff_t end_unchanged;
/*  368      |     8 */        modiff_count unchanged_modified;
/*  376      |     8 */        modiff_count overlay_unchanged_modified;
/*  384      |     4 */        INTERVAL intervals;
/*  388      |     4 */        struct Lisp_Marker *markers;
/*  392:31   |     4 */        bool_bf inhibit_shrinking : 1;
/*  392:30   |     4 */        bool_bf redisplay : 1;

                               /* total size (bytes):  104 */
                           } own_text;
/*  400      |     4 */    struct buffer_text *text;
/*  404      |     4 */    struct buffer *next;
/*  408      |     4 */    ptrdiff_t pt;
/*  412      |     4 */    ptrdiff_t pt_byte;
/*  416      |     4 */    ptrdiff_t begv;
/*  420      |     4 */    ptrdiff_t begv_byte;
/*  424      |     4 */    ptrdiff_t zv;
/*  428      |     4 */    ptrdiff_t zv_byte;
/*  432      |     4 */    struct buffer *base_buffer;
/*  436      |     4 */    int indirections;
/*  440      |     4 */    int window_count;
/*  444      |    50 */    char local_flags[50];
/* XXX  2-byte hole  */
/*  496      |    16 */    struct timespec {
/*  496      |     8 */        union {
/*                 4 */            time_t tv_sec;
/*                 4 */            __time32_t __tv32_sec;
/*                 8 */            __time64_t __tv64_sec;

                                   /* total size (bytes):    8 */
                               };
/*  504      |     4 */        long tv_nsec;

                               /* total size (bytes):   16 */
                           } modtime;
/*  512      |     4 */    off_t modtime_size;
/* XXX  4-byte hole  */
/*  520      |     8 */    modiff_count auto_save_modified;
/*  528      |     8 */    modiff_count display_error_modiff;
/*  536      |     4 */    time_t auto_save_failure_time;
/*  540      |     4 */    ptrdiff_t last_window_start;
/*  544      |     4 */    struct region_cache *newline_cache;
/*  548      |     4 */    struct region_cache *width_run_cache;
/*  552      |     4 */    struct region_cache *bidi_paragraph_cache;
/*  556:31   |     4 */    bool_bf prevent_redisplay_optimizations_p : 1;
/*  556:30   |     4 */    bool_bf clip_changed : 1;
/* XXX  6-bit hole   */
/* XXX  3-byte hole  */
/*  560      |     4 */    struct Lisp_Overlay *overlays_before;
/*  564      |     4 */    struct Lisp_Overlay *overlays_after;
/*  568      |     4 */    ptrdiff_t overlay_center;
/*  572      |     4 */    Lisp_Object undo_list_;
---Type <return> to continue, or q <return> to quit---

                           /* total size (bytes):  576 */
                         } *
(gdb)

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

> Date: [04-07-2019 08:48:21] <07 Apr 2019 18:48:21 +0300>
> From: Eli Zaretskii <eliz@gnu.org>
> To: Keith David Bershatsky <esq@lawlist.com>
> CC: emacs-devel@gnu.org,monnier@iro.umontreal.ca,schwab@linux-m68k.org,acm@muc.de,dancol@dancol.org,eggert@cs.ucla.edu
> Subject: Re: buffer.c/buffer.h:  How to add new buffer-local variables?
> 
> > Date:  Sat, 06 Apr 2019 19:50:14 -0700
> > From:  Keith David Bershatsky <esq@lawlist.com>
> > Cc:  emacs-devel@gnu.org,Stefan Monnier <monnier@iro.umontreal.ca>,Andreas Schwab <schwab@linux-m68k.org>,Alan Mackenzie <acm@muc.de>,Daniel Colascione <dancol@dancol.org>,Paul Eggert <eggert@cs.ucla.edu>
> >
> > Using an unsophisticated method of going back in time and building various versions of Emacs on the w32 platform, I tracked down the inability to add a few new buffer-local variables in buffer.c/buffer.h to a commit that occurred on 01/31/2019:  05d2fc7170fb66a87601b1c76ddae2c1b7b4b934.
> 
> Is your GDB version 8.1 or later?  If so, please do this:
> 
>   $ cd /path/to/emacs/src
>   $ gdb ./emacs.exe
>   ...
>   (gdb) start -Q
>   ...
>   (gdb) ptype /o current_buffer
> 
> and post here the result.  Please do this both in the build before and
> after the above commit, and let us see both results.


[-- Attachment #2: 05d2fc7170fb66a87601b1c76ddae2c1b7b4b934.txt --]
[-- Type: application/txt, Size: 7939 bytes --]

[-- Attachment #3: a68eee50eb515b28b448894299334afced26ef78.txt --]
[-- Type: application/txt, Size: 7888 bytes --]

Re: buffer.c/buffer.h: How to add new buffer-local variables?

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 865 bytes --]

Keith David Bershatsky wrote:
> Perhaps there is something that may stand out (to a trained programmer) in the 01/31/2019 commit ....

It did change the buffer struct layout, so it's a good candidate for a culprit.

For what it's worth, I cannot reproduce the problem in a 32-bit build under 
Fedora 29 x86-64 (GCC 8.3.1). I configured this way:

./configure CC=gcc -m32 -march=native --enable-gcc-warnings 
--without-imagemagick --without-gif --with-modules --enable-checking=yes,glyphs 
--enable-check-lisp-object-type --with-gnutls=no

and built Emacs master with the attached patch x.diff.

My guess is that the problem has something to do with the unportable assumptions 
we've long made about struct buffer layout. I am attaching ptype.txt, the output 
of the GDB command "ptype /o current_buffer" that Eli suggested; please compare 
it to your ptype output.

[-- Attachment #2: x.diff --]
[-- Type: text/x-patch, Size: 11218 bytes --]

diff --git a/src/buffer.c b/src/buffer.c
index ab47748191..2f8cb1d7c5 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -133,6 +133,69 @@ CHECK_OVERLAY (Lisp_Object x)
 
 /* These setters are used only in this file, so they can be private.
    The public setters are inline functions defined in buffer.h.  */
+
+
+/* *************************************************************************** */
+/* begin MULTIPLE-CURSORS */
+
+static void
+bset_mc_real_fake_cursor (struct buffer *b, Lisp_Object val)
+{
+  b->mc_real_fake_cursor_ = val;
+}
+
+static void
+bset_mc_conf (struct buffer *b, Lisp_Object val)
+{
+  b->mc_conf_ = val;
+}
+
+static void
+bset_mc_inactive_windows (struct buffer *b, Lisp_Object val)
+{
+  b->mc_inactive_windows_ = val;
+}
+
+static void
+bset_crosshairs (struct buffer *b, Lisp_Object val)
+{
+  b->crosshairs_ = val;
+}
+
+static void
+bset_ch_horizontal_ruler (struct buffer *b, Lisp_Object val)
+{
+  b->ch_horizontal_ruler_ = val;
+}
+
+static void
+bset_ch_vertical_ruler (struct buffer *b, Lisp_Object val)
+{
+  b->ch_vertical_ruler_ = val;
+}
+
+static void
+bset_ch_inactive_windows (struct buffer *b, Lisp_Object val)
+{
+  b->ch_inactive_windows_ = val;
+}
+
+static void
+bset_fc_visible (struct buffer *b, Lisp_Object val)
+{
+  b->fc_visible_ = val;
+}
+
+static void
+bset_fc_inactive_windows (struct buffer *b, Lisp_Object val)
+{
+  b->fc_inactive_windows_ = val;
+}
+
+/* end MULTIPLE-CURSORS */
+/* *************************************************************************** */
+
+
 static void
 bset_abbrev_mode (struct buffer *b, Lisp_Object val)
 {
@@ -5142,6 +5205,24 @@ init_buffer_once (void)
   bset_last_selected_window (&buffer_local_flags, make_fixnum (0));
 
   idx = 1;
+
+
+/* *************************************************************************** */
+/* MULTIPLE-CURSORS */
+
+  XSETFASTINT (BVAR (&buffer_local_flags, mc_real_fake_cursor), idx); ++idx;
+  XSETFASTINT (BVAR (&buffer_local_flags, mc_conf), idx); ++idx;
+  XSETFASTINT (BVAR (&buffer_local_flags, mc_inactive_windows), idx); ++idx;
+  XSETFASTINT (BVAR (&buffer_local_flags, crosshairs), idx); ++idx;
+  XSETFASTINT (BVAR (&buffer_local_flags, ch_horizontal_ruler), idx); ++idx;
+  XSETFASTINT (BVAR (&buffer_local_flags, ch_vertical_ruler), idx); ++idx;
+  XSETFASTINT (BVAR (&buffer_local_flags, ch_inactive_windows), idx); ++idx;
+  XSETFASTINT (BVAR (&buffer_local_flags, fc_visible), idx); ++idx;
+  XSETFASTINT (BVAR (&buffer_local_flags, fc_inactive_windows), idx); ++idx;
+
+/* *************************************************************************** */
+
+
   XSETFASTINT (BVAR (&buffer_local_flags, mode_line_format), idx); ++idx;
   XSETFASTINT (BVAR (&buffer_local_flags, abbrev_mode), idx); ++idx;
   XSETFASTINT (BVAR (&buffer_local_flags, overwrite_mode), idx); ++idx;
@@ -5228,6 +5309,24 @@ init_buffer_once (void)
   /* Must do these before making the first buffer! */
 
   /* real setup is done in bindings.el */
+
+
+/* *************************************************************************** */
+/* MULTIPLE-CURSORS */
+
+  bset_mc_real_fake_cursor (&buffer_defaults, Qt);
+  bset_mc_conf (&buffer_defaults, Qnil);
+  bset_mc_inactive_windows (&buffer_defaults, Qt);
+  bset_crosshairs (&buffer_defaults, Qnil);
+  bset_ch_horizontal_ruler (&buffer_defaults, Qt);
+  bset_ch_vertical_ruler (&buffer_defaults, Qt);
+  bset_ch_inactive_windows (&buffer_defaults, Qt);
+  bset_fc_visible (&buffer_defaults, Qnil);
+  bset_fc_inactive_windows (&buffer_defaults, Qt);
+
+/* *************************************************************************** */
+
+
   bset_mode_line_format (&buffer_defaults, build_pure_c_string ("%-"));
   bset_header_line_format (&buffer_defaults, Qnil);
   bset_abbrev_mode (&buffer_defaults, Qnil);
@@ -5464,6 +5563,23 @@ syms_of_buffer (void)
   staticpro (&QSFundamental);
   staticpro (&Vbuffer_alist);
 
+
+/* *************************************************************************** */
+/* MULTIPLE-CURSORS */
+
+  DEFSYM (Qmc_real_fake_cursor, "mc-real-fake-cursor");
+  DEFSYM (Qmc_conf, "mc-conf");
+  DEFSYM (Qmc_inactive_windows, "mc-inactive-windows");
+  DEFSYM (Qcrosshairs, "crosshairs");
+  DEFSYM (Qch_horizontal_ruler, "ch-horizontal-ruler");
+  DEFSYM (Qch_vertical_ruler, "ch-vertical-ruler");
+  DEFSYM (Qch_inactive_windows, "ch-inactive-windows");
+  DEFSYM (Qfc_visible, "fc-visible");
+  DEFSYM (Qfc_visible_inactive_window, "fc-inactive-window");
+
+/* *************************************************************************** */
+
+
   DEFSYM (Qchoice, "choice");
   DEFSYM (Qleft, "left");
   DEFSYM (Qright, "right");
@@ -5501,6 +5617,41 @@ syms_of_buffer (void)
   Fput (Qprotected_field, Qerror_message,
 	build_pure_c_string ("Attempt to modify a protected field"));
 
+
+/* *************************************************************************** */
+/* MULTIPLE-CURSORS */
+
+  DEFVAR_PER_BUFFER ("mc-real-fake-cursor", &BVAR (current_buffer, mc_real_fake_cursor), Qnil,
+    doc: /* A bufer-local variable to set the cursor type of the real fake cursor. */);
+
+  DEFVAR_PER_BUFFER ("mc-conf", &BVAR (current_buffer, mc_conf), Qnil,
+    doc: /* A bufer-local variable to store the value of the multiple cursors to be displayed
+during the next redisplay. */);
+
+  DEFVAR_PER_BUFFER ("mc-inactive-windows", &BVAR (current_buffer, mc_inactive_windows), Qnil,
+    doc: /* When non-nil, draw multiple cursors in inactive windows. */);
+
+  DEFVAR_PER_BUFFER ("crosshairs", &BVAR (current_buffer, crosshairs), Qnil,
+    doc: /* A bufer-local variable to activate/deactivate crosshairs. */);
+
+  DEFVAR_PER_BUFFER ("ch-horizontal-ruler", &BVAR (current_buffer, ch_horizontal_ruler), Qnil,
+    doc: /* A bufer-local variable to activate/deactivate the crosshairs horizontal ruler. */);
+
+  DEFVAR_PER_BUFFER ("ch-vertical-ruler", &BVAR (current_buffer, ch_vertical_ruler), Qnil,
+    doc: /* A bufer-local variable to activate/deactivate the crosshairs vertical ruler. */);
+
+  DEFVAR_PER_BUFFER ("ch-inactive-windows", &BVAR (current_buffer, ch_inactive_windows), Qnil,
+    doc: /* When non-nil, draw crosshairs in inactive windows. */);
+
+  DEFVAR_PER_BUFFER ("fc-visible", &BVAR (current_buffer, fc_visible), Qnil,
+    doc: /* A bufer-local variable to turn on/off the multiple cursors fill column. */);
+
+  DEFVAR_PER_BUFFER ("fc-inactive-windows", &BVAR (current_buffer, fc_inactive_windows), Qnil,
+    doc: /* When non-nil, draw multiple cursors fill column in inactive windows. */);
+
+/* *************************************************************************** */
+
+
   DEFVAR_PER_BUFFER ("header-line-format",
 		     &BVAR (current_buffer, header_line_format),
 		     Qnil,
diff --git a/src/buffer.h b/src/buffer.h
index 63b162161c..4dd91911da 100644
--- a/src/buffer.h
+++ b/src/buffer.h
@@ -636,6 +636,40 @@ struct buffer
      cache are enabled.  See search.c, indent.c and bidi.c for details.  */
   Lisp_Object cache_long_scans_;
 
+
+/* *************************************************************************** */
+/* MULTIPLE-CURSORS */
+
+  /* The cursor type of the real fake cursor. */
+  Lisp_Object mc_real_fake_cursor_;
+
+  /* The name of list used by multiple cursors for next redisplay. */
+  Lisp_Object mc_conf_;
+
+  /* Whether to draw multiple cursors in inactive windows. */
+  Lisp_Object mc_inactive_windows_;
+
+  /* The name of the buffer-local variable. */
+  Lisp_Object crosshairs_;
+
+  /* The name of the crosshairs horizontal ruler. */
+  Lisp_Object ch_horizontal_ruler_;
+
+  /* The name of the crosshairs vertical ruler. */
+  Lisp_Object ch_vertical_ruler_;
+
+  /* Whether to draw crosshairs in inactive windows. */
+  Lisp_Object ch_inactive_windows_;
+
+  /* The name of the buffer-local variable. */
+  Lisp_Object fc_visible_;
+
+  /* Whether to draw multiple cursors fill column in inactive windows. */
+  Lisp_Object fc_inactive_windows_;
+
+/* *************************************************************************** */
+
+
   /* If the width run cache is enabled, this table contains the
      character widths width_run_cache (see above) assumes.  When we
      do a thorough redisplay, we compare this against the buffer's
@@ -787,14 +821,21 @@ struct buffer
      an indirect buffer since it counts as its base buffer.  */
   int window_count;
 
+
+/* *************************************************************************** */
+/* MULTIPLE-CURSORS */
+
   /* A non-zero value in slot IDX means that per-buffer variable
      with index IDX has a local value in this buffer.  The index IDX
      for a buffer-local variable is stored in that variable's slot
      in buffer_local_flags as a Lisp integer.  If the index is -1,
      this means the variable is always local in all buffers.  */
-#define MAX_PER_BUFFER_VARS 50
+#define MAX_PER_BUFFER_VARS 60
   char local_flags[MAX_PER_BUFFER_VARS];
 
+/* *************************************************************************** */
+
+
   /* Set to the modtime of the visited file when read or written.
      modtime.tv_nsec == NONEXISTENT_MODTIME_NSECS means
      visited file was nonexistent.  modtime.tv_nsec ==
@@ -903,6 +944,69 @@ XBUFFER (Lisp_Object a)
 /* Most code should use these functions to set Lisp fields in struct
    buffer.  (Some setters that are private to a single .c file are
    defined as static in those files.)  */
+
+
+/* *************************************************************************** */
+/* begin MULTIPLE-CURSORS */
+
+INLINE void
+bset_blv_mc_real_fake_cursor (struct buffer *b, Lisp_Object val)
+{
+  b->mc_real_fake_cursor_ = val;
+}
+
+INLINE void
+bset_blv_mc_conf (struct buffer *b, Lisp_Object val)
+{
+  b->mc_conf_ = val;
+}
+
+INLINE void
+bset_blv_mc_inactive_windows (struct buffer *b, Lisp_Object val)
+{
+  b->mc_inactive_windows_ = val;
+}
+
+INLINE void
+bset_blv_crosshairs (struct buffer *b, Lisp_Object val)
+{
+  b->crosshairs_ = val;
+}
+
+INLINE void
+bset_blv_ch_horizontal_ruler (struct buffer *b, Lisp_Object val)
+{
+  b->ch_horizontal_ruler_ = val;
+}
+
+INLINE void
+bset_blv_ch_vertical_ruler (struct buffer *b, Lisp_Object val)
+{
+  b->ch_vertical_ruler_ = val;
+}
+
+INLINE void
+bset_blv_ch_inactive_windows (struct buffer *b, Lisp_Object val)
+{
+  b->ch_inactive_windows_ = val;
+}
+
+INLINE void
+bset_blv_fc_visible (struct buffer *b, Lisp_Object val)
+{
+  b->fc_visible_ = val;
+}
+
+INLINE void
+bset_blv_fc_inactive_windows (struct buffer *b, Lisp_Object val)
+{
+  b->fc_inactive_windows_ = val;
+}
+
+/* end MULTIPLE-CURSORS */
+/* *************************************************************************** */
+
+
 INLINE void
 bset_bidi_paragraph_direction (struct buffer *b, Lisp_Object val)
 {
diff --git a/src/pdumper.c b/src/pdumper.c
index b19f206d1b..0b60f79a6c 100644
--- a/src/pdumper.c
+++ b/src/pdumper.c
@@ -2770,9 +2770,6 @@ dump_hash_table (struct dump_context *ctx,
 static dump_off
 dump_buffer (struct dump_context *ctx, const struct buffer *in_buffer)
 {
-#if CHECK_STRUCTS && !defined HASH_buffer_2CEE653E74
-# error "buffer changed. See CHECK_STRUCTS comment."
-#endif
   struct buffer munged_buffer = *in_buffer;
   struct buffer *buffer = &munged_buffer;
 

[-- Attachment #3: ptype.txt --]
[-- Type: text/plain, Size: 8076 bytes --]

(gdb) ptype /o current_buffer
type = struct buffer {
/*    0      |     4 */    union vectorlike_header {
/*                 4 */        ptrdiff_t size;

                               /* total size (bytes):    4 */
                           } header;
/*    4      |     4 */    Lisp_Object name_;
/*    8      |     4 */    Lisp_Object filename_;
/*   12      |     4 */    Lisp_Object directory_;
/*   16      |     4 */    Lisp_Object backed_up_;
/*   20      |     4 */    Lisp_Object save_length_;
/*   24      |     4 */    Lisp_Object auto_save_file_name_;
/*   28      |     4 */    Lisp_Object read_only_;
/*   32      |     4 */    Lisp_Object mark_;
/*   36      |     4 */    Lisp_Object local_var_alist_;
/*   40      |     4 */    Lisp_Object major_mode_;
/*   44      |     4 */    Lisp_Object mode_name_;
/*   48      |     4 */    Lisp_Object mode_line_format_;
/*   52      |     4 */    Lisp_Object header_line_format_;
/*   56      |     4 */    Lisp_Object keymap_;
/*   60      |     4 */    Lisp_Object abbrev_table_;
/*   64      |     4 */    Lisp_Object syntax_table_;
/*   68      |     4 */    Lisp_Object category_table_;
/*   72      |     4 */    Lisp_Object case_fold_search_;
/*   76      |     4 */    Lisp_Object tab_width_;
/*   80      |     4 */    Lisp_Object fill_column_;
/*   84      |     4 */    Lisp_Object left_margin_;
/*   88      |     4 */    Lisp_Object auto_fill_function_;
/*   92      |     4 */    Lisp_Object downcase_table_;
/*   96      |     4 */    Lisp_Object upcase_table_;
/*  100      |     4 */    Lisp_Object case_canon_table_;
/*  104      |     4 */    Lisp_Object case_eqv_table_;
/*  108      |     4 */    Lisp_Object truncate_lines_;
/*  112      |     4 */    Lisp_Object word_wrap_;
/*  116      |     4 */    Lisp_Object ctl_arrow_;
/*  120      |     4 */    Lisp_Object bidi_display_reordering_;
/*  124      |     4 */    Lisp_Object bidi_paragraph_direction_;
/*  128      |     4 */    Lisp_Object bidi_paragraph_separate_re_;
/*  132      |     4 */    Lisp_Object bidi_paragraph_start_re_;
/*  136      |     4 */    Lisp_Object selective_display_;
/*  140      |     4 */    Lisp_Object selective_display_ellipses_;
/*  144      |     4 */    Lisp_Object minor_modes_;
/*  148      |     4 */    Lisp_Object overwrite_mode_;
/*  152      |     4 */    Lisp_Object abbrev_mode_;
/*  156      |     4 */    Lisp_Object display_table_;
/*  160      |     4 */    Lisp_Object mark_active_;
/*  164      |     4 */    Lisp_Object enable_multibyte_characters_;
/*  168      |     4 */    Lisp_Object buffer_file_coding_system_;
/*  172      |     4 */    Lisp_Object file_format_;
/*  176      |     4 */    Lisp_Object auto_save_file_format_;
/*  180      |     4 */    Lisp_Object cache_long_scans_;
/*  184      |     4 */    Lisp_Object mc_real_fake_cursor_;
/*  188      |     4 */    Lisp_Object mc_conf_;
/*  192      |     4 */    Lisp_Object mc_inactive_windows_;
/*  196      |     4 */    Lisp_Object crosshairs_;
/*  200      |     4 */    Lisp_Object ch_horizontal_ruler_;
/*  204      |     4 */    Lisp_Object ch_vertical_ruler_;
/*  208      |     4 */    Lisp_Object ch_inactive_windows_;
/*  212      |     4 */    Lisp_Object fc_visible_;
/*  216      |     4 */    Lisp_Object fc_inactive_windows_;
/*  220      |     4 */    Lisp_Object width_table_;
/*  224      |     4 */    Lisp_Object pt_marker_;
/*  228      |     4 */    Lisp_Object begv_marker_;
/*  232      |     4 */    Lisp_Object zv_marker_;
/*  236      |     4 */    Lisp_Object point_before_scroll_;
/*  240      |     4 */    Lisp_Object file_truename_;
/*  244      |     4 */    Lisp_Object invisibility_spec_;
/*  248      |     4 */    Lisp_Object last_selected_window_;
/*  252      |     4 */    Lisp_Object display_count_;
/*  256      |     4 */    Lisp_Object left_margin_cols_;
/*  260      |     4 */    Lisp_Object right_margin_cols_;
/*  264      |     4 */    Lisp_Object left_fringe_width_;
/*  268      |     4 */    Lisp_Object right_fringe_width_;
/*  272      |     4 */    Lisp_Object fringes_outside_margins_;
/*  276      |     4 */    Lisp_Object scroll_bar_width_;
/*  280      |     4 */    Lisp_Object scroll_bar_height_;
/*  284      |     4 */    Lisp_Object vertical_scroll_bar_type_;
/*  288      |     4 */    Lisp_Object horizontal_scroll_bar_type_;
/*  292      |     4 */    Lisp_Object indicate_empty_lines_;
/*  296      |     4 */    Lisp_Object indicate_buffer_boundaries_;
/*  300      |     4 */    Lisp_Object fringe_indicator_alist_;
/*  304      |     4 */    Lisp_Object fringe_cursor_alist_;
/*  308      |     4 */    Lisp_Object display_time_;
/*  312      |     4 */    Lisp_Object scroll_up_aggressively_;
/*  316      |     4 */    Lisp_Object scroll_down_aggressively_;
/*  320      |     4 */    Lisp_Object cursor_type_;
/*  324      |     4 */    Lisp_Object extra_line_spacing_;
/*  328      |     4 */    Lisp_Object cursor_in_non_selected_windows_;
/*  332      |   100 */    struct buffer_text {
/*  332      |     4 */        unsigned char *beg;
/*  336      |     4 */        ptrdiff_t gpt;
/*  340      |     4 */        ptrdiff_t z;
/*  344      |     4 */        ptrdiff_t gpt_byte;
/*  348      |     4 */        ptrdiff_t z_byte;
/*  352      |     4 */        ptrdiff_t gap_size;
/*  356      |     8 */        modiff_count modiff;
/*  364      |     8 */        modiff_count chars_modiff;
/*  372      |     8 */        modiff_count save_modiff;
/*  380      |     8 */        modiff_count overlay_modiff;
/*  388      |     8 */        modiff_count compact;
/*  396      |     4 */        ptrdiff_t beg_unchanged;
/*  400      |     4 */        ptrdiff_t end_unchanged;
/*  404      |     8 */        modiff_count unchanged_modified;
/*  412      |     8 */        modiff_count overlay_unchanged_modified;
/*  420      |     4 */        INTERVAL intervals;
/*  424      |     4 */        struct Lisp_Marker *markers;
/*  428: 7   |     1 */        bool_bf inhibit_shrinking : 1;
/*  428: 6   |     1 */        bool_bf redisplay : 1;
/* XXX  6-bit padding  */
/* XXX  3-byte padding */

                               /* total size (bytes):  100 */
                           } own_text;
/*  432      |     4 */    struct buffer_text *text;
/*  436      |     4 */    struct buffer *next;
/*  440      |     4 */    ptrdiff_t pt;
/*  444      |     4 */    ptrdiff_t pt_byte;
/*  448      |     4 */    ptrdiff_t begv;
/*  452      |     4 */    ptrdiff_t begv_byte;
/*  456      |     4 */    ptrdiff_t zv;
/*  460      |     4 */    ptrdiff_t zv_byte;
/*  464      |     4 */    struct buffer *base_buffer;
/*  468      |     4 */    int indirections;
/*  472      |     4 */    int window_count;
/*  476      |    60 */    char local_flags[60];
/*  536      |     8 */    struct timespec {
/*  536      |     4 */        __time_t tv_sec;
/*  540      |     4 */        __syscall_slong_t tv_nsec;

                               /* total size (bytes):    8 */
                           } modtime;
/*  544      |     8 */    off_t modtime_size;
/*  552      |     8 */    modiff_count auto_save_modified;
/*  560      |     8 */    modiff_count display_error_modiff;
/*  568      |     4 */    time_t auto_save_failure_time;
/*  572      |     4 */    ptrdiff_t last_window_start;
/*  576      |     4 */    struct region_cache *newline_cache;
/*  580      |     4 */    struct region_cache *width_run_cache;
/*  584      |     4 */    struct region_cache *bidi_paragraph_cache;
/*  588: 7   |     1 */    bool_bf prevent_redisplay_optimizations_p : 1;
/*  588: 6   |     1 */    bool_bf clip_changed : 1;
/*  588: 5   |     1 */    bool_bf inhibit_buffer_hooks : 1;
/* XXX  5-bit hole  */
/* XXX  3-byte hole */
/*  592      |     4 */    struct Lisp_Overlay *overlays_before;
/*  596      |     4 */    struct Lisp_Overlay *overlays_after;
/*  600      |     4 */    ptrdiff_t overlay_center;
/*  604      |     4 */    Lisp_Object undo_list_;

                           /* total size (bytes):  608 */
                         } *
(gdb)

Re: buffer.c/buffer.h: How to add new buffer-local variables?

[Keith David Bershatsky]

[-- Attachment #1: Type: text/plain, Size: 2724 bytes --]

Thank you, Paul, for taking the time to try and reproduce the behavior that I am experiencing on my end.

I am assuming that this round of tests is with a current version of the master branch (0b8117ed1abfc17bb0bc1690a8997736f1e8f98c) and _after_ applying the x.diff patch.  I built the Emacs master branch (current version) without any modifications so that the build could complete without errors, and then I applied the x.diff patch and built again and then performed the gdb test.  The build with the patch will crash on my end when garbage collection occurs; however, the ptype test is able to complete and I have attached a gdb printout 0b8117ed1abfc17bb0bc1690a8997736f1e8f98c.txt.  The current issue is beyond my current level of programming abilities to resolve, so other than performing a standard M-x diff between your ptype test (relabeled as ptype_by_paul.txt) and my own test (0b8117ed1abfc17
 bb0bc1690a8997736f1e8f98c.txt), I would need further guidance regarding how best to be of assistance.

My setup on Windows XP SP-3 didn't recognize a build option of -m32, so I just used the same old build options that I have used previously; i.e.,:

CFLAGS='-O0 -g3' ./configure \
--enable-checking='yes,glyphs' \
--enable-check-lisp-object-type \
--without-compress-install \
--without-makeinfo \
--with-gnutls=no \
--with-mailutils \
--without-makeinfo

Keith

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

> Date: [04-07-2019 22:23:07] <7 Apr 2019 22:23:07 -0700>
> From: Paul Eggert <eggert@cs.ucla.edu>
> To: Keith David Bershatsky <esq@lawlist.com>
> Cc: emacs-devel@gnu.org, Stefan Monnier <monnier@iro.umontreal.ca>,
> Andreas Schwab <schwab@linux-m68k.org>, Alan Mackenzie <acm@muc.de>,
> Daniel Colascione <dancol@dancol.org>
> Subject: Re: buffer.c/buffer.h: How to add new buffer-local variables?
> 
> Keith David Bershatsky wrote:
> > Perhaps there is something that may stand out (to a trained programmer) in the 01/31/2019 commit ....
> 
> It did change the buffer struct layout, so it's a good candidate for a culprit.
> 
> For what it's worth, I cannot reproduce the problem in a 32-bit build under Fedora 29 x86-64 (GCC 8.3.1). I configured this way:
> 
> ./configure CC=gcc -m32 -march=native --enable-gcc-warnings --without-imagemagick --without-gif --with-modules --enable-checking=yes,glyphs --enable-check-lisp-object-type --with-gnutls=no
> 
> and built Emacs master with the attached patch x.diff.
> 
> My guess is that the problem has something to do with the unportable assumptions we've long made about struct buffer layout. I am attaching ptype.txt, the output of the GDB command "ptype /o current_buffer" that Eli suggested; please compare it to your ptype output.


[-- Attachment #2: 0b8117ed1abfc17bb0bc1690a8997736f1e8f98c.txt --]
[-- Type: application/txt, Size: 8361 bytes --]

[-- Attachment #3: ptype_by_paul.txt --]
[-- Type: application/txt, Size: 8076 bytes --]

[-- Attachment #4: x.diff --]
[-- Type: application/diff, Size: 11218 bytes --]

Re: buffer.c/buffer.h: How to add new buffer-local variables?

[Eli Zaretskii]

On April 8, 2019 11:03:17 AM GMT+03:00, Keith David Bershatsky <esq@lawlist.com> wrote:
> Thank you, Paul, for taking the time to try and reproduce the behavior
> that I am experiencing on my end.
> 
> I am assuming that this round of tests is with a current version of
> the master branch (0b8117ed1abfc17bb0bc1690a8997736f1e8f98c) and
> _after_ applying the x.diff patch.  I built the Emacs master branch
> (current version) without any modifications so that the build could
> complete without errors, and then I applied the x.diff patch and built
> again and then performed the gdb test.  The build with the patch will
> crash on my end when garbage collection occurs; however, the ptype
> test is able to complete and I have attached a gdb printout
> 0b8117ed1abfc17bb0bc1690a8997736f1e8f98c.txt.  The current issue is
> beyond my current level of programming abilities to resolve, so other
> than performing a standard M-x diff between your ptype test (relabeled
> as ptype_by_paul.txt) and my own test
> (0b8117ed1abfc17bb0bc1690a8997736f1e8f98c.txt), I would need further
> guidance regarding how best to be of assistance.
> 
> My setup on Windows XP SP-3 didn't recognize a build option of -m32,
> so I just used the same old build options that I have used previously;
> i.e.,:
> 
> CFLAGS='-O0 -g3' ./configure \
> --enable-checking='yes,glyphs' \
> --enable-check-lisp-object-type \
> --without-compress-install \
> --without-makeinfo \
> --with-gnutls=no \
> --with-mailutils \
> --without-makeinfo
> 
> Keith
> 
> ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
> 
> > Date: [04-07-2019 22:23:07] <7 Apr 2019 22:23:07 -0700>
> > From: Paul Eggert <eggert@cs.ucla.edu>
> > To: Keith David Bershatsky <esq@lawlist.com>
> > Cc: emacs-devel@gnu.org, Stefan Monnier <monnier@iro.umontreal.ca>,
> > Andreas Schwab <schwab@linux-m68k.org>, Alan Mackenzie <acm@muc.de>,
> > Daniel Colascione <dancol@dancol.org>
> > Subject: Re: buffer.c/buffer.h: How to add new buffer-local
> variables?
> > 
> > Keith David Bershatsky wrote:
> > > Perhaps there is something that may stand out (to a trained
> programmer) in the 01/31/2019 commit ....
> > 
> > It did change the buffer struct layout, so it's a good candidate for
> a culprit.
> > 
> > For what it's worth, I cannot reproduce the problem in a 32-bit
> build under Fedora 29 x86-64 (GCC 8.3.1). I configured this way:
> > 
> > ./configure CC=gcc -m32 -march=native --enable-gcc-warnings
> --without-imagemagick --without-gif --with-modules
> --enable-checking=yes,glyphs --enable-check-lisp-object-type
> --with-gnutls=no
> > 
> > and built Emacs master with the attached patch x.diff.
> > 
> > My guess is that the problem has something to do with the unportable
> assumptions we've long made about struct buffer layout. I am attaching
> ptype.txt, the output of the GDB command "ptype /o current_buffer"
> that Eli suggested; please compare it to your ptype output.

The problem is caused by the 4-byte hole between the last Lisp_Object member of the buffer structure and the beginning of struct buffer_text.  It causes us to decide that a buffer has 83 Lisp components, whereas it actually has only 82.  The hole is left uninitialized, and causes the segfault when we try to use it as a valid object.

I guess we need to make BUFFER_LISP_SIZE smarter?

Re: buffer.c/buffer.h: How to add new buffer-local variables?

[Eli Zaretskii]

> Date: Mon, 08 Apr 2019 12:37:57 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: acm@muc.de, dancol@dancol.org, schwab@linux-m68k.org,
> 	monnier@iro.umontreal.ca
> 
> The problem is caused by the 4-byte hole between the last Lisp_Object member of the buffer structure and the beginning of struct buffer_text.  It causes us to decide that a buffer has 83 Lisp components, whereas it actually has only 82.  The hole is left uninitialized, and causes the segfault when we try to use it as a valid object.

To clarify, I meant this hole:

> /*  324      |     4 */    Lisp_Object extra_line_spacing_;
> /*  328      |     4 */    Lisp_Object cursor_in_non_selected_windows_;
> /* XXX  4-byte hole  */  <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
> /*  336      |   104 */    struct buffer_text {
> /*  336      |     4 */        unsigned char *beg;

Significantly, it doesn't show in what Paul saw in a 32-bit Fedora
build.  I guess the MS-Windows port of GCC has some special rules in
this case, perhaps triggered by the fact that some members of 'struct
buffer_text' are now 64-bit integers, and perhaps GCC on Windows wants
them 8-byte aligned?

> I guess we need to make BUFFER_LISP_SIZE smarter?

Not sure how, though.  Perhaps we should simply make BUFFER_LISP_SIZE
a literal number and request whoever extends 'struct buffer' to keep
that in sync manually?

Re: buffer.c/buffer.h: How to add new buffer-local variables?

[Andreas Schwab]

On Apr 08 2019, Eli Zaretskii <eliz@gnu.org> wrote:

> I guess we need to make BUFFER_LISP_SIZE smarter?

Perhaps adding a dummy member the same size as Lisp_Object, and using
that as the anchor?

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: buffer.c/buffer.h: How to add new buffer-local variables?

[Eli Zaretskii]

> From: Andreas Schwab <schwab@linux-m68k.org>
> Cc: emacs-devel@gnu.org, Keith David Bershatsky <esq@lawlist.com>, Paul Eggert <eggert@cs.ucla.edu>, monnier@iro.umontreal.ca, acm@muc.de, dancol@dancol.org
> Date: Mon, 08 Apr 2019 19:31:46 +0200
> 
> On Apr 08 2019, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> > I guess we need to make BUFFER_LISP_SIZE smarter?
> 
> Perhaps adding a dummy member the same size as Lisp_Object, and using
> that as the anchor?

Could be, although this will make the struct larger than it could have
been on some systems.

These complications are why I wonder whether they are worth the
occasional hard-to-debug bug.  How hard would it be to ask to update
the value when a Lisp member is added?

Re: buffer.c/buffer.h: How to add new buffer-local variables?

[Stefan Monnier]

>> Perhaps adding a dummy member the same size as Lisp_Object, and using
>> that as the anchor?
> Could be, although this will make the struct larger than it could have
> been on some systems.

We could also use the last Lisp_Object element as the anchor (taking its
offset plus its size as the "end offset").


        Stefan

Re: buffer.c/buffer.h: How to add new buffer-local variables?

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 691 bytes --]

On 4/8/19 11:33 AM, Stefan Monnier wrote:
> We could also use the last Lisp_Object element as the anchor (taking its
> offset plus its size as the "end offset").

Yes, that's a simple way to fix the problem. I was working on this idea
when I read your email. Also, I noticed that the problem could
potentially happen to any pseudovector, not just to buffers. I installed
the attached into master to fix the problem with other pseudovectors too.

This patch doesn't solve the general portability issue in this area, as
the C standard guarantees only that struct members are aligned and don't
overlap and are in increasing address order. But it should be enough to
fix the immediate problem.


[-- Attachment #2: 0001-Allow-gap-before-first-non-Lisp-pseudovec-member.patch --]
[-- Type: text/x-patch, Size: 22590 bytes --]

From 100ae28c35ad7788833992293417399ad19bf4db Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Mon, 8 Apr 2019 12:59:22 -0700
Subject: [PATCH] Allow gap before first non-Lisp pseudovec member

Problem reported by Keith David Bershatsky in:
https://lists.gnu.org/r/emacs-devel/2019-04/msg00259.html
Solution suggested by Stefan Monnier in:
https://lists.gnu.org/r/emacs-devel/2019-04/msg00282.html
* src/buffer.h (BUFFER_LISP_SIZE): Simplify by using PSEUDOVECSIZE.
(BUFFER_REST_SIZE): Simplify by using VECSIZE and BUFFER_LISP_SIZE.
* src/lisp.h (PSEUDOVECSIZE): Base it on the last Lisp field,
not the first non-Lisp field.  All callers changed.  Callers
without Lisp fields changed to use ALLOCATE_PLAIN_PSEUDOVECTOR.
(ALLOCATE_PLAIN_PSEUDOVECTOR): New macro.
---
 src/alloc.c        | 20 ++++++++++----------
 src/bignum.c       |  8 ++++----
 src/buffer.h       | 14 ++++++--------
 src/emacs-module.c |  2 +-
 src/fns.c          |  2 +-
 src/frame.c        |  3 ++-
 src/frame.h        |  7 +++----
 src/lisp.h         | 24 ++++++++++++++++--------
 src/pdumper.c      |  4 ++--
 src/process.c      |  3 ++-
 src/process.h      |  4 +---
 src/termhooks.h    |  2 +-
 src/terminal.c     |  4 ++--
 src/thread.c       |  8 ++++----
 src/thread.h       |  2 +-
 src/w32term.c      |  2 +-
 src/window.c       | 23 ++++++++++-------------
 src/window.h       |  5 ++---
 src/xterm.c        |  4 ++--
 src/xterm.h        |  2 +-
 src/xwidget.c      |  5 ++---
 src/xwidget.h      |  6 ++----
 22 files changed, 76 insertions(+), 78 deletions(-)

diff --git a/src/alloc.c b/src/alloc.c
index e48807c49a..dd783863be 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -3718,8 +3718,8 @@ Its value is void, and its function definition and property list are nil.  */)
 Lisp_Object
 make_misc_ptr (void *a)
 {
-  struct Lisp_Misc_Ptr *p = ALLOCATE_PSEUDOVECTOR (struct Lisp_Misc_Ptr, pointer,
-						   PVEC_MISC_PTR);
+  struct Lisp_Misc_Ptr *p = ALLOCATE_PLAIN_PSEUDOVECTOR (struct Lisp_Misc_Ptr,
+							 PVEC_MISC_PTR);
   p->pointer = a;
   return make_lisp_ptr (p, Lisp_Vectorlike);
 }
@@ -3729,7 +3729,7 @@ make_misc_ptr (void *a)
 Lisp_Object
 build_overlay (Lisp_Object start, Lisp_Object end, Lisp_Object plist)
 {
-  struct Lisp_Overlay *p = ALLOCATE_PSEUDOVECTOR (struct Lisp_Overlay, next,
+  struct Lisp_Overlay *p = ALLOCATE_PSEUDOVECTOR (struct Lisp_Overlay, plist,
 						  PVEC_OVERLAY);
   Lisp_Object overlay = make_lisp_ptr (p, Lisp_Vectorlike);
   OVERLAY_START (overlay) = start;
@@ -3743,8 +3743,8 @@ DEFUN ("make-marker", Fmake_marker, Smake_marker, 0, 0, 0,
        doc: /* Return a newly allocated marker which does not point at any place.  */)
   (void)
 {
-  struct Lisp_Marker *p = ALLOCATE_PSEUDOVECTOR (struct Lisp_Marker, buffer,
-						 PVEC_MARKER);
+  struct Lisp_Marker *p = ALLOCATE_PLAIN_PSEUDOVECTOR (struct Lisp_Marker,
+						       PVEC_MARKER);
   p->buffer = 0;
   p->bytepos = 0;
   p->charpos = 0;
@@ -3766,8 +3766,8 @@ build_marker (struct buffer *buf, ptrdiff_t charpos, ptrdiff_t bytepos)
   /* Every character is at least one byte.  */
   eassert (charpos <= bytepos);
 
-  struct Lisp_Marker *m = ALLOCATE_PSEUDOVECTOR (struct Lisp_Marker, buffer,
-						 PVEC_MARKER);
+  struct Lisp_Marker *m = ALLOCATE_PLAIN_PSEUDOVECTOR (struct Lisp_Marker,
+						       PVEC_MARKER);
   m->buffer = buf;
   m->charpos = charpos;
   m->bytepos = bytepos;
@@ -3821,8 +3821,8 @@ make_event_array (ptrdiff_t nargs, Lisp_Object *args)
 Lisp_Object
 make_user_ptr (void (*finalizer) (void *), void *p)
 {
-  struct Lisp_User_Ptr *uptr = ALLOCATE_PSEUDOVECTOR (struct Lisp_User_Ptr,
-						      finalizer, PVEC_USER_PTR);
+  struct Lisp_User_Ptr *uptr
+    = ALLOCATE_PLAIN_PSEUDOVECTOR (struct Lisp_User_Ptr, PVEC_USER_PTR);
   uptr->finalizer = finalizer;
   uptr->p = p;
   return make_lisp_ptr (uptr, Lisp_Vectorlike);
@@ -3945,7 +3945,7 @@ FUNCTION.  FUNCTION will be run once per finalizer object.  */)
   (Lisp_Object function)
 {
   struct Lisp_Finalizer *finalizer
-    = ALLOCATE_PSEUDOVECTOR (struct Lisp_Finalizer, prev, PVEC_FINALIZER);
+    = ALLOCATE_PSEUDOVECTOR (struct Lisp_Finalizer, function, PVEC_FINALIZER);
   finalizer->function = function;
   finalizer->prev = finalizer->next = NULL;
   finalizer_insert (&finalizers, finalizer);
diff --git a/src/bignum.c b/src/bignum.c
index 4118601e10..009d73118c 100644
--- a/src/bignum.c
+++ b/src/bignum.c
@@ -86,8 +86,8 @@ make_bignum_bits (size_t bits)
   if (integer_width < bits)
     overflow_error ();
 
-  struct Lisp_Bignum *b = ALLOCATE_PSEUDOVECTOR (struct Lisp_Bignum, value,
-						 PVEC_BIGNUM);
+  struct Lisp_Bignum *b = ALLOCATE_PLAIN_PSEUDOVECTOR (struct Lisp_Bignum,
+						       PVEC_BIGNUM);
   mpz_init (b->value);
   mpz_swap (b->value, mpz[0]);
   return make_lisp_ptr (b, Lisp_Vectorlike);
@@ -342,8 +342,8 @@ bignum_to_string (Lisp_Object num, int base)
 Lisp_Object
 make_bignum_str (char const *num, int base)
 {
-  struct Lisp_Bignum *b = ALLOCATE_PSEUDOVECTOR (struct Lisp_Bignum, value,
-						 PVEC_BIGNUM);
+  struct Lisp_Bignum *b = ALLOCATE_PLAIN_PSEUDOVECTOR (struct Lisp_Bignum,
+						       PVEC_BIGNUM);
   mpz_init (b->value);
   int check = mpz_set_str (b->value, num, base);
   eassert (check == 0);
diff --git a/src/buffer.h b/src/buffer.h
index 63b162161c..f42c3e97b9 100644
--- a/src/buffer.h
+++ b/src/buffer.h
@@ -741,8 +741,8 @@ struct buffer
      See `cursor-type' for other values.  */
   Lisp_Object cursor_in_non_selected_windows_;
 
-  /* No more Lisp_Object beyond this point.  Except undo_list,
-     which is handled specially in Fgarbage_collect.  */
+  /* No more Lisp_Object beyond cursor_in_non_selected_windows_.
+     Except undo_list, which is handled specially in Fgarbage_collect.  */
 
   /* This structure holds the coordinates of the buffer contents
      in ordinary buffers.  In indirect buffers, this is not used.  */
@@ -1019,14 +1019,12 @@ bset_width_table (struct buffer *b, Lisp_Object val)
    structure, make sure that this is still correct.  */
 
 #define BUFFER_LISP_SIZE						\
-  ((offsetof (struct buffer, own_text) - header_size) / word_size)
+  PSEUDOVECSIZE (struct buffer, cursor_in_non_selected_windows_)
 
-/* Size of the struct buffer part beyond leading Lisp_Objects, in word_size
-   units.  Rounding is needed for --with-wide-int configuration.  */
+/* Allocated size of the struct buffer part beyond leading
+   Lisp_Objects, in word_size units.  */
 
-#define BUFFER_REST_SIZE						\
-  ((((sizeof (struct buffer) - offsetof (struct buffer, own_text))	\
-     + (word_size - 1)) & ~(word_size - 1)) / word_size)
+#define BUFFER_REST_SIZE (VECSIZE (struct buffer) - BUFFER_LISP_SIZE)
 
 /* Initialize the pseudovector header of buffer object.  BUFFER_LISP_SIZE
    is required for GC, but BUFFER_REST_SIZE is set up just to be consistent
diff --git a/src/emacs-module.c b/src/emacs-module.c
index 2bb1062574..47ca3368c0 100644
--- a/src/emacs-module.c
+++ b/src/emacs-module.c
@@ -427,7 +427,7 @@ static struct Lisp_Module_Function *
 allocate_module_function (void)
 {
   return ALLOCATE_PSEUDOVECTOR (struct Lisp_Module_Function,
-                                min_arity, PVEC_MODULE_FUNCTION);
+                                documentation, PVEC_MODULE_FUNCTION);
 }
 
 #define XSET_MODULE_FUNCTION(var, ptr) \
diff --git a/src/fns.c b/src/fns.c
index b97b132b0f..c3202495da 100644
--- a/src/fns.c
+++ b/src/fns.c
@@ -3904,7 +3904,7 @@ static struct Lisp_Hash_Table *
 allocate_hash_table (void)
 {
   return ALLOCATE_PSEUDOVECTOR (struct Lisp_Hash_Table,
-				count, PVEC_HASH_TABLE);
+				index, PVEC_HASH_TABLE);
 }
 
 /* An upper bound on the size of a hash table index.  It must fit in
diff --git a/src/frame.c b/src/frame.c
index 6fdb7d0cbb..192ef4244f 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -798,7 +798,8 @@ adjust_frame_size (struct frame *f, int new_width, int new_height, int inhibit,
 static struct frame *
 allocate_frame (void)
 {
-  return ALLOCATE_ZEROED_PSEUDOVECTOR (struct frame, face_cache, PVEC_FRAME);
+  return ALLOCATE_ZEROED_PSEUDOVECTOR (struct frame, tool_bar_items,
+				       PVEC_FRAME);
 }
 
 struct frame *
diff --git a/src/frame.h b/src/frame.h
index ed62e7ace0..ec8f61465f 100644
--- a/src/frame.h
+++ b/src/frame.h
@@ -190,9 +190,6 @@ struct frame
   Lisp_Object current_tool_bar_string;
 #endif
 
-  /* Desired and current tool-bar items.  */
-  Lisp_Object tool_bar_items;
-
 #ifdef USE_GTK
   /* Where tool bar is, can be left, right, top or bottom.
      Except with GTK, the only supported position is `top'.  */
@@ -204,7 +201,9 @@ struct frame
   Lisp_Object font_data;
 #endif
 
-  /* Beyond here, there should be no more Lisp_Object components.  */
+  /* Desired and current tool-bar items.  */
+  Lisp_Object tool_bar_items;
+  /* tool_bar_items should be the last Lisp_Object member.  */
 
   /* Cache of realized faces.  */
   struct face_cache *face_cache;
diff --git a/src/lisp.h b/src/lisp.h
index a0a7cbdf51..681efc3b52 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -1904,9 +1904,9 @@ memclear (void *p, ptrdiff_t nbytes)
    at the end and we need to compute the number of Lisp_Object fields (the
    ones that the GC needs to trace).  */
 
-#define PSEUDOVECSIZE(type, nonlispfield)			\
-   (offsetof (type, nonlispfield) < header_size			\
-    ? 0 : (offsetof (type, nonlispfield) - header_size) / word_size)
+#define PSEUDOVECSIZE(type, lastlispfield)				\
+  (offsetof (type, lastlispfield) + word_size < header_size		\
+   ? 0 : (offsetof (type, lastlispfield) + word_size - header_size) / word_size)
 
 /* Compute A OP B, using the unsigned comparison operator OP.  A and B
    should be integer expressions.  This is not the same as
@@ -2109,11 +2109,14 @@ enum char_table_specials
     /* This is the number of slots that every char table must have.  This
        counts the ordinary slots and the top, defalt, parent, and purpose
        slots.  */
-    CHAR_TABLE_STANDARD_SLOTS = PSEUDOVECSIZE (struct Lisp_Char_Table, extras),
+    CHAR_TABLE_STANDARD_SLOTS
+      = (PSEUDOVECSIZE (struct Lisp_Char_Table, contents) - 1
+	 + (1 << CHARTAB_SIZE_BITS_0)),
 
-    /* This is an index of first Lisp_Object field in Lisp_Sub_Char_Table
+    /* This is the index of the first Lisp_Object field in Lisp_Sub_Char_Table
        when the latter is treated as an ordinary Lisp_Vector.  */
-    SUB_CHAR_TABLE_OFFSET = PSEUDOVECSIZE (struct Lisp_Sub_Char_Table, contents)
+    SUB_CHAR_TABLE_OFFSET
+      = PSEUDOVECSIZE (struct Lisp_Sub_Char_Table, contents) - 1
   };
 
 /* Sanity-check pseudovector layout.  */
@@ -2313,8 +2316,8 @@ struct Lisp_Hash_Table
      hash table size to reduce collisions.  */
   Lisp_Object index;
 
-  /* Only the fields above are traced normally by the GC.  The ones below
-     `count' are special and are either ignored by the GC or traced in
+  /* Only the fields above are traced normally by the GC.  The ones after
+     'index' are special and are either ignored by the GC or traced in
      a special way (e.g. because of weakness).  */
 
   /* Number of key/value entries in the table.  */
@@ -3940,6 +3943,11 @@ make_nil_vector (ptrdiff_t size)
 extern struct Lisp_Vector *allocate_pseudovector (int, int, int,
 						  enum pvec_type);
 
+/* Allocate uninitialized pseudovector with no Lisp_Object slots.  */
+
+#define ALLOCATE_PLAIN_PSEUDOVECTOR(type, tag) \
+  ((type *) allocate_pseudovector (VECSIZE (type), 0, 0, tag))
+
 /* Allocate partially initialized pseudovector where all Lisp_Object
    slots are set to Qnil but the rest (if any) is left uninitialized.  */
 
diff --git a/src/pdumper.c b/src/pdumper.c
index b19f206d1b..cb2915cb20 100644
--- a/src/pdumper.c
+++ b/src/pdumper.c
@@ -2702,7 +2702,7 @@ dump_hash_table (struct dump_context *ctx,
                  Lisp_Object object,
                  dump_off offset)
 {
-#if CHECK_STRUCTS && !defined (HASH_Lisp_Hash_Table_73C9BFB7D1)
+#if CHECK_STRUCTS && !defined HASH_Lisp_Hash_Table_EF95ED06FF
 # error "Lisp_Hash_Table changed. See CHECK_STRUCTS comment."
 #endif
   const struct Lisp_Hash_Table *hash_in = XHASH_TABLE (object);
@@ -2770,7 +2770,7 @@ dump_hash_table (struct dump_context *ctx,
 static dump_off
 dump_buffer (struct dump_context *ctx, const struct buffer *in_buffer)
 {
-#if CHECK_STRUCTS && !defined HASH_buffer_2CEE653E74
+#if CHECK_STRUCTS && !defined HASH_buffer_E34A11C6B9
 # error "buffer changed. See CHECK_STRUCTS comment."
 #endif
   struct buffer munged_buffer = *in_buffer;
diff --git a/src/process.c b/src/process.c
index 802ac02624..6770a5ed88 100644
--- a/src/process.c
+++ b/src/process.c
@@ -858,7 +858,8 @@ allocate_pty (char pty_name[PTY_NAME_SIZE])
 static struct Lisp_Process *
 allocate_process (void)
 {
-  return ALLOCATE_ZEROED_PSEUDOVECTOR (struct Lisp_Process, pid, PVEC_PROCESS);
+  return ALLOCATE_ZEROED_PSEUDOVECTOR (struct Lisp_Process, thread,
+				       PVEC_PROCESS);
 }
 
 static Lisp_Object
diff --git a/src/process.h b/src/process.h
index d66aa062a5..5e957c4298 100644
--- a/src/process.h
+++ b/src/process.h
@@ -117,9 +117,7 @@ struct Lisp_Process
 
     /* The thread a process is linked to, or nil for any thread.  */
     Lisp_Object thread;
-
-    /* After this point, there are no Lisp_Objects any more.  */
-    /* alloc.c assumes that `pid' is the first such non-Lisp slot.  */
+    /* After this point, there are no Lisp_Objects.  */
 
     /* Process ID.  A positive value is a child process ID.
        Zero is for pseudo-processes such as network or serial connections,
diff --git a/src/termhooks.h b/src/termhooks.h
index ca6782f461..a92b981110 100644
--- a/src/termhooks.h
+++ b/src/termhooks.h
@@ -408,7 +408,7 @@ struct terminal
      whether the mapping is available.  */
   Lisp_Object glyph_code_table;
 
-  /* All fields before `next_terminal' should be Lisp_Object and are traced
+  /* All earlier fields should be Lisp_Objects and are traced
      by the GC.  All fields afterwards are ignored by the GC.  */
 
   /* Chain of all terminal devices. */
diff --git a/src/terminal.c b/src/terminal.c
index 1d7a965dd2..0ee0121e35 100644
--- a/src/terminal.c
+++ b/src/terminal.c
@@ -264,8 +264,8 @@ get_named_terminal (const char *name)
 static struct terminal *
 allocate_terminal (void)
 {
-  return ALLOCATE_ZEROED_PSEUDOVECTOR
-    (struct terminal, next_terminal, PVEC_TERMINAL);
+  return ALLOCATE_ZEROED_PSEUDOVECTOR (struct terminal, glyph_code_table,
+				       PVEC_TERMINAL);
 }
 
 /* Create a new terminal object of TYPE and add it to the terminal list.  RIF
diff --git a/src/thread.c b/src/thread.c
index e51d614434..670680f2b0 100644
--- a/src/thread.c
+++ b/src/thread.c
@@ -267,7 +267,7 @@ informational only.  */)
   if (!NILP (name))
     CHECK_STRING (name);
 
-  mutex = ALLOCATE_PSEUDOVECTOR (struct Lisp_Mutex, mutex, PVEC_MUTEX);
+  mutex = ALLOCATE_PSEUDOVECTOR (struct Lisp_Mutex, name, PVEC_MUTEX);
   memset ((char *) mutex + offsetof (struct Lisp_Mutex, mutex),
 	  0, sizeof (struct Lisp_Mutex) - offsetof (struct Lisp_Mutex,
 						    mutex));
@@ -386,7 +386,7 @@ informational only.  */)
   if (!NILP (name))
     CHECK_STRING (name);
 
-  condvar = ALLOCATE_PSEUDOVECTOR (struct Lisp_CondVar, cond, PVEC_CONDVAR);
+  condvar = ALLOCATE_PSEUDOVECTOR (struct Lisp_CondVar, name, PVEC_CONDVAR);
   memset ((char *) condvar + offsetof (struct Lisp_CondVar, cond),
 	  0, sizeof (struct Lisp_CondVar) - offsetof (struct Lisp_CondVar,
 						      cond));
@@ -805,7 +805,7 @@ If NAME is given, it must be a string; it names the new thread.  */)
   if (!NILP (name))
     CHECK_STRING (name);
 
-  new_thread = ALLOCATE_PSEUDOVECTOR (struct thread_state, m_stack_bottom,
+  new_thread = ALLOCATE_PSEUDOVECTOR (struct thread_state, event_object,
 				      PVEC_THREAD);
   memset ((char *) new_thread + offset, 0,
 	  sizeof (struct thread_state) - offset);
@@ -1064,7 +1064,7 @@ static void
 init_main_thread (void)
 {
   main_thread.s.header.size
-    = PSEUDOVECSIZE (struct thread_state, m_stack_bottom);
+    = PSEUDOVECSIZE (struct thread_state, event_object);
   XSETPVECTYPE (&main_thread.s, PVEC_THREAD);
   main_thread.s.m_last_thing_searched = Qnil;
   main_thread.s.m_saved_last_thing_searched = Qnil;
diff --git a/src/thread.h b/src/thread.h
index 50f8f5cbe0..0514669a87 100644
--- a/src/thread.h
+++ b/src/thread.h
@@ -61,8 +61,8 @@ struct thread_state
   /* If we are waiting for some event, this holds the object we are
      waiting on.  */
   Lisp_Object event_object;
+  /* event_object must be the last Lisp field.  */
 
-  /* m_stack_bottom must be the first non-Lisp field.  */
   /* An address near the bottom of the stack.
      Tells GC how to save a copy of the stack.  */
   char const *m_stack_bottom;
diff --git a/src/w32term.c b/src/w32term.c
index 7dbeda7a71..bb1f0bad01 100644
--- a/src/w32term.c
+++ b/src/w32term.c
@@ -3896,7 +3896,7 @@ x_scroll_bar_create (struct window *w, int left, int top, int width, int height,
   HWND hwnd;
   SCROLLINFO si;
   struct scroll_bar *bar
-    = ALLOCATE_PSEUDOVECTOR (struct scroll_bar, top, PVEC_OTHER);
+    = ALLOCATE_PSEUDOVECTOR (struct scroll_bar, w32_widget_high, PVEC_OTHER);
   Lisp_Object barobj;
 
   block_input ();
diff --git a/src/window.c b/src/window.c
index be338c2af6..f911c0c7d4 100644
--- a/src/window.c
+++ b/src/window.c
@@ -4170,8 +4170,8 @@ temp_output_buffer_show (register Lisp_Object buf)
 static struct window *
 allocate_window (void)
 {
-  return ALLOCATE_ZEROED_PSEUDOVECTOR
-    (struct window, current_matrix, PVEC_WINDOW);
+  return ALLOCATE_ZEROED_PSEUDOVECTOR (struct window, mode_line_help_echo,
+				       PVEC_WINDOW);
 }
 
 /* Make new window, have it replace WINDOW in window-tree, and make
@@ -6710,7 +6710,8 @@ struct save_window_data
     Lisp_Object saved_windows;
 
     /* All fields above are traced by the GC.
-       From `frame-cols' down, the fields are ignored by the GC.  */
+       After saved_windows, the fields are ignored by the GC.  */
+
     /* We should be able to do without the following two.  */
     int frame_cols, frame_lines;
     /* These two should get eventually replaced by their pixel
@@ -7383,15 +7384,11 @@ redirection (see `redirect-frame-focus').  The variable
 saved by this function.  */)
   (Lisp_Object frame)
 {
-  Lisp_Object tem;
-  ptrdiff_t i, n_windows;
-  struct save_window_data *data;
   struct frame *f = decode_live_frame (frame);
-
-  n_windows = count_windows (XWINDOW (FRAME_ROOT_WINDOW (f)));
-  data = ALLOCATE_PSEUDOVECTOR (struct save_window_data, frame_cols,
-			       PVEC_WINDOW_CONFIGURATION);
-
+  ptrdiff_t n_windows = count_windows (XWINDOW (FRAME_ROOT_WINDOW (f)));
+  struct save_window_data *data
+    = ALLOCATE_PSEUDOVECTOR (struct save_window_data, saved_windows,
+			     PVEC_WINDOW_CONFIGURATION);
   data->frame_cols = FRAME_COLS (f);
   data->frame_lines = FRAME_LINES (f);
   data->frame_menu_bar_lines = FRAME_MENU_BAR_LINES (f);
@@ -7407,9 +7404,9 @@ saved by this function.  */)
   data->minibuf_selected_window = minibuf_level > 0 ? minibuf_selected_window : Qnil;
   data->root_window = FRAME_ROOT_WINDOW (f);
   data->focus_frame = FRAME_FOCUS_FRAME (f);
-  tem = make_uninit_vector (n_windows);
+  Lisp_Object tem = make_uninit_vector (n_windows);
   data->saved_windows = tem;
-  for (i = 0; i < n_windows; i++)
+  for (ptrdiff_t i = 0; i < n_windows; i++)
     ASET (tem, i, make_nil_vector (VECSIZE (struct saved_window)));
   save_window_save (FRAME_ROOT_WINDOW (f), XVECTOR (tem), 0);
   XSETWINDOW_CONFIGURATION (tem, data);
diff --git a/src/window.h b/src/window.h
index 4235a6eade..fdef407041 100644
--- a/src/window.h
+++ b/src/window.h
@@ -212,9 +212,8 @@ struct window
     /* The help echo text for this window.  Qnil if there's none.  */
     Lisp_Object mode_line_help_echo;
 
-    /* No Lisp data may follow below this point without changing
-       mark_object in alloc.c.  The member current_matrix must be the
-       first non-Lisp member.  */
+    /* No Lisp data may follow this point; mode_line_help_echo must be
+       the last Lisp member.  */
 
     /* Glyph matrices.  */
     struct glyph_matrix *current_matrix;
diff --git a/src/xterm.c b/src/xterm.c
index 2f830afe61..5aa3e3ff25 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -6611,8 +6611,8 @@ x_scroll_bar_create (struct window *w, int top, int left,
 		     int width, int height, bool horizontal)
 {
   struct frame *f = XFRAME (w->frame);
-  struct scroll_bar *bar
-    = ALLOCATE_PSEUDOVECTOR (struct scroll_bar, x_window, PVEC_OTHER);
+  struct scroll_bar *bar = ALLOCATE_PSEUDOVECTOR (struct scroll_bar, prev,
+						  PVEC_OTHER);
   Lisp_Object barobj;
 
   block_input ();
diff --git a/src/xterm.h b/src/xterm.h
index 972a10f4d4..c5ad38650c 100644
--- a/src/xterm.h
+++ b/src/xterm.h
@@ -897,7 +897,7 @@ struct scroll_bar
   /* The next and previous in the chain of scroll bars in this frame.  */
   Lisp_Object next, prev;
 
-  /* Fields from `x_window' down will not be traced by the GC.  */
+  /* Fields after 'prev' are not traced by the GC.  */
 
   /* The X window representing this scroll bar.  */
   Window x_window;
diff --git a/src/xwidget.c b/src/xwidget.c
index c56284928e..2486a2d4da 100644
--- a/src/xwidget.c
+++ b/src/xwidget.c
@@ -41,14 +41,13 @@ along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.  */
 static struct xwidget *
 allocate_xwidget (void)
 {
-  return ALLOCATE_PSEUDOVECTOR (struct xwidget, height, PVEC_XWIDGET);
+  return ALLOCATE_PSEUDOVECTOR (struct xwidget, script_callbacks, PVEC_XWIDGET);
 }
 
 static struct xwidget_view *
 allocate_xwidget_view (void)
 {
-  return ALLOCATE_PSEUDOVECTOR (struct xwidget_view, redisplayed,
-                                PVEC_XWIDGET_VIEW);
+  return ALLOCATE_PSEUDOVECTOR (struct xwidget_view, w, PVEC_XWIDGET_VIEW);
 }
 
 #define XSETXWIDGET(a, b) XSETPSEUDOVECTOR (a, b, PVEC_XWIDGET)
diff --git a/src/xwidget.h b/src/xwidget.h
index 8c598efb2e..1b6368daab 100644
--- a/src/xwidget.h
+++ b/src/xwidget.h
@@ -49,8 +49,7 @@ struct xwidget
 
   /* Vector of currently executing scripts with callbacks.  */
   Lisp_Object script_callbacks;
-
-  /* Here ends the Lisp part.  "height" is the marker field.  */
+  /* Here ends the Lisp part.  script_callbacks is the marker field.  */
 
   int height;
   int width;
@@ -68,8 +67,7 @@ struct xwidget_view
   union vectorlike_header header;
   Lisp_Object model;
   Lisp_Object w;
-
-  /* Here ends the lisp part.  "redisplayed" is the marker field.  */
+  /* Here ends the lisp part.  "w" is the marker field.  */
 
   /* If touched by redisplay.  */
   bool redisplayed;
-- 
2.20.1

Re: buffer.c/buffer.h: How to add new buffer-local variables?

[Michael Welsh Duggan]

I would suggest that there be a 

#define BUFFER_STRUCT_LAST_LISP_OBJECT cursor_in_non_selected_windows_

which would be properly commented and used in BUFFER_LISP_SIZE.  This
would, I think, make it easier to notice that this needs to change when
more items get added to the list.  I would also add a comment after the
cursor_in_non_selected_windows_ entry to the effect that the define
needs to be updated if more are added afterward.

Paul Eggert <eggert@cs.ucla.edu> writes:

> diff --git a/src/buffer.h b/src/buffer.h
> index 63b162161c..f42c3e97b9 100644
> --- a/src/buffer.h
> +++ b/src/buffer.h
> @@ -741,8 +741,8 @@ struct buffer
>       See `cursor-type' for other values.  */
>    Lisp_Object cursor_in_non_selected_windows_;
>  
> -  /* No more Lisp_Object beyond this point.  Except undo_list,
> -     which is handled specially in Fgarbage_collect.  */
> +  /* No more Lisp_Object beyond cursor_in_non_selected_windows_.
> +     Except undo_list, which is handled specially in Fgarbage_collect.  */
>  
>    /* This structure holds the coordinates of the buffer contents
>       in ordinary buffers.  In indirect buffers, this is not used.  */
> @@ -1019,14 +1019,12 @@ bset_width_table (struct buffer *b, Lisp_Object val)
>     structure, make sure that this is still correct.  */
>  
>  #define BUFFER_LISP_SIZE						\
> -  ((offsetof (struct buffer, own_text) - header_size) / word_size)
> +  PSEUDOVECSIZE (struct buffer, cursor_in_non_selected_windows_)
>  
> -/* Size of the struct buffer part beyond leading Lisp_Objects, in word_size
> -   units.  Rounding is needed for --with-wide-int configuration.  */
> +/* Allocated size of the struct buffer part beyond leading
> +   Lisp_Objects, in word_size units.  */
>  
> -#define BUFFER_REST_SIZE						\
> -  ((((sizeof (struct buffer) - offsetof (struct buffer, own_text))	\
> -     + (word_size - 1)) & ~(word_size - 1)) / word_size)
> +#define BUFFER_REST_SIZE (VECSIZE (struct buffer) - BUFFER_LISP_SIZE)
>  
>  /* Initialize the pseudovector header of buffer object.  BUFFER_LISP_SIZE
>     is required for GC, but BUFFER_REST_SIZE is set up just to be consistent

-- 
Michael Welsh Duggan
(md5i@md5i.com)

Re: buffer.c/buffer.h: How to add new buffer-local variables?

[Paul Eggert]

On 4/8/19 3:19 PM, Michael Welsh Duggan wrote:
> I would also add a comment after the
> cursor_in_non_selected_windows_ entry to the effect that the define
> needs to be updated if more are added afterward.

There's already a comment there to that effect, and similar comments
near similar boundaries in other structures. A patch to regularize
and/or clarify them would be welcome.

Not sure I want to add a macro, though, as it's one more thing to
maintain and it's not clear it's worth the hassle since one can get the
macro wrong too. (What I'd prefer is a static check of the rule, without
any macros.)

Re: buffer.c/buffer.h: How to add new buffer-local variables?

[Keith David Bershatsky]

Is it possible that a038df77de7b1aa2d73a6478493b8838b59e4982 broke the ability for Emacs to maintain the value of certain WINDOW Lisp_Object defined in window.h?

Here is a snippet from a modified window.h.  All of the WINDOW caches for the fake cursors are coming back as INVALID_LISP_OBJECT.  A test for NILP on those invalid lisp objects comes back TRUE, and subsequent functions fail when trying to process the object that reports as being "DEAD" in gdb printouts.  This happens on the NS and w32 ports of Emacs.  I have not tested X11, but I assume the result will be the same.

If it is highly unlikely that a038df77de7b1aa2d73a6478493b8838b59e4982 is the culprit, then I would imagine that the culprit is somewhere between March 28, 2019 and today.  If anyone would like me to start going back in time to test other comments between March 28, 2019 and today, please let me know ....

    /* An alist with parameters.  */
    Lisp_Object window_parameters;

    /* The help echo text for this window.  Qnil if there's none.  */
    Lisp_Object mode_line_help_echo;


/* *************************************************************************** */
/* MULTIPLE-CURSORS */

    Lisp_Object mc_temp_cache;

    /* The cache for multiple fake cursors. */
    Lisp_Object mc_cache;

    /* The cache for crosshairs. */
    Lisp_Object ch_cache;

    /* The cache for visible fill column. */
    Lisp_Object fc_cache;

    /* L.S.L. color vector used by crosshairs. */
    Lisp_Object ch_foreground;

    /* L.S.L. color vector used by visible fill column. */
    Lisp_Object fc_foreground;

/* *************************************************************************** */


    /* No Lisp data may follow this point; mode_line_help_echo must be
       the last Lisp member.  */

    /* Glyph matrices.  */
    struct glyph_matrix *current_matrix;
    struct glyph_matrix *desired_matrix;

Re: buffer.c/buffer.h: How to add new buffer-local variables?

[Keith David Bershatsky]

My bad ... I missed reading the new comment that "mode_line_help_echo must be the last Lisp member":

  /* No Lisp data may follow this point; mode_line_help_echo must be
     the last Lisp member.  */

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

> Date: [04-08-2019 17:40:33] <08 Apr 2019 17:40:33 -0700>
> From: Keith David Bershatsky <esq@lawlist.com>
> 
> Is it possible that a038df77de7b1aa2d73a6478493b8838b59e4982 broke the ability for Emacs to maintain the value of certain WINDOW Lisp_Object defined in window.h?
> 
> * * *

Re: buffer.c/buffer.h: How to add new buffer-local variables?

[Keith David Bershatsky]

I have successfully built Emacs (a038df77de7b1aa2d73a6478493b8838b59e4982) on the following platforms:

1.  w32:  Windows XP SP-3 (MinGW with ezwinports from a couple of years ago).

2.  X11:  Running on a machine with OSX 10.6.8.

3.  NS:  OSX 10.6.8.

All issues that I previously raised by virtue of this particular thread are now resolved.

Thank you very much to everyone for participating in this thread and working on these issues.

Greatly appreciated!!!

Keith

Re: buffer.c/buffer.h: How to add new buffer-local variables?

[Eli Zaretskii]

> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Mon, 8 Apr 2019 16:06:07 -0700
> 
> On 4/8/19 3:19 PM, Michael Welsh Duggan wrote:
> > I would also add a comment after the
> > cursor_in_non_selected_windows_ entry to the effect that the define
> > needs to be updated if more are added afterward.
> 
> There's already a comment there to that effect, and similar comments
> near similar boundaries in other structures. A patch to regularize
> and/or clarify them would be welcome.
> 
> Not sure I want to add a macro, though, as it's one more thing to
> maintain and it's not clear it's worth the hassle since one can get the
> macro wrong too.

I agree.  A macro doesn't necessarily tell what it's replaced with, so
when you read code which uses the macro, you many times need to look
up that macro to understand what it hides.  A comment doesn't have
this problem.