pointer aliasing causes core dump, with workaround

[qbarnes]

BPO	1433886
Nosy	@tim-one, @akuchling, @tiran
Files	typeobject.c.patch: Workaround patch for pointer aliasing problem typeobject.c-2.4.2.patch: Workaround patch for pointer aliasing problem for 2.4.2

^{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 = None
closed_at = <Date 2010-08-26.18:31:54.449>
created_at = <Date 2006-02-17.22:56:44.000>
labels = ['interpreter-core', 'type-crash']
title = 'pointer aliasing causes core dump, with workaround'
updated_at = <Date 2010-08-26.18:31:54.448>
user = 'https://bugs.python.org/qbarnes'

bugs.python.org fields:

activity = <Date 2010-08-26.18:31:54.448>
actor = 'benjamin.peterson'
assignee = 'none'
closed = True
closed_date = <Date 2010-08-26.18:31:54.449>
closer = 'benjamin.peterson'
components = ['Interpreter Core']
creation = <Date 2006-02-17.22:56:44.000>
creator = 'qbarnes'
dependencies = []
files = ['1904', '1905']
hgrepos = []
issue_num = 1433886
keywords = []
message_count = 10.0
messages = ['27563', '27564', '27565', '27566', '27567', '27568', '27569', '27570', '59208', '115005']
nosy_count = 5.0
nosy_names = ['tim.peters', 'akuchling', 'qbarnes', 'christian.heimes', 'BreamoreBoy']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'test needed'
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue1433886'
versions = []

[qbarnes]

When building 2.3.3 on Solaris using Studio 11 and
"-xalias_level=std" to enable ISO C99 pointer aliasing
rules, the interpreter would core dump when running
test_descr2.py. From examining the source, I believe
this problem exists in 2.4.2 as well, but did not
verify it.

I was able to simplify the code to invoke the problem
down to:
====

class C1(object):
   def __new__(cls):
       return super(C1, cls).__new__(cls)

a = C1()

====

I tracked the problem to super_init() in
Objects/typeobject.c.

The local variable "obj" is of type "PyObject *" and
"type" is of type "PyTypeObject *". In this function,
both variables can be pointing to the same object in
memory. Since the pointers are not of compatible
types, the compiler ends up optimizing out a memory
load between Py_INCREF(obj) and Py_INCREF(type) caching
a previously read value. This causes the object
reference counter to only be incremented once instead
of twice. When the object is deallocated, the
interpreter blows up.

A workaround is to cast one of the pointers to the
others type so that the compiler can see the pointer
aliasing potential. This is what I did in the patch.

What I suspect needs to be done as a more global fix is
to discontinue use of the PyObject_VAR_HEAD macro hack.
Casting between structures containing this macro
creates ISO C99 pointer aliasing violations. The
contents of the macro needs to be in its own structure
which is referenced so a compliant compiler can know of
possible aliasing.

[akuchling]

Logged In: YES
user_id=11375

I have the vague impression that Python's code doesn't
conform to strict aliasing, and it therefore must always be
compiled with GCC's -fno-strict-aliasing option or the
equivalent. Your fix might avoid that one problem, but
there might be other subtle bugs.

Assigning to Tim Peters, who will know if the
-fno-strict-aliasing statement is correct.

[qbarnes]

Logged In: YES
user_id=288734

There is no reason why the source code for Python cannot
conform to ISO C99 pointer aliasing rules. If pointers are
being improperly aliased violating the C standard, it's just
bad programming practice.

The only reason to use a switch like GCC's
-fno-strict-aliasing or Solaris' -xalias_level=any is for
old legacy code, not well-programmed, actively maintained code.

[tim-one]

Logged In: YES
user_id=31435

Yes, Python must be compiled with -fno-strict-aliasing (or
moral equivalent).

No, that's not going to change before Python 3000 (if even
then). All Python object structs conceptually extend the
PyObject struct, including (but not limited to) object
structs conceptually extending the PyVarObject struct (which
in turn conceptually extends the PyObject struct). This is
uniquitous in the core source code, and in all 3rd-party
extension modules defining new Python objects in C.

Note that Python doesn't much care about C99. It was
written against K&R C, and went nearly a decade before
requiring C89 (it took that long for C89-conforming
compilers to show up on all interesting Python platforms).
Best guess is that it will never require C99 (because best
guess is that C99-conforming compilers will never show up on
all interesting Python platforms).

That said, I don't see anything to object to in adding
fiddly casting patches to make sure compilers don't try to
exploit the trivial optimization opportunities that assuming
no aliasing in the Python source may allow.

[qbarnes]

Logged In: YES
user_id=288734

It doesn't matter which standard is used, C99 or C89, the
aliasing violation is the same.

An object is being accessed through two different aggregate
type aliases. The language in C99 section 6.5 is the same
in this regards as C89 section 3.3.

(Actually, C99 is slightly looser than C89 since it allows
effective type aliasing which C89 does not have.)

[tim-one]

Logged In: YES
user_id=31435

Sorry, but it still doesn't really matter what any version
of the C standard says here: the /conceptual/ extension of
the PyObject struct by all Python object structs is the
source of the problem, and is ubiquitous both within the
core and throughout countless 3rd-party extension modules.
This isn't realistically fixable before Python 3000. In the
meantime, use of the platform equivalent to gcc's
-fno-strict-aliasing option is necessary.

[qbarnes]

Logged In: YES
user_id=288734

I wasn't demanding the bug be fixed in the present source
base. I understand that's impractical due to the invasive
nature of the PyObject_VAR_HEAD macro trick.

I was just trying to clarify that the trick is an invalid
coding practice even under C89. I didn't want people
assuming it was ok because of them thinking it was only a
problem under C99 and later standards while safe under C89.

[tim-one]

Logged In: YES
user_id=31435

Ah, OK, and I have no argument with that. Well ;-), I'd
state it more strongly: there are > 150 places in the core
that stick PyObject_HEAD at the start of a struct
declaration, and so, e.g., there are potential aliasing
problems even between PyObject* and PyIntObject* pointers.
IOW, PyObject_VAR_HEAD isn't the whole story here (or even
most of it).

[tiran]

Should we fix the general issue in Python 3.0 now that Python 3.0 is
fast apace? CAN we fix it?

[BreamoreBoy]

Can a core dev please look at this now that we're at 3.2 alapha, thanks. Or has it already been fixed?