Merge "Fix race condition updating local map data." (863d8e11) · Commits · e / os / android_system_core

debuggerd/tombstone.cpp

+1 −0

Original line number	Diff line number	Diff line
		@@ -368,6 +368,7 @@ static void dump_all_maps(Backtrace* backtrace, BacktraceMap* map, log_t* log, p
		ALOGE("Cannot get siginfo for %d: %s\n", tid, strerror(errno));
		}

		ScopedBacktraceMapIteratorLock lock(map);
		_LOG(log, logtype::MAPS, "\n");
		if (!print_fault_address_marker) {
		_LOG(log, logtype::MAPS, "memory map:\n");

+20 −0

Original line number	Diff line number	Diff line
		@@ -71,6 +71,12 @@ public:
		bool IsWritable(uintptr_t pc) { return GetFlags(pc) & PROT_WRITE; }
		bool IsExecutable(uintptr_t pc) { return GetFlags(pc) & PROT_EXEC; }

		// In order to use the iterators on this object, a caller must
		// call the LockIterator and UnlockIterator function to guarantee
		// that the data does not change while it's being used.
		virtual void LockIterator() {}
		virtual void UnlockIterator() {}

		typedef std::deque<backtrace_map_t>::iterator iterator;
		iterator begin() { return maps_.begin(); }
		iterator end() { return maps_.end(); }
		@@ -102,4 +108,18 @@ protected:
		pid_t pid_;
		};

		class ScopedBacktraceMapIteratorLock {
		public:
		explicit ScopedBacktraceMapIteratorLock(BacktraceMap* map) : map_(map) {
		map->LockIterator();
		}

		~ScopedBacktraceMapIteratorLock() {
		map_->UnlockIterator();
		}

		private:
		BacktraceMap* map_;
		};

		#endif // _BACKTRACE_BACKTRACE_MAP_H

+2 −2

Original line number	Diff line number	Diff line
		@@ -36,8 +36,8 @@ BacktraceMap::~BacktraceMap() {
		}

		void BacktraceMap::FillIn(uintptr_t addr, backtrace_map_t* map) {
		for (BacktraceMap::const_iterator it = begin();
		it != end(); ++it) {
		ScopedBacktraceMapIteratorLock lock(this);
		for (BacktraceMap::const_iterator it = begin(); it != end(); ++it) {
		if (addr >= it->start && addr < it->end) {
		map = it;
		return;

+15 −3

Original line number	Diff line number	Diff line
		@@ -14,6 +14,7 @@
		* limitations under the License.
		*/

		#include <pthread.h>
		#include <stdint.h>
		#include <stdlib.h>
		#include <sys/types.h>
		@@ -72,6 +73,7 @@ bool UnwindMapRemote::Build() {
		}

		UnwindMapLocal::UnwindMapLocal() : UnwindMap(getpid()), map_created_(false) {
		pthread_rwlock_init(&map_lock_, nullptr);
		}

		UnwindMapLocal::~UnwindMapLocal() {
		@@ -82,9 +84,14 @@ UnwindMapLocal::~UnwindMapLocal() {
		}

		bool UnwindMapLocal::GenerateMap() {
		// Lock so that multiple threads cannot modify the maps data at the
		// same time.
		pthread_rwlock_wrlock(&map_lock_);

		// It's possible for the map to be regenerated while this loop is occurring.
		// If that happens, get the map again, but only try at most three times
		// before giving up.
		bool generated = false;
		for (int i = 0; i < 3; i++) {
		maps_.clear();

		@@ -110,12 +117,17 @@ bool UnwindMapLocal::GenerateMap() {
		}
		// Check to see if the map changed while getting the data.
		if (ret != -UNW_EINVAL) {
		return true;
		generated = true;
		break;
		}
		}

		pthread_rwlock_unlock(&map_lock_);

		if (!generated) {
		BACK_LOGW("Unable to generate the map.");
		return false;
		}
		return generated;
		}

		bool UnwindMapLocal::Build() {

+6 −0

Original line number	Diff line number	Diff line
		@@ -17,6 +17,7 @@
		#ifndef _LIBBACKTRACE_UNWIND_MAP_H
		#define _LIBBACKTRACE_UNWIND_MAP_H

		#include <pthread.h>
		#include <stdint.h>
		#include <sys/types.h>

		@@ -56,10 +57,15 @@ public:

		void FillIn(uintptr_t addr, backtrace_map_t* map) override;

		void LockIterator() override { pthread_rwlock_rdlock(&map_lock_); }
		void UnlockIterator() override { pthread_rwlock_unlock(&map_lock_); }

		private:
		bool GenerateMap();

		bool map_created_;

		pthread_rwlock_t map_lock_;
		};

		#endif // _LIBBACKTRACE_UNWIND_MAP_H