#4943 Manual page breaking causing assertion failure

Thomas Morley - 46 minutes ago

Inline assembler fallback for _FPU_SETCW() missing in MINGW libraries

Issue 4943
As Issue 4943 on Windows compilation was triggered by
missing _FPU_SETCW() in the MINGW libraries, an alternate call to
initiate the X87 FPU setup as an inline-assembler command is added
- for MINGW 32 Bit compilation only.

https://codereview.appspot.com/575600043/

On Sat, 9 Jan 2016 at 09:55 Chris Yate chrisyate@gmail.com wrote:

So, it turns out this was easy to minimise the code. File is attached.
It's still ~70 bars but only one voice, which is the dynamics staff of a
piano score (without anything else), so it's only silent music.

"AutoPageBreaksOff" in the score block causes the exception failure.

I note some "insane spring distance" warnings.These don't happen in the
real thing.

Chris

Note: According to Phil H's own tests: "This started in 2.19.21. 2.19.20 is OK."

Update: an instance of this affects also non-Windows systems, see comment below.

[Anonymous]

Error reproduced by James L on Windows 8.1 - screenshot of error attached.

[Anonymous]

Does not occur in Linux builds.

[David Kastrup]

Pretty sure that "This started in 2.19.21. 2.19.20 is OK." is a red herring since 2.19.21 was the version where assertions were enabled for more than just --disable-optimising.

So I'll assume that the error is older than that. Probably lots older.

[Carl Sorensen]

I see the same assertion failure on OSX 10.8.5

[Carl Sorensen]

Here's a tiny example that shows the problem.

The assertion failure is triggered with 18 bars (in both 6/8 and 4/4 time) but not with 17.

[Thomas Morley]

Arrgh.
I misclicked and deleted the attachment.
Can it be restored?

[Trevor Daniels]

Fortunately I created a backup a couple of weeks ago, so yes it can. It should be back in its proper place now.

[Thomas Morley]

Great. Thank tons.

Would it be possible to configure the settings that only the author and admins (you and Phil) can delete?

[Trevor Daniels]

Thomas Morley wrote Friday, January 06, 2017 6:50 PM

Great. Thank tons.

No problem - lucky I took a year-end backup though!

Would it be possible to configure the settings that only the author and admins (you and Phil) can delete?

I don't think so. Deletion seems to be part of Update permissions, which all Devs and Members have. Does seem rather surprising though.

Trevor

[Carl Sorensen]

And I confirm that the assertion failure does not happen on LilyDev.

[Valentin Villenave]

A similar crash has been reported on -user-fr, which I was able to trim down to the minimal example below (commenting any line solves the bug). Interestingly though, this one’s reproducible even on GNU/Linux. (Whereas the previous snippets in this ticket aren’t.)

\version "2.19.80"
% 2.18.2 doesn’t crash, whilst 2.21.0 still does.

\header { title = "foo bar" }

{
  \pageBreak
  c'1 \break
  c' \break
  \repeat unfold 4 c'
}

Here’s the log on GNU/Linux:

page-breaking.cc:1052: void Page_breaking::line_divisions_rec(vsize, const Line_division&, const Line_division&, Page_breaking::Line_division*): Assertion `my_index == 0' failed.
Aborted (core dumped)

See also the attached Windows screenshot, which looks a lot like what had previously been reported. (Except it appears to be a different assertion.)

[Thomas Morley]

Some findings by Arnold can be found at:
http://lilypond.1069038.n5.nabble.com/Issue-4934-analysis-indicates-new-issue-td224851.html
regrettable there is a typo: 4934 <-> 4943 in his post.

I'll quote it here entirely:

Hello,
as I got this assertion failure on multiple scores on Win7 (on scores which
I created in sequence) I did tried to analyse this problem.

All my effected scores had one system per page. On the last page the
system is compressed.
If I apply tiny changes to the global-staff-size, there is a chance of 50 %
the lilypond run will succeed or it will terminate with assertion from
function
Page_breaking::min_page_count() in file lily/page-breaking.cc.
This problem does occur on Windows (mingw compilation), but not on
Linux compilations (executed in a LILYDEV VM on my Windows computer).

Tracing this function ‘Page_breaking::min_page_count()’ by additional
text output in the LILYDEV VM did show, the critical code must be in
line 1178. The comparison there is sensitive to rounding errors between
80 bit floating point and 64 bit floating point variables, a typical code
problem in IA32 compilations for Windows, where function return values
are passed back in the 80 bit floating point register of the X87 arithmetic
coprocessor.
If the last page contains a single compressed system, the stored (double)
value ‘cur_rod_height’ and the function return value (in X87 register in
IA32 architecture) from ‘cached_line_details_.back().full_height()’ were
equal in the Linux version.

Together with ‘harm6’ at the German Lilypond Forum it got a MINGW
compilation for Windows with tracing text output. It did prove what I
expected:

(1) The assertion is thrown, because the function return value of
‘cached_line_details_.back().full_height()’ is ‘more exact’ (longer
mantissa)
than the double value stored in cur_rod_height. The function return value
is a 80 bit floating point value, and not rounded to the mantissa length
of the 64 bit floating point value it’s going to be compared with.

(2) The check in line 1178 of lily/page-breaking.cc, which is:
&& cur_rod_height > cached_line_details_.back ().full_height ())
can be replaced with:
&& page_starter + 1 != cached_line_details_.size())
which does not compare (inexact) floating point numbers anymore to
check if there is only one system on the last page, to solve issue 4934.

Beside that, I found:

(3) In the past, rounding to 64 bit floating point numbers in the
MINGW compilation was forced by _FPU_SETCW() in function
configure_fpu() from file lily\main.cc.
In this MINGW test compilation this code was not reached!
As this can cause many other peculiar problems, not so simple to
detect as this issue 3934, a new issue should be opened:

/++++++++++++++++++
Repair the broken support of configure_fpu() in the
mingw installation of GUB
/++++++++++++++++++

For a more easy check during GUB compilation I suggest to add
a conditional compile time error (or warning if you prefer) into
lily/main.cc to the empty dummy function configure_fpu():

... starting at line 202 of lily/main.cc ...

 #include <fpu_control.h>
 static void
 configure_fpu ()
 {
   fpu_control_t fpu_control = 0x027f;
   _FPU_SETCW (fpu_control);
 }

 #else

 static void
 configure_fpu ()
 {
+/* throw compilation error if MINGW 32bit with x87 */
+#if defined (__MINGW32__) && defined (__code_model_32__) &&
!defined(__SSE2_MATH__)
+#pragma GCC error "No FPU CONTROL in MINGW compilation required, but NOT
found"
+#endif
 }

 #endif /* defined(__x86__) || defined(__i386__) */

Furthermore, there are also command line options reported for MINGW
to set the floating point precision.
But notice, ‘-mpc64’ did not help for a simple test program on my MINGW
compilation in the LILYDEV VM. That might be the same origin as support
of configure_fpu() is broken in the GUB.
Instead ‘-march=pentium4 -mfpmath=sse -msse2' did work, but this
drops support for older CPU types.

ArnoldTheresius

[Thomas Morley]

Inline assembler fallback for _FPU_SETCW() missing in MINGW libraries

Issue 4943
As Issue 4943 on Windows compilation was triggered by
missing _FPU_SETCW() in the MINGW libraries, an alternate call to
initiate the X87 FPU setup as an inline-assembler command is added
- for MINGW 32 Bit compilation only.

http://codereview.appspot.com/577450043

[Anonymous]

Passes make, make check and a full make doc.

[Anonymous]

Patch on countdown for Feb 2nd

[Anonymous]

Patch on countdown for Feb 2nd

[Thomas Morley]

Inline assembler fallback for _FPU_SETCW() missing in x86 platforms

Issue 4943
As Issue 4943 on x86 platform compilations was triggered by
missing _FPU_SETCW(), an alternate call to initiate the
X87 FPU setup as an inline-assembler command is added.

http://codereview.appspot.com/575600043

[Thomas Morley]

New patch at
https://codereview.appspot.com/575600043/