Failing GMP compatibility tests on Linux with GCC

creachadair commented 5 years ago

Several tests of unsigned integer conversion do not work on the signbug branch, where I have been working to make the Docker-based Linux test include make check as well as make test.

Repro

cd imath
git checkout signbug
docker build -f tests/linux/Dockerfile .

Expected

All tests pass.

Observed

Total
  Tests: 1302531. Passes: 1302519. Failures: 12.
Failing Tests:
mpz_get_ui|-1
mpz_get_ui|-2147483648
mpz_get_ui|-32768
mpz_get_ui|-2147483647
mpz_get_ui|-32767
mpz_get_ui|-2147483649
mpz_get_ui|-32769
mpz_get_ui|-45
mpz_get_ui|-1
mpz_get_ui|-25
mpz_get_ui|-9454
mpz_get_ui|-479051

Notes

# uname -a
Linux 0a92db5e9814 4.9.125-linuxkit #1 SMP Fri Sep 7 08:20:28 UTC 2018 x86_64 Linux

# gcc --version
gcc (Alpine 6.4.0) 6.4.0
Copyright (C) 2017 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.

All the failing examples are for mpz_get_ui, and appear to be related to the handling of sign. The documentation stipulates that

If op is too big to fit an unsigned long then just the least significant bits that do fit are returned. The sign of op is ignored, only the absolute value is used.

The code at least superficially appears correct, but it's possible there is some size issue on this platform.

creachadair commented 5 years ago

Here are all thempz_get_ui tests that were run. Note that not all of them failed:

$ docker run --rm -it 1aa0a9359752 grep -E '^mpz_get_ui\|-' tests/gmp-compat-test/random.tests
mpz_get_ui|-0
mpz_get_ui|-1
mpz_get_ui|-9223372036854775808
mpz_get_ui|-2147483648
mpz_get_ui|-32768
mpz_get_ui|-9223372036854775807
mpz_get_ui|-2147483647
mpz_get_ui|-32767
mpz_get_ui|-9223372036854775809
mpz_get_ui|-2147483649
mpz_get_ui|-32769
mpz_get_ui|-45
mpz_get_ui|-1
mpz_get_ui|-25
mpz_get_ui|-9454
mpz_get_ui|-858601313164665
mpz_get_ui|-640548354329213637
mpz_get_ui|-68769130978627842
mpz_get_ui|-125290025931
mpz_get_ui|-479051
mpz_get_ui|-4826373731961063416102855225686204704570

creachadair commented 5 years ago

 # ./runtest failed.tests 
Running tests in failed.tests
FAIL: 2@mpz_get_ui  1  !=  94519345283073 
FAIL: 2@mpz_get_ui|-1

FAIL: 4@mpz_get_ui  2147483648  !=  94521492766720 
FAIL: 4@mpz_get_ui|-2147483648

FAIL: 5@mpz_get_ui  32768  !=  94519345315840 
FAIL: 5@mpz_get_ui|-32768

FAIL: 7@mpz_get_ui  2147483647  !=  94521492766719 
FAIL: 7@mpz_get_ui|-2147483647

FAIL: 8@mpz_get_ui  32767  !=  94519345315839 
FAIL: 8@mpz_get_ui|-32767

FAIL: 10@mpz_get_ui  2147483649  !=  94521492766721 
FAIL: 10@mpz_get_ui|-2147483649

FAIL: 11@mpz_get_ui  32769  !=  94519345315841 
FAIL: 11@mpz_get_ui|-32769

FAIL: 12@mpz_get_ui  45  !=  94519345283117 
FAIL: 12@mpz_get_ui|-45

FAIL: 13@mpz_get_ui  1  !=  94519345283073 
FAIL: 13@mpz_get_ui|-1

FAIL: 14@mpz_get_ui  25  !=  94519345283097 
FAIL: 14@mpz_get_ui|-25

FAIL: 15@mpz_get_ui  9454  !=  94519345292526 
FAIL: 15@mpz_get_ui|-9454

FAIL: 20@mpz_get_ui  479051  !=  94519345762123 
FAIL: 20@mpz_get_ui|-479051

  Tests: 21. Passes: 9. Failures: 12.

creachadair commented 5 years ago

The same exact tests fail in a container built from ubuntu:latest, so I'm fair sure this is not due to a quirk in Alpine.

creachadair commented 5 years ago

The common theme among all the failures is the values fit within 32 bits in magnitude. Longer values, even negative ones, fail. This suggests there's some corner case around the handling of single-digit values.

creachadair commented 5 years ago

Ok, so the problem is that the packing routine does not consider the magnitude of the given value. It requests conversion, which fails with MP_RANGE because op is negative: But in these cases, it is not the the magnitude that's too large, it's the sign.

Nevertheless, the get_long_bits routine assumes the magnitude is the problem and starts packing digits based on how many fit into an unsigned long and doesn't account for op->used at all. Since the digits at indices >= op->used are garbage, this is grabbing uninitialized values. In general this is a potential buffer overflow—but in the test cases MP_ALLOC(op) is always big enough.

creachadair / imath