OSDir


[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Python-Dev] Python 2.7, long double vs allocator alignment, GCC 8 on x86-64


I'm curious if changing the obmalloc.c ALIGNMENT and ALIGNMENT_SHIFT
defines is sufficient to avoid ABI breakage.

-gps

On Tue, Jan 30, 2018 at 1:20 PM Gregory P. Smith <greg at krypto.org> wrote:

> The proper fix for this in the code would likely break ABI compatibility
> (ie: not possible in python 2.7 or any other stable release).
>
> Clang's UBSAN (undefined behavior sanitizer) has been flagging this one
> for a long time.
>
> In Python 3 a double is used instead of long double since 2012 as I did
> some digging at the time:
> https://github.com/python/cpython/commit/e348c8d154cf6342c79d627ebfe89dfe9de23817
>
> -gps
>
> On Tue, Jan 30, 2018 at 10:59 AM Florian Weimer <fw at deneb.enyo.de> wrote:
>
>> I hope this is the right list for this kind of question.  We recently
>> tried to build Python 2.6 with GCC 8, and ran into this issue:
>>
>>   <https://bugzilla.redhat.com/show_bug.cgi?id=1540316>
>>
>> Also quoting for context:
>>
>> | PyInstance_NewRaw contains this code:
>> |
>> |     inst = PyObject_GC_New(PyInstanceObject, &PyInstance_Type);
>> |     if (inst == NULL) {
>> |         Py_DECREF(dict);
>> |         return NULL;
>> |     }
>> |     inst->in_weakreflist = NULL;
>> |     Py_INCREF(klass);
>> |     inst->in_class = (PyClassObject *)klass;
>> |     inst->in_dict = dict;
>> |     _PyObject_GC_TRACK(inst);
>> |
>> | _PyObject_GC_TRACK expands to:
>> |
>> | #define _PyObject_GC_TRACK(o) do { \
>> |     PyGC_Head *g = _Py_AS_GC(o); \
>> |     if (g->gc.gc_refs != _PyGC_REFS_UNTRACKED) \
>> |         Py_FatalError("GC object already tracked"); \
>> | ?
>> |
>> | Via:
>> |
>> | #define _Py_AS_GC(o) ((PyGC_Head *)(o)-1)
>> |
>> | We get to this:
>> |
>> | /* GC information is stored BEFORE the object structure. */
>> | typedef union _gc_head {
>> |     struct {
>> |         union _gc_head *gc_next;
>> |         union _gc_head *gc_prev;
>> |         Py_ssize_t gc_refs;
>> |     } gc;
>> |     long double dummy;  /* force worst-case alignment */
>> | } PyGC_Head;
>> |
>> | PyGC_Head has 16-byte alignment.  The net result is that
>> |
>> |     _PyObject_GC_TRACK(inst);
>> |
>> | promises to the compiler that inst is properly aligned for the
>> | PyGC_Head type, but it is not: PyObject_GC_New returns a pointer which
>> | is only 8-byte-aligned.
>> |
>> | Objects/obmalloc.c contains this:
>> |
>> | /*
>> |  * Alignment of addresses returned to the user. 8-bytes alignment works
>> |  * on most current architectures (with 32-bit or 64-bit address busses).
>> |  * The alignment value is also used for grouping small requests in size
>> |  * classes spaced ALIGNMENT bytes apart.
>> |  *
>> |  * You shouldn't change this unless you know what you are doing.
>> |  */
>> | #define ALIGNMENT               8               /* must be 2^N */
>> | #define ALIGNMENT_SHIFT         3
>> | #define ALIGNMENT_MASK          (ALIGNMENT - 1)
>> |
>> | So either the allocator alignment needs to be increased, or the
>> | PyGC_Head alignment needs to be decreased.
>>
>> Is this a known issue?  As far as I can see, it has not been fixed on
>> the 2.7 branch.
>>
>> (Store merging is a relatively new GCC feature.  Among other things,
>> this means that on x86-64, for sufficiently aligned pointers, vector
>> instructions are used to update multiple struct fields at once.  These
>> vector instructions can trigger alignment traps, similar to what
>> happens on some other architectures for scalars.)
>> _______________________________________________
>> Python-Dev mailing list
>> Python-Dev at python.org
>> https://mail.python.org/mailman/listinfo/python-dev
>> Unsubscribe:
>> https://mail.python.org/mailman/options/python-dev/greg%40krypto.org
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/python-dev/attachments/20180130/37bc015d/attachment.html>