[WIP] [GR-57232] C warnigs as errors, take 2.

steve-s · steve-s · commit cce612f99ee7 · 2024-08-24T21:44:12.000Z
PullRequest: graalpython/3442
diff --git a/graalpython/com.oracle.graal.python.cext/CMakeLists.txt b/graalpython/com.oracle.graal.python.cext/CMakeLists.txt
@@ -28,7 +28,6 @@
 #
 cmake_minimum_required(VERSION 3.22)
 project(com.oracle.graal.python.cext)
-set(CMAKE_COMPILE_WARNING_AS_ERROR ON)
 
 function(require_var var)
     if (NOT DEFINED ${var})
@@ -61,7 +60,11 @@ set(TARGET_LIBPYTHON "python-${LLVM_MODE}")
 # common variables and compile/link options (for all build targets)
 ######################################################################
 
-set(CFLAGS_WARNINGS -Wno-int-to-pointer-cast -Wno-int-conversion -Wno-void-pointer-to-int-cast
+set(CFLAGS_WARNINGS -Wall -Werror -Wno-unused-function -Wno-unused-variable -Wno-unused-const-variable
+                    -Wno-tautological-constant-out-of-range-compare   # fileutils.c: wchar_t > MAX_UNICODE is always false on Windows
+                    -Wno-unused-but-set-variable                      # sqlite.c: BOOL bRc
+                    -Wno-ignored-pragmas                              # sre.c: #pragma optimize("agtw", on)
+                    -Wno-int-to-pointer-cast -Wno-int-conversion -Wno-void-pointer-to-int-cast
                     -Wno-incompatible-pointer-types-discards-qualifiers -Wno-pointer-type-mismatch
                     -Wno-braced-scalar-init -Wno-deprecated-declarations)
 
diff --git a/graalpython/com.oracle.graal.python.cext/src/capi.h b/graalpython/com.oracle.graal.python.cext/src/capi.h
@@ -254,7 +254,8 @@ static inline int get_method_flags_wrapper(int flags) {
     return JWRAPPER_UNSUPPORTED;
 }
 
-void register_native_slots(PyTypeObject* managed_class, PyGetSetDef* getsets, PyMemberDef* members);
+// looked up by NFI, so exported
+PyAPI_FUNC(void) register_native_slots(PyTypeObject* managed_class, PyGetSetDef* getsets, PyMemberDef* members);
 
 // export the SizeT arg parse functions, because we use them in contrast to cpython on windows for core modules that we link dynamically
 PyAPI_FUNC(int) _PyArg_Parse_SizeT(PyObject *, const char *, ...);
diff --git a/graalpython/com.oracle.graal.python.cext/src/import.c b/graalpython/com.oracle.graal.python.cext/src/import.c
@@ -2551,7 +2551,10 @@ static struct PyModuleDef imp_module = {
 PyMODINIT_FUNC
 PyInit__imp(void)
 {
-    return PyModuleDef_Init(&imp_module);
+    // GraalPy change: we intrinsify this module in Java, no one should call this directly,
+    // so to be safe we bail out
+    printf("Function PyInit__imp not implemented in GraalPy. The _imp module is available, but not implemented as extension.\n");
+    exit(-1);
 }
 
 
diff --git a/graalpython/com.oracle.graal.python.cext/src/longobject.c b/graalpython/com.oracle.graal.python.cext/src/longobject.c
@@ -1981,7 +1981,10 @@ PyLong_FromString(const char *str, char **pend, int base)
         overflow = 1;
     }
 
-    if (overflow) {
+    if (overflow || (error_if_nonzero && value != 0)) {
+        if (error_if_nonzero) {
+            base = 0;
+        }
         PyObject* string = PyUnicode_FromStringAndSize(numberStart, digits);
         PyObject* result = GraalPyTruffleLong_FromString(string, base, negative);
         Py_DecRef(string);
diff --git a/graalpython/com.oracle.graal.python.cext/src/memoryobject.c b/graalpython/com.oracle.graal.python.cext/src/memoryobject.c
@@ -754,7 +754,6 @@ PyObject *
 PyMemoryView_FromBuffer(const Py_buffer *info)
 {
     // GraalPy change: different implementation
-    Py_ssize_t ndim = info->ndim;
     if (info->buf == NULL) {
         PyErr_SetString(PyExc_ValueError,
             "PyMemoryView_FromBuffer(): info->buf must not be NULL");
@@ -3334,7 +3333,6 @@ PyTruffle_MemoryViewFromObject(PyObject *v, int flags)
         if (PyObject_GetBuffer(v, buffer, flags) < 0) {
             return NULL;
         }
-        Py_ssize_t ndim = buffer->ndim;
         int needs_release = 0;
         if (buffer->obj != NULL) {
             PyBufferProcs *pb;
diff --git a/graalpython/com.oracle.graal.python.cext/src/obmalloc.c b/graalpython/com.oracle.graal.python.cext/src/obmalloc.c
@@ -84,7 +84,6 @@ int PyTruffle_AllocMemory(size_t size) {
 	    PyTruffle_Log(PY_TRUFFLE_LOG_CONFIG, "PyTruffle_AllocMemory: exceeding PyTruffle_NativeMemoryGCBarrier (%lu) with allocation of size %lu, current PyTruffle_AllocatedMemory: %lu\n", PyTruffle_NativeMemoryGCBarrier, size, PyTruffle_AllocatedMemory);
 
 	    size_t delay = 0;
-	    int iteration = 0;
 	    for (int iteration = 0; iteration < MAX_COLLECTION_RETRIES; iteration++) {
 	    	GraalPyTruffle_TriggerGC(delay);
 	    	delay += COLLECTION_DELAY_INCREMENT;
diff --git a/graalpython/com.oracle.graal.python.cext/src/structseq.c b/graalpython/com.oracle.graal.python.cext/src/structseq.c
@@ -532,7 +532,8 @@ _PyStructSequence_InitType(PyTypeObject *type, PyStructSequence_Desc *desc,
 #endif // GraalPy change
 
     if (PyType_Ready(type) < 0) {
-        PyMem_Free(members);
+        // GraalPy change: not initialized
+        // PyMem_Free(members);
         return -1;
     }
     Py_INCREF(type);
diff --git a/graalpython/com.oracle.graal.python.cext/src/tupleobject.c b/graalpython/com.oracle.graal.python.cext/src/tupleobject.c
@@ -114,7 +114,6 @@ PyTuple_GetItem(PyObject *op, Py_ssize_t i) {
         PyErr_BadInternalCall();
         return NULL;
     }
-    PyObject *res;
     PyObject **ob_item;
     if (points_to_py_handle_space(op)) {
         const PyObject *ptr = pointer_to_stub((PyObject *) op);
@@ -1363,7 +1362,6 @@ PyTruffleTuple_GetItems(PyObject *op)
 #ifdef GRAALVM_PYTHON_LLVM_MANAGED
     return GraalPy_get_PyTupleObject_ob_item((PyTupleObject*) op);
 #else /* GRAALVM_PYTHON_LLVM_MANAGED */
-    PyObject *res;
     PyObject **ob_item;
     if (points_to_py_handle_space(op)) {
         GraalPyVarObject *ptr = (GraalPyVarObject *) pointer_to_stub(op);
diff --git a/graalpython/com.oracle.graal.python.cext/src/typeobject.c b/graalpython/com.oracle.graal.python.cext/src/typeobject.c
@@ -5721,7 +5721,7 @@ type_add_members(PyTypeObject *type)
     PyObject *dict = type->tp_dict;
     for (; memb->name != NULL; memb++) {
         // GraalPy change
-        if (GraalPyTruffleType_AddMember(type, type->tp_dict, memb->name, memb->type, memb->offset, (memb->flags & READONLY) == 0, memb->doc) < 0)
+        if (GraalPyTruffleType_AddMember(type, dict, memb->name, memb->type, memb->offset, (memb->flags & READONLY) == 0, memb->doc) < 0)
             return -1;
     }
     return 0;
@@ -5740,7 +5740,7 @@ type_add_getset(PyTypeObject *type)
     for (; gsp->name != NULL; gsp++) {
         // GraalPy change
         if (GraalPyTruffleType_AddGetSet(type,
-                        type->tp_dict,
+                        dict,
                         gsp->name,
                         gsp->get,
                         gsp->set,
diff --git a/graalpython/com.oracle.graal.python.hpy.llvm/CMakeLists.txt b/graalpython/com.oracle.graal.python.hpy.llvm/CMakeLists.txt
@@ -28,7 +28,6 @@
 #
 cmake_minimum_required(VERSION 3.22)
 project(com.oracle.graal.python.hpy.llvm)
-set(CMAKE_COMPILE_WARNING_AS_ERROR ON)
 
 function(require_var var)
   if (NOT DEFINED ${var})
diff --git a/graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_long.py b/graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_long.py
@@ -359,6 +359,8 @@ class TestPyLong(CPyExtTestCase):
     test_PyLong_FromString = CPyExtFunction(
         lambda args: int(args[0], args[1]),
         lambda: (
+            ("00", 0),
+            ("03", 0),
             ("  12 ", 10),
             ("  12abg13 ", 22),
             ("12", 0),
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/CApiFunction.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/CApiFunction.java
@@ -770,7 +770,9 @@ public final class CApiFunction {
     @CApiBuiltin(name = "PyImport_ImportFrozenModule", ret = Int, args = {ConstCharPtrAsTruffleString}, call = NotImplemented)
     @CApiBuiltin(name = "PyImport_ImportFrozenModuleObject", ret = Int, args = {PyObject}, call = NotImplemented)
     @CApiBuiltin(name = "PyImport_ReloadModule", ret = PyObject, args = {PyObject}, call = NotImplemented)
-    @CApiBuiltin(name = "PyInit__imp", ret = PyObject, args = {}, call = NotImplemented)
+    // Not implemented, we just use handwritten stub (in import.c) to preserve the C attributes
+    // (PyMODINIT_FUNC vs PyAPI_FUNC)
+    @CApiBuiltin(name = "PyInit__imp", ret = PyObject, args = {}, call = Ignored)
     @CApiBuiltin(name = "PyInterpreterState_Clear", ret = Void, args = {PyInterpreterState}, call = NotImplemented)
     @CApiBuiltin(name = "PyInterpreterState_Delete", ret = Void, args = {PyInterpreterState}, call = NotImplemented)
     @CApiBuiltin(name = "PyInterpreterState_Get", ret = PyInterpreterState, args = {}, call = NotImplemented)
diff --git a/graalpython/python-liblzma/CMakeLists.txt b/graalpython/python-liblzma/CMakeLists.txt
@@ -80,12 +80,12 @@ target_compile_definitions(${TARGET_LZMASUPPORT} PRIVATE NDEBUG)
 
 if(WIN32)
     target_compile_options(${TARGET_LZMA} PRIVATE /Z7 /O2)
-    target_compile_options(${TARGET_LZMASUPPORT} PRIVATE /Z7 /O2 /WX)
+    target_compile_options(${TARGET_LZMASUPPORT} PRIVATE /Z7 /O2 /W3 /WX)
 else()
     target_compile_options(${TARGET_LZMA} PRIVATE -fPIC -DPIC -g -O2)
     target_compile_definitions(${TARGET_LZMA} PRIVATE PIC)
 
-    target_compile_options(${TARGET_LZMASUPPORT} PRIVATE -g -O3)
+    target_compile_options(${TARGET_LZMASUPPORT} PRIVATE -g -O3 -Wall -Werror)
 endif()
 
 # don't install into the system but into the MX project's output dir
diff --git a/graalpython/python-liblzma/src/lzma.c b/graalpython/python-liblzma/src/lzma.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2021, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * The Universal Permissive License (UPL), Version 1.0
@@ -298,14 +298,6 @@ static off_heap_buffer* lzma_allocate_buffer(size_t items)
     return o;
 }
 
-static off_heap_buffer* lzma_create_copy_buffer(Byte *src, ssize_t len) {
-    off_heap_buffer *dest = lzma_allocate_buffer(len);
-    if (dest && len > 0) {
-        memcpy(dest->buf, src, len);
-    }
-    return dest; 
-}
-
 static void lzma_release_buffer(off_heap_buffer *o) {
     if (!o) {
         return;
@@ -329,14 +321,6 @@ static void lzma_release_buffer(off_heap_buffer *o) {
     free(o);
 }
 
-static off_heap_buffer* lzma_get_ref(off_heap_buffer* o) {
-    if (o) {
-        LOG_FINEST("off_heap_buffer(ref_count: %zu + 1)\n", o->ref_count);
-        o->ref_count++;
-    }
-    return o;
-}
-
 // nfi_function: name('createStream') map('lzmast_stream*', 'POINTER')
 lzmast_stream *lzma_create_lzmast_stream() {
     lzmast_stream *lzmast = (lzmast_stream *) calloc(1, sizeof(lzmast_stream));
@@ -621,7 +605,7 @@ int lzma_set_filter_spec_bcj(lzmast_stream *lzmast, int fidx, int64_t* opts) {
 
 // nfi_function: name('encodeFilter') map('lzmast_stream*', 'POINTER')
 int lzma_encode_filter_spec(lzmast_stream *lzmast, int64_t* opts) {
-    lzma_ret lzret;
+    lzma_ret lzret = LZMA_PROG_ERROR;
     uint32_t encoded_size;
     lzma_filter filter;
 

Original file line number	Diff line number	Diff line change
`@@ -254,7 +254,8 @@ static inline int get_method_flags_wrapper(int flags) {`
`254`	`254`	`return JWRAPPER_UNSUPPORTED;`
`255`	`255`	`}`
`256`	`256`
`257`		`-void register_native_slots(PyTypeObject* managed_class, PyGetSetDef* getsets, PyMemberDef* members);`
	`257`	`+// looked up by NFI, so exported`
	`258`	`+PyAPI_FUNC(void) register_native_slots(PyTypeObject* managed_class, PyGetSetDef* getsets, PyMemberDef* members);`
`258`	`259`
`259`	`260`	`// export the SizeT arg parse functions, because we use them in contrast to cpython on windows for core modules that we link dynamically`
`260`	`261`	`PyAPI_FUNC(int) _PyArg_Parse_SizeT(PyObject , const char , ...);`
Original file line number	Diff line number	Diff line change
`@@ -2551,7 +2551,10 @@ static struct PyModuleDef imp_module = {`
`2551`	`2551`	`PyMODINIT_FUNC`
`2552`	`2552`	`PyInit__imp(void)`
`2553`	`2553`	`{`
`2554`		`- return PyModuleDef_Init(&imp_module);`
	`2554`	`+ // GraalPy change: we intrinsify this module in Java, no one should call this directly,`
	`2555`	`+ // so to be safe we bail out`
	`2556`	`+ printf("Function PyInit__imp not implemented in GraalPy. The _imp module is available, but not implemented as extension.\n");`
	`2557`	`+ exit(-1);`
`2555`	`2558`	`}`
`2556`	`2559`
`2557`	`2560`
Original file line number	Diff line number	Diff line change
`@@ -532,7 +532,8 @@ _PyStructSequence_InitType(PyTypeObject type, PyStructSequence_Desc desc,`
`532`	`532`	`#endif // GraalPy change`
`533`	`533`
`534`	`534`	`if (PyType_Ready(type) < 0) {`
`535`		`- PyMem_Free(members);`
	`535`	`+ // GraalPy change: not initialized`
	`536`	`+ // PyMem_Free(members);`
`536`	`537`	`return -1;`
`537`	`538`	`}`
`538`	`539`	`Py_INCREF(type);`
Original file line number	Diff line number	Diff line change
`@@ -28,7 +28,6 @@`
`28`	`28`	`#`
`29`	`29`	`cmake_minimum_required(VERSION 3.22)`
`30`	`30`	`project(com.oracle.graal.python.hpy.llvm)`
`31`		`-set(CMAKE_COMPILE_WARNING_AS_ERROR ON)`
`32`	`31`
`33`	`32`	`function(require_var var)`
`34`	`33`	`if (NOT DEFINED ${var})`