MemorySanitizer: use-of-uninitialized-value issue due to accessing destroyed class member

[yanjunxiang-google]

Accessing destroyed class member causing MemorySanitizer: use-of-uninitialized-value issue

Signed-off-by: Yanjun Xiang yanjunxiang@google.com

Commit Message:
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

[yanjunxiang-google]

For reviewers:

==7987==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 in Envoy::AccessLog::AccessLogFileImpl::doWrite(Envoy::Buffer::Instance&) source/common/access_log/access_log_manager_impl.cc
#1 in Envoy::AccessLog::AccessLogFileImpl::~AccessLogFileImpl() source/common/access_log/access_log_manager_impl.cc:91:7
#2 in std::__msan::__shared_ptr_emplace<Envoy::AccessLog::AccessLogFileImpl, std::__msan::allocatorEnvoy::AccessLog::AccessLogFileImpl >::__on_zero_shared() include/c++/v1/__memory/shared_ptr.h:315:24
#3 in __release_shared include/c++/v1/__memory/shared_ptr.h:177:9
#4 in __release_shared include/c++/v1/__memory/shared_ptr.h:219:27
#5 in ~shared_ptr include/c++/v1/__memory/shared_ptr.h:706:23
#6 in Envoy::Extensions::AccessLoggers::File::FileAccessLog::~FileAccessLog() source/extensions/access_loggers/common/file_access_log_impl.h:14:7
#7 in std::__msan::__shared_ptr_emplace<Envoy::Extensions::AccessLoggers::File::FileAccessLog, std::__msan::allocatorEnvoy::Extensions::AccessLoggers::File::FileAccessLog >::__on_zero_shared() include/c++/v1/__memory/shared_ptr.h:315:24
#8 in __release_shared include/c++/v1/__memory/shared_ptr.h:177:9
#9 in __release_shared include/c++/v1/__memory/shared_ptr.h:219:27
#10 in shared_ptr include/c++/v1/__memory/shared_ptr.h:706:23
#11 in destroy<std::__msan::shared_ptrEnvoy::AccessLog::Instance, void, void> include/c++/v1/__memory/allocator_traits.h:319:15
#12 in clear include/c++/v1/list:749:13
#13 in std::__msan::__list_imp<std::__msan::shared_ptrEnvoy::AccessLog::Instance, std::__msan::allocator<std::__msan::shared_ptrEnvoy::AccessLog::Instance > >::__list_imp() include/c++/v1/list:728:3
#14 in Envoy::Server::AdminImpl::~AdminImpl() source/server/admin/admin.h:64:7
#15 in Envoy::Server::AdminImpl::~AdminImpl() source/server/admin/admin.h:64:7
#16 in operator() include/c++/v1/__memory/unique_ptr.h:57:5
#17 in ~unique_ptr include/c++/v1/__memory/unique_ptr.h:275:7
#18 in Envoy::Server::InstanceImpl::~InstanceImpl() source/server/server.cc:163:1
#19 in Envoy::IntegrationTestServerImpl::createAndRunEnvoyServer(Envoy::OptionsImpl&, Envoy::Event::TimeSystem&, std::__msan::shared_ptr<Envoy::Network::Address::Instance const>, Envoy::ListenerHooks&, Envoy::Thread::BasicLockable&, Envoy::Server::ComponentFactory&, std::__msan::unique_ptr<Envoy::Random::RandomGenerator, std::__msan::default_deleteEnvoy::Random::RandomGenerator >&&, std::__msan::optional<std::__msan::reference_wrapperEnvoy::ProcessObject >, std::__msan::shared_ptrEnvoy::Buffer::WatermarkFactory) test/integration/server.cc:243:3
#20 in Envoy::IntegrationTestServer::threadRoutine(Envoy::Network::Address::IpVersion, std::__msan::optional, std::__msan::optional<std::__msan::reference_wrapperEnvoy::ProcessObject >, Envoy::Server::FieldValidationConfig, unsigned int, std::__msan::chrono::duration<long long, std::__msan::ratio<1l, 1l> >, Envoy::Server::DrainStrategy, std::__msan::shared_ptrEnvoy::Buffer::WatermarkFactory) test/integration/server.cc:201:3
#21 in operator() test/integration/server.cc:108:5
#22 in __invoke<(lambda at test/integration/server.cc:105:47) &> include/c++/v1/type_traits:3493:23
#23 in __call<(lambda at test/integration/server.cc:105:47) &> include/c++/v1/__functional/invoke.h:61:9
#24 in operator() include/c++/v1/__functional/function.h:232:12
#25 in void std::__msan::__function::__policy_invoker<void ()>::__call_impl<std::__msan::__function::__default_alloc_func<Envoy::IntegrationTestServer::start(Envoy::Network::Address::IpVersion, std::__msan::function<void ()>, std::__msan::optional, bool, std::__msan::optional<std::__msan::reference_wrapperEnvoy::ProcessObject >, Envoy::Server::FieldValidationConfig, unsigned int, std::__msan::chrono::duration<long long, std::__msan::ratio<1l, 1l> >, Envoy::Server::DrainStrategy, std::__msan::shared_ptrEnvoy::Buffer::WatermarkFactory)::$_2, void ()> >(std::__msan::__function::__policy_storage const*) include/c++/v1/__functional/function.h:713:16
#26 in operator() include/c++/v1/__functional/function.h:845:16
#27 in operator() include/c++/v1/__functional/function.h:1186:12
#28 in operator() source/common/common/posix/thread_impl.cc:49:11
#29 in Envoy::Thread::ThreadImplPosix::ThreadImplPosix(std::__msan::function<void ()>, std::__msan::optionalEnvoy::Thread::Options const&)::'lambda'(void*)::__invoke(void*) source/common/common/posix/thread_impl.cc:48:9
#30 in start_thread (/usr/grte/v5/lib64/libpthread.so.0)
#31 in clone (/usr/grte/v5/lib64/libc.so.6)

Memory was marked as uninitialized
#0 in __sanitizer_dtor_callback llvm-project/compiler-rt/lib/msan/msan_interceptors.cpp:940:5
#1 in Envoy::AccessLog::AccessLogManagerImpl::~AccessLogManagerImpl() source/common/access_log/access_log_manager_impl.cc:22:1
#2 in Envoy::Server::InstanceImpl::~InstanceImpl() source/server/server.cc:163:1
#3 in Envoy::IntegrationTestServerImpl::createAndRunEnvoyServer(Envoy::OptionsImpl&, Envoy::Event::TimeSystem&, std::__msan::shared_ptr<Envoy::Network::Address::Instance const>, Envoy::ListenerHooks&, Envoy::Thread::BasicLockable&, Envoy::Server::ComponentFactory&, std::__msan::unique_ptr<Envoy::Random::RandomGenerator, std::__msan::default_deleteEnvoy::Random::RandomGenerator >&&, std::__msan::optional<std::__msan::reference_wrapperEnvoy::ProcessObject >, std::__msan::shared_ptrEnvoy::Buffer::WatermarkFactory) test/integration/server.cc:243:3
#4 in Envoy::IntegrationTestServer::threadRoutine(Envoy::Network::Address::IpVersion, std::__msan::optional, std::__msan::optional<std::__msan::reference_wrapperEnvoy::ProcessObject >, Envoy::Server::FieldValidationConfig, unsigned int, std::__msan::chrono::duration<long long, std::__msan::ratio<1l, 1l> >, Envoy::Server::DrainStrategy, std::__msan::shared_ptrEnvoy::Buffer::WatermarkFactory) test/integration/server.cc:201:3
#5 in operator() test/integration/server.cc:108:5
#6 in __invoke<(lambda at test/integration/server.cc:105:47) &> include/c++/v1/type_traits:3493:23
#7 in __call<(lambda at test/integration/server.cc:105:47) &> include/c++/v1/__functional/invoke.h:61:9
#8 in operator() include/c++/v1/__functional/function.h:232:12
#9 in void std::__msan::__function::__policy_invoker<void ()>::__call_impl<std::__msan::__function::__default_alloc_func<Envoy::IntegrationTestServer::start(Envoy::Network::Address::IpVersion, std::__msan::function<void ()>, std::__msan::optional, bool, std::__msan::optional<std::__msan::reference_wrapperEnvoy::ProcessObject >, Envoy::Server::FieldValidationConfig, unsigned int, std::__msan::chrono::duration<long long, std::__msan::ratio<1l, 1l> >, Envoy::Server::DrainStrategy, std::__msan::shared_ptrEnvoy::Buffer::WatermarkFactory)::$_2, void ()> >(std::__msan::__function::__policy_storage const*) include/c++/v1/__functional/function.h:713:16
#10 in operator() include/c++/v1/__functional/function.h:845:16
#11 in operator() include/c++/v1/__functional/function.h:1186:12
#12 in operator() source/common/common/posix/thread_impl.cc:49:11
#13 in Envoy::Thread::ThreadImplPosix::ThreadImplPosix(std::__msan::function<void ()>, std::__msan::optionalEnvoy::Thread::Options const&)::'lambda'(void*)::__invoke(void*) source/common/common/posix/thread_impl.cc:48:9
#14 in start_thread (/usr/grte/v5/lib64/libpthread.so.0)
Important frames only:

==7987==WARNING: MemorySanitizer: use-of-uninitialized-value
// Destroyes admin_ which calls into

envoy/source/common/access_log/access_log_manager_impl.cc

Line 116 in 052f94e

stats_.write_completed_.inc();

.
#0 in Envoy::AccessLog::AccessLogFileImpl::doWrite(Envoy::Buffer::Instance&) source/common/access_log/access_log_manager_impl.cc
#15 in Envoy::Server::AdminImpl::~AdminImpl() source/server/admin/admin.h:64:7
#18 in Envoy::Server::InstanceImpl::~InstanceImpl() source/server/server.cc:163:1

Memory was marked as uninitialized
// Destroyes access_log_manager_.
#1 in Envoy::AccessLog::AccessLogManagerImpl::~AccessLogManagerImpl() source/common/access_log/access_log_manager_impl.cc:22:1
#2 in Envoy::Server::InstanceImpl::~InstanceImpl() source/server/server.cc:163:1
access_log_manager_ member is being destroyed before admin_.

The fix is just to reorder members.

[yanjunxiang-google]

/assign @vitalybuka @wbpcode

[repokitteh-read-only]

@vitalybuka cannot be assigned to this issue.

🐱

Caused by: a #20913 (comment) was created by @yanjunxiang-google.

see: more, trace.

[vitalybuka]

LGTM

[wbpcode]

LGTM. Thanks cc @vitalybuka @yanjunxiang-google