Skip to content

Commit a726835

Browse files
committed
[GR-80744] Don't crash when memoryview is asked for native buffer of interop object
PullRequest: graalpython/4970
2 parents ca2f2e3 + 2bc6805 commit a726835

15 files changed

Lines changed: 465 additions & 320 deletions

File tree

‎graalpython/com.oracle.graal.python.cext/src/memoryobject.c‎

Lines changed: 6 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -186,9 +186,9 @@ PyTypeObject _PyManagedBuffer_Type = {
186186

187187
/* In the process of breaking reference cycles mbuf_release() can be
188188
called before memory_release(). */
189-
// GraalPy change: don't use the manager
189+
// GraalPy change: upcall
190190
#define BASE_INACCESSIBLE(mv) \
191-
(((PyMemoryViewObject *)mv)->flags&_Py_MEMORYVIEW_RELEASED)
191+
GraalPyPrivate_MemoryViewIsReleased((PyObject *)(mv))
192192

193193
#define CHECK_RELEASED(mv) \
194194
if (BASE_INACCESSIBLE(mv)) { \
@@ -1513,88 +1513,13 @@ memoryview_toreadonly_impl(PyMemoryViewObject *self)
15131513
Py_LOCAL_SYMBOL int // GraalPy change: hidden for capi.c
15141514
memory_getbuf(PyMemoryViewObject *self, Py_buffer *view, int flags)
15151515
{
1516-
Py_buffer *base = &self->view;
1517-
int baseflags = self->flags;
1518-
1519-
CHECK_RELEASED_INT(self);
1520-
CHECK_RESTRICTED_INT(self);
1521-
1522-
/* start with complete information */
1523-
*view = *base;
1524-
view->obj = NULL;
1525-
1526-
if (REQ_WRITABLE(flags) && base->readonly) {
1527-
PyErr_SetString(PyExc_BufferError,
1528-
"memoryview: underlying buffer is not writable");
1529-
return -1;
1530-
}
1531-
if (!REQ_FORMAT(flags)) {
1532-
/* NULL indicates that the buffer's data type has been cast to 'B'.
1533-
view->itemsize is the _previous_ itemsize. If shape is present,
1534-
the equality product(shape) * itemsize = len still holds at this
1535-
point. The equality calcsize(format) = itemsize does _not_ hold
1536-
from here on! */
1537-
view->format = NULL;
1538-
}
1539-
1540-
if (REQ_C_CONTIGUOUS(flags) && !MV_C_CONTIGUOUS(baseflags)) {
1541-
PyErr_SetString(PyExc_BufferError,
1542-
"memoryview: underlying buffer is not C-contiguous");
1543-
return -1;
1544-
}
1545-
if (REQ_F_CONTIGUOUS(flags) && !MV_F_CONTIGUOUS(baseflags)) {
1546-
PyErr_SetString(PyExc_BufferError,
1547-
"memoryview: underlying buffer is not Fortran contiguous");
1548-
return -1;
1549-
}
1550-
if (REQ_ANY_CONTIGUOUS(flags) && !MV_ANY_CONTIGUOUS(baseflags)) {
1551-
PyErr_SetString(PyExc_BufferError,
1552-
"memoryview: underlying buffer is not contiguous");
1553-
return -1;
1554-
}
1555-
if (!REQ_INDIRECT(flags) && (baseflags & _Py_MEMORYVIEW_PIL)) {
1556-
PyErr_SetString(PyExc_BufferError,
1557-
"memoryview: underlying buffer requires suboffsets");
1558-
return -1;
1559-
}
1560-
if (!REQ_STRIDES(flags)) {
1561-
if (!MV_C_CONTIGUOUS(baseflags)) {
1562-
PyErr_SetString(PyExc_BufferError,
1563-
"memoryview: underlying buffer is not C-contiguous");
1564-
return -1;
1565-
}
1566-
view->strides = NULL;
1567-
}
1568-
if (!REQ_SHAPE(flags)) {
1569-
/* PyBUF_SIMPLE or PyBUF_WRITABLE: at this point buf is C-contiguous,
1570-
so base->buf = ndbuf->data. */
1571-
if (view->format != NULL) {
1572-
/* PyBUF_SIMPLE|PyBUF_FORMAT and PyBUF_WRITABLE|PyBUF_FORMAT do
1573-
not make sense. */
1574-
PyErr_Format(PyExc_BufferError,
1575-
"memoryview: cannot cast to unsigned bytes if the format flag "
1576-
"is present");
1577-
return -1;
1578-
}
1579-
/* product(shape) * itemsize = len and calcsize(format) = itemsize
1580-
do _not_ hold from here on! */
1581-
view->ndim = 1;
1582-
view->shape = NULL;
1583-
}
1584-
1585-
1586-
view->obj = Py_NewRef(self);
1587-
self->exports++;
1588-
1589-
return 0;
1516+
return GraalPyPrivate_MemoryViewGetBuffer((PyObject *)self, view, flags);
15901517
}
15911518

15921519
Py_LOCAL_SYMBOL void // GraalPy change: hidden for capi.c
15931520
memory_releasebuf(PyMemoryViewObject *self, Py_buffer *view)
15941521
{
1595-
self->exports--;
1596-
return;
1597-
/* PyBuffer_Release() decrements view->obj after this function returns. */
1522+
GraalPyPrivate_MemoryViewReleaseBuffer((PyObject *)self, view);
15981523
}
15991524

16001525
#if 0 // GraalPy change
@@ -3525,11 +3450,11 @@ GraalPyPrivate_ReleaseBuffer(Py_buffer* buffer)
35253450
Py_buffer *
35263451
GraalPyMemoryView_GET_BUFFER(PyObject *op)
35273452
{
3528-
return &_PyMemoryView_CAST(op)->view;
3453+
return GraalPyPrivate_MemoryViewBuffer(op);
35293454
}
35303455

35313456
PyObject *
35323457
GraalPyMemoryView_GET_BASE(PyObject *op)
35333458
{
3534-
return _PyMemoryView_CAST(op)->view.obj;
3459+
return GraalPyPrivate_MemoryViewBase(op);
35353460
}

‎graalpython/com.oracle.graal.python.test/src/com/oracle/graal/python/test/UnavailableFrameLocationTest.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -108,14 +108,14 @@ assert list(proxy) == []
108108
ReadFrameNode.AllPythonFramesSelector.INSTANCE, 0, CallerFlags.ALL_FRAME_FLAGS);
109109
assertNotNull(result);
110110
MaterializeFrameNode materialize = MaterializeFrameNode.getUncached();
111-
assertThrows(AssertionError.class, () -> materialize.execute(null, false, false, result.frame()));
111+
assertThrows(AssertionError.class, () -> materialize.execute(null, false, 0, result.frame()));
112112
BytecodeFrame captured = null;
113113
if (captureFirst) {
114-
captured = materialize.execute(result.callNode(), true, true, result.frame()).getBytecodeFrame();
114+
captured = materialize.execute(result.callNode(), true, CallerFlags.NEEDS_LOCALS, result.frame()).getBytecodeFrame();
115115
assertNotNull(captured);
116116
}
117117
Node unavailable = PythonLanguage.get(null).unavailableSafepointLocation;
118-
PFrame pyFrame = materialize.execute(unavailable, true, true, result.frame());
118+
PFrame pyFrame = materialize.execute(unavailable, true, CallerFlags.NEEDS_LOCALS, result.frame());
119119
assertNull(pyFrame.getBytecodeNode());
120120
assertEquals(-1, pyFrame.getBci());
121121
assertEquals(-1, pyFrame.getLine());
@@ -126,7 +126,7 @@ assert list(proxy) == []
126126
checkLocals.execute(context.asValue(pyFrame), captureFirst);
127127

128128
// A later observation with a valid location must resume normal locals capture.
129-
assertSame(pyFrame, materialize.execute(result.callNode(), true, true, result.frame()));
129+
assertSame(pyFrame, materialize.execute(result.callNode(), true, CallerFlags.NEEDS_LOCALS, result.frame()));
130130
assertNotNull(pyFrame.getBytecodeNode());
131131
assertNotNull(pyFrame.getBytecodeFrame());
132132
assertTrue(pyFrame.hasMaterializedFrame() || pyFrame.syncsLocals());
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
# Copyright (c) 2026, Oracle and/or its affiliates. All rights reserved.
2+
# DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
3+
#
4+
# The Universal Permissive License (UPL), Version 1.0
5+
#
6+
# Subject to the condition set forth below, permission is hereby granted to any
7+
# person obtaining a copy of this software, associated documentation and/or
8+
# data (collectively the "Software"), free of charge and under any and all
9+
# copyright rights in the Software, and any and all patent rights owned or
10+
# freely licensable by each licensor hereunder covering either (i) the
11+
# unmodified Software as contributed to or provided by such licensor, or (ii)
12+
# the Larger Works (as defined below), to deal in both
13+
#
14+
# (a) the Software, and
15+
#
16+
# (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if
17+
# one is included with the Software each a "Larger Work" to which the Software
18+
# is contributed by such licensors),
19+
#
20+
# without restriction, including without limitation the rights to copy, create
21+
# derivative works of, display, perform, and distribute the Software and make,
22+
# use, sell, offer for sale, import, export, have made, and have sold the
23+
# Software and the Larger Work(s), and to sublicense the foregoing rights on
24+
# either these or other terms.
25+
#
26+
# This license is subject to the following condition:
27+
#
28+
# The above copyright notice and either this complete permission notice or at a
29+
# minimum a reference to the UPL must be included in all copies or substantial
30+
# portions of the Software.
31+
#
32+
# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
33+
# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
34+
# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
35+
# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
36+
# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
37+
# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
38+
# SOFTWARE.
39+
40+
from unittest import TestCase, skipUnless
41+
42+
from . import CPyExtType, GRAALPYTHON
43+
44+
if GRAALPYTHON:
45+
import java
46+
from __graalpython__ import is_native
47+
else:
48+
is_native = False
49+
50+
51+
@skipUnless(GRAALPYTHON and not is_native, "requires GraalPy Java interop")
52+
class TestInteropBuffer(TestCase):
53+
@classmethod
54+
def setUpClass(cls):
55+
cls.reader = CPyExtType(
56+
"InteropBufferReader",
57+
"""
58+
static PyObject* read_buffer(PyObject* self, PyObject* obj) {
59+
Py_buffer view;
60+
if (PyObject_GetBuffer(obj, &view, PyBUF_SIMPLE) < 0) {
61+
return NULL;
62+
}
63+
PyObject* result = PyBytes_FromStringAndSize((const char*)view.buf, view.len);
64+
PyBuffer_Release(&view);
65+
return result;
66+
}
67+
""",
68+
tp_methods='{"read_buffer", read_buffer, METH_O, ""}',
69+
)()
70+
71+
def setUp(self):
72+
self.buffer = java.type("java.nio.ByteBuffer").allocate(4)
73+
for i, value in enumerate(b"test"):
74+
self.buffer.put(i, value)
75+
76+
def test_getbuffer(self):
77+
self.assertRaises(TypeError, self.reader.read_buffer, self.buffer)
78+
79+
def test_getbuffer_memoryview(self):
80+
with memoryview(self.buffer) as view:
81+
# memoryview supports interop objects, but sending them to native causes a copy with a warning
82+
self.assertEqual(self.reader.read_buffer(view), b"test")

‎graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_memoryview.py‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -466,6 +466,79 @@ class TestPyMemoryView(CPyExtTestCase):
466466

467467

468468
class TestObject(unittest.TestCase):
469+
def test_memoryview_native_exports(self):
470+
TestType = CPyExtType(
471+
"TestMemoryViewNativeExports",
472+
"""
473+
static PyObject *acquire(PyObject *self, PyObject *args) {
474+
PyObject *obj;
475+
int flags;
476+
if (!PyArg_ParseTuple(args, "Oi", &obj, &flags)) {
477+
return NULL;
478+
}
479+
TestMemoryViewNativeExportsObject *holder = (TestMemoryViewNativeExportsObject *)self;
480+
if (holder->view.obj != NULL) {
481+
PyErr_SetString(PyExc_RuntimeError, "buffer already acquired");
482+
return NULL;
483+
}
484+
if (PyObject_GetBuffer(obj, &holder->view, flags) < 0) {
485+
return NULL;
486+
}
487+
Py_buffer *v = &holder->view;
488+
return Py_BuildValue("nniiii", v->len, v->itemsize, v->ndim,
489+
v->format != NULL, v->shape != NULL, v->strides != NULL);
490+
}
491+
static PyObject *drop(PyObject *self, PyObject *unused) {
492+
PyBuffer_Release(&((TestMemoryViewNativeExportsObject *)self)->view);
493+
Py_RETURN_NONE;
494+
}
495+
static PyObject *read_buffer(PyObject *self, PyObject *unused) {
496+
Py_buffer *v = &((TestMemoryViewNativeExportsObject *)self)->view;
497+
PyObject *result = PyBytes_FromStringAndSize(NULL, v->len);
498+
if (result != NULL && PyBuffer_ToContiguous(PyBytes_AS_STRING(result), v, v->len, 'C') < 0) {
499+
Py_CLEAR(result);
500+
}
501+
return result;
502+
}
503+
""",
504+
cmembers='Py_buffer view;',
505+
tp_methods='''
506+
{"acquire", acquire, METH_VARARGS, ""},
507+
{"drop", drop, METH_NOARGS, ""},
508+
{"read", read_buffer, METH_NOARGS, ""}
509+
''',
510+
)
511+
first, second = TestType(), TestType()
512+
mv = memoryview(bytearray(b'abcdefgh'))[1:7].cast('H', shape=[1, 3])
513+
assert first.acquire(mv, 0x11c) == (6, 2, 2, 1, 1, 1) # PyBUF_FULL_RO
514+
assert second.acquire(mv, 0) == (6, 2, 1, 0, 0, 0) # PyBUF_SIMPLE
515+
assert first.read() == b'bcdefg'
516+
with self.assertRaises(BufferError):
517+
mv.release()
518+
first.drop()
519+
with self.assertRaises(BufferError):
520+
mv.release()
521+
second.drop()
522+
mv.release()
523+
with self.assertRaises(ValueError):
524+
first.acquire(mv, 0x11c)
525+
526+
for mv, flags in [(memoryview(b'abc'), 1), # PyBUF_WRITABLE
527+
(memoryview(b'abcdef')[::2], 0),
528+
(memoryview(b'abcdef')[::2], 0x38), # PyBUF_C_CONTIGUOUS
529+
(memoryview(b'abcdef')[::2], 0x58), # PyBUF_F_CONTIGUOUS
530+
(memoryview(b'abcdef')[::2], 0x98), # PyBUF_ANY_CONTIGUOUS
531+
(memoryview(b'abc'), 4)]: # PyBUF_SIMPLE | PyBUF_FORMAT
532+
with self.assertRaises(BufferError):
533+
first.acquire(mv, flags)
534+
mv.release() # Failed acquisition must not leave an export behind.
535+
536+
mv = memoryview(bytearray(b'abcdef'))[::2]
537+
assert first.acquire(mv, 0x11c) == (3, 1, 1, 1, 1, 1)
538+
assert first.read() == b'ace'
539+
first.drop()
540+
mv.release()
541+
469542
def test_memoryview_fromobject_multidim(self):
470543
TestType = CPyExtType(
471544
"TestMemoryViewMultidim",

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/PythonLanguage.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@
101101
import com.oracle.graal.python.pegparser.sst.StmtTy;
102102
import com.oracle.graal.python.pegparser.sst.TypeParamTy;
103103
import com.oracle.graal.python.pegparser.tokenizer.SourceRange;
104+
import com.oracle.graal.python.runtime.CallerFlags;
104105
import com.oracle.graal.python.runtime.GilNode;
105106
import com.oracle.graal.python.runtime.IndirectCallData.BoundaryCallData;
106107
import com.oracle.graal.python.runtime.IndirectCallData.InteropCallData;
@@ -752,7 +753,7 @@ public ExecutableNode parse(InlineParsingRequest request) {
752753
@Override
753754
public Object execute(VirtualFrame frame) {
754755
Object[] arguments = PArguments.create();
755-
PFrame pFrame = materializeFrameNode.execute(this, false, true, frame);
756+
PFrame pFrame = materializeFrameNode.execute(this, false, CallerFlags.NEEDS_LOCALS, frame);
756757
Object pLocals = getFrameLocalsNode.executeCached(frame, pFrame, true);
757758
PArguments.setSpecialArgument(arguments, pLocals);
758759
PArguments.setCodeObject(arguments, PFactory.createCode(getLanguage(PythonLanguage.class), callTarget));

0 commit comments

Comments
 (0)