bug#21132: 25.0.50; Emacs on Windows crashes evaluating x-frame-geometry in batch mode

bug#21132: 25.0.50; Emacs on Windows crashes evaluating x-frame-geometry in batch mode

[Francis Litterio]

Emacs built from the latest source on Windows crashes when invoked
as follows:

  emacs.exe -Q -batch --eval="(x-frame-geometry (selected-frame))"

This is consistently reproduceable.
--
Fran Litterio
flitterio -at- gmail.com



In GNU Emacs 25.0.50.4 (i686-pc-mingw32)
 of 2015-06-30 on PUPPY
Repository revision: 5f004117f5bcab9171eaddb2867393ed69ae49bf
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --prefix=c:/apps/emacs --without-x --without-xpm
 --without-png --without-jpeg --without-tiff --without-gif'

Configured features:
SOUND NOTIFY ACL TOOLKIT_SCROLL_BARS

Important settings:
  value of $LANG: C.ISO-8859-1
  locale-coding-system: cp1252

Major mode: Text

Minor modes in effect:
  erc-list-mode: t
  erc-menu-mode: t
  erc-ring-mode: t
  erc-networks-mode: t
  erc-pcomplete-mode: t
  erc-track-mode: t
  erc-track-minor-mode: t
  erc-match-mode: t
  erc-button-mode: t
  erc-fill-mode: t
  erc-netsplit-mode: t
  erc-irccontrols-mode: t
  erc-noncommands-mode: t
  erc-move-to-prompt-mode: t
  erc-readonly-mode: t
  diff-auto-refine-mode: t
  show-paren-mode: t
  save-place-mode: t
  icomplete-mode: t
  savehist-mode: t
  shell-dirtrack-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  auto-fill-function: do-auto-fill
  transient-mark-mode: t
  abbrev-mode: t

Recent messages:
This is a test
Type C-x 1 to remove help window.  
Quit [2 times]
Buffer buried.
Type "q" to delete help window.
Saving file c:/franl/todo.txt...
Wrote c:/franl/todo.txt
Mark set [2 times]
Quit
Type C-x 1 to remove help window.  
report-emacs-bug is on <menu-bar> <help-menu> <send-emacs-bug-report>

Load-path shadows:
None found.

Features:
(shadow mail-extr emacsbug debug jka-compr eieio-opt speedbar sb-image
ezimage dframe find-func conf-mode sh-script smie executable edmacro
kmacro misearch multi-isearch server sort gnus-draft gnus-agent
gnus-srvr nnvirtual nndraft nnmh gnus-msg gnus-cite canlock gnus-art
mm-uu mml2015 epg-config mm-view mml-smime smime dig mailcap gnus-async
gnus-score score-mode gnus-cache gnus-sum fpl-moo fpl-react erc-notify
erc-truncate erc-log erc-dcc erc-list erc-menu erc-join erc-ring
erc-networks erc-pcomplete erc-track erc-match erc-button erc-fill
erc-stamp erc-netsplit erc-goodies erc erc-backend erc-compat thingatpt
help-mode source-safe ediff-merg ediff-wind ediff-diff ediff-mult
ediff-help ediff-init ediff-util ediff grep python json ielm pp
sgml-mode csharp-mode cc-langs cl smtpmail sendmail nntp gnus-group
gnus-undo gnus-start gnus-cloud nnimap nnmail mail-source utf7 netrc
parse-time gnus-spec gnus-int gnus-range message rfc822 mml mml-sec
mm-decode mm-bodies mm-encode mail-parse rfc2231 rfc2047 rfc2045
ietf-drums mailabbrev gmm-utils mailheader gnus-win nnoo gnus gnus-ems
nnheader mail-utils wid-edit etags xref vc vc-dispatcher dired-aux hexl
smerge-mode diff-mode easy-mmode paren man info compile apropos tramp
tramp-compat tramp-loaddefs trampver format-spec advice saveplace
icomplete savehist browse-url shell pcomplete warnings arc-mode
archive-mode ange-ftp socks network-stream nsm auth-source cl-macs
cl-seq eieio byte-opt gv bytecomp byte-compile cl-extra seq cconv
eieio-core cl-loaddefs pcase cl-lib gnus-util mm-util help-fns
mail-prsvr password-cache starttls tls dired cc-mode cc-fonts easymenu
cc-guess cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars cc-defs
comint ansi-color ring calc-ext calc calc-loaddefs calc-macs time-stamp
time-date mule-util tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel dos-w32 ls-lisp disp-table w32-win w32-vars
term/common-win tool-bar dnd fontset image regexp-opt fringe
tabulated-list newcomment elisp-mode lisp-mode prog-mode register page
menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core frame cl-generic cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese charscript case-table epa-hook jka-cmpr-hook help
simple abbrev minibuffer cl-preloaded nadvice loaddefs button faces
cus-face macroexp files text-properties overlay sha1 md5 base64 format
env code-pages mule custom widget hashtable-print-readable backquote
w32notify w32 multi-tty make-network-process emacs)

Memory information:
((conses 8 413719 46357)
 (symbols 32 43443 0)
 (miscs 32 183 1822)
 (strings 16 95444 23311)
 (string-bytes 1 2954158)
 (vectors 8 49918)
 (vector-slots 4 1628135 52046)
 (floats 8 431 743)
 (intervals 28 10264 655)
 (buffers 516 36))

bug#21132: 25.0.50; Emacs on Windows crashes evaluating x-frame-geometry in batch mode

[Eli Zaretskii]

> From: flitterio@gmail.com (Francis Litterio)
> Date: Sat, 25 Jul 2015 12:03:42 -0400
> 
> 
> Emacs built from the latest source on Windows crashes when invoked
> as follows:
> 
>   emacs.exe -Q -batch --eval="(x-frame-geometry (selected-frame))"

Thanks, fixed.

I believe similar changes are needed in xfns.c and in nsfns.m, could
someone with access to the relevant OSes please check and apply?

bug#21132: 25.0.50; Emacs on Windows crashes evaluating x-frame-geometry in batch mode

[martin rudalics]

 >> Emacs built from the latest source on Windows crashes when invoked
 >> as follows:
 >>
 >>    emacs.exe -Q -batch --eval="(x-frame-geometry (selected-frame))"
 >
 > Thanks, fixed.

Do you have any simple guideline how to find similar instances of this
bug?  And what is the difference between !FRAME_INITIAL_P (f) and
FRAME_W32_WINDOW (f)?  I suppose that !FRAME_INITIAL_P (f) implies
FRAME_W32_WINDOW (f).  So there are probably cases when we use
FRAME_W32_WINDOW (f) and FRAME_INITIAL_P (f) holds.  Right?

And do we prefer (FRAME_W32_WINDOW (f) != 0) to (!FRAME_W32_WINDOW (f))?
Also I believe that I should replace FRAME_X_WINDOW by FRAME_W32_WINDOW.

 > I believe similar changes are needed in xfns.c and in nsfns.m, could
 > someone with access to the relevant OSes please check and apply?

I'll look into these.

Thanks for the quick fix, martin

bug#21132: 25.0.50; Emacs on Windows crashes evaluating x-frame-geometry in batch mode

[Eli Zaretskii]

> Date: Sat, 25 Jul 2015 20:17:39 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: 21132@debbugs.gnu.org
> 
>  >> Emacs built from the latest source on Windows crashes when invoked
>  >> as follows:
>  >>
>  >>    emacs.exe -Q -batch --eval="(x-frame-geometry (selected-frame))"
>  >
>  > Thanks, fixed.
> 
> Do you have any simple guideline how to find similar instances of this
> bug?

Look for unconditional uses of FRAME_W32_WINDOW in code that is not
guaranteed to be invoked in a GUI session.  Functions exposed to Lisp
are an obvious candidate, as in this case.

> And what is the difference between !FRAME_INITIAL_P (f) and
> FRAME_W32_WINDOW (f)?

FRAME_W32_WINDOW is not a predicate.  You probably meant
FRAME_INITIAL_P and FRAME_W32_P.  The difference is the method used to
output text.

> I suppose that !FRAME_INITIAL_P (f) implies
> FRAME_W32_WINDOW (f).

No, it implies (on w32) FRAME_W32_P or FRAME_TERMCAP_P.

> So there are probably cases when we use
> FRAME_W32_WINDOW (f) and FRAME_INITIAL_P (f) holds.  Right?

If there are, we will crash.

> And do we prefer (FRAME_W32_WINDOW (f) != 0) to (!FRAME_W32_WINDOW (f))?

We prefer !FRAME_W32_P (f)

> Also I believe that I should replace FRAME_X_WINDOW by FRAME_W32_WINDOW.

Yes.

>  > I believe similar changes are needed in xfns.c and in nsfns.m, could
>  > someone with access to the relevant OSes please check and apply?
> 
> I'll look into these.

Thanks.

bug#21132: 25.0.50; Emacs on Windows crashes evaluating x-frame-geometry in batch mode

[martin rudalics]

 >> And do we prefer (FRAME_W32_WINDOW (f) != 0) to (!FRAME_W32_WINDOW (f))?
 >
 > We prefer !FRAME_W32_P (f)

x_set_foreground_color, x_set_background_color and x_set_mouse_color use

if (FRAME_W32_WINDOW (f) != 0)

Should these be changed?  In x_change_tool_bar_height and
w32_set_title_bar_text we use

if (FRAME_W32_WINDOW (f))

Should these be changed too?

 >>   > I believe similar changes are needed in xfns.c and in nsfns.m, could
 >>   > someone with access to the relevant OSes please check and apply?
 >>
 >> I'll look into these.

I fixed these hopefully.  The Gtk build always crashed when invoked with
-nw so I now have `x-frame-geometry' return nil for terminal frames.

martin

bug#21132: 25.0.50; Emacs on Windows crashes evaluating x-frame-geometry in batch mode

[Eli Zaretskii]

> Date: Sun, 26 Jul 2015 13:27:40 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: flitterio@gmail.com, 21132@debbugs.gnu.org
> 
>  >> And do we prefer (FRAME_W32_WINDOW (f) != 0) to (!FRAME_W32_WINDOW (f))?
>  >
>  > We prefer !FRAME_W32_P (f)
> 
> x_set_foreground_color, x_set_background_color and x_set_mouse_color use
> 
> if (FRAME_W32_WINDOW (f) != 0)
> 
> Should these be changed?

No, I don't think so, because these are handlers for w32 frame
parameters, and I see no way they could be called from Lisp, except in
that context.  Am I missing something?

> In x_change_tool_bar_height and w32_set_title_bar_text we use
> 
> if (FRAME_W32_WINDOW (f))
> 
> Should these be changed too?

No, for the same reasons.

>  >>   > I believe similar changes are needed in xfns.c and in nsfns.m, could
>  >>   > someone with access to the relevant OSes please check and apply?
>  >>
>  >> I'll look into these.
> 
> I fixed these hopefully.  The Gtk build always crashed when invoked with
> -nw so I now have `x-frame-geometry' return nil for terminal frames.

Thanks.

bug#21132: 25.0.50; Emacs on Windows crashes evaluating x-frame-geometry in batch mode

[martin rudalics]

 >>   >> And do we prefer (FRAME_W32_WINDOW (f) != 0) to (!FRAME_W32_WINDOW (f))?
 >>   >
 >>   > We prefer !FRAME_W32_P (f)
 >>
 >> x_set_foreground_color, x_set_background_color and x_set_mouse_color use
 >>
 >> if (FRAME_W32_WINDOW (f) != 0)
 >>
 >> Should these be changed?
 >
 > No, I don't think so, because these are handlers for w32 frame
 > parameters, and I see no way they could be called from Lisp, except in
 > that context.  Am I missing something?

No.  I asked because of your preference stated above.  Although in all
the cases I cited we probably just care about whether the frame exists
at all.  Yet I would feel better with a more stringent predicate that
would combine say, FRAME_W32_WINDOW and FRAME_W32_P.

martin

bug#21132: 25.0.50; Emacs on Windows crashes evaluating x-frame-geometry in batch mode

[Eli Zaretskii]

> Date: Mon, 27 Jul 2015 18:02:38 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: flitterio@gmail.com, 21132@debbugs.gnu.org
> 
>  >>   >> And do we prefer (FRAME_W32_WINDOW (f) != 0) to (!FRAME_W32_WINDOW (f))?
>  >>   >
>  >>   > We prefer !FRAME_W32_P (f)
>  >>
>  >> x_set_foreground_color, x_set_background_color and x_set_mouse_color use
>  >>
>  >> if (FRAME_W32_WINDOW (f) != 0)
>  >>
>  >> Should these be changed?
>  >
>  > No, I don't think so, because these are handlers for w32 frame
>  > parameters, and I see no way they could be called from Lisp, except in
>  > that context.  Am I missing something?
> 
> No.  I asked because of your preference stated above.  Although in all
> the cases I cited we probably just care about whether the frame exists
> at all.  Yet I would feel better with a more stringent predicate that
> would combine say, FRAME_W32_WINDOW and FRAME_W32_P.

When FRAME_W32_P returns false, FRAME_W32_WINDOW will crash.

bug#21132: 25.0.50; Emacs on Windows crashes evaluating x-frame-geometry in batch mode

[martin rudalics]

 > When FRAME_W32_P returns false, FRAME_W32_WINDOW will crash.

The question is whether always for any frame f holds FRAME_W32_P (f) =>
FRAME_W32_WINDOW (f).  If that is the case we could do away with using
FRAME_W32_WINDOW as/in predicate(s).

martin

bug#21132: 25.0.50; Emacs on Windows crashes evaluating x-frame-geometry in batch mode

[Eli Zaretskii]

> Date: Mon, 27 Jul 2015 18:34:45 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: flitterio@gmail.com, 21132@debbugs.gnu.org
> 
>  > When FRAME_W32_P returns false, FRAME_W32_WINDOW will crash.
> 
> The question is whether always for any frame f holds FRAME_W32_P (f) =>
> FRAME_W32_WINDOW (f).

I don't know.  FRAME_W32_WINDOW returns a window handle, whereas
FRAME_W32_P only tells you can access that handle, but doesn't
necessarily ensure that handle will be non-NULL.

bug#21132: 25.0.50; Emacs on Windows crashes evaluating x-frame-geometry in batch mode

[martin rudalics]

 > Emacs built from the latest source on Windows crashes when invoked
 > as follows:
 >
 >    emacs.exe -Q -batch --eval="(x-frame-geometry (selected-frame))"
 >
 > This is consistently reproduceable.

On Windows the function has been renamed to `w32-frame-geometry'.  The
underlying bug should have been fixed on all platforms.  Closing this
bug.

Thanks, martin