sfc-gh-bhannel opened a new pull request, #2702: URL: https://github.com/apache/orc/pull/2702
`getTimezoneByFilename` overwrites existing entries, potentially causing causing callers which already took a pointer to that entry to end up with a dangling pointer. I think the best fix is to only insert for new keys, since if the key already exists, the value is correct - this is a cache for a pure function. I think the issue may have been introduced in https://github.com/apache/orc/commit/481aadf4bbcaf2ebc36e862dafa6a628b94283d2 (ORC-2011). I wrote a new unit test for the issue, `TimestampAliasCacheEviction.readerSurvivesAliasCacheEviction`. Without the fix, this test segfaults due to a dangling pointer. ASAN gives good diagnostics: ``` $ cmake /tmp/orc-tz-race \ -DBUILD_JAVA=OFF -DBUILD_TOOLS=OFF -DBUILD_CPP_TESTS=ON \ -DCMAKE_BUILD_TYPE=RelWithDebInfo \ -DCMAKE_CXX_FLAGS="-fsanitize=address" \ -DCMAKE_EXE_LINKER_FLAGS="-fsanitize=address" $ make -j$(nproc) orc-test $ ASAN_OPTIONS=detect_leaks=0 /tmp/orc-tz-race-build-asan/c++/test/orc-test --gtest_filter='TimestampAliasCacheEviction*' ... [ RUN ] TimestampAliasCacheEviction.readerSurvivesAliasCacheEviction ================================================================= ==917813==ERROR: AddressSanitizer: heap-use-after-free on address 0xffff94413b30 at pc 0x000000e65bf4 bp 0xfffff4e1ce60 sp 0xfffff4e1ce50 READ of size 8 at 0xffff94413b30 thread T0 #0 0xe65bf0 in orc::TimestampColumnReader::next(orc::ColumnVectorBatch&, unsigned long, char*) /tmp/orc-tz-race/c++/src/ColumnReader.cc:336 #1 0xe65208 in void orc::StructColumnReader::nextInternal<false>(orc::ColumnVectorBatch&, unsigned long, char*) /tmp/orc-tz-race/c++/src/ColumnReader.cc:868 #2 0xe65208 in orc::StructColumnReader::next(orc::ColumnVectorBatch&, unsigned long, char*) /tmp/orc-tz-race/c++/src/ColumnReader.cc:849 #3 0xc9a700 in orc::RowReaderImpl::next(orc::ColumnVectorBatch&) /tmp/orc-tz-race/c++/src/Reader.cc:1417 #4 0xc9a700 in orc::RowReaderImpl::next(orc::ColumnVectorBatch&) /tmp/orc-tz-race/c++/src/Reader.cc:1392 #5 0xd28f0c in orc::TimestampAliasCacheEviction_readerSurvivesAliasCacheEviction_Test::TestBody() /tmp/orc-tz-race/c++/test/TestWriter.cc:2708 #6 0x14bdb20 in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:2664 #7 0x14bdb20 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:2700 #8 0x14864b0 in testing::Test::Run() /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:2739 #9 0x1486ff0 in testing::Test::Run() /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:2730 #10 0x1486ff0 in testing::TestInfo::Run() /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:2885 #11 0x1489cc0 in testing::TestSuite::Run() /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:3063 #12 0x14a2cc0 in testing::internal::UnitTestImpl::RunAllTests() /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:6054 #13 0x1487464 in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:2664 #14 0x1487464 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:2700 #15 0x1487464 in testing::UnitTest::Run() /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:5594 #16 0x44ee74 in RUN_ALL_TESTS() /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/include/gtest/gtest.h:2334 #17 0x44ee74 in main /tmp/orc-tz-race/c++/test/TestDriver.cc:30 #18 0xffff9882453c in __libc_start_call_main (/lib64/libc.so.6+0x2753c) #19 0xffff98824614 in __libc_start_main_alias_2 (/lib64/libc.so.6+0x27614) #20 0x49562c in _start (/tmp/orc-tz-race-build-asan/c++/test/orc-test+0x49562c) 0xffff94413b30 is located 16 bytes inside of 72-byte region [0xffff94413b20,0xffff94413b68) freed by thread T0 here: #0 0xffff98d50ecc in operator delete(void*, unsigned long) (/lib64/libasan.so.6+0xafecc) #1 0xf95444 in __gnu_cxx::new_allocator<std::_Sp_counted_ptr_inplace<orc::LazyTimezone, std::allocator<orc::LazyTimezone>, (__gnu_cxx::_Lock_policy)2> >::deallocate(std::_Sp_counted_ptr_inplace<orc::LazyTimezone, std::allocator<orc::LazyTimezone>, (__gnu_cxx::_Lock_policy)2>*, unsigned long) /usr/include/c++/11/ext/new_allocator.h:145 #2 0xf95444 in std::allocator_traits<std::allocator<std::_Sp_counted_ptr_inplace<orc::LazyTimezone, std::allocator<orc::LazyTimezone>, (__gnu_cxx::_Lock_policy)2> > >::deallocate(std::allocator<std::_Sp_counted_ptr_inplace<orc::LazyTimezone, std::allocator<orc::LazyTimezone>, (__gnu_cxx::_Lock_policy)2> >&, std::_Sp_counted_ptr_inplace<orc::LazyTimezone, std::allocator<orc::LazyTimezone>, (__gnu_cxx::_Lock_policy)2>*, unsigned long) /usr/include/c++/11/bits/alloc_traits.h:496 #3 0xf95444 in std::__allocated_ptr<std::allocator<std::_Sp_counted_ptr_inplace<orc::LazyTimezone, std::allocator<orc::LazyTimezone>, (__gnu_cxx::_Lock_policy)2> > >::~__allocated_ptr() /usr/include/c++/11/bits/allocated_ptr.h:74 #4 0xf95444 in std::_Sp_counted_ptr_inplace<orc::LazyTimezone, std::allocator<orc::LazyTimezone>, (__gnu_cxx::_Lock_policy)2>::_M_destroy() /usr/include/c++/11/bits/shared_ptr_base.h:538 #5 0xf9325c in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release() /usr/include/c++/11/bits/shared_ptr_base.h:184 #6 0xf9325c in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release() /usr/include/c++/11/bits/shared_ptr_base.h:161 #7 0xf9325c in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count() /usr/include/c++/11/bits/shared_ptr_base.h:705 #8 0xf9325c in std::__shared_ptr<orc::Timezone, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr() /usr/include/c++/11/bits/shared_ptr_base.h:1154 #9 0xf9325c in std::enable_if<std::__sp_compatible_with<orc::LazyTimezone*, orc::Timezone*>::value, std::__shared_ptr<orc::Timezone, (__gnu_cxx::_Lock_policy)2>&>::type std::__shared_ptr<orc::Timezone, (__gnu_cxx::_Lock_policy)2>::operator=<orc::LazyTimezone>(std::__shared_ptr<orc::LazyTimezone, (__gnu_cxx::_Lock_policy)2>&&) /usr/include/c++/11/bits/shared_ptr_base.h:1258 #10 0xf9325c in std::enable_if<std::is_assignable<std::__shared_ptr<orc::Timezone, (__gnu_cxx::_Lock_policy)2>&, std::shared_ptr<orc::LazyTimezone> >::value, std::shared_ptr<orc::Timezone>&>::type std::shared_ptr<orc::Timezone>::operator=<orc::LazyTimezone>(std::shared_ptr<orc::LazyTimezone>&&) /usr/include/c++/11/bits/shared_ptr.h:393 #11 0xf9325c in orc::getTimezoneByFilename(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) /tmp/orc-tz-race/c++/src/Timezone.cc:776 #12 0xf94144 in orc::getTimezoneByName(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) /tmp/orc-tz-race/c++/src/Timezone.cc:809 #13 0xd28e5c in orc::TimestampAliasCacheEviction_readerSurvivesAliasCacheEviction_Test::TestBody() /tmp/orc-tz-race/c++/test/TestWriter.cc:2705 #14 0x14bdb20 in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:2664 #15 0x14bdb20 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:2700 #16 0x14864b0 in testing::Test::Run() /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:2739 #17 0x1486ff0 in testing::Test::Run() /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:2730 #18 0x1486ff0 in testing::TestInfo::Run() /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:2885 #19 0x1489cc0 in testing::TestSuite::Run() /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:3063 #20 0x14a2cc0 in testing::internal::UnitTestImpl::RunAllTests() /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:6054 #21 0x1487464 in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:2664 #22 0x1487464 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:2700 #23 0x1487464 in testing::UnitTest::Run() /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:5594 #24 0x44ee74 in RUN_ALL_TESTS() /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/include/gtest/gtest.h:2334 #25 0x44ee74 in main /tmp/orc-tz-race/c++/test/TestDriver.cc:30 #26 0xffff9882453c in __libc_start_call_main (/lib64/libc.so.6+0x2753c) #27 0xffff98824614 in __libc_start_main_alias_2 (/lib64/libc.so.6+0x27614) #28 0x49562c in _start (/tmp/orc-tz-race-build-asan/c++/test/orc-test+0x49562c) ``` I cannot file a tracking ticket because the [ORC Jira instance](https://issues.apache.org/jira/projects/ORC/issues) does not allow public signup. Generated-by: Claude Sonnet 4.6, but a human (me) reviewed everything throughly before putting up this pR. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
