[C++][R] R Sanitizer nightly error

[jonkeane]

Describe the bug, including details regarding any error messages, version, and platform.

We introduced a sanitizer error sometime between commit d7a5777 and 7e2245c which is showing up in our test-ubuntu-r-sanitizer extended test.

Start test: Handling string data with embedded nuls
  'test-scalar.R:90:3' [success]
/arrow/cpp/src/arrow/scalar.h:144:43: runtime error: downcast of address 0x6080003875b0 which does not point to an object of type 'BinaryScalar'
0x6080003875b0: note: object is of type 'arrow::BaseBinaryScalar'
 01 00 00 00  e0 88 ff eb 14 7f 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  b0 2b 00 00
              ^~~~~~~~~~~~~~~~~~~~~~~
              vptr for 'arrow::BaseBinaryScalar'
    #0 0x7f14d93d6d8d in arrow::internal::ArraySpanFillFromScalarScratchSpace<arrow::BinaryScalar>::ArraySpanFillFromScalarScratchSpace() /arrow/cpp/src/arrow/scalar.h:144
    #1 0x7f14d93d6d8d in arrow::BinaryScalar::BaseBinaryScalar(std::shared_ptr<arrow::Buffer>, std::shared_ptr<arrow::DataType>) /arrow/cpp/src/arrow/scalar.h:281
    #2 0x7f14d93d6d8d in void std::_Construct<arrow::BinaryScalar, std::shared_ptr<arrow::Buffer>, std::shared_ptr<arrow::DataType> >(arrow::BinaryScalar*, std::shared_ptr<arrow::Buffer>&&, std::shared_ptr<arrow::DataType>&&) /usr/include/c++/12/bits/stl_construct.h:119
    #3 0x7f14d93d6d8d in void std::allocator_traits<std::allocator<void> >::construct<arrow::BinaryScalar, std::shared_ptr<arrow::Buffer>, std::shared_ptr<arrow::DataType> >(std::allocator<void>&, arrow::BinaryScalar*, std::shared_ptr<arrow::Buffer>&&, std::shared_ptr<arrow::DataType>&&) /usr/include/c++/12/bits/alloc_traits.h:635
    #4 0x7f14d93d6d8d in std::_Sp_counted_ptr_inplace<arrow::BinaryScalar, std::allocator<void>, (__gnu_cxx::_Lock_policy)2>::_Sp_counted_ptr_inplace<std::shared_ptr<arrow::Buffer>, std::shared_ptr<arrow::DataType> >(std::allocator<void>, std::shared_ptr<arrow::Buffer>&&, std::shared_ptr<arrow::DataType>&&) /usr/include/c++/12/bits/shared_ptr_base.h:604
    #5 0x7f14d93d6d8d in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::__shared_count<arrow::BinaryScalar, std::allocator<void>, std::shared_ptr<arrow::Buffer>, std::shared_ptr<arrow::DataType> >(arrow::BinaryScalar*&, std::_Sp_alloc_shared_tag<std::allocator<void> >, std::shared_ptr<arrow::Buffer>&&, std::shared_ptr<arrow::DataType>&&) /usr/include/c++/12/bits/shared_ptr_base.h:971
    #6 0x7f14d93d6d8d in std::__shared_ptr<arrow::BinaryScalar, (__gnu_cxx::_Lock_policy)2>::__shared_ptr<std::allocator<void>, std::shared_ptr<arrow::Buffer>, std::shared_ptr<arrow::DataType> >(std::_Sp_alloc_shared_tag<std::allocator<void> >, std::shared_ptr<arrow::Buffer>&&, std::shared_ptr<arrow::DataType>&&) /usr/include/c++/12/bits/shared_ptr_base.h:1712
    #7 0x7f14d93d6d8d in std::shared_ptr<arrow::BinaryScalar>::shared_ptr<std::allocator<void>, std::shared_ptr<arrow::Buffer>, std::shared_ptr<arrow::DataType> >(std::_Sp_alloc_shared_tag<std::allocator<void> >, std::shared_ptr<arrow::Buffer>&&, std::shared_ptr<arrow::DataType>&&) /usr/include/c++/12/bits/shared_ptr.h:464
    #8 0x7f14d93d6d8d in std::shared_ptr<std::enable_if<!std::is_array<arrow::BinaryScalar>::value, arrow::BinaryScalar>::type> std::make_shared<arrow::BinaryScalar, std::shared_ptr<arrow::Buffer>, std::shared_ptr<arrow::DataType> >(std::shared_ptr<arrow::Buffer>&&, std::shared_ptr<arrow::DataType>&&) /usr/include/c++/12/bits/shared_ptr.h:1010
    #9 0x7f14d93d6d8d in arrow::Status arrow::MakeScalarImpl<std::shared_ptr<arrow::Buffer>&&>::Visit<arrow::BinaryType, arrow::BinaryScalar, std::shared_ptr<arrow::Buffer>, void>(arrow::BinaryType const&) /arrow/cpp/src/arrow/scalar.h:895
    #10 0x7f14d93d6d8d in arrow::Status arrow::VisitTypeInline<arrow::MakeScalarImpl<std::shared_ptr<arrow::Buffer>&&>>(arrow::DataType const&, arrow::MakeScalarImpl<std::shared_ptr<arrow::Buffer>&&>*) /arrow/cpp/src/arrow/visit_type_inline.h:54
    #11 0x7f14d93db8b4 in arrow::MakeScalarImpl<std::shared_ptr<arrow::Buffer>&&>::Finish() && /arrow/cpp/src/arrow/scalar.h:926
    #12 0x7f14d93db8b4 in arrow::Result<std::shared_ptr<arrow::Scalar> > arrow::MakeScalar<std::shared_ptr<arrow::Buffer> >(std::shared_ptr<arrow::DataType>, std::shared_ptr<arrow::Buffer>&&) /arrow/cpp/src/arrow/scalar.h:859
    #13 0x7f14ddc05b8b in arrow::internal::ScalarFromArraySlotImpl::Finish(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) /arrow/cpp/src/arrow/array/array_base.cc:174
    #14 0x7f14ddc06e3a in arrow::Status arrow::internal::ScalarFromArraySlotImpl::Visit<arrow::BinaryType>(arrow::BaseBinaryArray<arrow::BinaryType> const&) /arrow/cpp/src/arrow/array/array_base.cc:87
    #15 0x7f14ddcc30e8 in arrow::Status arrow::VisitArrayInline<arrow::internal::ScalarFromArraySlotImpl>(arrow::Array const&, arrow::internal::ScalarFromArraySlotImpl*) /arrow/cpp/src/arrow/visit_array_inline.h:55
    #16 0x7f14ddcd43bf in arrow::internal::ScalarFromArraySlotImpl::Finish() && /arrow/cpp/src/arrow/array/array_base.cc:198
    #17 0x7f14ddbea09c in arrow::Array::GetScalar(long) const /arrow/cpp/src/arrow/array/array_base.cc:213
    #18 0x7f14d837679a in Array__GetScalar(std::shared_ptr<arrow::Array> const&, long) /tmp/RtmpXxxFU4/R.INSTALL5049c82dca/arrow/src/scalar.cpp:40
    #19 0x7f14d7c25068 in _arrow_Array__GetScalar /tmp/RtmpXxxFU4/R.INSTALL5049c82dca/arrow/src/arrowExports.cpp:5129
    #20 0x7f151870d228 in R_doDotCall /tmp/r-source/src/main/dotcode.c:757
    #21 0x7f1518810e0e in bcEval_loop /tmp/r-source/src/main/eval.c:8691
    #22 0x7f15187d353a in bcEval /tmp/r-source/src/main/eval.c:7524
    #23 0x7f15187b04b2 in Rf_eval /tmp/r-source/src/main/eval.c:1167
    #24 0x7f15187b6de6 in R_execClosure /tmp/r-source/src/main/eval.c:2398
    #25 0x7f15187b64e6 in applyClosure_core /tmp/r-source/src/main/eval.c:2311
    #26 0x7f15187b663f in Rf_applyClosure /tmp/r-source/src/main/eval.c:2333
    #27 0x7f15187b1774 in Rf_eval /tmp/r-source/src/main/eval.c:1285
    #28 0x7f15189bc563 in R_DispatchOrEvalSP /tmp/r-source/src/main/subset.c:657
    #29 0x7f15189c016a in do_subset3 /tmp/r-source/src/main/subset.c:1265
    #30 0x7f15187b0f37 in Rf_eval /tmp/r-source/src/main/eval.c:1237
    #31 0x7f15187b0bbd in Rf_eval /tmp/r-source/src/main/eval.c:1226
    #32 0x7f15187bfc1d in do_set /tmp/r-source/src/main/eval.c:3582
    #33 0x7f15187b0f37 in Rf_eval /tmp/r-source/src/main/eval.c:1237
    #34 0x7f15187bc39d in do_begin /tmp/r-source/src/main/eval.c:3010
    #35 0x7f15187b0f37 in Rf_eval /tmp/r-source/src/main/eval.c:1237
    #36 0x7f15187c2308 in do_eval /tmp/r-source/src/main/eval.c:3956
    #37 0x7f15187e5e9f in bcEval_loop /tmp/r-source/src/main/eval.c:8141
    #38 0x7f15187d353a in bcEval /tmp/r-source/src/main/eval.c:7524
    #39 0x7f15187b04b2 in Rf_eval /tmp/r-source/src/main/eval.c:1167
    #40 0x7f15187b6de6 in R_execClosure /tmp/r-source/src/main/eval.c:2398
    #41 0x7f15187b64e6 in applyClosure_core /tmp/r-source/src/main/eval.c:2311
    #42 0x7f15187b663f in Rf_applyClosure /tmp/r-source/src/main/eval.c:2333
    #43 0x7f15187b1774 in Rf_eval /tmp/r-source/src/main/eval.c:1285
    #44 0x7f15187c25c3 in do_eval /tmp/r-source/src/main/eval.c:3974
    #45 0x7f15187e5e9f in bcEval_loop /tmp/r-source/src/main/eval.c:8141
    #46 0x7f15187d353a in bcEval /tmp/r-source/src/main/eval.c:7524
    #47 0x7f15187b04b2 in Rf_eval /tmp/r-source/src/main/eval.c:1167
    #48 0x7f15187b6de6 in R_execClosure /tmp/r-source/src/main/eval.c:2398
    #49 0x7f15187b64e6 in applyClosure_core /tmp/r-source/src/main/eval.c:2311
    #50 0x7f15187b663f in Rf_applyClosure /tmp/r-source/src/main/eval.c:2333
    #51 0x7f15187b7dbe in R_forceAndCall /tmp/r-source/src/main/eval.c:2465
    #52 0x7f151861f204 in do_lapply /tmp/r-source/src/main/apply.c:75
    #53 0x7f15188b0764 in do_internal /tmp/r-source/src/main/names.c:1409
    #54 0x7f15187e6406 in bcEval_loop /tmp/r-source/src/main/eval.c:8161
    #55 0x7f15187d353a in bcEval /tmp/r-source/src/main/eval.c:7524
    #56 0x7f15187b04b2 in Rf_eval /tmp/r-source/src/main/eval.c:1167
    #57 0x7f15187b6de6 in R_execClosure /tmp/r-source/src/main/eval.c:2398
    #58 0x7f15187b64e6 in applyClosure_core /tmp/r-source/src/main/eval.c:2311
    #59 0x7f15187b663f in Rf_applyClosure /tmp/r-source/src/main/eval.c:2333
    #60 0x7f15187b1774 in Rf_eval /tmp/r-source/src/main/eval.c:1285
    #61 0x7f151886ceb4 in Rf_ReplIteration /tmp/r-source/src/main/main.c:262
    #62 0x7f151886d39f in R_ReplConsole /tmp/r-source/src/main/main.c:314
    #63 0x7f151886fb69 in run_Rmainloop /tmp/r-source/src/main/main.c:1216
    #64 0x7f151886fb83 in Rf_mainloop /tmp/r-source/src/main/main.c:1223
    #65 0x55d279d4e238 in main /tmp/r-source/src/main/Rmain.c:29
    #66 0x7f1517c31d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #67 0x7f1517c31e3f in __libc_start_main_impl ../csu/libc-start.c:392
    #68 0x55d279d4e104 in _start (/usr/local/RDsan/lib/R/bin/exec/R+0x1104)

https://github.com/ursacomputing/crossbow/actions/runs/8809030102/job/24179167935#step:7:13701

Possible PRs that add this based on components: #40237 #41354 #41115 #40356

Component(s)

C++

[jonkeane]

I suspect this is also what is responsible for the test-fedore-r-clang-sanitizer as well:

https://github.com/ursacomputing/crossbow/actions/runs/8809030613/job/24179194857

[jonkeane]

Doing some more digging into those PRs, it does look like

#40237 did add one of the lines that the sanitizer is complaining about:

https://github.com/apache/arrow/pull/40237/files#diff-0df9ec77ca57be897102ef78fc030c4b2fea9b161bab5719a4957bb57224ae9bR144

cc @zanmato1984 @bkietz

[zanmato1984]

Thank you for reporting and locating the issue. I confirm that it is introduced by my PR #40237.

The cause is that we are down-casting a CRTP base class's this to the derived class in base class's constructor. UBSAN's "vptr" checking treats it as an UB. I'm not entirely sure if this is true-positive, since I don't find a clear statement about this case in C++ standard (I'm not a language lawyer so maybe there is, I appreciate if someone can point it to me).

Additionally, C++ UBSAN has suppressed the "vptr" checking for some reason [1]. This is why this could've been discovered by C++ UBSAN but has not.

If we consider it a false-positive, then we can simply add a suppression in [1]. Otherwise I can make the FillScratchSpace method static to avoid this down-casting, which should have no performance impact, nor extra complexity. Update: Making FillScratchSpace static won't be trivial, it requires lots of restructuring of the code. I prefer to explore if it is a false-positive first.

What do you think? @bkietz

[1]

arrow/cpp/cmake_modules/san-config.cmake

Line 33 in 4b2cf22

# - disable 'vptr' because of RTTI issues across shared libraries (?)

[2] https://github.com/apache/arrow/blob/main/r/tools/ubsan.supp

[zanmato1984]

PR #41421 filed to solve this issue "the hard way".

[zanmato1984]

Hi @jonkeane , could you help to trigger the questioning CI job in #41421 to verify the fix? I don't think I have the permission and if I do, I don't know how either. Thank you :)

[bkietz]

Issue resolved by pull request 41421
#41421