From f5a45fe8272017cfed2617632e5aa8bb49b15f4c Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sun, 22 Sep 2019 12:34:35 +0300 Subject: [PATCH 1/6] bpo-38249: Expand Py_UNREACHABLE() to __builtin_unreachable() in the release mode. --- Include/pymacro.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Include/pymacro.h b/Include/pymacro.h index c080fb164e353a..8ab593623220ca 100644 --- a/Include/pymacro.h +++ b/Include/pymacro.h @@ -118,6 +118,8 @@ "We've reached an unreachable state. Anything is possible.\n" \ "The limits were in our heads all along. Follow your dreams.\n" \ "https://xkcd.com/2200") +#elif defined(__GNUC__) || defined(__clang__) +#define Py_UNREACHABLE() __builtin_unreachable() #else #define Py_UNREACHABLE() \ Py_FatalError("Unreachable C code path reached") From 30f8a4d62351831ce6e423e6f12718fe52429046 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sun, 8 Mar 2020 20:43:41 +0200 Subject: [PATCH 2/6] Handle non-unreachable cases. --- Modules/_tracemalloc.c | 2 +- Objects/abstract.c | 15 ++++++--------- Objects/stringlib/eq.h | 9 +++++---- Python/formatter_unicode.c | 2 +- Python/peephole.c | 6 +++++- Python/pytime.c | 9 ++++----- 6 files changed, 22 insertions(+), 21 deletions(-) diff --git a/Modules/_tracemalloc.c b/Modules/_tracemalloc.c index ddf6ef4e11dd4f..17b7a29ee57e60 100644 --- a/Modules/_tracemalloc.c +++ b/Modules/_tracemalloc.c @@ -712,7 +712,7 @@ tracemalloc_realloc(void *ctx, void *ptr, size_t new_size) The GIL and the table lock ensures that only one thread is allocating memory. */ - Py_UNREACHABLE(); + Py_FatalError("Memory allocation failed."); } TABLES_UNLOCK(); } diff --git a/Objects/abstract.c b/Objects/abstract.c index accd72d5f28e5d..f9be4227890ec7 100644 --- a/Objects/abstract.c +++ b/Objects/abstract.c @@ -1551,18 +1551,15 @@ PyNumber_Float(PyObject *o) PyObject * PyNumber_ToBase(PyObject *n, int base) { - PyObject *res = NULL; + if (!(base == 2 || base == 8 || base == 10 || base == 16)) { + PyErr_SetString(PyExc_SystemError, + "PyNumber_ToBase: base must be 2, 8, 10 or 16"); + return NULL; + } PyObject *index = PyNumber_Index(n); - if (!index) return NULL; - if (PyLong_Check(index)) - res = _PyLong_Format(index, base); - else - /* It should not be possible to get here, as - PyNumber_Index already has a check for the same - condition */ - PyErr_SetString(PyExc_ValueError, "PyNumber_ToBase: index not int"); + PyObject *res = _PyLong_Format(index, base); Py_DECREF(index); return res; } diff --git a/Objects/stringlib/eq.h b/Objects/stringlib/eq.h index ff22f913712e8d..9c1058b86cbedb 100644 --- a/Objects/stringlib/eq.h +++ b/Objects/stringlib/eq.h @@ -6,13 +6,14 @@ Py_LOCAL_INLINE(int) unicode_eq(PyObject *aa, PyObject *bb) { + assert(PyUnicode_Check(aa)); + assert(PyUnicode_Check(bb)); + assert(PyUnicode_IS_READY(aa)); + assert(PyUnicode_IS_READY(bb)); + PyUnicodeObject *a = (PyUnicodeObject *)aa; PyUnicodeObject *b = (PyUnicodeObject *)bb; - if (PyUnicode_READY(a) == -1 || PyUnicode_READY(b) == -1) { - Py_UNREACHABLE(); - } - if (PyUnicode_GET_LENGTH(a) != PyUnicode_GET_LENGTH(b)) return 0; if (PyUnicode_GET_LENGTH(a) == 0) diff --git a/Python/formatter_unicode.c b/Python/formatter_unicode.c index 55ed59d36898d1..841b25a43fce34 100644 --- a/Python/formatter_unicode.c +++ b/Python/formatter_unicode.c @@ -574,7 +574,7 @@ calc_number_widths(NumberFieldWidths *spec, Py_ssize_t n_prefix, spec->n_lpadding = n_padding; break; default: - /* Shouldn't get here, but treat it as '>' */ + /* Shouldn't get here */ Py_UNREACHABLE(); } } diff --git a/Python/peephole.c b/Python/peephole.c index baa217ad02d1d2..84de1abc175476 100644 --- a/Python/peephole.c +++ b/Python/peephole.c @@ -511,8 +511,12 @@ PyCode_Optimize(PyObject *code, PyObject* consts, PyObject *names, if (instrsize(j) > ilen) { goto exitUnchanged; } - assert(ilen <= INT_MAX); /* If instrsize(j) < ilen, we'll emit EXTENDED_ARG 0 */ + if (ilen > 4) { + /* Can only happen when PyCode_Optimize() is called with + malformed bytecode. */ + goto exitUnchanged; + } write_op_arg(codestr + h, opcode, j, (int)ilen); h += ilen; } diff --git a/Python/pytime.c b/Python/pytime.c index 9b2b74af5c0438..33b14f489b79b1 100644 --- a/Python/pytime.c +++ b/Python/pytime.c @@ -746,7 +746,7 @@ _PyTime_GetSystemClock(void) _PyTime_t t; if (pygettimeofday(&t, NULL, 0) < 0) { /* should not happen, _PyTime_Init() checked the clock at startup */ - Py_UNREACHABLE(); + Py_FatalError("pygettimeofday() failed."); } return t; } @@ -775,8 +775,7 @@ pymonotonic(_PyTime_t *tp, _Py_clock_info_t *info, int raise) _PyTime_overflow(); return -1; } - /* Hello, time traveler! */ - Py_UNREACHABLE(); + Py_FatalError("Hello, time traveler!"); } *tp = t * MS_TO_NS; @@ -918,7 +917,7 @@ _PyTime_GetMonotonicClock(void) if (pymonotonic(&t, NULL, 0) < 0) { /* should not happen, _PyTime_Init() checked that monotonic clock at startup */ - Py_UNREACHABLE(); + Py_FatalError("pymonotonic() failed."); } return t; } @@ -1019,7 +1018,7 @@ _PyTime_GetPerfCounter(void) { _PyTime_t t; if (_PyTime_GetPerfCounterWithInfo(&t, NULL)) { - Py_UNREACHABLE(); + Py_FatalError("_PyTime_GetPerfCounterWithInfo() failed."); } return t; } From 21093b44b5f0aa76ed713a8c4e0ff53cc0d2bd18 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sun, 8 Mar 2020 20:58:32 +0200 Subject: [PATCH 3/6] Support Intel and Microsoft compilers. --- Include/pymacro.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Include/pymacro.h b/Include/pymacro.h index 8ab593623220ca..e4dc8407860386 100644 --- a/Include/pymacro.h +++ b/Include/pymacro.h @@ -118,8 +118,10 @@ "We've reached an unreachable state. Anything is possible.\n" \ "The limits were in our heads all along. Follow your dreams.\n" \ "https://xkcd.com/2200") -#elif defined(__GNUC__) || defined(__clang__) +#elif defined(__GNUC__) || defined(__clang__) || defined(__INTEL_COMPILER) #define Py_UNREACHABLE() __builtin_unreachable() +#elif defined(_MSC_VER) +#define Py_UNREACHABLE() __assume(0) #else #define Py_UNREACHABLE() \ Py_FatalError("Unreachable C code path reached") From 5610ff73d21bf0347d603534c359cfafa52552f9 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Mon, 9 Mar 2020 20:10:14 +0200 Subject: [PATCH 4/6] Apply suggestions from code review Co-Authored-By: Victor Stinner --- Include/pymacro.h | 2 +- Modules/_tracemalloc.c | 2 +- Python/pytime.c | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Include/pymacro.h b/Include/pymacro.h index e4dc8407860386..d90a54c6c2f96d 100644 --- a/Include/pymacro.h +++ b/Include/pymacro.h @@ -119,7 +119,7 @@ "The limits were in our heads all along. Follow your dreams.\n" \ "https://xkcd.com/2200") #elif defined(__GNUC__) || defined(__clang__) || defined(__INTEL_COMPILER) -#define Py_UNREACHABLE() __builtin_unreachable() +# define Py_UNREACHABLE() __builtin_unreachable() #elif defined(_MSC_VER) #define Py_UNREACHABLE() __assume(0) #else diff --git a/Modules/_tracemalloc.c b/Modules/_tracemalloc.c index 17b7a29ee57e60..74c09e4d48535c 100644 --- a/Modules/_tracemalloc.c +++ b/Modules/_tracemalloc.c @@ -712,7 +712,7 @@ tracemalloc_realloc(void *ctx, void *ptr, size_t new_size) The GIL and the table lock ensures that only one thread is allocating memory. */ - Py_FatalError("Memory allocation failed."); + Py_FatalError("tracemalloc_realloc() failed to allocate a trace"); } TABLES_UNLOCK(); } diff --git a/Python/pytime.c b/Python/pytime.c index 33b14f489b79b1..9bf7025c0be817 100644 --- a/Python/pytime.c +++ b/Python/pytime.c @@ -746,7 +746,7 @@ _PyTime_GetSystemClock(void) _PyTime_t t; if (pygettimeofday(&t, NULL, 0) < 0) { /* should not happen, _PyTime_Init() checked the clock at startup */ - Py_FatalError("pygettimeofday() failed."); + Py_FatalError("pygettimeofday() failed"); } return t; } @@ -775,7 +775,7 @@ pymonotonic(_PyTime_t *tp, _Py_clock_info_t *info, int raise) _PyTime_overflow(); return -1; } - Py_FatalError("Hello, time traveler!"); + Py_FatalError("pymonotonic: integer overflow"); } *tp = t * MS_TO_NS; @@ -917,7 +917,7 @@ _PyTime_GetMonotonicClock(void) if (pymonotonic(&t, NULL, 0) < 0) { /* should not happen, _PyTime_Init() checked that monotonic clock at startup */ - Py_FatalError("pymonotonic() failed."); + Py_FatalError("pymonotonic() failed"); } return t; } @@ -1018,7 +1018,7 @@ _PyTime_GetPerfCounter(void) { _PyTime_t t; if (_PyTime_GetPerfCounterWithInfo(&t, NULL)) { - Py_FatalError("_PyTime_GetPerfCounterWithInfo() failed."); + Py_FatalError("_PyTime_GetPerfCounterWithInfo() failed"); } return t; } From c28d09e478dd574a022e382d0914087d49142d16 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Mon, 9 Mar 2020 20:10:25 +0200 Subject: [PATCH 5/6] Address review comments. --- Include/pymacro.h | 10 +++++----- Modules/_tracemalloc.c | 2 +- Python/pytime.c | 7 ++++--- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/Include/pymacro.h b/Include/pymacro.h index e4dc8407860386..856cae774d61c7 100644 --- a/Include/pymacro.h +++ b/Include/pymacro.h @@ -101,7 +101,7 @@ #endif #if defined(RANDALL_WAS_HERE) -#define Py_UNREACHABLE() \ +# define Py_UNREACHABLE() \ Py_FatalError( \ "If you're seeing this, the code is in what I thought was\n" \ "an unreachable state.\n\n" \ @@ -113,17 +113,17 @@ "I'm so sorry.\n" \ "https://xkcd.com/2200") #elif defined(Py_DEBUG) -#define Py_UNREACHABLE() \ +# define Py_UNREACHABLE() \ Py_FatalError( \ "We've reached an unreachable state. Anything is possible.\n" \ "The limits were in our heads all along. Follow your dreams.\n" \ "https://xkcd.com/2200") #elif defined(__GNUC__) || defined(__clang__) || defined(__INTEL_COMPILER) -#define Py_UNREACHABLE() __builtin_unreachable() +# define Py_UNREACHABLE() __builtin_unreachable() #elif defined(_MSC_VER) -#define Py_UNREACHABLE() __assume(0) +# define Py_UNREACHABLE() __assume(0) #else -#define Py_UNREACHABLE() \ +# define Py_UNREACHABLE() \ Py_FatalError("Unreachable C code path reached") #endif diff --git a/Modules/_tracemalloc.c b/Modules/_tracemalloc.c index 17b7a29ee57e60..74c09e4d48535c 100644 --- a/Modules/_tracemalloc.c +++ b/Modules/_tracemalloc.c @@ -712,7 +712,7 @@ tracemalloc_realloc(void *ctx, void *ptr, size_t new_size) The GIL and the table lock ensures that only one thread is allocating memory. */ - Py_FatalError("Memory allocation failed."); + Py_FatalError("tracemalloc_realloc() failed to allocate a trace"); } TABLES_UNLOCK(); } diff --git a/Python/pytime.c b/Python/pytime.c index 33b14f489b79b1..b953c08f54aeae 100644 --- a/Python/pytime.c +++ b/Python/pytime.c @@ -746,7 +746,7 @@ _PyTime_GetSystemClock(void) _PyTime_t t; if (pygettimeofday(&t, NULL, 0) < 0) { /* should not happen, _PyTime_Init() checked the clock at startup */ - Py_FatalError("pygettimeofday() failed."); + Py_FatalError("pygettimeofday() failed"); } return t; } @@ -775,7 +775,8 @@ pymonotonic(_PyTime_t *tp, _Py_clock_info_t *info, int raise) _PyTime_overflow(); return -1; } - Py_FatalError("Hello, time traveler!"); + /* Hello, time traveler! */ + Py_FatalError("pymonotonic: integer overflow"); } *tp = t * MS_TO_NS; @@ -917,7 +918,7 @@ _PyTime_GetMonotonicClock(void) if (pymonotonic(&t, NULL, 0) < 0) { /* should not happen, _PyTime_Init() checked that monotonic clock at startup */ - Py_FatalError("pymonotonic() failed."); + Py_FatalError("pymonotonic() failed"); } return t; } From 0afeb6e25d690a5fe087cdea3cef5180ef2f585d Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Mon, 9 Mar 2020 20:20:52 +0200 Subject: [PATCH 6/6] Update docs. --- Doc/c-api/intro.rst | 15 ++++++++++++++- .../2020-03-09-20-27-19.bpo-38249.IxYbQy.rst | 2 ++ 2 files changed, 16 insertions(+), 1 deletion(-) create mode 100644 Misc/NEWS.d/next/C API/2020-03-09-20-27-19.bpo-38249.IxYbQy.rst diff --git a/Doc/c-api/intro.rst b/Doc/c-api/intro.rst index d08d4f97a308e7..5a99631bbbdcfb 100644 --- a/Doc/c-api/intro.rst +++ b/Doc/c-api/intro.rst @@ -107,11 +107,24 @@ complete listing. .. c:macro:: Py_UNREACHABLE() - Use this when you have a code path that you do not expect to be reached. + Use this when you have a code path that cannot be reached by design. For example, in the ``default:`` clause in a ``switch`` statement for which all possible values are covered in ``case`` statements. Use this in places where you might be tempted to put an ``assert(0)`` or ``abort()`` call. + In release mode, the macro helps the compiler to optimize the code, and + avoids a warning about unreachable code. For example, the macro is + implemented with ``__builtin_unreachable()`` on GCC in release mode. + + A use for ``Py_UNREACHABLE()`` is following a call a function that + never returns but that is not declared :c:macro:`_Py_NO_RETURN`. + + If a code path is very unlikely code but can be reached under exceptional + case, this macro must not be used. For example, under low memory condition + or if a system call returns a value out of the expected range. In this + case, it's better to report the error to the caller. If the error cannot + be reported to caller, :c:func:`Py_FatalError` can be used. + .. versionadded:: 3.7 .. c:macro:: Py_ABS(x) diff --git a/Misc/NEWS.d/next/C API/2020-03-09-20-27-19.bpo-38249.IxYbQy.rst b/Misc/NEWS.d/next/C API/2020-03-09-20-27-19.bpo-38249.IxYbQy.rst new file mode 100644 index 00000000000000..e209c8bd7e6085 --- /dev/null +++ b/Misc/NEWS.d/next/C API/2020-03-09-20-27-19.bpo-38249.IxYbQy.rst @@ -0,0 +1,2 @@ +:c:macro:`Py_UNREACHABLE` is now implemented with +``__builtin_unreachable()`` and analogs in release mode.