Page MenuHomePhabricator

cext: make revlog.c PY_SSIZE_T_CLEAN
ClosedPublic

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

Details

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
This revision was automatically updated to reflect the committed changes.

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);
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.