From 2f0358257dac4b2ddae091a5ebfbf25e045a9fd0 Mon Sep 17 00:00:00 2001 From: hidehiko Date: Tue, 23 Jun 2015 01:07:54 -0700 Subject: [PATCH] Reland "Non-SFI mode: Switch to newlib. (patchset #4 id:60001 of https://codereview.chromium.org/1184683002/)" This reverts commit a947f3b0181f51a94a18bd80227960037a936c70. TEST=Run bots. Run autotests locally. BUG=421748, 501272 Review URL: https://codereview.chromium.org/1193103003 Cr-Commit-Position: refs/heads/master@{#335646} --- chrome/test/nacl/nacl_browsertest_util.cc | 2 +- chrome/test/nacl/nacl_browsertest_util.h | 43 +++++++++++++++++-------------- chrome/test/ppapi/ppapi_browsertest.cc | 7 ++--- chrome/test/ppapi/ppapi_test.cc | 4 +-- chrome/test/ppapi/ppapi_test.h | 8 +++--- components/nacl/common/nacl_paths.cc | 13 +++++----- 6 files changed, 41 insertions(+), 36 deletions(-) diff --git a/chrome/test/nacl/nacl_browsertest_util.cc b/chrome/test/nacl/nacl_browsertest_util.cc index bd79bdf94d68..3ecb690e5556 100644 --- a/chrome/test/nacl/nacl_browsertest_util.cc +++ b/chrome/test/nacl/nacl_browsertest_util.cc @@ -301,7 +301,7 @@ void NaClBrowserTestNonSfiMode::SetUpCommandLine( void NaClBrowserTestTransitionalNonSfi::SetUpCommandLine( base::CommandLine* command_line) { NaClBrowserTestNonSfiMode::SetUpCommandLine(command_line); - command_line->AppendSwitch(switches::kUseNaClHelperNonSfi); + command_line->AppendSwitchASCII(switches::kUseNaClHelperNonSfi, "false"); } base::FilePath::StringType NaClBrowserTestStatic::Variant() { diff --git a/chrome/test/nacl/nacl_browsertest_util.h b/chrome/test/nacl/nacl_browsertest_util.h index b40bf89ba878..cfe4971133b9 100644 --- a/chrome/test/nacl/nacl_browsertest_util.h +++ b/chrome/test/nacl/nacl_browsertest_util.h @@ -137,11 +137,14 @@ class NaClBrowserTestPnaclNonSfi : public NaClBrowserTestBase { base::FilePath::StringType Variant() override; }; -// "Transitional" here means that this uses nacl_helper_nonsfi which has -// nacl_helper feature for Non-SFI mode, but statically linked to newlib -// instead of using host glibc. It is still under development. -// TODO(hidehiko): Switch NonSfi tests to use nacl_helper_nonsfi, when -// it is launched officially. +// "Transitional" here means that this uses nacl_helper in Non-SFI mode. +// nacl_helper_nonsfi, which is replacing nacl_helper in Non-SFI mode, is being +// launched. In the meanwhile, nacl_helper in Non-SFI is still kept just in +// case. When the launching is successfully done, it will be removed. +// "Transitional" tests are for ensuring compatibility between those two +// binaries. +// TODO(hidehiko): Remove the tests when nacl_helper in Non-SFI mode is +// removed. class NaClBrowserTestPnaclTransitionalNonSfi : public NaClBrowserTestPnaclNonSfi { public: @@ -154,8 +157,8 @@ class NaClBrowserTestNonSfiMode : public NaClBrowserTestBase { base::FilePath::StringType Variant() override; }; -// TODO(hidehiko): Switch NonSfi tests to use nacl_helper_nonsfi, when -// it is launched officially. See NaClBrowserTestPnaclTransitionalNonSfi +// TODO(hidehiko): Remove this when clean-up to drop Non-SFI support from +// nacl_helper is done. See NaClBrowserTestPnaclTransitionalNonSfi // for more details. class NaClBrowserTestTransitionalNonSfi : public NaClBrowserTestNonSfiMode { public: @@ -199,38 +202,38 @@ class NaClBrowserTestGLibcExtension : public NaClBrowserTestGLibc { # define MAYBE_GLIBC(test_name) DISABLED_##test_name #endif -// Sanitizers internally use some syscalls which non-SFI NaCl disallows. +// Currently, we only support it on x86-32 or ARM architecture. +// TODO(hidehiko,mazda): Enable this on x86-64, too, when it is supported. #if defined(OS_LINUX) && !defined(ADDRESS_SANITIZER) && \ !defined(THREAD_SANITIZER) && !defined(MEMORY_SANITIZER) && \ - !defined(LEAK_SANITIZER) + !defined(LEAK_SANITIZER) && \ + (defined(ARCH_CPU_X86) || defined(ARCH_CPU_ARMEL)) # define MAYBE_NONSFI(test_case) test_case #else # define MAYBE_NONSFI(test_case) DISABLED_##test_case #endif -// Currently, we only support it on x86-32 or ARM architecture. -// TODO(hidehiko,mazda): Enable this on x86-64, too, when it is supported. +// Sanitizers internally use some syscalls which non-SFI NaCl disallows. #if defined(OS_LINUX) && !defined(ADDRESS_SANITIZER) && \ !defined(THREAD_SANITIZER) && !defined(MEMORY_SANITIZER) && \ - !defined(LEAK_SANITIZER) && \ - (defined(ARCH_CPU_X86) || defined(ARCH_CPU_ARMEL)) + !defined(LEAK_SANITIZER) # define MAYBE_TRANSITIONAL_NONSFI(test_case) test_case #else # define MAYBE_TRANSITIONAL_NONSFI(test_case) DISABLED_##test_case #endif -// Currently, translation from pexe to non-sfi nexe is supported only for -// x86-32 or ARM binary. -#if defined(OS_LINUX) && (defined(ARCH_CPU_X86) || defined(ARCH_CPU_ARMEL)) +// Similar to MAYBE_NONSFI, this is available only on x86-32, x86-64 or +// ARM linux. +#if defined(OS_LINUX) && \ + (defined(ARCH_CPU_X86_FAMILY) || defined(ARCH_CPU_ARMEL)) # define MAYBE_PNACL_NONSFI(test_case) test_case #else # define MAYBE_PNACL_NONSFI(test_case) DISABLED_##test_case #endif -// Similar to MAYBE_TRANSITIONAL_NONSFI, this is available only on x86-32, -// x86-64 or ARM linux. -#if defined(OS_LINUX) && \ - (defined(ARCH_CPU_X86_FAMILY) || defined(ARCH_CPU_ARMEL)) +// Currently, translation from pexe to non-sfi nexe is supported only for +// x86-32 or ARM binary. +#if defined(OS_LINUX) && (defined(ARCH_CPU_X86) || defined(ARCH_CPU_ARMEL)) # define MAYBE_PNACL_TRANSITIONAL_NONSFI(test_case) test_case #else # define MAYBE_PNACL_TRANSITIONAL_NONSFI(test_case) DISABLED_##test_case diff --git a/chrome/test/ppapi/ppapi_browsertest.cc b/chrome/test/ppapi/ppapi_browsertest.cc index 76bcc8d2f29f..9225f0f214e4 100644 --- a/chrome/test/ppapi/ppapi_browsertest.cc +++ b/chrome/test/ppapi/ppapi_browsertest.cc @@ -1400,14 +1400,15 @@ class NonSfiPackagedAppTest : public PackagedAppTest { } }; -// TODO(hidehiko): Switch for NonSfi tests to use nacl_helper_nonsfi, when -// it is launched officially. See NaClBrowserTestPnaclTransitionalNonSfi +// TODO(hidehiko): Remove this when clean-up to drop Non-SFI support from +// nacl_helper is done. See NaClBrowserTestPnaclTransitionalNonSfi // for more details. class TransitionalNonSfiPackagedAppTest : public NonSfiPackagedAppTest { public: void SetUpCommandLine(base::CommandLine* command_line) override { NonSfiPackagedAppTest::SetUpCommandLine(command_line); - command_line->AppendSwitch(switches::kUseNaClHelperNonSfi); + command_line->AppendSwitchASCII(switches::kUseNaClHelperNonSfi, + "false"); } }; diff --git a/chrome/test/ppapi/ppapi_test.cc b/chrome/test/ppapi/ppapi_test.cc index 6e9fc0356049..11e287d2917b 100644 --- a/chrome/test/ppapi/ppapi_test.cc +++ b/chrome/test/ppapi/ppapi_test.cc @@ -433,7 +433,7 @@ void PPAPINaClPNaClTransitionalNonSfiTest::SetUpCommandLine( base::CommandLine* command_line) { PPAPINaClPNaClNonSfiTest::SetUpCommandLine(command_line); #if !defined(DISABLE_NACL) - command_line->AppendSwitch(switches::kUseNaClHelperNonSfi); + command_line->AppendSwitchASCII(switches::kUseNaClHelperNonSfi, "false"); #endif } @@ -447,7 +447,7 @@ void PPAPIPrivateNaClPNaClTransitionalNonSfiTest::SetUpCommandLine( base::CommandLine* command_line) { PPAPIPrivateNaClPNaClNonSfiTest::SetUpCommandLine(command_line); #if !defined(DISABLE_NACL) - command_line->AppendSwitch(switches::kUseNaClHelperNonSfi); + command_line->AppendSwitchASCII(switches::kUseNaClHelperNonSfi, "false"); #endif } diff --git a/chrome/test/ppapi/ppapi_test.h b/chrome/test/ppapi/ppapi_test.h index 0110ca589c0d..92805f4703f8 100644 --- a/chrome/test/ppapi/ppapi_test.h +++ b/chrome/test/ppapi/ppapi_test.h @@ -182,8 +182,8 @@ class PPAPINaClPNaClNonSfiTest : public PPAPINaClTest { const std::string& test_case) override; }; -// TODO(hidehiko): Switch NonSfi tests to use nacl_helper_nonsfi, when -// it is launched officially. See NaClBrowserTestPnaclTransitionalNonSfi +// TODO(hidehiko): Remove this when clean-up to drop Non-SFI support from +// nacl_helper is done. See NaClBrowserTestPnaclTransitionalNonSfi // for more details. class PPAPINaClPNaClTransitionalNonSfiTest : public PPAPINaClPNaClNonSfiTest { public: @@ -195,8 +195,8 @@ class PPAPIPrivateNaClPNaClNonSfiTest : public PPAPINaClPNaClNonSfiTest { void SetUpCommandLine(base::CommandLine* command_line) override; }; -// TODO(hidehiko): Switch NonSfi tests to use nacl_helper_nonsfi, when -// it is launched officially. See NaClBrowserTestPnaclTransitionalNonSfi +// TODO(hidehiko): Remove this when clean-up to drop Non-SFI support from +// nacl_helper is done. See NaClBrowserTestPnaclTransitionalNonSfi // for more details. class PPAPIPrivateNaClPNaClTransitionalNonSfiTest : public PPAPIPrivateNaClPNaClNonSfiTest { diff --git a/components/nacl/common/nacl_paths.cc b/components/nacl/common/nacl_paths.cc index 442d1493689c..f7093800bc1f 100644 --- a/components/nacl/common/nacl_paths.cc +++ b/components/nacl/common/nacl_paths.cc @@ -40,12 +40,13 @@ bool PathProvider(int key, base::FilePath* result) { case FILE_NACL_HELPER: return GetNaClHelperPath(kInternalNaClHelperFileName, result); case FILE_NACL_HELPER_NONSFI: - if (!base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kUseNaClHelperNonSfi)) { - // Currently nacl_helper_nonsfi is disabled, so use nacl_helper - // in Non-SFI mode instead. - // TODO(hidehiko): Remove this code path after nacl_helper_nonsfi - // is supported. + if (base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII( + switches::kUseNaClHelperNonSfi) == "false") { + // nacl_helper_nonsfi is disabled by the flag. So, use nacl_helper + // in Non-SFI mode instead. This is old behavior just in case. + // TODO(hidehiko): Remove this code path, when the old feature is + // cleaned after nacl_helper_nonsfi is officially launched and + // its stability is confirmed. return GetNaClHelperPath(kInternalNaClHelperFileName, result); } return GetNaClHelperPath(kInternalNaClHelperNonSfiFileName, result); -- 2.11.4.GIT