Skip to content

test_maxint64 fails on 32-bit systems due to assumption that 64-bit fits into "long" #49227

Closed
Closed
test_maxint64 fails on 32-bit systems due to assumption that 64-bit fits into "long"#49227
Assignees
pitrou
Labels
buildThe build process and cross-build

Description

@lkcl
lkcl
mannequin
opened on Jan 17, 2009
BPO 4977
Nosy @loewis, @pitrou

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = 'https://github.com/pitrou'
closed_at = <Date 2009-04-11.08:37:27.928>
created_at = <Date 2009-01-17.22:00:59.054>
labels = ['build']
title = 'test_maxint64 fails on 32-bit systems due to assumption that 64-bit fits into "long"'
updated_at = <Date 2009-04-11.08:37:27.926>
user = 'https://bugs.python.org/lkcl'

bugs.python.org fields:

activity = <Date 2009-04-11.08:37:27.926>
actor = 'pitrou'
assignee = 'pitrou'
closed = True
closed_date = <Date 2009-04-11.08:37:27.928>
closer = 'pitrou'
components = ['Build']
creation = <Date 2009-01-17.22:00:59.054>
creator = 'lkcl'
dependencies = []
files = []
hgrepos = []
issue_num = 4977
keywords = []
message_count = 11.0
messages = ['80052', '80058', '80059', '80236', '80243', '80245', '80247', '80251', '80253', '80277', '85842']
nosy_count = 3.0
nosy_names = ['loewis', 'lkcl', 'pitrou']
pr_nums = []
priority = 'normal'
resolution = 'works for me'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue4977'
versions = ['Python 2.5']

Activity

  1. lkcl commented on Jan 17, 2009

    @lkcl
    lkclmannequin
    on Jan 17, 2009
    MannequinAuthor

    the assumption is made that the result will fit into a PyInt.
    obviously, on a 32-bit system, where SIZEOF_LONG is 4 bytes,
    that ain't happening.

    a complex test would be something like this:

    if len <= 9: it's an int, definitely.
if len > 10: it's a long, definitely.
if len == 10, and first char is a "-", it's an int, definitely
if len == 10, and first char is 5,6,7,8,9, it's a long, definitely.
if len == 10, and first char is 0,1,2,3, it's an int, definitely.
if len == 10, and first char is 4, it _might_ be a long, but it might
also be an int, so... uh... let's assume it's a long!

    and you know what? xxxx that for a game of soldiers: just use
    "if len >= 10" assume it's a long :)

    diff --git a/Modules/cPickle.c b/Modules/cPickle.c
    index 537276c..e56f8e5 100644
    --- a/Modules/cPickle.c
    +++ b/Modules/cPickle.c
    @@ -3143,7 +3143,15 @@ load_int(Unpicklerobject *self)
    errno = 0;
    l = strtol(s, &endptr, 0);

    -   if (errno || (*endptr != '\n') || (endptr[1] != '\0')) {
+   if (errno || (*endptr != '\n') || (endptr[1] != '\0')
+#if SIZEOF_LONG == 4
+        /* integers of 10 chars or over could be bigger than 32-bit.
+         * just keep it simple: 10 or more chars, it goes into
+         * a PyLong.
+         */
+        || (len >= 10)
+#endif
+       ) {
        /* Hm, maybe we've got something long.  Let's try reading
           it as a Python long object. */
        errno = 0;
  2. lkcl
    added
    buildThe build process and cross-build
    on Jan 17, 2009

  3. mdickinson commented on Jan 17, 2009

    @mdickinson
    mdickinson
    on Jan 17, 2009
    Member

    From the included patch, I assume you're talking about a failure of
    test_pickle.

    I'm not seeing any such failure on my (32-bit) system, and the code in
    Modules/cPickle.c looks correct to me. I suspect that the problem is that
    in your setup, strtol is failing to set errno correctly on overflow.

  4. mdickinson commented on Jan 17, 2009

    @mdickinson
    mdickinson
    on Jan 17, 2009
    Member

    You might try replacing the strtol call with a call to PyOS_strtol.
    Please let us know if this works!

  5. mdickinson commented on Jan 20, 2009

    @mdickinson
    mdickinson
    on Jan 20, 2009
    Member

    Closing as invalid.

  6. mdickinson
    closed this as completedon Jan 20, 2009
  7. mdickinson
    added
    invalid
    on Jan 20, 2009

  8. pitrou commented on Jan 20, 2009

    @pitrou
    pitrou
    on Jan 20, 2009
    Member

    Reopening at OP's request.

  9. pitrou
    reopened this on Jan 20, 2009
  10. pitrou
    removed
    invalid
    on Jan 20, 2009

  11. pitrou commented on Jan 20, 2009

    @pitrou
    pitrou
    on Jan 20, 2009
    Member

    Two comments:

    • please be more specific when mentioning a problem (when reading your
      bug report, I had a hard time figuring out what "test_maxint64" could be)
    • I hope you're testing against trunk or at least 2.6, because the 2.5
      branch will only receive security fixes

  12. mdickinson commented on Jan 20, 2009

    @mdickinson
    mdickinson
    on Jan 20, 2009
    Member

    Luke,

    I closed this as invalid because it's not a Python bug: your system's
    strtol is buggy, so your bug report should go elsewhere.

  13. mdickinson commented on Jan 20, 2009

    @mdickinson
    mdickinson
    on Jan 20, 2009
    Member

    Antoine, I leave it to you to decide what to do with this.

    As far as I can see, the only action that one might want to take
    as a result of this issue is to replace strtol with PyOS_strol
    in the codebase (in the struct module, and there's also an
    occurrence in Objects/floatobject.c in the fromhex method).

    I recommend against this though, for a few reasons:

    1. PyOS_strtol is likely slower than strtol.

    2. PyOS_strtol is itself buggy: for example, it incorrectly
      accepts strings like "- 3", with whitespace between the sign
      and the unsigned part. This is the reason that Python accepts
      '- 3' as an input to int; this is a Python bug that can't be
      changed in 2.x for compatibility reasons. It's fixed in 3.x.

    3. There's nothing wrong with the existing code, and changing
      it risks introducing new bugs. It's the platform that's buggy here:
      its strtol method doesn't comply with the C89 standard (or the C99
      standard, for that matter).

    It's true that Python has code to work around problems with standards
    compliance on some platforms, and if this were a supported platform
    then I'd be okay with making the change. As it is, though, I think this
    should be reclosed (as invalid, or won't fix if that makes people feel
    happier).

    BTW, Luke, note that you can still add comments to an issue even after
    the issue has been closed: people subscribed to that issue (via nosy)
    will still see the messages.

  14. mdickinson
    assigned
    pitrou
    on Jan 20, 2009

  15. lkcl commented on Jan 20, 2009

    @lkcl
    lkclmannequin
    on Jan 20, 2009
    MannequinAuthor

    hiya folks,

    lots of comments here. in no particular order:

    1. thanks for reopening the bug

    2. apologies for not being clearer - it's Lib/test/test_testzip.py
      specifically the TestZip64InSmallFiles case that's failing.

    3. it's not yet _clear_ whether strtol is the cause of the
      problem - i haven't yet got round to testing that because
      i'm in the middle of a rebuild trying to get msvcr80
      assemblies to work, and i'll need to back out of that and
      rebuild with msvcrt or msvcrt71 first.

    4. i'm not yet certain as to whether it's the mingw 3.4.5
      _compiler_ that's broken (!) there's some empirical evidence
      suggesting that it might well be, but again, that's not
      yet determined

    5. under wine, strtol goes through to msvcrt, which goes through
      to ntdll, which goes through to the _unix_ system's strtol.
      wine is compiled with -m32, so it _should_ still all be doing
      32-bit stuff, even though i'm using a 64-bit host (debian/amd64).

    on balance, i'd very much appreciate this being kept open so that i
    can keep track of determining what the heck is going on - i have
    about five or six different things to investigate.

  16. loewis commented on Jan 20, 2009

    @loewis
    loewismannequin
    on Jan 20, 2009
    Mannequin

    i'd very much appreciate this being kept open so that i
    can keep track of determining what the heck is going on

    Please understand that the purpose of the bug tracker is not to keep
    your personal progress notes. A tracker item, in theory, must be
    formulated such that it can immediately be responded to. If not, we may
    ask for further information (closing it when it is not forthcoming), or
    close it right aways as invalid.

    I'm tempted to close this report immediately as completely confused and
    lacking focus. Please clarify immediately

    a) what action you are performing
    b) what is happening
    c) what you expect to happen instead
    d) why you believe this is a problem with Python (and not, say, with
    your target system)

    Please be *EXTREMELY* cautious when using the bug tracker, as any
    message you post will consume somebody else's time.

  17. pitrou commented on Apr 11, 2009

    @pitrou
    pitrou
    on Apr 11, 2009
    Member

    Closing since I haven't seen any sign of such failures on a 64-bit build.

  18. pitrou
    closed this as completedon Apr 11, 2009
  19. ezio-melotti
    transferred this issue fromon Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

buildThe build process and cross-build

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions