From 6b19df3268fc517d1d3eedf61748ef6d81ff412e Mon Sep 17 00:00:00 2001 From: "John \"Juce\" Bruce" Date: Fri, 17 Jun 2022 15:54:36 -0700 Subject: [PATCH] Defer creation of default_config_ (This is a merge of http://go/wvgerrit/152969.) C++ makes absolutely no guarantees about the order of initialization of global variables in different compilation units. The class-scope static WvCdmTestBase::default_config_ in test_base.cpp invokes the ConfigTestEnv constructor on creation, which depends on the prior initialization of several file-scope static variables in config_test_env.cpp. Since those are different compilation units, there is no guarantee that they will initialize in the correct order to avoid referencing uninitialized memory. This is one of the reasons Google Style really encourages people not to have global-scope variables with complex types. As it happens, on all our internal platforms, these files get linked in such a way that the variables get initialized in the right order and there is no crash. But that's not guaranteed, and some partners have reported crashes here. In at least one case, the "right" linker order was platform-dependent, and the partner ended up having to maintain separate linker orders for separate platforms. This patch defers default_config_ initialization until WvCdmTestBase::Initialize() is called. By that time, all static variables will be initialized, so it will be safe to reference them. Bug: 173252165 Test: x86-64 Test: build_and_run_all_unit_tests.sh Change-Id: If31128a999c7d6945f47293ca57f08e43d8274de --- .../cdm/core/test/keybox_ota_test.cpp | 4 +- libwvdrmengine/cdm/core/test/reboot_test.h | 2 +- libwvdrmengine/cdm/core/test/test_base.cpp | 43 ++++++++++--------- libwvdrmengine/cdm/core/test/test_base.h | 7 +-- 4 files changed, 30 insertions(+), 26 deletions(-) diff --git a/libwvdrmengine/cdm/core/test/keybox_ota_test.cpp b/libwvdrmengine/cdm/core/test/keybox_ota_test.cpp index 39e7ca72..66046417 100644 --- a/libwvdrmengine/cdm/core/test/keybox_ota_test.cpp +++ b/libwvdrmengine/cdm/core/test/keybox_ota_test.cpp @@ -40,7 +40,7 @@ class CdmOtaKeyboxTest : public ::testing::Test { } void Provision(TestCdmEngine* cdm_engine) { - ConfigTestEnv config = WvCdmTestBase::default_config_; + ConfigTestEnv config = *WvCdmTestBase::default_config_; CdmCertificateType cert_type = kCertificateWidevine; std::string cert_authority; CdmProvisioningRequest prov_request; @@ -114,7 +114,7 @@ TEST_F(CdmOtaKeyboxTest, BasicTest) { } CdmSessionId session_id; - ConfigTestEnv config = WvCdmTestBase::default_config_; + ConfigTestEnv config = *WvCdmTestBase::default_config_; CdmResponseType status = cdm_engine.OpenSession(config.key_system(), nullptr, nullptr, &session_id); diff --git a/libwvdrmengine/cdm/core/test/reboot_test.h b/libwvdrmengine/cdm/core/test/reboot_test.h index 0a9234be..fae4993e 100644 --- a/libwvdrmengine/cdm/core/test/reboot_test.h +++ b/libwvdrmengine/cdm/core/test/reboot_test.h @@ -34,7 +34,7 @@ class RebootTest : public WvCdmTestBaseWithEngine { static bool ParseDump(const std::string& dump, std::map* data); - static int test_pass() { return default_config_.test_pass(); } + static int test_pass() { return default_config_->test_pass(); } // Load a previously saved time. Returns 0 if the value does not exist or // cannot be parsed. diff --git a/libwvdrmengine/cdm/core/test/test_base.cpp b/libwvdrmengine/cdm/core/test/test_base.cpp index c51eb908..f2c931eb 100644 --- a/libwvdrmengine/cdm/core/test/test_base.cpp +++ b/libwvdrmengine/cdm/core/test/test_base.cpp @@ -179,7 +179,7 @@ bool ExtractSignedMessage(const std::string& response, } // namespace -ConfigTestEnv WvCdmTestBase::default_config_(kContentProtectionUatServer); +std::unique_ptr WvCdmTestBase::default_config_; bool WvCdmTestBase::use_qa_test_keybox_ = false; void WvCdmTestBase::StripeBuffer(std::vector* buffer, size_t size, @@ -309,7 +309,7 @@ void WvCdmTestBase::InstallTestRootOfTrust() { } WvCdmTestBase::WvCdmTestBase() - : config_(default_config_), binary_provisioning_(false) { + : config_(*default_config_), binary_provisioning_(false) { const ::testing::TestInfo* const test_info = ::testing::UnitTest::GetInstance()->current_test_info(); @@ -469,6 +469,8 @@ bool WvCdmTestBase::Initialize(int argc, const char* const argv[], bool show_usage = false; int verbosity = 0; + default_config_.reset(new ConfigTestEnv(kContentProtectionUatServer)); + // Skip the first element, which is the program name. const std::vector args(argv + 1, argv + argc); for (const std::string& arg : args) { @@ -482,8 +484,8 @@ bool WvCdmTestBase::Initialize(int argc, const char* const argv[], wvutil::TestSleep::set_real_sleep(false); } else if (arg == "--qa_provisioning") { use_qa_test_keybox_ = true; - default_config_.set_provisioning_service_certificate( - default_config_.QAProvisioningServiceCertificate()); + default_config_->set_provisioning_service_certificate( + default_config_->QAProvisioningServiceCertificate()); } else if (arg.find("--gtest") == 0) { // gtest arguments will be passed to gtest by the main program. continue; @@ -500,33 +502,34 @@ bool WvCdmTestBase::Initialize(int argc, const char* const argv[], const std::string arg_value = arg.substr(index + 1); if (arg_prefix == "--license_server_id") { if (arg_value == "gp") { - default_config_ = ConfigTestEnv(kGooglePlayServer); + default_config_.reset(new ConfigTestEnv(kGooglePlayServer)); } else if (arg_value == "cp") { - default_config_ = ConfigTestEnv(kContentProtectionUatServer); + default_config_.reset(new ConfigTestEnv(kContentProtectionUatServer)); } else if (arg_value == "st") { - default_config_ = ConfigTestEnv(kContentProtectionStagingServer); + default_config_.reset( + new ConfigTestEnv(kContentProtectionStagingServer)); } else { std::cerr << "Invalid license server id: " << arg_value << std::endl; show_usage = true; break; } } else if (arg_prefix == "--keyid") { - default_config_.set_key_id(arg_value); + default_config_->set_key_id(arg_value); } else if (arg_prefix == "--service_certificate") { const std::string certificate(wvutil::a2bs_hex(arg_value)); - default_config_.set_license_service_certificate(certificate); + default_config_->set_license_service_certificate(certificate); } else if (arg_prefix == "--provisioning_certificate") { const std::string certificate(wvutil::a2bs_hex(arg_value)); - default_config_.set_provisioning_service_certificate(certificate); + default_config_->set_provisioning_service_certificate(certificate); } else if (arg_prefix == "--license_server_url") { - default_config_.set_license_server(arg_value); + default_config_->set_license_server(arg_value); } else if (arg_prefix == "--renewal_server_url") { - default_config_.set_renewal_server(arg_value); + default_config_->set_renewal_server(arg_value); } else if (arg_prefix == "--provisioning_server_url") { - default_config_.set_provisioning_server(arg_value); + default_config_->set_provisioning_server(arg_value); } else if (arg_prefix == "--pass") { - default_config_.set_test_pass(std::stoi(arg_value)); - std::cout << "Running test pass " << default_config_.test_pass() + default_config_->set_test_pass(std::stoi(arg_value)); + std::cout << "Running test pass " << default_config_->test_pass() << std::endl; } else if (arg_prefix == "--initial_time") { if (wvutil::TestSleep::real_sleep()) { @@ -537,7 +540,7 @@ bool WvCdmTestBase::Initialize(int argc, const char* const argv[], wvutil::TestSleep::SetFakeClock(stol(arg_value)); } } else if (arg_prefix == "--test_data_path") { - default_config_.set_test_data_path(arg_value); + default_config_->set_test_data_path(arg_value); } else { std::cerr << "Unknown argument " << arg_prefix << std::endl; show_usage = true; @@ -556,12 +559,12 @@ bool WvCdmTestBase::Initialize(int argc, const char* const argv[], // Displays server url, port and key Id being used std::cout << std::endl; std::cout << "Default Provisioning Server: " - << default_config_.provisioning_server() << std::endl; - std::cout << "Default License Server: " << default_config_.license_server() + << default_config_->provisioning_server() << std::endl; + std::cout << "Default License Server: " << default_config_->license_server() << std::endl; - std::cout << "Default Renewal Server: " << default_config_.renewal_server() + std::cout << "Default Renewal Server: " << default_config_->renewal_server() << std::endl; - std::cout << "Default KeyID: " << default_config_.key_id() << std::endl + std::cout << "Default KeyID: " << default_config_->key_id() << std::endl << std::endl; // Figure out which tests are appropriate for OEMCrypto, based on features diff --git a/libwvdrmengine/cdm/core/test/test_base.h b/libwvdrmengine/cdm/core/test/test_base.h index 7606ad37..68372c3a 100644 --- a/libwvdrmengine/cdm/core/test/test_base.h +++ b/libwvdrmengine/cdm/core/test/test_base.h @@ -7,6 +7,7 @@ #include +#include #include #include @@ -63,9 +64,9 @@ class WvCdmTestBase : public ::testing::Test { static std::string SignHMAC(const std::string& message, const std::vector& key); - // The default test configuration. This is influenced by command line - // arguments before any tests are created. - static ConfigTestEnv default_config_; + // The default test configuration. This is created in Initialize() and + // influenced by command line arguments before any tests are created. + static std::unique_ptr default_config_; // If the tests should use the QA test keybox. static bool use_qa_test_keybox_;