This is an archive of the discontinued Mercurial Phabricator instance.

Differential D6196

cext: make revlog.c PY_SSIZE_T_CLEAN
ClosedPublic

Authored by indygreg on Apr 4 2019, 6:31 PM.

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHGb01bbb8ff1f2: cext: make revlog.c PY_SSIZE_T_CLEAN
Summary

Without this, Python 3.8 emits a deprecation warning, as using
int for # values is deprecated. Many existing modules use
PY_SSIZE_T_CLEAN, so this shouldn't be contentious.

I audited the file for all # formatters and verified we are
using Py_ssize_t everywhere now.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

indygreg created this revision.Apr 4 2019, 6:31 PM
pulkit accepted this revision.Apr 5 2019, 7:37 AM
Closed by commit rHGb01bbb8ff1f2: cext: make revlog.c PY_SSIZE_T_CLEAN (authored by indygreg). · Explain WhyApr 5 2019, 7:57 AM
This revision was automatically updated to reflect the committed changes.
mharbison72 added a subscriber: mharbison72.Apr 8 2019, 12:32 PM

I don't have time to look into this, but this makes the mac builds very unhappy. (The parent runs fine.) Somehow the bot is still running, but recording various segfaults. When I tried to investigate, the build process fails because python hg version is segfaulting. Here's the output of that, with the initial python frames removed:

Process:               Python [45726]
Path:                  /Library/Frameworks/Python.framework/Versions/2.7/Resources/Python.app/Contents/MacOS/Python
Identifier:            Python
Version:               2.7.15 (2.7.15)
Code Type:             X86-64 (Native)
Parent Process:        ??? [45575]
Responsible:           Python [45726]
User ID:               501

Date/Time:             2019-04-08 12:18:07.791 -0400
OS Version:            Mac OS X 10.13.4 (17E202)
Report Version:        12
Anonymous UUID:        6CDCE0B6-3B02-6776-850A-1BB3329186FD


Time Awake Since Boot: 1500000 seconds

System Integrity Protection: enabled

Crashed Thread:        0  Dispatch queue: com.apple.main-thread

Exception Type:        EXC_BAD_ACCESS (SIGSEGV)
Exception Codes:       KERN_INVALID_ADDRESS at 0x0000000106656ffc
Exception Note:        EXC_CORPSE_NOTIFY

Termination Signal:    Segmentation fault: 11
Termination Reason:    Namespace SIGNAL, Code 0xb
Terminating Process:   exc handler [0]

VM Regions Near 0x106656ffc:
    MALLOC_SMALL           0000000102800000-0000000103000000 [ 8192K] rw-/rwx SM=PRV  
--> 
    __TEXT                 0000000106657000-00000001066a2000 [  300K] r-x/rwx SM=COW  A� [/usr/lib/dyld]

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   libsystem_platform.dylib      	0x00007fff796655e6 _platform_memmove$VARIANT$Nehalem + 486
1   org.python.python             	0x0000000100068694 PyString_FromStringAndSize + 260
2   org.python.python             	0x00000001000ea4f1 do_mkvalue + 1329
3   org.python.python             	0x00000001000eb3a3 do_mktuple + 115
4   org.python.python             	0x00000001000eb761 va_build_value + 737
5   org.python.python             	0x00000001000eb837 _Py_BuildValue_SizeT + 167
6   parsers.so                    	0x00000001007de05a revlog_module_init + 218 (revlog.c:3020)
7   parsers.so                    	0x00000001007dacf2 initparsers + 258 (parsers.c:688)
8   org.python.python             	0x00000001000e4f21 _PyImport_LoadDynamicModule + 177
9   org.python.python             	0x00000001000e337b import_submodule + 315
10  org.python.python             	0x00000001000e383a load_next + 234
11  org.python.python             	0x00000001000e3b33 PyImport_ImportModuleLevel + 339
...
94  org.python.python             	0x000000010010803d Py_Main + 3101
95  org.python.python             	0x0000000100000f14 0x100000000 + 3860

Thread 0 crashed with X86 Thread State (64-bit):
  rax: 0x00000001066f5024  rbx: 0x00000001066f5000  rcx: 0x0000000000000008  rdx: 0x0000000005e712cc
  rdi: 0x000000010c566330  rsi: 0x000000010665703c  rbp: 0x00007ffeefbfa0f0  rsp: 0x00007ffeefbfa0f0
   r8: 0x0000000003000001   r9: 0x0000000000000003  r10: 0x0000000000000000  r11: 0x00000002066f5038
  r12: 0x0000000100000014  r13: 0x0000000100000014  r14: 0x00000001007e5d30  r15: 0x00000000007e5000
  rip: 0x00007fff796655e6  rfl: 0x0000000000010206  cr2: 0x0000000106656ffc
  
Logical CPU:     2
Error Code:      0x00000004
Trap Number:     14
yuja added a subscriber: yuja.Apr 9 2019, 9:04 AM
4   org.python.python             	0x00000001000eb761 va_build_value + 737
5   org.python.python             	0x00000001000eb837 _Py_BuildValue_SizeT + 167

Maybe we shouldn't trust the Python doc too much, which says 's#' of
Py_BuildValue() takes an int.

https://docs.python.org/2/c-api/arg.html#c.Py_BuildValue

Can you test if this patch fixes the problem?

diff --git a/mercurial/cext/parsers.c b/mercurial/cext/parsers.c
--- a/mercurial/cext/parsers.c
+++ b/mercurial/cext/parsers.c
@@ -184,7 +184,8 @@ static PyObject *parse_dirstate(PyObject
 		goto quit;
 	}
 
-	parents = Py_BuildValue(PY23("s#s#", "y#y#"), str, 20, str + 20, 20);
+	parents = Py_BuildValue(PY23("s#s#", "y#y#"), str, (Py_ssize_t)20,
+	                        str + 20, (Py_ssize_t)20);
 	if (!parents) {
 		goto quit;
 	}
diff --git a/mercurial/cext/revlog.c b/mercurial/cext/revlog.c
--- a/mercurial/cext/revlog.c
+++ b/mercurial/cext/revlog.c
@@ -366,7 +366,7 @@ static PyObject *index_get(indexObject *
 
 	entry = Py_BuildValue(tuple_format, offset_flags, comp_len, uncomp_len,
 	                      base_rev, link_rev, parent_1, parent_2, c_node_id,
-	                      20);
+	                      (Py_ssize_t)20);
 
 	if (entry) {
 		PyObject_GC_UnTrack(entry);
@@ -3017,8 +3017,9 @@ void revlog_module_init(PyObject *mod)
 	PyModule_AddObject(mod, "nodetree", (PyObject *)&nodetreeType);
 
 	if (!nullentry) {
-		nullentry = Py_BuildValue(PY23("iiiiiiis#", "iiiiiiiy#"), 0, 0,
-		                          0, -1, -1, -1, -1, nullid, 20);
+		nullentry =
+		    Py_BuildValue(PY23("iiiiiiis#", "iiiiiiiy#"), 0, 0, 0, -1,
+		                  -1, -1, -1, nullid, (Py_ssize_t)20);
 	}
 	if (nullentry)
 		PyObject_GC_UnTrack(nullentry);
mharbison72 added a comment.Apr 9 2019, 12:51 PM
In D6196#90521, @yuja wrote:
4   org.python.python             	0x00000001000eb761 va_build_value + 737
5   org.python.python             	0x00000001000eb837 _Py_BuildValue_SizeT + 167

Maybe we shouldn't trust the Python doc too much, which says 's#' of
Py_BuildValue() takes an int.
https://docs.python.org/2/c-api/arg.html#c.Py_BuildValue
Can you test if this patch fixes the problem?

diff --git a/mercurial/cext/parsers.c b/mercurial/cext/parsers.c
--- a/mercurial/cext/parsers.c
+++ b/mercurial/cext/parsers.c
@@ -184,7 +184,8 @@ static PyObject *parse_dirstate(PyObject
 		goto quit;
 	}
-	parents = Py_BuildValue(PY23("s#s#", "y#y#"), str, 20, str + 20, 20);
+	parents = Py_BuildValue(PY23("s#s#", "y#y#"), str, (Py_ssize_t)20,
+	                        str + 20, (Py_ssize_t)20);
 	if (!parents) {
 		goto quit;
 	}
diff --git a/mercurial/cext/revlog.c b/mercurial/cext/revlog.c
--- a/mercurial/cext/revlog.c
+++ b/mercurial/cext/revlog.c
@@ -366,7 +366,7 @@ static PyObject *index_get(indexObject *
 	entry = Py_BuildValue(tuple_format, offset_flags, comp_len, uncomp_len,
 	                      base_rev, link_rev, parent_1, parent_2, c_node_id,
-	                      20);
+	                      (Py_ssize_t)20);
 	if (entry) {
 		PyObject_GC_UnTrack(entry);
@@ -3017,8 +3017,9 @@ void revlog_module_init(PyObject *mod)
 	PyModule_AddObject(mod, "nodetree", (PyObject *)&nodetreeType);
 	if (!nullentry) {
-		nullentry = Py_BuildValue(PY23("iiiiiiis#", "iiiiiiiy#"), 0, 0,
-		                          0, -1, -1, -1, -1, nullid, 20);
+		nullentry =
+		    Py_BuildValue(PY23("iiiiiiis#", "iiiiiiiy#"), 0, 0, 0, -1,
+		                  -1, -1, -1, nullid, (Py_ssize_t)20);
 	}
 	if (nullentry)
 		PyObject_GC_UnTrack(nullentry);

That cleared the whole test suite, thanks.

Revision Contents
Changeset List

PathPackages
Mmercurial/cext/revlog.c (3 lines)

Diff 14669

mercurial/cext/revlog.c

/* /*
parsers.c - efficient content parsing parsers.c - efficient content parsing
Copyright 2008 Matt Mackall <mpm@selenic.com> and others Copyright 2008 Matt Mackall <mpm@selenic.com> and others
This software may be used and distributed according to the terms of This software may be used and distributed according to the terms of
the GNU General Public License, incorporated herein by reference. the GNU General Public License, incorporated herein by reference.
*/ */
#define PY_SSIZE_T_CLEAN
#include <Python.h> #include <Python.h>
#include <assert.h> #include <assert.h>
#include <ctype.h> #include <ctype.h>
#include <limits.h> #include <limits.h>
#include <stddef.h> #include <stddef.h>
#include <stdlib.h> #include <stdlib.h>
#include <string.h> #include <string.h>
self->ntrev = -1; self->ntrev = -1;
} }
return 0; return 0;
} }
static PyObject *index_partialmatch(indexObject *self, PyObject *args) static PyObject *index_partialmatch(indexObject *self, PyObject *args)
{ {
const char *fullnode; const char *fullnode;
int nodelen; Py_ssize_t nodelen;
char *node; char *node;
int rev, i; int rev, i;
if (!PyArg_ParseTuple(args, PY23("s#", "y#"), &node, &nodelen)) if (!PyArg_ParseTuple(args, PY23("s#", "y#"), &node, &nodelen))
return NULL; return NULL;
if (nodelen < 1) { if (nodelen < 1) {
PyErr_SetString(PyExc_ValueError, "key too short"); PyErr_SetString(PyExc_ValueError, "key too short");