bug#29040: emacs-26 crash due to misaligned longjmp buffer in 64-bit MSYS2/MinGW-W64 build

bug#29040: emacs-26 crash due to misaligned longjmp buffer in 64-bit MSYS2/MinGW-W64 build

[Richard Copley]

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

When I build from the current emacs-26 branch with the current 64-bit
mingw-w64 toolchain from MSYS2, with optimization, Emacs sometimes
crashes with a segfault on typing C-g.

As far as I can tell, the current 64-bit pretest is not affected.

The attached file "servicelistpage.txt" helps to reproduce the crash.
It was created by the OP in this thread on help-gnu-emacs:

<http://lists.gnu.org/archive/html/help-gnu-emacs/2017-10/msg00089.html>

To reproduce the build with the 64-bit MinGW-W64 toolchain from MSYS2,

 * Save a backup of your MSYS2 installation, if it is in working order.
 * Update MSYS2.
 * In MSYS2 MINGW64 shell in the emacs repo:

git reset --hard 68182a47
git clean -xfd
./autogen.sh
./configure --with-modules --without-pop 'CFLAGS=-O1 -ggdb3'
make -j8 -O

Then, to reproduce the crash from "src/emacs -Q servicelistpage.txt":

 * Wait for the buffer to be displayed.
 * Type [C-g].

Partial GDB backtrace (full backtrace attached):

Thread 1 (Thread 5480.0x1e9c):
#0  0x00007ffaa1b693a0 in ntdll!RtlCaptureContext ()
   from C:\WINDOWS\SYSTEM32\ntdll.dll
No symbol table info available.
#1  0x00007ffaa1ad8f27 in ntdll!RtlUnwindEx ()
   from C:\WINDOWS\SYSTEM32\ntdll.dll
No symbol table info available.
#2  0x00007ffaa0671f4a in msvcrt!_setjmpex ()
   from C:\WINDOWS\System32\msvcrt.dll
No symbol table info available.
#3  0x00000004000b1a9a in quit_throw_to_read_char (
    from_signal=from_signal@entry=false) at keyboard.c:10548
No locals.

The faulting instruction in ntdll!RtlCaptureContext (frame #0) is

0x00007ffaa1b693a0 <+384>:   movaps 0x60(%rax),%xmm0

The memory operand should be 16-byte aligned but it is not. That is
the cause of the segfault. I think the following extract from the GDB
session shows the problem. The value in %rax is 0x4005CDAE8 (not
16-byte aligned).

(gdb) p $rax
$1 = 17185954536
(gdb) up
#1  0x00007ffaa1ad8f27 in ntdll!RtlUnwindEx ()
   from C:\WINDOWS\SYSTEM32\ntdll.dll
(gdb) up
#2  0x00007ffaa0671f4a in msvcrt!_setjmpex ()
   from C:\WINDOWS\System32\msvcrt.dll
(gdb) up
#3  0x00000004000b1a9a in quit_throw_to_read_char (
    from_signal=from_signal@entry=false) at keyboard.c:10548
10548     sys_longjmp (getcjmp, 1);
(gdb) p &getcjmp
$2 = (sys_jmp_buf *) 0x4005cdae8 <main_thread+224>

In the help-gnu-emacs thread, Eli said:

  [...] we should ask the MinGW64 developers for advice about using
  longjmp. Most probably, something in that area has changed in recent
  releases of their runtime [...]

Eli, my apologies, but I don't think I understand the issues well
enough to have a productive discussion with the MinGW-W64 developers
myself.

[-- Attachment #2: servicelistpage-1.txt --]
[-- Type: text/plain, Size: 28 bytes --]

"hello":""

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

$ gdb --quiet -ex run --args src/emacs.exe -Q "C:\Users\buster\AppData\Local\Temp\servicelistpage-1.txt"
Reading symbols from src/emacs.exe...done.
Starting program: C:\projects\emacs\src\emacs.exe -Q C:\Users\buster\AppData\Local\Temp\servicelistpage-1.txt
[New Thread 5480.0x1e9c]
[New Thread 5480.0x3bcc]
[New Thread 5480.0x1e04]
[New Thread 5480.0x38a4]
[New Thread 5480.0x28a0]
[New Thread 5480.0x2a2c]

Thread 1 received signal SIGSEGV, Segmentation fault.
0x00007ffaa1b693a0 in ntdll!RtlCaptureContext ()
   from C:\WINDOWS\SYSTEM32\ntdll.dll
(gdb) thread apply all bt full

Thread 6 (Thread 5480.0x2a2c):
#0  0x00007ffa9f001144 in win32u!NtUserGetMessage ()
   from C:\WINDOWS\System32\win32u.dll
No symbol table info available.
#1  0x00007ffaa1862dd6 in USER32!GetMessageW ()
   from C:\WINDOWS\System32\user32.dll
No symbol table info available.
#2  0x000000040017b2d7 in w32_msg_pump (msg_buf=msg_buf@entry=0x4bafec0)
    at w32fns.c:3255
        msg = {
          hwnd = 0x0,
          message = 0,
          wParam = 0,
          lParam = 0,
          time = 1213933250,
          pt = {
            x = 1136,
            y = 1066
          }
        }
        result = <optimized out>
        focus_window = <optimized out>
#3  0x000000040017b389 in w32_msg_worker (arg=<optimized out>)
    at w32fns.c:3478
        msg = {
          hwnd = 0x0,
          message = 0,
          wParam = 0,
          lParam = 0,
          time = 0,
          pt = {
            x = 0,
            y = 0
          }
        }
        dummy_buf = {
          next = 0x0,
          w32msg = {
            msg = {
              hwnd = 0x0,
              message = 0,
              wParam = 0,
              lParam = 0,
              time = 0,
              pt = {
                x = 0,
                y = 0
              }
            },
            dwModifiers = 0,
            rect = {
              left = 0,
              top = 0,
              right = 0,
              bottom = 0
            }
          },
          result = 0,
          completed = 0
        }
#4  0x00007ffaa0da2774 in KERNEL32!BaseThreadInitThunk ()
   from C:\WINDOWS\System32\kernel32.dll
No symbol table info available.
#5  0x00007ffaa1b30d51 in ntdll!RtlUserThreadStart ()
   from C:\WINDOWS\SYSTEM32\ntdll.dll
No symbol table info available.
#6  0x0000000000000000 in ?? ()
No symbol table info available.
Backtrace stopped: previous frame inner to this frame (corrupt stack?)

Thread 5 (Thread 5480.0x28a0):
#0  0x00007ffaa1b65a24 in ntdll!ZwDelayExecution ()
   from C:\WINDOWS\SYSTEM32\ntdll.dll
No symbol table info available.
#1  0x00007ffa9e037287 in SleepEx () from C:\WINDOWS\System32\KernelBase.dll
No symbol table info available.
#2  0x00000004001a89c2 in timer_loop (arg=0x401adfd60 <real_itimer>)
    at w32proc.c:397
        sleep_time = <optimized out>
        handler = <optimized out>
        now = <optimized out>
        expire = <optimized out>
        reload = <optimized out>
        itimer = 0x401adfd60 <real_itimer>
        which = <optimized out>
        crit = 0x401adfca0 <crit_prof>
        hth = 0x0
#3  0x00007ffaa0da2774 in KERNEL32!BaseThreadInitThunk ()
   from C:\WINDOWS\System32\kernel32.dll
No symbol table info available.
#4  0x00007ffaa1b30d51 in ntdll!RtlUserThreadStart ()
   from C:\WINDOWS\SYSTEM32\ntdll.dll
No symbol table info available.
#5  0x0000000000000000 in ?? ()
No symbol table info available.
Backtrace stopped: previous frame inner to this frame (corrupt stack?)

Thread 4 (Thread 5480.0x38a4):
#0  0x00007ffaa1b68c34 in ntdll!ZwWaitForWorkViaWorkerFactory ()
   from C:\WINDOWS\SYSTEM32\ntdll.dll
No symbol table info available.
#1  0x00007ffaa1b01553 in ntdll!TpReleaseWork ()
   from C:\WINDOWS\SYSTEM32\ntdll.dll
No symbol table info available.
#2  0x00007ffaa0da2774 in KERNEL32!BaseThreadInitThunk ()
   from C:\WINDOWS\System32\kernel32.dll
No symbol table info available.
#3  0x00007ffaa1b30d51 in ntdll!RtlUserThreadStart ()
   from C:\WINDOWS\SYSTEM32\ntdll.dll
No symbol table info available.
#4  0x0000000000000000 in ?? ()
No symbol table info available.
Backtrace stopped: previous frame inner to this frame (corrupt stack?)

Thread 3 (Thread 5480.0x1e04):
#0  0x00007ffaa1b68c34 in ntdll!ZwWaitForWorkViaWorkerFactory ()
   from C:\WINDOWS\SYSTEM32\ntdll.dll
No symbol table info available.
#1  0x00007ffaa1b01553 in ntdll!TpReleaseWork ()
   from C:\WINDOWS\SYSTEM32\ntdll.dll
No symbol table info available.
#2  0x00007ffaa0da2774 in KERNEL32!BaseThreadInitThunk ()
   from C:\WINDOWS\System32\kernel32.dll
No symbol table info available.
#3  0x00007ffaa1b30d51 in ntdll!RtlUserThreadStart ()
   from C:\WINDOWS\SYSTEM32\ntdll.dll
No symbol table info available.
#4  0x0000000000000000 in ?? ()
No symbol table info available.
Backtrace stopped: previous frame inner to this frame (corrupt stack?)

Thread 2 (Thread 5480.0x3bcc):
#0  0x00007ffaa1b68c34 in ntdll!ZwWaitForWorkViaWorkerFactory ()
   from C:\WINDOWS\SYSTEM32\ntdll.dll
No symbol table info available.
#1  0x00007ffaa1b01553 in ntdll!TpReleaseWork ()
   from C:\WINDOWS\SYSTEM32\ntdll.dll
No symbol table info available.
#2  0x00007ffaa0da2774 in KERNEL32!BaseThreadInitThunk ()
   from C:\WINDOWS\System32\kernel32.dll
No symbol table info available.
#3  0x00007ffaa1b30d51 in ntdll!RtlUserThreadStart ()
   from C:\WINDOWS\SYSTEM32\ntdll.dll
No symbol table info available.
#4  0x0000000000000000 in ?? ()
No symbol table info available.
Backtrace stopped: previous frame inner to this frame (corrupt stack?)

Thread 1 (Thread 5480.0x1e9c):
#0  0x00007ffaa1b693a0 in ntdll!RtlCaptureContext ()
   from C:\WINDOWS\SYSTEM32\ntdll.dll
No symbol table info available.
#1  0x00007ffaa1ad8f27 in ntdll!RtlUnwindEx ()
   from C:\WINDOWS\SYSTEM32\ntdll.dll
No symbol table info available.
#2  0x00007ffaa0671f4a in msvcrt!_setjmpex ()
   from C:\WINDOWS\System32\msvcrt.dll
No symbol table info available.
#3  0x00000004000b1a9a in quit_throw_to_read_char (
    from_signal=from_signal@entry=false) at keyboard.c:10548
No locals.
#4  0x00000004000b3db5 in kbd_buffer_get_event (end_time=0x0,
    used_mouse_menu=0x0, kbp=<synthetic pointer>) at keyboard.c:3790
        obj = <optimized out>
#5  read_event_from_main_queue (used_mouse_menu=0x0, local_getcjmp=0xbff258,
    end_time=0x0) at keyboard.c:2151
        c = {
          i = 0
        }
        save_jump = {{
            Part = {0, 0}
          } <repeats 16 times>}
        kb = <optimized out>
#6  read_decoded_event_from_main_queue (used_mouse_menu=0x0, prev_event=...,
    local_getcjmp=0xbff258, end_time=0x0) at keyboard.c:2214
No locals.
#7  read_char (commandflag=120, commandflag@entry=1, map=..., map@entry=...,
    prev_event=..., used_mouse_menu=0x0, used_mouse_menu@entry=0xbff44b,
    end_time=end_time@entry=0x0) at keyboard.c:2802
        c = <optimized out>
        jmpcount = 0
        local_getcjmp = {{
            Part = {12579624, 17185452969}
          }, {
            Part = {12578304, 12579624}
          }, {
            Part = {17185461184, 17185861600}
          }, {
            Part = {534528, 3}
          }, {
            Part = {0, 0}
          }, {
            Part = {17180601996, 3843995738016}
          }, {
            Part = {0, 0}
          }, {
            Part = {0, 0}
          }, {
            Part = {0, 0}
          }, {
            Part = {0, 0}
          }, {
            Part = {0, 0}
          }, {
            Part = {0, 0}
          }, {
            Part = {0, 0}
          }, {
            Part = {0, 0}
          }, {
            Part = {0, 0}
          }, {
            Part = {0, 0}
          }}
        save_jump = {{
            Part = {17185907520, 17185869328}
          }, {
            Part = {61768, 80389168}
          }, {
            Part = {2, 17185131016}
          }, {
            Part = {12579296, 0}
          }, {
            Part = {0, 17181005805}
          }, {
            Part = {0, 17185907520}
          }, {
            Part = {35840, 17188240544}
          }, {
            Part = {61768, 120}
          }, {
            Part = {17185131016, 12579296}
          }, {
            Part = {0, 17181009037}
          }, {
            Part = {0, 80389168}
          }, {
            Part = {1, 15}
          }, {
            Part = {5, 17188880288}
          }, {
            Part = {3002368, 17180939646}
          }, {
            Part = {0, 80389168}
          }, {
            Part = {0, 15}
          }}
        tem = <optimized out>
        save = <optimized out>
        previous_echo_area_message = {
          i = 0
        }
        also_record = {
          i = 0
        }
        reread = false
        recorded = false
        polling_stopped_here = true
        orig_kboard = 0x1
#8  0x00000004000b61a3 in read_key_sequence (keybuf=keybuf@entry=0xbff580,
    bufsize=bufsize@entry=30, prompt=..., prompt@entry=...,
    dont_downcase_last=dont_downcase_last@entry=false,
    can_return_switch_frame=can_return_switch_frame@entry=true,
    fix_current_buffer=fix_current_buffer@entry=true,
    prevent_redisplay=prevent_redisplay@entry=false) at keyboard.c:9147
        interrupted_kboard = 0x41c4c50
        interrupted_frame = 0x400a3fa20 <dumped_data+4594080>
        key = <optimized out>
        used_mouse_menu = false
        echo_local_start = 0
        last_real_key_start = 0
        keys_local_start = <optimized out>
        new_binding = <optimized out>
        t = 0
        echo_start = 0
        keys_start = 0
        current_binding = {
          i = 17188072051
        }
        first_event = {
          i = 0
        }
        first_unbound = 31
        mock_input = 0
        fkey = {
          parent = {
            i = 17187443171
          },
          map = {
            i = 17187443171
          },
          start = 0,
          end = 0
        }
        keytran = {
          parent = {
            i = 17186169491
          },
          map = {
            i = 17186169491
          },
          start = 0,
          end = 0
        }
        indec = {
          parent = {
            i = 17187443187
          },
          map = {
            i = 17187443187
          },
          start = 0,
          end = 0
        }
        shift_translated = false
        delayed_switch_frame = {
          i = 0
        }
        original_uppercase = {
          i = 0
        }
        original_uppercase_position = -1
        dummyflag = false
        starting_buffer = 0x4caa430
        fake_prefixed_keys = {
          i = 0
        }
#9  0x00000004000b6ee3 in command_loop_1 () at keyboard.c:1368
        cmd = <optimized out>
        keybuf = {{
            i = 0
          }, {
            i = 17181005805
          }, {
            i = 21392506257408078
          }, {
            i = 17185908864
          }, {
            i = 37184
          }, {
            i = 17188240544
          }, {
            i = 0
          }, {
            i = 120
          }, {
            i = 17185131016
          }, {
            i = 12580352
          }, {
            i = 17186479395
          }, {
            i = 17181009037
          }, {
            i = 1460728
          }, {
            i = 0
          }, {
            i = 17185861600
          }, {
            i = 1
          }, {
            i = 5
          }, {
            i = 37184
          }, {
            i = 0
          }, {
            i = 17186190821
          }, {
            i = 0
          }, {
            i = 17185861600
          }, {
            i = 3
          }, {
            i = 17187765203
          }, {
            i = 17185861600
          }, {
            i = 12580608
          }, {
            i = 0
          }, {
            i = 17181024997
          }, {
            i = 17185861600
          }, {
            i = 64312368
          }}
        i = <optimized out>
        prev_modiff = 0
        prev_buffer = 0x0
#10 0x0000000400115d7a in internal_condition_case (
    bfun=bfun@entry=0x4000b6aec <command_loop_1>, handlers=...,
    handlers@entry=..., hfun=hfun@entry=0x4000ad1bb <cmd_error>)
    at eval.c:1332
        val = {
          i = 17185954536
        }
        c = 0x3d55430
#11 0x00000004000a9a87 in command_loop_2 (ignore=...) at keyboard.c:1110
        val = <optimized out>
#12 0x0000000400115ceb in internal_catch (tag=..., tag@entry=...,
    func=func@entry=0x4000a9a63 <command_loop_2>, arg=arg@entry=...)
    at eval.c:1097
        val = {
          i = 17185954536
        }
        c = 0x3d552a0
#13 0x00000004000a99b4 in command_loop () at keyboard.c:1089
No locals.
#14 0x0000000000000000 in ?? ()
No symbol table info available.
Backtrace stopped: previous frame inner to this frame (corrupt stack?)
(gdb)

bug#29040: emacs-26 crash due to misaligned longjmp buffer in 64-bit MSYS2/MinGW-W64 build

[Eli Zaretskii]

> From: Richard Copley <rcopley@gmail.com>
> Date: Sat, 28 Oct 2017 14:40:41 +0100
> 
> In the help-gnu-emacs thread, Eli said:
> 
>   [...] we should ask the MinGW64 developers for advice about using
>   longjmp. Most probably, something in that area has changed in recent
>   releases of their runtime [...]
> 
> Eli, my apologies, but I don't think I understand the issues well
> enough to have a productive discussion with the MinGW-W64 developers
> myself.

Just ask them to read this bug report and ask questions if they don't
understand the issue well enough.

Thanks.

bug#29040: Trouble with misaligned jmp_buf in 64-bit MinGW-64 runtime, in Emacs 26

[Richard Copley]

Hello MinGW-W64,

I'm writing on behalf of the Emacs developers. They are keen to have
your advice on the proper way to call setjmp with the current version
of the 64-bit MinGW-W64 runtime.

I tried to summarise the current issue in Emacs bug #29040. Would you
mind having a look at the report, please?

<https://debbugs.gnu.org/cgi/bugreport.cgi?bug=29040>

I have CC'd the bug's email address on the Emacs bug tracker.

Warm regards,
Richard.

bug#29040: Emacs 25 hangs on windows arbitrarily during search of a unicode file

[Paul Eggert]

> +#define THREAD_ALIGNMENT COMMON_MULTIPLE (alignof (max_align_t), GCALIGNMENT)
> +
> +static struct thread_state alignas (THREAD_ALIGNMENT) main_thread;

Three questions:

The Gnulib manual 
<https://www.gnu.org/software/gnulib/manual/html_node/stdalign_002eh.html> says, 
"Some compilers require the operand of _Alignas/alignas to be a single integer 
constant, not an expression: MSVC 7.0 through at least 10.0." (Also, 
<https://stackoverflow.com/questions/7895869/cross-platform-alignx-macro> 
suggests that this issue may also occur with VS2013, though VS2015 should be OK 
since it supports alignas.) Are any of these older compilers of concern when 
building Emacs? If so, the above change won't work for them.

Why does the above code use 'alignof (max_align_t)' rather than 'alignof (struct 
thread_state)'? Doesn't the latter suffice?

C11 requires that a compiler must issue a diagnostic for a declaration like 
'struct thread_state alignas (8) foo;' if the alignment of plain 'struct state' 
is greater than 8. Can I take it from this bug report that the Microsoft 
compilers do not conform to that part of the standard? If so, we should add 
checks to verify this requirement where it is assumed elsewhere.

bug#29040: Emacs 25 hangs on windows arbitrarily during search of a unicode file

[Richard Copley]

On 2 November 2017 at 06:02, Paul Eggert <eggert@cs.ucla.edu> wrote:
>> +#define THREAD_ALIGNMENT COMMON_MULTIPLE (alignof (max_align_t),
>> GCALIGNMENT)
>> +
>> +static struct thread_state alignas (THREAD_ALIGNMENT) main_thread;
>
> Three questions:
>
> The Gnulib manual
> <https://www.gnu.org/software/gnulib/manual/html_node/stdalign_002eh.html>
> says, "Some compilers require the operand of _Alignas/alignas to be a single
> integer constant, not an expression: MSVC 7.0 through at least 10.0." (Also,
> <https://stackoverflow.com/questions/7895869/cross-platform-alignx-macro>
> suggests that this issue may also occur with VS2013, though VS2015 should be
> OK since it supports alignas.) Are any of these older compilers of concern
> when building Emacs?

I don't think so. How would one go about building Emacs with MSVC anyway?

>If so, the above change won't work for them.

> Why does the above code use 'alignof (max_align_t)' rather than 'alignof
> (struct thread_state)'? Doesn't the latter suffice?

For information, in the 64-bit GCC 7.2 toolchain from MinGW-W64, the two
are equal in value.

> C11 requires that a compiler must issue a diagnostic for a declaration like
> 'struct thread_state alignas (8) foo;' if the alignment of plain 'struct
> state' is greater than 8. Can I take it from this bug report that the
> Microsoft compilers do not conform to that part of the standard?

Microsoft compilers weren't mentioned at all. GCC didn't issue
a diagnostic.

> If so, we should add checks to verify this requirement where it is assumed elsewhere.

I think that is a good idea.

Thanks.

bug#29040: Emacs 25 hangs on windows arbitrarily during search of a unicode file

[Noam Postavsky]

Richard Copley <rcopley@gmail.com> writes:

> I don't think so. How would one go about building Emacs with MSVC anyway?

AFAIK, it used to be possible via nt/config.bat, but we stopped
supporting it in 24.4 and removed the code for it entirely in 25.1.

bug#29040: Emacs 25 hangs on windows arbitrarily during search of a unicode file

[Eli Zaretskii]

> Cc: rcopley@gmail.com, 29040@debbugs.gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Wed, 1 Nov 2017 23:02:39 -0700
> 
> > +#define THREAD_ALIGNMENT COMMON_MULTIPLE (alignof (max_align_t), GCALIGNMENT)
> > +
> > +static struct thread_state alignas (THREAD_ALIGNMENT) main_thread;
> 
> Three questions:
> 
> The Gnulib manual 
> <https://www.gnu.org/software/gnulib/manual/html_node/stdalign_002eh.html> says, 
> "Some compilers require the operand of _Alignas/alignas to be a single integer 
> constant, not an expression: MSVC 7.0 through at least 10.0." (Also, 
> <https://stackoverflow.com/questions/7895869/cross-platform-alignx-macro> 
> suggests that this issue may also occur with VS2013, though VS2015 should be OK 
> since it supports alignas.) Are any of these older compilers of concern when 
> building Emacs? If so, the above change won't work for them.

We've stopped supporting MSVC long ago.  Only GCC is supported for
building on MS-Windows.

> Why does the above code use 'alignof (max_align_t)' rather than 'alignof (struct 
> thread_state)'? Doesn't the latter suffice?

No special reason, I just did the same as you did in aligning
pseudovectors, when we had a similar problem the last time.  Feel free
to change if you think it's better.

> C11 requires that a compiler must issue a diagnostic for a declaration like 
> 'struct thread_state alignas (8) foo;' if the alignment of plain 'struct state' 
> is greater than 8. Can I take it from this bug report that the Microsoft 
> compilers do not conform to that part of the standard? If so, we should add 
> checks to verify this requirement where it is assumed elsewhere.

Again, this is GCC, so what MS compilers do is not interesting.

The problem was caused by your changes in 015f0bb, which were for
OpenVMS:

  diff --git a/src/thread.c b/src/thread.c
  index c03cdda..6f12d79 100644
  --- a/src/thread.c
  +++ b/src/thread.c
  @@ -26,7 +26,7 @@ along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.  */
   #include "coding.h"
   #include "syssignal.h"

  -static struct thread_state main_thread;
  +static struct thread_state alignas (GCALIGNMENT) main_thread;

   struct thread_state *current_thread = &main_thread;

This downgraded the alignment on 64-bit MS-Windows hosts, so I
upgraded it back.  If using alignas with an argument > 8 is
non-portable, then I guess we will have to make your OpenVMS fix
conditional on just that platform.  Or do you see a cleaner fix?

bug#29040: Emacs 25 hangs on windows arbitrarily during search of a unicode file

[Paul Eggert]

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

On 11/02/2017 08:50 AM, Eli Zaretskii wrote:
> do you see a cleaner fix?

Yes, we can stop using alignas entirely, since it doesn't work the way I 
expected. I thought that it could only increase alignment, and that it 
was a no-op if it specified a decreased alignment: this is how 
__attribute__ ((aligned (8))) works. However, I now see that C11 says 
that a compiler is supposed to report an error if alignas specifies a 
decreased alignment. So I installed the attached patch to stop using 
alignas.

> We've stopped supporting MSVC long ago.
OK. Can we then simplify the source a little bit, in the 'master' 
branch, as a low-priority task? I was thinking of something like this:

* Remove my_endbss_static in lastfile.c, since we no longer need to 
worry about the Alpha MSVC linker.

* Remove the the _MSC_VER-specific code in lisp.h's definitions of 
ENUM_BF and DEFUN and in regex.c's definition of re_char and const_re_char.


[-- Attachment #2: 0001-Fix-alignment-portability-problems.patch --]
[-- Type: text/x-patch, Size: 6661 bytes --]

From f8e2d438bfd4923b30ff878e5834e1d8e4061397 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Thu, 2 Nov 2017 13:06:38 -0700
Subject: [PATCH] Fix alignment portability problems

Do not assume that the natural alignment of Lisp objects is a
multiple of GCALIGNMENT.  This improves on the portability of the
recent fix for Bug#29040.
* lib-src/make-docfile.c (close_emacs_globals):
* src/buffer.c (buffer_defaults, buffer_local_symbols):
* src/lisp.h (DEFUN):
* src/thread.c (main_thread):
Use GCALIGNED, not alignas (GCALIGNMENT).
* src/alloc.c (COMMON_MULTIPLE):
Move back here from lisp.h, since it is no longer used elsewhere.
* src/lisp.h (GCALIGNMENT): No longer a macro, since we need not
worry about MSVC.  Omit no-longer-needed consistency check.
* src/thread.c (THREAD_ALIGNMENT): Remove.
---
 lib-src/make-docfile.c |  2 +-
 src/alloc.c            |  6 ++++++
 src/buffer.c           |  4 ++--
 src/lisp.h             | 25 ++++++++-----------------
 src/thread.c           |  4 +---
 5 files changed, 18 insertions(+), 23 deletions(-)

diff --git a/lib-src/make-docfile.c b/lib-src/make-docfile.c
index 69c7f37a17..0ea3f7b6b6 100644
--- a/lib-src/make-docfile.c
+++ b/lib-src/make-docfile.c
@@ -668,7 +668,7 @@ close_emacs_globals (ptrdiff_t num_symbols)
 	   "extern\n"
 	   "#endif\n"
 	   "struct {\n"
-	   "  struct Lisp_Symbol alignas (GCALIGNMENT) s;\n"
+	   "  struct Lisp_Symbol GCALIGNED s;\n"
 	   "} lispsym[%td];\n"),
 	  num_symbols);
 }
diff --git a/src/alloc.c b/src/alloc.c
index ff93956109..0fc79fe68a 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -621,6 +621,12 @@ buffer_memory_full (ptrdiff_t nbytes)
 #endif
 }
 
+/* A common multiple of the positive integers A and B.  Ideally this
+   would be the least common multiple, but there's no way to do that
+   as a constant expression in C, so do the best that we can easily do.  */
+#define COMMON_MULTIPLE(a, b) \
+  ((a) % (b) == 0 ? (a) : (b) % (a) == 0 ? (b) : (a) * (b))
+
 #ifndef XMALLOC_OVERRUN_CHECK
 #define XMALLOC_OVERRUN_CHECK_OVERHEAD 0
 #else
diff --git a/src/buffer.c b/src/buffer.c
index 9635733fcf..15735a298a 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -61,7 +61,7 @@ struct buffer *all_buffers;
    Setting the default value also goes through the alist of buffers
    and stores into each buffer that does not say it has a local value.  */
 
-struct buffer alignas (GCALIGNMENT) buffer_defaults;
+struct buffer GCALIGNED buffer_defaults;
 
 /* This structure marks which slots in a buffer have corresponding
    default values in buffer_defaults.
@@ -84,7 +84,7 @@ struct buffer buffer_local_flags;
 /* This structure holds the names of symbols whose values may be
    buffer-local.  It is indexed and accessed in the same way as the above.  */
 
-struct buffer alignas (GCALIGNMENT) buffer_local_symbols;
+struct buffer GCALIGNED buffer_local_symbols;
 
 /* Return the symbol of the per-buffer variable at offset OFFSET in
    the buffer structure.  */
diff --git a/src/lisp.h b/src/lisp.h
index 43b3ec618f..bf9db591bd 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -228,14 +228,12 @@ extern bool suppress_checking EXTERNALLY_VISIBLE;
 
    USE_LSB_TAG not only requires the least 3 bits of pointers returned by
    malloc to be 0 but also needs to be able to impose a mult-of-8 alignment
-   on the few static Lisp_Objects used: lispsym, all the defsubr, and
-   the two special buffers buffer_defaults and buffer_local_symbols.  */
+   on the few static Lisp_Objects used, all of which are aligned via
+   the GCALIGN macro defined below.  */
 
 enum Lisp_Bits
   {
-    /* 2**GCTYPEBITS.  This must be a macro that expands to a literal
-       integer constant, for MSVC.  */
-#define GCALIGNMENT 8
+    GCALIGNMENT = 1 << GCTYPEBITS,
 
     /* Number of bits in a Lisp_Object value, not counting the tag.  */
     VALBITS = EMACS_INT_WIDTH - GCTYPEBITS,
@@ -247,10 +245,6 @@ enum Lisp_Bits
     FIXNUM_BITS = VALBITS + 1
   };
 
-#if GCALIGNMENT != 1 << GCTYPEBITS
-# error "GCALIGNMENT and GCTYPEBITS are inconsistent"
-#endif
-
 /* The maximum value that can be stored in a EMACS_INT, assuming all
    bits other than the type bits contribute to a nonnegative signed value.
    This can be used in #if, e.g., '#if USE_LSB_TAG' below expands to an
@@ -277,18 +271,15 @@ DEFINE_GDB_SYMBOL_END (VALMASK)
 error !;
 #endif
 
+/* Declare an object to have an address that is a multiple of
+   GCALIGNMENT.  alignas is not suitable here, as it fails if the
+   object's natural alignment exceeds GCALIGNMENT.  */
 #ifdef HAVE_STRUCT_ATTRIBUTE_ALIGNED
 # define GCALIGNED __attribute__ ((aligned (GCALIGNMENT)))
 #else
 # define GCALIGNED /* empty */
 #endif
 
-/* A common multiple of the positive integers A and B.  Ideally this
-   would be the least common multiple, but there's no way to do that
-   as a constant expression in C, so do the best that we can easily do.  */
-#define COMMON_MULTIPLE(a, b) \
-  ((a) % (b) == 0 ? (a) : (b) % (a) == 0 ? (b) : (a) * (b))
-
 /* Some operations are so commonly executed that they are implemented
    as macros, not functions, because otherwise runtime performance would
    suffer too much when compiling with GCC without optimization.
@@ -2946,7 +2937,7 @@ CHECK_NUMBER_CDR (Lisp_Object x)
 #ifdef _MSC_VER
 #define DEFUN(lname, fnname, sname, minargs, maxargs, intspec, doc)	\
    Lisp_Object fnname DEFUN_ARGS_ ## maxargs ;				\
-   static struct Lisp_Subr alignas (GCALIGNMENT) sname =		\
+   static struct Lisp_Subr GCALIGNED sname =				\
    { { (PVEC_SUBR << PSEUDOVECTOR_AREA_BITS)				\
        | (sizeof (struct Lisp_Subr) / sizeof (EMACS_INT)) },		\
       { (Lisp_Object (__cdecl *)(void))fnname },                        \
@@ -2954,7 +2945,7 @@ CHECK_NUMBER_CDR (Lisp_Object x)
    Lisp_Object fnname
 #else  /* not _MSC_VER */
 #define DEFUN(lname, fnname, sname, minargs, maxargs, intspec, doc)	\
-   static struct Lisp_Subr alignas (GCALIGNMENT) sname =		\
+   static struct Lisp_Subr GCALIGNED sname =				\
      { { PVEC_SUBR << PSEUDOVECTOR_AREA_BITS },				\
        { .a ## maxargs = fnname },					\
        minargs, maxargs, lname, intspec, 0};				\
diff --git a/src/thread.c b/src/thread.c
index 7a670ba410..03f5b31855 100644
--- a/src/thread.c
+++ b/src/thread.c
@@ -26,9 +26,7 @@ along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.  */
 #include "coding.h"
 #include "syssignal.h"
 
-#define THREAD_ALIGNMENT COMMON_MULTIPLE (alignof (max_align_t), GCALIGNMENT)
-
-static struct thread_state alignas (THREAD_ALIGNMENT) main_thread;
+static struct thread_state GCALIGNED main_thread;
 
 struct thread_state *current_thread = &main_thread;
 
-- 
2.13.6

bug#29040: Emacs 25 hangs on windows arbitrarily during search of a unicode file

[Eli Zaretskii]

> Cc: rcopley@gmail.com, 29040@debbugs.gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Thu, 2 Nov 2017 13:35:16 -0700
> 
> Yes, we can stop using alignas entirely, since it doesn't work the way I 
> expected. I thought that it could only increase alignment, and that it 
> was a no-op if it specified a decreased alignment: this is how 
> __attribute__ ((aligned (8))) works. However, I now see that C11 says 
> that a compiler is supposed to report an error if alignas specifies a 
> decreased alignment. So I installed the attached patch to stop using 
> alignas.

Thanks, but I think the resulting code is confusing.  E.g.:

  static struct thread_state GCALIGNED main_thread;

This looks like we ask main_thread to be 8-byte aligned, which would
bring back the problems we had before.  Requiring readers to read the
C Standard is not a good thing.

Can we come up with a clearer code, please?

> * Remove my_endbss_static in lastfile.c, since we no longer need to 
> worry about the Alpha MSVC linker.

I don't know anything about that code, and I don't think anyone does.
Who will tell us that the comment is still accurate?

> * Remove the the _MSC_VER-specific code in lisp.h's definitions of 
> ENUM_BF and DEFUN and in regex.c's definition of re_char and const_re_char.

I don't think ENUM_BF is only for MSVC.  The condition there mentions
other compilers.

Anything that is conditioned by _MSC_VER can go.

bug#29040: Emacs 25 hangs on windows arbitrarily during search of a unicode file

[Paul Eggert]

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

Eli Zaretskii wrote:
> Thanks, but I think the resulting code is confusing.  E.g.:
> 
>    static struct thread_state GCALIGNED main_thread;
> 
> This looks like we ask main_thread to be 8-byte aligned, which would
> bring back the problems we had before.

I attempted to clarify this by installing the first attached patch into the 
emacs-26 branch. This patch adds a comment to the definition of GCALIGNED. If 
the name "GCALIGNED" itself is confusing, we can systematically change the name 
to something else.  (The name doesn't confuse me, but perhaps I'm too close to 
the code.)

> I don't know anything about that code, and I don't think anyone does.
> Who will tell us that the comment is still accurate?

Since MSVC is no longer relevant, it doesn't matter whether the comment about 
MSVC is accurate, and we can remove it.

> Anything that is conditioned by _MSC_VER can go.

I did that by installing the second attached patch into 'master'. This affects 
only the mainline code; I didn't fiddle with the MS-Windows-only files since I 
can't easily test them.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-src-lisp.h-GCALIGNED-Clarify-comment-Bug-29040.patch --]
[-- Type: text/x-patch; name="0001-src-lisp.h-GCALIGNED-Clarify-comment-Bug-29040.patch", Size: 982 bytes --]

From a87ce5c4b41ca8146c764b1186a6162c069b1933 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Thu, 2 Nov 2017 20:04:25 -0700
Subject: [PATCH] * src/lisp.h (GCALIGNED): Clarify comment (Bug#29040).

---
 src/lisp.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/lisp.h b/src/lisp.h
index bf9db59..a71ba22 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -272,8 +272,9 @@ error !;
 #endif
 
 /* Declare an object to have an address that is a multiple of
-   GCALIGNMENT.  alignas is not suitable here, as it fails if the
-   object's natural alignment exceeds GCALIGNMENT.  */
+   GCALIGNMENT.  This is a no-op if the object's natural alignment is
+   already a multiple of GCALIGNMENT.  alignas is not suitable here,
+   as it fails if the object's natural alignment exceeds GCALIGNMENT.  */
 #ifdef HAVE_STRUCT_ATTRIBUTE_ALIGNED
 # define GCALIGNED __attribute__ ((aligned (GCALIGNMENT)))
 #else
-- 
2.7.4


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0001-Simplify-by-assuming-_MSC_VER.patch --]
[-- Type: text/x-patch; name="0001-Simplify-by-assuming-_MSC_VER.patch", Size: 8025 bytes --]

From 5b4ff53ab4579532b1b9eba233c135fee274d674 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Thu, 2 Nov 2017 21:54:23 -0700
Subject: [PATCH] Simplify by assuming !_MSC_VER.

Emacs has not been portable to MSVC for some time (Bug#29040#57).
* admin/CPP-DEFINES (_MSC_VER): Remove.
* src/lisp.h (ENUM_BF, DEFUN):
* src/regex.c (re_char):
Simplify by assuming _MSC_VER is not defined.
* src/regex.c (const_re_char): Remove.
All uses replaced by re_char.
---
 admin/CPP-DEFINES |  1 -
 src/lastfile.c    |  3 ---
 src/lisp.h        | 16 ++--------------
 src/regex.c       | 32 +++++++++++++-------------------
 4 files changed, 15 insertions(+), 37 deletions(-)

diff --git a/admin/CPP-DEFINES b/admin/CPP-DEFINES
index 10b558d..eb3eadf 100644
--- a/admin/CPP-DEFINES
+++ b/admin/CPP-DEFINES
@@ -19,7 +19,6 @@ __DJGPP_MINOR__ Minor version number of the DJGPP library; used only in msdos.c
 DOS_NT		Compiling for either the MS-DOS or native MS-Windows port.
 WINDOWSNT	Compiling the native MS-Windows (W32) port.
 __MINGW32__	Compiling the W32 port with the MinGW or MinGW-w64 ports of GCC.
-_MSC_VER	Compiling the W32 port with the Microsoft C compiler.
 MINGW_W64	Compiling the W32 port with the MinGW-w64 port of GCC.
 DARWIN_OS	Compiling on macOS or pure Darwin (and using s/darwin.h).
 SOLARIS2
diff --git a/src/lastfile.c b/src/lastfile.c
index 2901f14..1302279 100644
--- a/src/lastfile.c
+++ b/src/lastfile.c
@@ -49,9 +49,6 @@ char my_edata[] = "End of Emacs initialized data";
    isn't always a separate section in NT executables).  */
 char my_endbss[1];
 
-/* The Alpha MSVC linker globally segregates all static and public bss
-   data, so we must take both into account to determine the true extent
-   of the bss area used by Emacs.  */
 static char _my_endbss[1];
 char * my_endbss_static = _my_endbss;
 
diff --git a/src/lisp.h b/src/lisp.h
index a71ba22..1a8c3a0 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -418,9 +418,8 @@ error !;
 #define case_Lisp_Int case Lisp_Int0: case Lisp_Int1
 
 /* Idea stolen from GDB.  Pedantic GCC complains about enum bitfields,
-   MSVC doesn't support them, and xlc and Oracle Studio c99 complain
-   vociferously about them.  */
-#if (defined __STRICT_ANSI__ || defined _MSC_VER || defined __IBMC__ \
+   and xlc and Oracle Studio c99 complain vociferously about them.  */
+#if (defined __STRICT_ANSI__ || defined __IBMC__ \
      || (defined __SUNPRO_C && __STDC__))
 #define ENUM_BF(TYPE) unsigned int
 #else
@@ -2935,23 +2934,12 @@ CHECK_NUMBER_CDR (Lisp_Object x)
 
 /* This version of DEFUN declares a function prototype with the right
    arguments, so we can catch errors with maxargs at compile-time.  */
-#ifdef _MSC_VER
-#define DEFUN(lname, fnname, sname, minargs, maxargs, intspec, doc)	\
-   Lisp_Object fnname DEFUN_ARGS_ ## maxargs ;				\
-   static struct Lisp_Subr GCALIGNED sname =				\
-   { { (PVEC_SUBR << PSEUDOVECTOR_AREA_BITS)				\
-       | (sizeof (struct Lisp_Subr) / sizeof (EMACS_INT)) },		\
-      { (Lisp_Object (__cdecl *)(void))fnname },                        \
-       minargs, maxargs, lname, intspec, 0};				\
-   Lisp_Object fnname
-#else  /* not _MSC_VER */
 #define DEFUN(lname, fnname, sname, minargs, maxargs, intspec, doc)	\
    static struct Lisp_Subr GCALIGNED sname =				\
      { { PVEC_SUBR << PSEUDOVECTOR_AREA_BITS },				\
        { .a ## maxargs = fnname },					\
        minargs, maxargs, lname, intspec, 0};				\
    Lisp_Object fnname
-#endif
 
 /* defsubr (Sname);
    is how we define the symbol for function `name' at start-up time.  */
diff --git a/src/regex.c b/src/regex.c
index 330f2f7..d3d910d 100644
--- a/src/regex.c
+++ b/src/regex.c
@@ -519,13 +519,7 @@ ptrdiff_t emacs_re_safe_alloca = MAX_ALLOCA;
 #endif
 
 /* Type of source-pattern and string chars.  */
-#ifdef _MSC_VER
-typedef unsigned char re_char;
-typedef const re_char const_re_char;
-#else
 typedef const unsigned char re_char;
-typedef re_char const_re_char;
-#endif
 
 typedef char boolean;
 
@@ -2403,7 +2397,7 @@ do {									\
   } while (0)
 
 static reg_errcode_t
-regex_compile (const_re_char *pattern, size_t size,
+regex_compile (re_char *pattern, size_t size,
 #ifdef emacs
 # define syntax RE_SYNTAX_EMACS
 	       bool posix_backtracking,
@@ -3728,7 +3722,7 @@ insert_op2 (re_opcode_t op, unsigned char *loc, int arg1, int arg2, unsigned cha
    least one character before the ^.  */
 
 static boolean
-at_begline_loc_p (const_re_char *pattern, const_re_char *p, reg_syntax_t syntax)
+at_begline_loc_p (re_char *pattern, re_char *p, reg_syntax_t syntax)
 {
   re_char *prev = p - 2;
   boolean odd_backslashes;
@@ -3769,7 +3763,7 @@ at_begline_loc_p (const_re_char *pattern, const_re_char *p, reg_syntax_t syntax)
    at least one character after the $, i.e., `P < PEND'.  */
 
 static boolean
-at_endline_loc_p (const_re_char *p, const_re_char *pend, reg_syntax_t syntax)
+at_endline_loc_p (re_char *p, re_char *pend, reg_syntax_t syntax)
 {
   re_char *next = p;
   boolean next_backslash = *next == '\\';
@@ -3813,7 +3807,7 @@ group_in_compile_stack (compile_stack_type compile_stack, regnum_t regnum)
    Return -1 if fastmap was not updated accurately.  */
 
 static int
-analyze_first (const_re_char *p, const_re_char *pend, char *fastmap,
+analyze_first (re_char *p, re_char *pend, char *fastmap,
 	       const int multibyte)
 {
   int j, k;
@@ -4555,7 +4549,7 @@ static int bcmp_translate (re_char *s1, re_char *s2,
 /* If the operation is a match against one or more chars,
    return a pointer to the next operation, else return NULL.  */
 static re_char *
-skip_one_char (const_re_char *p)
+skip_one_char (re_char *p)
 {
   switch (*p++)
     {
@@ -4597,7 +4591,7 @@ skip_one_char (const_re_char *p)
 
 /* Jump over non-matching operations.  */
 static re_char *
-skip_noops (const_re_char *p, const_re_char *pend)
+skip_noops (re_char *p, re_char *pend)
 {
   int mcnt;
   while (p < pend)
@@ -4628,7 +4622,7 @@ skip_noops (const_re_char *p, const_re_char *pend)
    character (i.e. without any translations).  UNIBYTE denotes whether c is
    unibyte or multibyte character. */
 static bool
-execute_charset (const_re_char **pp, unsigned c, unsigned corig, bool unibyte)
+execute_charset (re_char **pp, unsigned c, unsigned corig, bool unibyte)
 {
   re_char *p = *pp, *rtp = NULL;
   bool not = (re_opcode_t) *p == charset_not;
@@ -4692,8 +4686,8 @@ execute_charset (const_re_char **pp, unsigned c, unsigned corig, bool unibyte)
 
 /* Non-zero if "p1 matches something" implies "p2 fails".  */
 static int
-mutually_exclusive_p (struct re_pattern_buffer *bufp, const_re_char *p1,
-		      const_re_char *p2)
+mutually_exclusive_p (struct re_pattern_buffer *bufp, re_char *p1,
+		      re_char *p2)
 {
   re_opcode_t op2;
   const boolean multibyte = RE_MULTIBYTE_P (bufp);
@@ -4931,8 +4925,8 @@ WEAK_ALIAS (__re_match_2, re_match_2)
 /* This is a separate function so that we can force an alloca cleanup
    afterwards.  */
 static regoff_t
-re_match_2_internal (struct re_pattern_buffer *bufp, const_re_char *string1,
-		     size_t size1, const_re_char *string2, size_t size2,
+re_match_2_internal (struct re_pattern_buffer *bufp, re_char *string1,
+		     size_t size1, re_char *string2, size_t size2,
 		     ssize_t pos, struct re_registers *regs, ssize_t stop)
 {
   /* General temporaries.  */
@@ -6222,10 +6216,10 @@ re_match_2_internal (struct re_pattern_buffer *bufp, const_re_char *string1,
    bytes; nonzero otherwise.  */
 
 static int
-bcmp_translate (const_re_char *s1, const_re_char *s2, register ssize_t len,
+bcmp_translate (re_char *s1, re_char *s2, ssize_t len,
 		RE_TRANSLATE_TYPE translate, const int target_multibyte)
 {
-  register re_char *p1 = s1, *p2 = s2;
+  re_char *p1 = s1, *p2 = s2;
   re_char *p1_end = s1 + len;
   re_char *p2_end = s2 + len;
 
-- 
2.7.4

bug#29040: Emacs 25 hangs on windows arbitrarily during search of a unicode file

[Eli Zaretskii]

> Cc: rcopley@gmail.com, 29040@debbugs.gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Thu, 2 Nov 2017 22:03:17 -0700
> 
> > I don't know anything about that code, and I don't think anyone does.
> > Who will tell us that the comment is still accurate?
> 
> Since MSVC is no longer relevant, it doesn't matter whether the comment about 
> MSVC is accurate, and we can remove it.

Alpha was never relevant for the MS-Windows builds, either, but we
still had this code since about forever.

But my point was that I'm not sure this isn't needed for the GCC build
on Windows.  I don't know enough about how GNU ld generates MS-Windows
executables to tell whether this could be so.  If you feel strongly
about getting rid of those 2 lines, I guess it's okay to try that on
master.

bug#29040: Emacs 25 hangs on windows arbitrarily during search of a unicode file

[Paul Eggert]

Eli Zaretskii wrote:
> If you feel strongly
> about getting rid of those 2 lines, I guess it's okay to try that on
> master.

I don't feel that strongly. I left the code alone, and merely deleted the 
comment about the Alpha MSVC (which is obsolete and no longer relevant).

bug#29040: Emacs 25 hangs on windows arbitrarily during search of a unicode file

[Eli Zaretskii]

> Cc: rcopley@gmail.com, 29040@debbugs.gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Thu, 2 Nov 2017 22:03:17 -0700
> 
> I attempted to clarify this by installing the first attached patch into the 
> emacs-26 branch. This patch adds a comment to the definition of GCALIGNED. If 
> the name "GCALIGNED" itself is confusing, we can systematically change the name 
> to something else.  (The name doesn't confuse me, but perhaps I'm too close to 
> the code.)

Seems like this causes trouble to some versions of GCC.  Compiling the
current master on fencepost.gnu.org with GCC 4.8.4 fails with the
following diagnostic:

  In file included from lisp.h:783:0,
		   from vm-limit.c:21:
  globals.h:1095:10: error: requested alignment is not an integer constant
     struct Lisp_Symbol GCALIGNED s;
	    ^
  In file included from vm-limit.c:21:0:
  lisp.h:1149:3: error: requested alignment is not an integer constant
     };
     ^
  lisp.h:1262:3: error: requested alignment is not an integer constant
     };
     ^

This GCC identifies itself as

  $ gcc --version
  gcc (Ubuntu 4.8.4-2ubuntu1~14.04.3) 4.8.4
  Copyright (C) 2013 Free Software Foundation, Inc.
  This is free software; see the source for copying conditions.  There is NO
  warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

bug#29040: Emacs 25 hangs on windows arbitrarily during search of a unicode file

[Paul Eggert]

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

Eli Zaretskii wrote:
>    globals.h:1095:10: error: requested alignment is not an integer constant
>       struct Lisp_Symbol GCALIGNED s;
> 	    ^


Ouch, I had thought that only older MSVC had the problem. Sorry about that. I 
installed the attached into emacs-26 and merged it to master.

[-- Attachment #2: 0001-Change-GCALIGNMENT-back-to-an-integer-literal.patch --]
[-- Type: text/x-patch, Size: 1438 bytes --]

From c911b27aff8dd15cd47a063dc958e6dae74dde06 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Fri, 3 Nov 2017 02:20:15 -0700
Subject: [PATCH] Change GCALIGNMENT back to an integer literal

* src/lisp.h (GCALIGNMENT): Change it back to a macro
that expands to a literal integer constant, for older GCC.
I had mistakenly thought that only MSVC had the problem.
Problem repored by Eli Zaretskii (Bug#29040#69).
---
 src/lisp.h | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/lisp.h b/src/lisp.h
index a71ba22..4dd4720 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -233,7 +233,9 @@ extern bool suppress_checking EXTERNALLY_VISIBLE;
 
 enum Lisp_Bits
   {
-    GCALIGNMENT = 1 << GCTYPEBITS,
+    /* 2**GCTYPEBITS.  This must be a macro that expands to a literal
+       integer constant, for older versions of GCC (through at least 4.9).  */
+#define GCALIGNMENT 8
 
     /* Number of bits in a Lisp_Object value, not counting the tag.  */
     VALBITS = EMACS_INT_WIDTH - GCTYPEBITS,
@@ -245,6 +247,10 @@ enum Lisp_Bits
     FIXNUM_BITS = VALBITS + 1
   };
 
+#if GCALIGNMENT != 1 << GCTYPEBITS
+# error "GCALIGNMENT and GCTYPEBITS are inconsistent"
+#endif
+
 /* The maximum value that can be stored in a EMACS_INT, assuming all
    bits other than the type bits contribute to a nonnegative signed value.
    This can be used in #if, e.g., '#if USE_LSB_TAG' below expands to an
-- 
2.7.4

bug#29040: Emacs 25 hangs on windows arbitrarily during search of a unicode file

[Eli Zaretskii]

> Cc: 29040@debbugs.gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Fri, 3 Nov 2017 02:25:17 -0700
> 
> Eli Zaretskii wrote:
> >    globals.h:1095:10: error: requested alignment is not an integer constant
> >       struct Lisp_Symbol GCALIGNED s;
> > 	    ^
> 
> 
> Ouch, I had thought that only older MSVC had the problem. Sorry about that. I 
> installed the attached into emacs-26 and merged it to master.

Thanks, this fixed the build on fencepost.