Workaround for GMP crash on Cygwin (regression introduced by PR #3434)

[embray]

Description

Adds ENSURE_BAG which on the relevant platform (Cygwin 64-bit, at least) simply zero-fills the contents of a bag before using it.

This is perhaps a naive attempt and I'm not sure it covers every possible case, but it does at least fix the test case from #3434 using Bernoulli().

Needs proper documentation, but I'll add that if/when this approach is approved.

Text for release notes

Fixed crash on 64-bit Cygwin that could occur in some integer operations.

Checklist for pull request reviewers

• proper formatting

If your code contains kernel C code, run clang-format on it; the
simplest way is to use git clang-format, e.g. like this (don't
forget to commit the resulting changes):

git clang-format $(git merge-base master)

• usage of relevant labels

  1. either release notes: not needed or release notes: to be added
  2. at least one of the labels bug or enhancement or new feature
  3. for changes meant to be backported to stable-4.X add the backport-to-4.X label
  4. consider adding any of the labels build system, documentation, kernel, library, tests

• runnable tests

• adequate pull request title

• well formulated text for release notes

• relevant documentation updates

• sensible comments in the code

[embray]

(Rebased on current master branch; mine was quite a bit behind it seems.)

[coveralls]

Coverage increased (+0.05%) to 85.038% when pulling 95a011a on embray:cygwin/issue-3434 into 7ab3b05 on gap-system:master.

[ChrisJefferson]

Thanks. I like this PR. While we might be able to make it more efficient (by only touching each page), I'd like to keep it simple, as Cygwin is already poorly tested, and this is a very subtle issue.

Once it is fixed in GMP, we might want to think about how to selectively enable it, or require the up to date version.

[fingolfin]

Thank you! I have some minor change requests. But overall, this seems very sensible to me.

[embray]

Thanks. I like this PR. While we might be able to make it more efficient (by only touching each page), I'd like to keep it simple, as Cygwin is already poorly tested, and this is a very subtle issue.

This is actually what I tried first, but as you previously suspected it was trickier than I had expected to get the compiler to not just optimize it away. E.g. volatile had basically not effect with a local variable. I'm sure it can be done but I didn't want to spend a lot of time messing with it.

[embray]

Once it is fixed in GMP, we might want to think about how to selectively enable it, or require the up to date version.

Well, so far at least one of the GMP developers has been supportive of the idea: https://gmplib.org/list-archives/gmp-bugs/2019-May/004544.html GMP has some good version macros so it would be easy to disable based on a version check if/when fixed. I should probably look into how to check if GMP is actually MPIR, because MPIR actually doesn't have this problem as far as I can tell. In MPIR they rewrote everything to use the YASM assembler, which has implicit support for adding the stack unwinding metadata on Windows.

[fingolfin]

Editorial note: Since this is a fix for a recent regression that was in no GAP release, there is no need for release notes.

[codecov]

Codecov Report

Merging #3435 into master will decrease coverage by 2.72%.
The diff coverage is 88.88%.

@@            Coverage Diff             @@
##           master    #3435      +/-   ##
==========================================
- Coverage    80.2%   77.47%   -2.73%     
==========================================
  Files         695      692       -3     
  Lines      342574   340498    -2076     
==========================================
- Hits       274750   263810   -10940     
- Misses      67824    76688    +8864

Impacted Files	Coverage Δ
src/gasman.h	67.44% <0%> (-25.42%)	⬇️
src/integer.c	97.52% <100%> (-1.02%)	⬇️
src/modules.h	16.66% <0%> (-83.34%)	⬇️
lib/contfrac.gi	31.42% <0%> (-67.15%)	⬇️
src/saveload.c	3.27% <0%> (-64.93%)	⬇️
src/libgap-api.c	0% <0%> (-62.58%)	⬇️
src/cyclotom.h	50% <0%> (-50%)	⬇️
src/records.h	50% <0%> (-50%)	⬇️
src/gapstate.h	42.85% <0%> (-42.86%)	⬇️
src/vec8bit.h	51.78% <0%> (-42.86%)	⬇️
... and 230 more

[embray]

That said, would there be any objection to backporting this and #3335 to the 4.10-stable branch? If so for some reason that's fine. I just wondered.

[fingolfin]

@embray at this point, I am not sure there will be any further release in the 4.10 line; we plan to release GAP 4.11 in May.

[embray]

Ah, ok. Fair enough.

[ChrisJefferson]

Thanks again for tracking this down.

[slel]

@ChrisJefferson @fingolfin could this be backported in GAP 4.10
so it becomes part of GAP 4.10.3?