python / python/cpython

Allocated size miscalculated in ASDL sequence constructor

Open
#129,933 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

interpreter-core topic-parser type-bug
Dominant language
Python
Stars
77.2k
Forks
35.9k
PR merge metrics
PR metrics pending

Description

In the macro generating ASDL sequence constructor in pycore_asdl.h, the extra allocated size is miscalculated as sizeof(TYPE *) * (size - 1), while the correct one should be sizeof(TYPE) * (size - 1):

#define GENERATE_ASDL_SEQ_CONSTRUCTOR(NAME, TYPE) \
asdl_ ## NAME ## _seq *_Py_asdl_ ## NAME ## _seq_new(Py_ssize_t size, PyArena *arena) \
{ \
    asdl_ ## NAME ## _seq *seq = NULL; \
    size_t n; \
    /* check size is sane */ \
    if (size < 0 || \
        (size && (((size_t)size - 1) > (SIZE_MAX / sizeof(void *))))) { \
        PyErr_NoMemory(); \
        return NULL; \
    } \
    n = (size ? (sizeof(TYPE *) * (size - 1)) : 0); \                       // <----------------------------- this line
    /* check if size can be added safely */ \
    if (n > SIZE_MAX - sizeof(asdl_ ## NAME ## _seq)) { \
        PyErr_NoMemory(); \
        return NULL; \
    } \
    n += sizeof(asdl_ ## NAME ## _seq); \
    seq = (asdl_ ## NAME ## _seq *)_PyArena_Malloc(arena, n); \
    if (!seq) { \
        PyErr_NoMemory(); \
        return NULL; \
    } \
    memset(seq, 0, n); \
    seq->size = size; \
    seq->elements = (void**)seq->typed_elements; \
    return seq; \
}

A sample struct definition and usage taken from the source code:

// pycore_asdl.h
#define asdl_seq_GET(S, I) _Py_RVALUE((S)->typed_elements[(I)])

// pycore_ast.h
typedef struct {
    _ASDL_SEQ_HEAD
    expr_ty typed_elements[1];
} asdl_expr_seq;

// Python-ast.c
GENERATE_ASDL_SEQ_CONSTRUCTOR(expr, expr_ty)

// ast.c - validate_exprs
asdl_expr_seq *exprs
expr_ty expr = asdl_seq_GET(exprs, i);

This shows that the stored elements have type TYPE (i.e. expr_ty), not TYPE *. So the extra allocated memory should be sizeof(TYPE) * (size - 1) instead of sizeof(TYPE *) * (size - 1).

This hasn't cause problem yet because in every uses of GENERATE_ASDL_SEQ_CONSTRUCTOR, TYPE is either int or some pointer, so sizeof(TYPE*)'s are not smaller than sizeof(TYPE). But it can cause confusion and potential bug in the future.

Linked PRs
  • gh-129934

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in Include/internal/pycore_asdl.h at GENERATE_ASDL_SEQ_CONSTRUCTOR and review the linked work in PR gh-129934. Verify the allocation calculation matches the typed element storage, then run the relevant CPython test suite or ASDL/AST tests to confirm the constructor remains correct.

Written by the indexing model from the issue text.

Assessment

Tech stack
c
Domain
backend
Issue type
Bug
Difficulty
1/5
Estimated time
Under an hour
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.