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
Objects/longobject.c not compiled without long long #60481
Comments
Preprocessor directives emit error in PyLong_FromVoidPtr: PyLong_FromVoidPtr: sizeof(void*) > sizeof(long), but no long long if HAVE_LONG_LONG not defined. Here is a patch which adds missing branch (lost somewhere in 2->3 translation). Also removed non-needed optimization which can cause undefined behavior (C Standard not guarantee (long long)NULL == 0). |
First part of the patch looks fine to me. For the second part, I don't see any undefined behaviour here. Can you explain where the undefined behaviour comes from? And are you sure that this is really just an optimization? It looks as though it might be deliberately there to make sure that the conversion still produces a Python 0 even on systems where the NULL pointer *doesn't* give 0 when converted to an integer. |
Actually, I think that special case needs to be added to the first branch as well. |
If on some platform (uintptr_t)NULL != 0, then some other address can be What about (Py_uintptr_t)p - (Py_uintptr_t)(void *)NULL? Then we should change |
This doesn't seem very likely, since then the C implementation wouldn't roundtrip when converting that other pointer to an integer and back to a pointer. (C99 6.3.2.3 says that (void *)0 is a null pointer constant.) The code you removed is not undefined behaviour, and is not just an optimization---removing it would change the semantics of PyLong_FromVoidPtr. There may be code that depends on PyLong_FromVoidPtr(NULL) being 0. I believe the code should stay.
What about it? What's the relevance to this issue? |
"0" lexeme is not 0 integer. In expression "(void*)0" "0" means null pointer.
I think it was wrong implementation, even if only in theoretical
This provides the invariant PyLong_FromVoidPtr(NULL) == PyLong_FromLong(0) and |
Ah yes, true. C99 6.3.2.3 specifies an "integer *constant* expression". So I was mistaken in thinking that converting an arbitrary integer-valued expression with value 0 to (void*) must always give a NULL pointer.
Okay, but do you agree that (1) there's no undefined behaviour, and (2) removing this code has the potential to change results on platforms where converting NULL to an integer doesn't give zero? If you agree with those two things, then this is a behaviour change, not a bugfix, and you should open a new issue aimed at 3.4 for that.
What's your justification for this belief?
Sorry, but this is making no sense to me. You wrote: "(Py_uintptr_t)p - (Py_uintptr_t)(void *)NULL". Is that supposed to be existing code that you're proposing changing? An example to prove some point (what point?) Code you're proposing to add somewhere? (If so, where are you proposing to add it?) Why are you subtracting these two pointers? What's p? I'm finding it hard to make sense of what you're writing. |
Undefined behavior (or may be just the wrong behavior), we obtain later, when
Can you show the contrexample? I can not. If such a platform existed, perhaps
I mean using PyLong_FromUnsignedLongLong((unsigned PY_LONG_LONG) |
So maybe the fix should be to special case zero in PyLong_AsVoidPtr, and turn 0L back into NULL there? I think it's just too risky to change the current behaviour in 3.2 and 3.3, given the number of places that PyLong_FromVoidPtr is used. Unlike you, I don't have confidence that there are no current or future platforms that don't map NULL to 0. It might be reasonable to consider a change for 3.4.
Okay, thanks for explaining. |
New changeset bedb2903d71e by Mark Dickinson in branch '3.2': New changeset 8ce04be1321c by Mark Dickinson in branch '3.3': New changeset 02e57069f43e by Mark Dickinson in branch 'default': |
I've fixed the missing branch in 3.2, 3.3 and 3.4. |
Yes, of course. If we have a special case in PyLong_FromVoidPtr(), it is wrong The patch long_fromvoidptr_2.patch contains such changes (as you want). But this changes are buggy too, because now PyLong_FromVoidPtr() and The patch long_fromvoidptr_3.patch is more consistent in this sense. Now both
Buggy behaviour changed if we fix the bug.
I just want to say that I'm not sure whether to fix this bug. But if we fix it |
Let's keep it simple and just drop the special case for 3.4. |
I close this issue as the main bug was fixed. For incidental bug I open bpo-16280. |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: