diff --git a/libbacktrace/Backtrace.cpp b/libbacktrace/Backtrace.cpp index afe518c5b..e18dbf3db 100644 --- a/libbacktrace/Backtrace.cpp +++ b/libbacktrace/Backtrace.cpp @@ -14,7 +14,6 @@ * limitations under the License. */ -#include #include #include #include @@ -135,34 +134,6 @@ Backtrace* Backtrace::Create(pid_t pid, pid_t tid, BacktraceMap* map) { tid = pid; } - if (pid == getpid()) { - return new UnwindCurrent(pid, tid, map); - } else { - return new UnwindPtrace(pid, tid, map); - } -} - -Backtrace* Backtrace::CreateNew(pid_t pid, pid_t tid, BacktraceMap* map) { - if (pid == BACKTRACE_CURRENT_PROCESS) { - pid = getpid(); - if (tid == BACKTRACE_CURRENT_THREAD) { - tid = gettid(); - } - } else if (tid == BACKTRACE_CURRENT_THREAD) { - tid = pid; - } - - if (map == nullptr) { -// This would cause the wrong type of map object to be created, so disallow. -#if defined(__ANDROID__) - __assert2(__FILE__, __LINE__, __PRETTY_FUNCTION__, - "Backtrace::CreateNew() must be called with a real map pointer."); -#else - BACK_LOGE("Backtrace::CreateNew() must be called with a real map pointer."); - abort(); -#endif - } - if (pid == getpid()) { return new UnwindStackCurrent(pid, tid, map); } else { diff --git a/libbacktrace/UnwindMap.cpp b/libbacktrace/UnwindMap.cpp index 0b8232b2f..3cab0d1b0 100644 --- a/libbacktrace/UnwindMap.cpp +++ b/libbacktrace/UnwindMap.cpp @@ -146,24 +146,3 @@ void UnwindMapLocal::FillIn(uintptr_t addr, backtrace_map_t* map) { } } } - -//------------------------------------------------------------------------- -// BacktraceMap create function. -//------------------------------------------------------------------------- -BacktraceMap* BacktraceMap::Create(pid_t pid, bool uncached) { - BacktraceMap* map; - - if (uncached) { - // Force use of the base class to parse the maps when this call is made. - map = new BacktraceMap(pid); - } else if (pid == getpid()) { - map = new UnwindMapLocal(); - } else { - map = new UnwindMapRemote(pid); - } - if (!map->Build()) { - delete map; - return nullptr; - } - return map; -} diff --git a/libbacktrace/UnwindStackMap.cpp b/libbacktrace/UnwindStackMap.cpp index e7e5e4c43..25e5002ba 100644 --- a/libbacktrace/UnwindStackMap.cpp +++ b/libbacktrace/UnwindStackMap.cpp @@ -103,7 +103,7 @@ std::shared_ptr UnwindStackMap::GetProcessMemory() { //------------------------------------------------------------------------- // BacktraceMap create function. //------------------------------------------------------------------------- -BacktraceMap* BacktraceMap::CreateNew(pid_t pid, bool uncached) { +BacktraceMap* BacktraceMap::Create(pid_t pid, bool uncached) { BacktraceMap* map; if (uncached) { diff --git a/libbacktrace/backtrace_benchmarks.cpp b/libbacktrace/backtrace_benchmarks.cpp index 2593e0530..bb4134fec 100644 --- a/libbacktrace/backtrace_benchmarks.cpp +++ b/libbacktrace/backtrace_benchmarks.cpp @@ -149,12 +149,6 @@ static void BM_create_map(benchmark::State& state) { } BENCHMARK(BM_create_map); -static void BM_create_map_new(benchmark::State& state) { - CreateMap(state, BacktraceMap::CreateNew); -} -BENCHMARK(BM_create_map_new); - - using BacktraceCreateFn = decltype(Backtrace::Create); static void CreateBacktrace(benchmark::State& state, BacktraceMap* map, BacktraceCreateFn fn) { @@ -170,10 +164,4 @@ static void BM_create_backtrace(benchmark::State& state) { } BENCHMARK(BM_create_backtrace); -static void BM_create_backtrace_new(benchmark::State& state) { - std::unique_ptr backtrace_map(BacktraceMap::CreateNew(getpid())); - CreateBacktrace(state, backtrace_map.get(), Backtrace::CreateNew); -} -BENCHMARK(BM_create_backtrace_new); - BENCHMARK_MAIN(); diff --git a/libbacktrace/backtrace_test.cpp b/libbacktrace/backtrace_test.cpp index c3e5da09b..0a60ec4f3 100644 --- a/libbacktrace/backtrace_test.cpp +++ b/libbacktrace/backtrace_test.cpp @@ -368,20 +368,6 @@ TEST(libbacktrace, ptrace_trace) { ASSERT_EQ(waitpid(pid, &status, 0), pid); } -TEST(libbacktrace, ptrace_trace_new) { - pid_t pid; - if ((pid = fork()) == 0) { - ASSERT_NE(test_level_one(1, 2, 3, 4, nullptr, nullptr), 0); - _exit(1); - } - VerifyProcTest(pid, BACKTRACE_CURRENT_THREAD, ReadyLevelBacktrace, VerifyLevelDump, - Backtrace::CreateNew, BacktraceMap::CreateNew); - - kill(pid, SIGKILL); - int status; - ASSERT_EQ(waitpid(pid, &status, 0), pid); -} - TEST(libbacktrace, ptrace_max_trace) { pid_t pid; if ((pid = fork()) == 0) { @@ -396,20 +382,6 @@ TEST(libbacktrace, ptrace_max_trace) { ASSERT_EQ(waitpid(pid, &status, 0), pid); } -TEST(libbacktrace, ptrace_max_trace_new) { - pid_t pid; - if ((pid = fork()) == 0) { - ASSERT_NE(test_recursive_call(MAX_BACKTRACE_FRAMES + 10, nullptr, nullptr), 0); - _exit(1); - } - VerifyProcTest(pid, BACKTRACE_CURRENT_THREAD, ReadyMaxBacktrace, VerifyMaxDump, - Backtrace::CreateNew, BacktraceMap::CreateNew); - - kill(pid, SIGKILL); - int status; - ASSERT_EQ(waitpid(pid, &status, 0), pid); -} - static void VerifyProcessIgnoreFrames(Backtrace* bt_all, create_func_t create_func, map_create_func_t map_create_func) { std::unique_ptr map(map_create_func(bt_all->Pid(), false)); @@ -440,20 +412,6 @@ TEST(libbacktrace, ptrace_ignore_frames) { ASSERT_EQ(waitpid(pid, &status, 0), pid); } -TEST(libbacktrace, ptrace_ignore_frames_new) { - pid_t pid; - if ((pid = fork()) == 0) { - ASSERT_NE(test_level_one(1, 2, 3, 4, nullptr, nullptr), 0); - _exit(1); - } - VerifyProcTest(pid, BACKTRACE_CURRENT_THREAD, ReadyLevelBacktrace, VerifyProcessIgnoreFrames, - Backtrace::CreateNew, BacktraceMap::CreateNew); - - kill(pid, SIGKILL); - int status; - ASSERT_EQ(waitpid(pid, &status, 0), pid); -} - // Create a process with multiple threads and dump all of the threads. static void* PtraceThreadLevelRun(void*) { EXPECT_NE(test_level_one(1, 2, 3, 4, nullptr, nullptr), 0); @@ -517,45 +475,6 @@ TEST(libbacktrace, ptrace_threads) { FinishRemoteProcess(pid); } -TEST(libbacktrace, ptrace_threads_new) { - pid_t pid; - if ((pid = fork()) == 0) { - for (size_t i = 0; i < NUM_PTRACE_THREADS; i++) { - pthread_attr_t attr; - pthread_attr_init(&attr); - pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED); - - pthread_t thread; - ASSERT_TRUE(pthread_create(&thread, &attr, PtraceThreadLevelRun, nullptr) == 0); - } - ASSERT_NE(test_level_one(1, 2, 3, 4, nullptr, nullptr), 0); - _exit(1); - } - - // Check to see that all of the threads are running before unwinding. - std::vector threads; - uint64_t start = NanoTime(); - do { - usleep(US_PER_MSEC); - threads.clear(); - GetThreads(pid, &threads); - } while ((threads.size() != NUM_PTRACE_THREADS + 1) && ((NanoTime() - start) <= 5 * NS_PER_SEC)); - ASSERT_EQ(threads.size(), static_cast(NUM_PTRACE_THREADS + 1)); - - ASSERT_TRUE(ptrace(PTRACE_ATTACH, pid, 0, 0) == 0); - WaitForStop(pid); - for (std::vector::const_iterator it = threads.begin(); it != threads.end(); ++it) { - // Skip the current forked process, we only care about the threads. - if (pid == *it) { - continue; - } - VerifyProcTest(pid, *it, ReadyLevelBacktrace, VerifyLevelDump, Backtrace::CreateNew, - BacktraceMap::CreateNew); - } - - FinishRemoteProcess(pid); -} - void VerifyLevelThread(void*) { std::unique_ptr backtrace(Backtrace::Create(getpid(), gettid())); ASSERT_TRUE(backtrace.get() != nullptr); @@ -1663,7 +1582,7 @@ TEST(libbacktrace, unwind_disallow_device_map_local) { munmap(device_map, DEVICE_MAP_SIZE); } -TEST(libbacktrace, unwind_disallow_device_map_remote_new) { +TEST(libbacktrace, unwind_disallow_device_map_remote) { void* device_map; SetupDeviceMap(&device_map); @@ -1672,9 +1591,7 @@ TEST(libbacktrace, unwind_disallow_device_map_remote_new) { CreateRemoteProcess(&pid); // Now create an unwind object. - std::unique_ptr map(BacktraceMap::CreateNew(pid)); - ASSERT_TRUE(map.get() != nullptr); - std::unique_ptr backtrace(Backtrace::CreateNew(pid, pid, map.get())); + std::unique_ptr backtrace(Backtrace::Create(pid, pid)); UnwindFromDevice(backtrace.get(), device_map); @@ -1832,18 +1749,10 @@ TEST(libbacktrace, unwind_remote_through_signal_using_handler) { UnwindThroughSignal(false, Backtrace::Create, BacktraceMap::Create); } -TEST(libbacktrace, unwind_remote_through_signal_using_handler_new) { - UnwindThroughSignal(false, Backtrace::CreateNew, BacktraceMap::CreateNew); -} - TEST(libbacktrace, unwind_remote_through_signal_using_action) { UnwindThroughSignal(true, Backtrace::Create, BacktraceMap::Create); } -TEST(libbacktrace, unwind_remote_through_signal_using_action_new) { - UnwindThroughSignal(true, Backtrace::CreateNew, BacktraceMap::CreateNew); -} - static void TestFrameSkipNumbering(create_func_t create_func, map_create_func_t map_create_func) { std::unique_ptr map(map_create_func(getpid(), false)); std::unique_ptr backtrace(create_func(getpid(), gettid(), map.get())); @@ -1856,19 +1765,17 @@ TEST(libbacktrace, unwind_frame_skip_numbering) { TestFrameSkipNumbering(Backtrace::Create, BacktraceMap::Create); } -TEST(libbacktrace, unwind_frame_skip_numbering_new) { - TestFrameSkipNumbering(Backtrace::CreateNew, BacktraceMap::CreateNew); -} - #if defined(ENABLE_PSS_TESTS) #include "GetPss.h" #define MAX_LEAK_BYTES (32*1024UL) static void CheckForLeak(pid_t pid, pid_t tid) { + std::unique_ptr map(BacktraceMap::Create(pid)); + // Do a few runs to get the PSS stable. for (size_t i = 0; i < 100; i++) { - Backtrace* backtrace = Backtrace::Create(pid, tid); + Backtrace* backtrace = Backtrace::Create(pid, tid, map.get()); ASSERT_TRUE(backtrace != nullptr); ASSERT_TRUE(backtrace->Unwind(0)); ASSERT_EQ(BACKTRACE_UNWIND_NO_ERROR, backtrace->GetError()); @@ -1879,7 +1786,7 @@ static void CheckForLeak(pid_t pid, pid_t tid) { // Loop enough that even a small leak should be detectable. for (size_t i = 0; i < 4096; i++) { - Backtrace* backtrace = Backtrace::Create(pid, tid); + Backtrace* backtrace = Backtrace::Create(pid, tid, map.get()); ASSERT_TRUE(backtrace != nullptr); ASSERT_TRUE(backtrace->Unwind(0)); ASSERT_EQ(BACKTRACE_UNWIND_NO_ERROR, backtrace->GetError()); diff --git a/libbacktrace/include/backtrace/Backtrace.h b/libbacktrace/include/backtrace/Backtrace.h index 274c64bb6..73a58b5ee 100644 --- a/libbacktrace/include/backtrace/Backtrace.h +++ b/libbacktrace/include/backtrace/Backtrace.h @@ -94,8 +94,6 @@ public: // If map is NULL, then create the map and manage it internally. // If map is not NULL, the map is still owned by the caller. static Backtrace* Create(pid_t pid, pid_t tid, BacktraceMap* map = NULL); - // Same as above, but uses a different underlying unwinder. - static Backtrace* CreateNew(pid_t pid, pid_t tid, BacktraceMap* map = NULL); // Create an offline Backtrace object that can be used to do an unwind without a process // that is still running. If cache_file is set to true, then elf information will be cached diff --git a/libbacktrace/include/backtrace/BacktraceMap.h b/libbacktrace/include/backtrace/BacktraceMap.h index 84e71327d..d0783929a 100644 --- a/libbacktrace/include/backtrace/BacktraceMap.h +++ b/libbacktrace/include/backtrace/BacktraceMap.h @@ -56,8 +56,6 @@ public: // Passing a map created with uncached set to true to Backtrace::Create() // is unsupported. static BacktraceMap* Create(pid_t pid, bool uncached = false); - // Same as above, but is compatible with the new unwinder. - static BacktraceMap* CreateNew(pid_t pid, bool uncached = false); static BacktraceMap* Create(pid_t pid, const std::vector& maps);