From e73c36364a89472ce1bcac9044493c752f0579c0 Mon Sep 17 00:00:00 2001 From: Jerry Ling Date: Fri, 25 Mar 2022 13:43:15 -0400 Subject: [PATCH 1/7] fix `===` when encountering null pointer --- src/builtins.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/builtins.c b/src/builtins.c index f8a78485551ad..d80b20a5f47ff 100644 --- a/src/builtins.c +++ b/src/builtins.c @@ -94,20 +94,22 @@ static int NOINLINE compare_fields(const jl_value_t *a, const jl_value_t *b, jl_ else { jl_datatype_t *ft = (jl_datatype_t*)jl_field_type_concrete(dt, f); if (jl_is_uniontype(ft)) { - uint8_t asel = ((uint8_t*)ao)[jl_field_size(dt, f) - 1]; - uint8_t bsel = ((uint8_t*)bo)[jl_field_size(dt, f) - 1]; + size_t idx = jl_field_size(dt, f) - 1; + uint8_t asel = ((uint8_t*)ao)[idx]; + uint8_t bsel = ((uint8_t*)bo)[idx]; if (asel != bsel) return 0; ft = (jl_datatype_t*)jl_nth_union_component((jl_value_t*)ft, asel); } else if (ft->layout->first_ptr >= 0) { - // If the field is a inline immutable that can be can be undef - // we need to check to check for undef first since undef struct + // If the field is a inline immutable that can be undef + // we need to check for undef first since undef struct // may have fields that are different but should still be treated as equal. - jl_value_t *ptra = ((jl_value_t**)ao)[ft->layout->first_ptr]; - jl_value_t *ptrb = ((jl_value_t**)bo)[ft->layout->first_ptr]; - if (ptra == NULL && ptrb == NULL) { - return 1; + size_t idx = ft->layout->first_ptr; + jl_value_t *ptra = ((jl_value_t**)ao)[idx]; + jl_value_t *ptrb = ((jl_value_t**)bo)[idx]; + if ((ptra != NULL && ptrb == NULL) || (ptra == NULL && ptrb != NULL)) { + return 0; } } if (!ft->layout->haspadding) { From b2e704d58e3806459d8a06b95acbde973da85690 Mon Sep 17 00:00:00 2001 From: Jerry Ling Date: Fri, 25 Mar 2022 13:48:19 -0400 Subject: [PATCH 2/7] Update builtins.c --- src/builtins.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/builtins.c b/src/builtins.c index d80b20a5f47ff..4f84594c83eb7 100644 --- a/src/builtins.c +++ b/src/builtins.c @@ -105,7 +105,7 @@ static int NOINLINE compare_fields(const jl_value_t *a, const jl_value_t *b, jl_ // If the field is a inline immutable that can be undef // we need to check for undef first since undef struct // may have fields that are different but should still be treated as equal. - size_t idx = ft->layout->first_ptr; + int32_t idx = ft->layout->first_ptr; jl_value_t *ptra = ((jl_value_t**)ao)[idx]; jl_value_t *ptrb = ((jl_value_t**)bo)[idx]; if ((ptra != NULL && ptrb == NULL) || (ptra == NULL && ptrb != NULL)) { From 5a77107110356e1d31384ca8eee23bd14de6f46c Mon Sep 17 00:00:00 2001 From: Jerry Ling Date: Fri, 25 Mar 2022 14:37:58 -0400 Subject: [PATCH 3/7] Update src/builtins.c Co-authored-by: Jameson Nash --- src/builtins.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/builtins.c b/src/builtins.c index 4f84594c83eb7..b627eed34f8cd 100644 --- a/src/builtins.c +++ b/src/builtins.c @@ -108,7 +108,7 @@ static int NOINLINE compare_fields(const jl_value_t *a, const jl_value_t *b, jl_ int32_t idx = ft->layout->first_ptr; jl_value_t *ptra = ((jl_value_t**)ao)[idx]; jl_value_t *ptrb = ((jl_value_t**)bo)[idx]; - if ((ptra != NULL && ptrb == NULL) || (ptra == NULL && ptrb != NULL)) { + if ((ptra == NULL) != (ptrb == NULL)) { return 0; } } From bd2efbd65238a6913606ace799a0adf180b57ca4 Mon Sep 17 00:00:00 2001 From: Jerry Ling Date: Fri, 25 Mar 2022 17:44:30 -0400 Subject: [PATCH 4/7] fix test and add tests --- src/builtins.c | 3 +++ test/core.jl | 8 ++++++++ 2 files changed, 11 insertions(+) diff --git a/src/builtins.c b/src/builtins.c index b627eed34f8cd..d3e6781da336a 100644 --- a/src/builtins.c +++ b/src/builtins.c @@ -111,6 +111,9 @@ static int NOINLINE compare_fields(const jl_value_t *a, const jl_value_t *b, jl_ if ((ptra == NULL) != (ptrb == NULL)) { return 0; } + else { + continue; + } } if (!ft->layout->haspadding) { if (!bits_equal(ao, bo, ft->size)) diff --git a/test/core.jl b/test/core.jl index 50332c1d8f6b6..b9aaea3a87a8d 100644 --- a/test/core.jl +++ b/test/core.jl @@ -92,6 +92,14 @@ let abcd = ABCDconst(1, 2, 3, 4) @test (1, 2, "not constant", 4) === (abcd.a, abcd.b, abcd.c, abcd.d) end +# test `===` handling null pointer in struct #44712 +struct N44712 + a::Some{Any} + b::Int + AB() = new() +end + +@test unsafe_load(Ptr{N44712}(pointer(Int[0,1]))) !== unsafe_load(Ptr{N44712}(pointer(Int[0,2]))) f47(x::Vector{Vector{T}}) where {T} = 0 @test_throws MethodError f47(Vector{Vector}()) From fa22c34807a24d6d29e143422bddf3b3ce86d5af Mon Sep 17 00:00:00 2001 From: Jerry Ling Date: Fri, 25 Mar 2022 21:22:07 -0400 Subject: [PATCH 5/7] Update test/core.jl Co-authored-by: Jameson Nash --- test/core.jl | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/test/core.jl b/test/core.jl index b9aaea3a87a8d..52964a05e1784 100644 --- a/test/core.jl +++ b/test/core.jl @@ -96,10 +96,13 @@ end struct N44712 a::Some{Any} b::Int - AB() = new() + N44712() = new() +end +let a = Int[0, 1], b = Int[0, 2] + GC.@preserve a b begin + @test unsafe_load(Ptr{N44712}(pointer(a))) !== unsafe_load(Ptr{N44712}(pointer(b))) + end end - -@test unsafe_load(Ptr{N44712}(pointer(Int[0,1]))) !== unsafe_load(Ptr{N44712}(pointer(Int[0,2]))) f47(x::Vector{Vector{T}}) where {T} = 0 @test_throws MethodError f47(Vector{Vector}()) From e4b18a7c879b4c7658fc99178a68ca25f3277e69 Mon Sep 17 00:00:00 2001 From: Jerry Ling Date: Mon, 28 Mar 2022 15:29:50 -0400 Subject: [PATCH 6/7] Update src/builtins.c Co-authored-by: Jameson Nash --- src/builtins.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/builtins.c b/src/builtins.c index d3e6781da336a..ecef920c7316b 100644 --- a/src/builtins.c +++ b/src/builtins.c @@ -111,8 +111,8 @@ static int NOINLINE compare_fields(const jl_value_t *a, const jl_value_t *b, jl_ if ((ptra == NULL) != (ptrb == NULL)) { return 0; } - else { - continue; + else if (ptra == NULL) { // implies ptrb == NULL + continue; // skip this field (it is #undef) } } if (!ft->layout->haspadding) { From 9067814294aba9d7e73142ca1e743a63311b5a95 Mon Sep 17 00:00:00 2001 From: Jerry Ling Date: Mon, 28 Mar 2022 19:04:01 -0400 Subject: [PATCH 7/7] add one more test --- test/core.jl | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/core.jl b/test/core.jl index 52964a05e1784..d7c6149fa609d 100644 --- a/test/core.jl +++ b/test/core.jl @@ -104,6 +104,9 @@ let a = Int[0, 1], b = Int[0, 2] end end +# another possible issue in #44712 +@test (("", 0),) !== (("", 1),) + f47(x::Vector{Vector{T}}) where {T} = 0 @test_throws MethodError f47(Vector{Vector}()) @test f47(Vector{Vector{Int}}()) == 0