8000 bpo-37540: vectorcall: keyword names must be strings (GH-14682) · python/cpython@0567786 · GitHub
[go: up one dir, main page]
Skip to content

Commit 0567786

Browse files
jdemeyermiss-islington
jdemeyer
authored and
miss-islington
committed
bpo-37540: vectorcall: keyword names must be strings (GH-14682)
The fact that keyword names are strings is now part of the vectorcall and `METH_FASTCALL` protocols. The biggest concrete change is that `_PyStack_UnpackDict` now checks that and raises `TypeError` if not. CC @markshannon @vstinner https://bugs.python.org/issue37540
1 parent f3cb68f commit 0567786

File tree

10 files changed

+43
-46
lines changed

10 files changed

+43
-46
lines changed

‎Doc/c-api/object.rst

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -400,8 +400,8 @@ Object Protocol
400400
:c:func:`PyVectorcall_NARGS(nargsf) <PyVectorcall_NARGS>`.
401401
402402
*kwnames* can be either NULL (no keyword arguments) or a tuple of keyword
403-
names. In the latter case, the values of the keyword arguments are stored
404-
in *args* after the positional arguments.
403+
names, which must be strings. In the latter case, the values of the keyword
404+
arguments are stored in *args* after the positional arguments.
405405
The number of keyword arguments does not influence *nargsf*.
406406
407407
*kwnames* must contain only objects of type ``str`` (not a subclass),

‎Doc/c-api/structures.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,7 @@ also keyword arguments. So there are a total of 6 calling conventions:
204204
Keyword arguments are passed the same way as in the vectorcall protocol:
205205
there is an additional fourth :c:type:`PyObject\*` parameter
206206
which is a tuple representing the names of the keyword arguments
207+
(which are guaranteed to be strings)
207208
or possibly *NULL* if there are no keywords. The values of the keyword
208209
arguments are stored in the *args* array, after the positional arguments.
209210

‎Doc/library/dis.rst

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1142,8 +1142,10 @@ All of the following opcodes use their arguments.
11421142

11431143
Calls a callable object with positional (if any) and keyword arguments.
11441144
*argc* indicates the total number of positional and keyword arguments.
1145-
The top element on the stack contains a tuple of keyword argument names.
1146-
Below that are keyword arguments in the order corresponding to the tuple.
1145+
The top element on the stack contains a tuple with the names of the
1146+
keyword arguments, which must be strings.
1147+
Below that are the values for the keyword arguments,
1148+
in the order corresponding to the tuple.
11471149
Below that are positional arguments, with the right-most parameter on
11481150
top. Below the arguments is a callable object to call.
11491151
``CALL_FUNCTION_KW`` pops all arguments and the callable object off the stack,

‎Include/cpython/abstract.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,7 @@ _PyVectorcall_Function(PyObject *callable)
8888
of keyword arguments does not change nargsf). kwnames can also be NULL if
8989
there are no keyword arguments.
9090
91-
keywords must only contains str strings (no subclass), and all keys must
92-
be unique.
91+
keywords must only contain strings and all keys must be unique.
9392
9493
Return the result on success. Raise an exception and return NULL on
9594
error. */

‎Lib/test/test_extcall.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@
237237
>>> f(**{1:2})
238238
T A3E2 raceback (most recent call last):
239239
...
240-
TypeError: f() keywords must be strings
240+
TypeError: keywords must be strings
241241
242242
>>> h(**{'e': 2})
243243
Traceback (most recent call last):

‎Lib/test/test_unpack_ex.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@
256256
>>> f(**{1: 3}, **{1: 5})
257257
Traceback (most recent call last):
258258
...
259-
TypeError: f() keywords must be strings
259+
TypeError: f() got multiple values for keyword argument '1'
260260
261261
Unpacking non-sequence
262262

‎Misc/NEWS.d/next/C API/2019-07-10-12-27-28.bpo-37540.E8Z773.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
The vectorcall protocol now requires that the caller passes only strings as
2+
keyword names.

‎Objects/call.c

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -322,8 +322,7 @@ _PyFunction_Vectorcall(PyObject *func, PyObject* const* stack,
322322
assert(nargs >= 0);
323323
assert(kwnames == NULL || PyTuple_CheckExact(kwnames));
324324
assert((nargs == 0 && nkwargs == 0) || stack != NULL);
325-
/* kwnames must only contains str strings, no subclass, and all keys must
326-
be unique */
325+
/* kwnames must only contain strings and all keys must be unique */
327326

328327
if (co->co_kwonlyargcount == 0 && nkwargs == 0 &&
329328
(co->co_flags & ~PyCF_MASK) == (CO_OPTIMIZED | CO_NEWLOCALS | CO_NOFREE))
@@ -943,12 +942,12 @@ _PyStack_AsDict(PyObject *const *values, PyObject *kwnames)
943942
vector; return NULL with exception set on error. Return the keyword names
944943
tuple in *p_kwnames.
945944
946-
The newly allocated argument vector supports PY_VECTORCALL_ARGUMENTS_OFFSET.
945+
This also checks that all keyword names are strings. If not, a TypeError is
946+
raised.
947947
948-
When done, you must call _PyStack_UnpackDict_Free(stack, nargs, kwnames)
948+
The newly allocated argument vector supports PY_VECTORCALL_ARGUMENTS_OFFSET.
949949
950-
The type of keyword keys is not checked, these checks should be done
951-
later (ex: _PyArg_ParseStackAndKeywords). */
950+
When done, you must call _PyStack_UnpackDict_Free(stack, nargs, kwnames) */
952951
static PyObject *const *
953952
_PyStack_UnpackDict(PyObject *const *args, Py_ssize_t nargs, PyObject *kwargs,
954953
PyObject **p_kwnames)
@@ -994,14 +993,28 @@ _PyStack_UnpackDict(PyObject *const *args, Py_ssize_t nargs, PyObject *kwargs,
994993
called in the performance critical hot code. */
995994
Py_ssize_t pos = 0, i = 0;
996995
PyObject *key, *value;
996+
unsigned long keys_are_strings = Py_TPFLAGS_UNICODE_SUBCLASS;
997997
while (PyDict_Next(kwargs, &pos, &key, &value)) {
998+
keys_are_strings &= Py_TYPE(key)->tp_flags;
998999
Py_INCREF(key);
9991000
Py_INCREF(value);
10001001
PyTuple_SET_ITEM(kwnames, i, key);
10011002
kwstack[i] = value;
10021003
i++;
10031004
}
10041005

1006+
/* keys_are_strings has the value Py_TPFLAGS_UNICODE_SUBCLASS if that
1007+
* flag is set for all keys. Otherwise, keys_are_strings equals 0.
1008+
* We do this check once at the end instead of inside the loop above
1009+
* because it simplifies the deallocation in the failing case.
1010+
* It happens to also make the loop above slightly more efficient. */
1011+
if (!keys_are_strings) {
1012+
PyErr_SetString(PyExc_TypeError,
1013+
"keywords must be strings");
1014+
_PyStack_UnpackDict_Free(stack, nargs, kwnames);
1015+
return NULL;
1016+
}
1017+
10051018
*p_kwnames = kwnames;
10061019
return stack;
10071020
}

‎Python/ceval.c

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3504,7 +3504,9 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag)
35043504
PyObject **sp, *res, *names;
35053505

35063506
names = POP();
3507-
assert(PyTuple_CheckExact(names) && PyTuple_GET_SIZE(names) <= oparg);
3507+
assert(PyTuple_Check(names));
3508+
assert(PyTuple_GET_SIZE(names) <= oparg);
3509+
/* We assume without checking that names contains only strings */
35083510
sp = stack_pointer;
35093511
res = call_function(tstate, &sp, oparg, names);
35103512
stack_pointer = sp;
@@ -5372,20 +5374,12 @@ format_kwargs_error(PyThreadState *tstate, PyObject *func, PyObject *kwargs)
53725374
_PyErr_Fetch(tstate, &exc, &val, &tb);
53735375
if (val && PyTuple_Check(val) && PyTuple_GET_SIZE(val) == 1) {
53745376
PyObject *key = PyTuple_GET_ITEM(val, 0);
5375-
if (!PyUnicode_Check(key)) {
5376-
_PyErr_Format(tstate, PyExc_TypeError,
5377-
"%.200s%.200s keywords must be strings",
5378-
PyEval_GetFuncName(func),
5379-
PyEval_GetFuncDesc(func));
5380-
}
5381-
else {
5382-
_PyErr_Format(tstate, PyExc_TypeError,
5383-
"%.200s%.200s got multiple "
5384-
"values for keyword argument '%U'",
5385-
PyEval_GetFuncName(func),
5386-
PyEval_GetFuncDesc(func),
5387-
key);
5388-
}
5377+
_PyErr_Format(tstate, PyExc_TypeError,
5378+
"%.200s%.200s got multiple "
5379+
"values for keyword argument '%S'",
5380+
PyEval_GetFuncName(func),
5381+
PyEval_GetFuncDesc(func),
5382+
key);
53895383
Py_XDECREF(exc);
53905384
Py_XDECREF(val);
53915385
Py_XDECREF(tb);

‎Python/getargs.c

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2043,11 +2043,7 @@ find_keyword(PyObject *kwnames, PyObject *const *kwstack, PyObject *key)
20432043
if (kwname == key) {
20442044
return kwstack[i];
20452045
}
2046-
if (!PyUnicode_Check(kwname)) {
2047-
/* ignore non-string keyword keys:
2048-
an error will be raised below */
2049-
continue;
2050-
}
2046+
assert(PyUnicode_Check(kwname));
20512047
if (_PyUnicode_EQ(kwname, key)) {
20522048
return kwstack[i];
20532049
}
@@ -2275,16 +2271,11 @@ vgetargskeywordsfast_impl(PyObject *const *args, Py_ssize_t nargs,
22752271
j++;
22762272
}
22772273

2278-
if (!PyUnicode_Check(keyword)) {
2279-
PyErr_SetString(PyExc_TypeError,
2280-
"keywords must be strings");
2281-
return cleanreturn(0, &freelist);
2282-
}
22832274
match = PySequence_Contains(kwtuple, keyword);
22842275
if (match <= 0) {
22852276
if (!match) {
22862277
PyErr_Format(PyExc_TypeError,
2287-
"'%U' is an invalid keyword "
2278+
"'%S' is an invalid keyword "
22882279
"argument for %.200s%s",
22892280
keyword,
22902281
(parser->fname == NULL) ? "this function" : parser->fname,
@@ -2505,16 +2496,11 @@ _PyArg_UnpackKeywords(PyObject *const *args, Py_ssize_t nargs,
25052496
j++;
25062497
}
25072498

2508-
if (!PyUnicode_Check(keyword)) {
2509-
PyErr_SetString(PyExc_TypeError,
2510-
"keywords must be strings");
2511-
return NULL;
2512-
}
25132499
match = PySequence_Contains(kwtuple, keyword);
25142500
if (match <= 0) {
25152501
if (!match) {
25162502
PyErr_Format(PyExc_TypeError,
2517-
"'%U' is an invalid keyword "
2503+
"'%S' is an invalid keyword "
25182504
"argument for %.200s%s",
25192505
keyword,
25202506
(parser->fname == NULL) ? "this function" : parser->fname,

0 commit comments

Comments
 (0)
0