Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ZlibStreamError due to size mismatch of data types (Linux) #39

Open
sthenic opened this issue Mar 26, 2019 · 9 comments
Open

ZlibStreamError due to size mismatch of data types (Linux) #39

sthenic opened this issue Mar 26, 2019 · 9 comments
Assignees
Labels

Comments

@sthenic
Copy link

sthenic commented Mar 26, 2019

I was trying out the zlib wrapper on my Linux machine (amd64, zlib 1.2.11) with the following program:

import zip/zlib

let c = compress("Compress me!")

Compilation goes well but the program yields the following run-time error:

Error: unhandled exception: zlib version mismatch! [ZlibStreamError]

I went digging through the source code and I think I've figured out what's going on.

The version mismatch exception is (in this case) caused by zlib's deflateInit2() returning Z_VERSION_ERROR. According to the docs, that can happen for two reasons:

  • the requested version is not compatible with the library version or
  • the z_stream size differs from that used by the library.

The wrapper defines the ZStream object to represent z_stream and passes sizeof(ZStream) as the size that gets validated by the library. That causes a mismatch on my system since the library expects a 112-byte struct but instead gets one that's 88 bytes.

There is a zlib function called zlibCompileFlags() that defines the sizes of the types used by the library. Compile the following with gcc:

#include <stdio.h>
#include <zlib.h>

unsigned int get_zlib_size(unsigned long flag)
{
    switch (flag)
    {
    case 0:
    case 1:
    case 2:
        return (1u << (1 + flag));
    default:
        break;
    }

    printf("Unknown size flag '%lu'.\n", flag);
    return 0;
}

int main(int argc, const char *argv[])
{
    z_stream s;
    printf("Size of z_stream: %lu.\n", sizeof(s));

    unsigned long cflags = zlibCompileFlags();

    printf("zlib compile flags:\n");
    printf("  size of uInt: %u.\n", get_zlib_size(cflags & 0x3));
    printf("  size of uLong: %u.\n", get_zlib_size((cflags >> 2) & 0x3));
    printf("  size of voidpf: %u.\n", get_zlib_size((cflags >> 4) & 0x3));
    printf("  size of z_off_t: %u.\n", get_zlib_size((cflags >> 6) & 0x3));

    return 0;
}

For me that yields the output:

Size of z_stream: 112.
zlib compile flags:
  size of uInt: 4.
  size of uLong: 8.
  size of voidpf: 8.
  size of z_off_t: 8.

Merge request #35 changed the data type for Ulong to int32 instead of Nim's int type. This should be the source of the issue on my machine and other's too, I'm sure.

I'm guessing a solution has to work around the fact that #35 was needed on Windows, without breaking the wrapper for other platforms. Just changing the types back to int or int64 works for me. Additionally, looking at the output from zlibCompileFlags(), there seems to be a mismatch for the ZoffT type too.

@Nindaleth
Copy link

Nindaleth commented May 8, 2019

Is it then a matter of making this change or is something else needed?

when defined(windows):
  type
    Ulong* = uint32
    Ulongf* = uint32
    ZOffT* = int32
else:
  type
    Ulong* = int
    Ulongf* = int
    ZOffT* = int

EDIT: Added the ZOffT*, too. I don't have any Windows instalation to verify if the 64-bit zlib there matches the type definition.

@Araq
Copy link
Member

Araq commented May 8, 2019

I don't know.

@mrhdias
Copy link

mrhdias commented May 16, 2019

Is it then a matter of making this change or is something else needed?

when defined(windows):
  type
    Ulong* = uint32
    Ulongf* = uint32
    ZOffT* = int32
else:
  type
    Ulong* = int
    Ulongf* = int
    ZOffT* = int

EDIT: Added the ZOffT*, too. I don't have any Windows instalation to verify if the 64-bit zlib there matches the type definition.

I have the same problem, but this solution works on Linux x64.

@ChanderG
Copy link

ChanderG commented Jun 28, 2019

Can confirm that HEAD is not working on Linux with the same error and switching to 07c03b3 (ie removing the commit in #35) solves this issue.

@PMunch
Copy link

PMunch commented Jul 3, 2019

Had this issue as well, the fix from @Nindaleth works for me as well. Could this be applied to master, or is there something holding this back?

krux02 added a commit to krux02/zip that referenced this issue Jul 19, 2019
@krux02 krux02 mentioned this issue Apr 6, 2020
@krux02 krux02 added the wontfix label Apr 8, 2020
@krux02
Copy link
Contributor

krux02 commented Apr 8, 2020

This bug won't be fixed. See: #42 (review)

@Araq
Copy link
Member

Araq commented Apr 10, 2020

It will be fixed but properly.

@snej
Copy link
Contributor

snej commented Jun 19, 2020

See dup #53. (And the dup I would have filed today if I hadn't searched first...) Zlib is pretty much dead in the water on macOS (and iOS, and probably Linux(?)) due to this.

The fix is simply to change the int types to the standard C-based ones, e.g.:

  Uint* = cuint
  Ulong* = culong
  Ulongf* = culong
  Pulongf* = ptr Ulongf

ZOffT is a bit trickier because it looks like the C version is conditionally defined and often ends up as a typedef of off_t (which is weird because off_t is for files, not memory), and off_t varies unpredictably between 32- and 64-bit on different OSs. But that's less critical because there are only a few functions using that.

snej added a commit to snej/zip that referenced this issue Jun 19, 2020
Integer types in zlib.h were not properly translated into Nim. Most
seriously, `Ulong` and `Ulongf` (which are both `unsigned long`)
were translated to `int32`, which is wrong on LP64 systems like
macOS, iOS and I think 64-bit Linux too.

This causes the Nim `ZStream` struct to be much shorter than the real
`z_stream` (88 vs 120 bytes), and that causes the size comparison in
the inflate/deflateInit functions to fail with Z_VERSION_ERROR, which
is actually a good thing because if it kept going it would be smashing
memory, since Nim didn't allocate enough space for its struct.

I've fixed the types to the best of my knowledge, using the standard
C-based types like `cint` and `culong`. The only iffy one is `ZOffT`
which has a complicated conditional definition in C -- I think it's
supposed to match `off_t`, whose size depends not on the ABI but on
whether the filesystem supports 64-bit files... I made it a `clong`
since most systems handle 64-bit files nowadays.

Fixes nim-lang#23, nim-lang#39, nim-lang#53
@krux02
Copy link
Contributor

krux02 commented Jun 22, 2020

@snej

ZOffT is a bit trickier because it looks like the C version is conditionally defined and often ends up as a typedef of off_t (which is weird because off_t is for files, not memory), and off_t varies unpredictably between 32- and 64-bit on different OSs. But that's less critical because there are only a few functions using that.

I had a solution that addressed exactly that. Just take a look at my last comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

9 participants