From eebaa9bfc593d5a46b293c1abd929fbfbfd28199 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Mon, 9 Mar 2020 20:49:52 +0200 Subject: [PATCH] bpo-38249: Expand Py_UNREACHABLE() to __builtin_unreachable() in the release mode. (GH-16329) Co-authored-by: Victor Stinner --- Doc/c-api/intro.rst | 15 ++++++++++++++- Include/pymacro.h | 10 +++++++--- .../2020-03-09-20-27-19.bpo-38249.IxYbQy.rst | 2 ++ Modules/_tracemalloc.c | 2 +- Objects/stringlib/eq.h | 9 +++++---- Python/formatter_unicode.c | 2 +- Python/peephole.c | 6 +++++- Python/pytime.c | 8 ++++---- 8 files changed, 39 insertions(+), 15 deletions(-) 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 d08d4f97a30..5a99631bbbd 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/Include/pymacro.h b/Include/pymacro.h index c080fb164e3..856cae774d6 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,13 +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() +#elif defined(_MSC_VER) +# define Py_UNREACHABLE() __assume(0) #else -#define Py_UNREACHABLE() \ +# define Py_UNREACHABLE() \ Py_FatalError("Unreachable C code path reached") #endif 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 00000000000..e209c8bd7e6 --- /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. diff --git a/Modules/_tracemalloc.c b/Modules/_tracemalloc.c index ddf6ef4e11d..74c09e4d485 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("tracemalloc_realloc() failed to allocate a trace"); } TABLES_UNLOCK(); } diff --git a/Objects/stringlib/eq.h b/Objects/stringlib/eq.h index ff22f913712..9c1058b86cb 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 55ed59d3689..841b25a43fc 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 baa217ad02d..84de1abc175 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 9b2b74af5c0..6affccbeffa 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; } @@ -776,7 +776,7 @@ pymonotonic(_PyTime_t *tp, _Py_clock_info_t *info, int raise) return -1; } /* Hello, time traveler! */ - Py_UNREACHABLE(); + Py_FatalError("pymonotonic: integer overflow"); } *tp = t * MS_TO_NS; @@ -918,7 +918,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 +1019,7 @@ _PyTime_GetPerfCounter(void) { _PyTime_t t; if (_PyTime_GetPerfCounterWithInfo(&t, NULL)) { - Py_UNREACHABLE(); + Py_FatalError("_PyTime_GetPerfCounterWithInfo() failed"); } return t; }