From 8f54d34c02206f6d8976e6e7916b51055a0f7a19 Mon Sep 17 00:00:00 2001 From: Nathan Daly Date: Tue, 10 Nov 2020 17:45:53 -0500 Subject: [PATCH 1/6] Fix segfault in static_show, by using correct `vt` instead of `typeof(v)` This fixes a segfault introduced in #38049. Co-Authored-By: Sacha Verweij --- src/rtutils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rtutils.c b/src/rtutils.c index 01a47228f3c0a..a9a7a3797acbb 100644 --- a/src/rtutils.c +++ b/src/rtutils.c @@ -999,7 +999,7 @@ static size_t jl_static_show_x_(JL_STREAM *out, jl_value_t *v, jl_datatype_t *vt n += jl_static_show_x(out, *(jl_value_t**)v, depth); n += jl_printf(out, ")"); } - else if (jl_function_type && jl_isa(v, (jl_value_t*)jl_function_type)) { + else if (jl_function_type && jl_subtype(vt, (jl_value_t*)jl_function_type)) { // v is function instance (an instance of a Function type). jl_datatype_t *dv = (jl_datatype_t*)vt; jl_sym_t *sym = dv->name->mt->name; From 2e5f22c1d7f2f3e0872b95c03cdec45ea083d352 Mon Sep 17 00:00:00 2001 From: Nathan Daly Date: Fri, 13 Nov 2020 00:13:23 -0500 Subject: [PATCH 2/6] Fix illegal GC-violating call to `jl_subtype` in static_show Implement custom iterative check via `->super` --- src/rtutils.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/rtutils.c b/src/rtutils.c index a9a7a3797acbb..1dc86540c7dc4 100644 --- a/src/rtutils.c +++ b/src/rtutils.c @@ -641,6 +641,19 @@ static size_t jl_static_show_x_sym_escaped(JL_STREAM *out, jl_sym_t *name) JL_NO return n; } +// `jl_static_show()` cannot call `jl_subtype()`, for the GC reasons +// explained in the comment on `jl_static_show_x_()`, below. +// This function checks if `vt <: Function` without triggering GC. +static int jl_static_is_function_(jl_datatype_t *vt) { + while (vt != jl_any_type) { + if (vt == jl_function_type) { + return 1; + } + vt = vt->super; + } + return 0; +} + // `v` might be pointing to a field inlined in a structure therefore // `jl_typeof(v)` may not be the same with `vt` and only `vt` should be // used to determine the type of the value. @@ -999,7 +1012,7 @@ static size_t jl_static_show_x_(JL_STREAM *out, jl_value_t *v, jl_datatype_t *vt n += jl_static_show_x(out, *(jl_value_t**)v, depth); n += jl_printf(out, ")"); } - else if (jl_function_type && jl_subtype(vt, (jl_value_t*)jl_function_type)) { + else if (jl_function_type && jl_static_is_function_(vt)) { // v is function instance (an instance of a Function type). jl_datatype_t *dv = (jl_datatype_t*)vt; jl_sym_t *sym = dv->name->mt->name; From 81aa68a50d63c2e48f9e445d74bbdf8c5b2f23da Mon Sep 17 00:00:00 2001 From: Nathan Daly Date: Fri, 13 Nov 2020 00:41:49 -0500 Subject: [PATCH 3/6] PR feedback: JL_NOTSAFEPOINT, jl_function_type --- src/rtutils.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/rtutils.c b/src/rtutils.c index 1dc86540c7dc4..2c4ae0b38196e 100644 --- a/src/rtutils.c +++ b/src/rtutils.c @@ -644,7 +644,10 @@ static size_t jl_static_show_x_sym_escaped(JL_STREAM *out, jl_sym_t *name) JL_NO // `jl_static_show()` cannot call `jl_subtype()`, for the GC reasons // explained in the comment on `jl_static_show_x_()`, below. // This function checks if `vt <: Function` without triggering GC. -static int jl_static_is_function_(jl_datatype_t *vt) { +static int jl_static_is_function_(jl_datatype_t *vt) JL_NOTSAFEPOINT { + if (!jl_function_type) { // Make sure there's a Function type defined. + return 0; + } while (vt != jl_any_type) { if (vt == jl_function_type) { return 1; @@ -1012,7 +1015,7 @@ static size_t jl_static_show_x_(JL_STREAM *out, jl_value_t *v, jl_datatype_t *vt n += jl_static_show_x(out, *(jl_value_t**)v, depth); n += jl_printf(out, ")"); } - else if (jl_function_type && jl_static_is_function_(vt)) { + else if (jl_static_is_function_(vt)) { // v is function instance (an instance of a Function type). jl_datatype_t *dv = (jl_datatype_t*)vt; jl_sym_t *sym = dv->name->mt->name; From 2479af73e936d434fbef8c1967ef1f380a95a040 Mon Sep 17 00:00:00 2001 From: Nathan Daly Date: Mon, 16 Nov 2020 10:58:22 -0500 Subject: [PATCH 4/6] Add error checking to `jl_static_is_function_`: null and cycles --- src/rtutils.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/rtutils.c b/src/rtutils.c index 2c4ae0b38196e..4b8f6d04a6b94 100644 --- a/src/rtutils.c +++ b/src/rtutils.c @@ -648,11 +648,17 @@ static int jl_static_is_function_(jl_datatype_t *vt) JL_NOTSAFEPOINT { if (!jl_function_type) { // Make sure there's a Function type defined. return 0; } + int _iter_count = 0; // To prevent infinite loops from corrupt type objects. while (vt != jl_any_type) { - if (vt == jl_function_type) { + if (vt == NULL) { + jl_error("static_show: Nullptr encountered inside datatype."); + } else if (_iter_count > 10000) { + jl_error("static_show: Exit after 10,000 iterations of supertype(). Presuming invalid cycle in datatype object."); + } else if (vt == jl_function_type) { return 1; } vt = vt->super; + _iter_count += 1; } return 0; } From 7e2d96736ae56baa3e11c515e16f20b0edbbafb1 Mon Sep 17 00:00:00 2001 From: Nathan Daly Date: Mon, 16 Nov 2020 12:55:16 -0500 Subject: [PATCH 5/6] Print warning to STDERR instead of throwing exception Since throwing an exception would allocate, which is illegal from the `jl_static_*` function(s). ``` julia> ccall(:jl_static_show, Cvoid, (Ptr{Cvoid}, Any), p.in, x) ERROR: static_show: Exit after 10,000 iterations of supertype(). Presuming invalid cycle in datatype object. ``` --- src/rtutils.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/rtutils.c b/src/rtutils.c index 4b8f6d04a6b94..2c58cda9a22ed 100644 --- a/src/rtutils.c +++ b/src/rtutils.c @@ -651,9 +651,13 @@ static int jl_static_is_function_(jl_datatype_t *vt) JL_NOTSAFEPOINT { int _iter_count = 0; // To prevent infinite loops from corrupt type objects. while (vt != jl_any_type) { if (vt == NULL) { - jl_error("static_show: Nullptr encountered inside datatype."); + jl_printf((JL_STREAM*)STDERR_FILENO, + "ERROR: static_show: Nullptr encountered inside datatype.\n"); + return 0; } else if (_iter_count > 10000) { - jl_error("static_show: Exit after 10,000 iterations of supertype(). Presuming invalid cycle in datatype object."); + jl_printf((JL_STREAM*)STDERR_FILENO, + "ERROR: static_show: Exit after 10,000 iterations of supertype(). Presuming invalid cycle in datatype object.\n"); + return 0; } else if (vt == jl_function_type) { return 1; } From aa77e3528eabbdf18d9ec92f543e5a8ee46440ed Mon Sep 17 00:00:00 2001 From: Nathan Daly Date: Wed, 18 Nov 2020 11:40:43 -0500 Subject: [PATCH 6/6] Remove WARNING logs on `jl_static_is_function_()` If we ecounter a cycle or a NULL super type before encountering a Function super type, then this type is not <: Function. --- src/rtutils.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/rtutils.c b/src/rtutils.c index 2c58cda9a22ed..03fe6f461d564 100644 --- a/src/rtutils.c +++ b/src/rtutils.c @@ -651,12 +651,10 @@ static int jl_static_is_function_(jl_datatype_t *vt) JL_NOTSAFEPOINT { int _iter_count = 0; // To prevent infinite loops from corrupt type objects. while (vt != jl_any_type) { if (vt == NULL) { - jl_printf((JL_STREAM*)STDERR_FILENO, - "ERROR: static_show: Nullptr encountered inside datatype.\n"); return 0; } else if (_iter_count > 10000) { - jl_printf((JL_STREAM*)STDERR_FILENO, - "ERROR: static_show: Exit after 10,000 iterations of supertype(). Presuming invalid cycle in datatype object.\n"); + // We are very likely stuck in a cyclic datastructure, so we assume this is + // _not_ a Function. return 0; } else if (vt == jl_function_type) { return 1;