From 32e8a679b54e4bb32e0190bb8c04934e32f6ed4e Mon Sep 17 00:00:00 2001 From: Horst Schirmeier Date: Tue, 13 Jan 2015 14:48:22 +0100 Subject: [PATCH 1/3] CMake: Fail* version number handling fixed The project version number is now not user-editable anymore, and custom values get overwritten by force. Without this change, CMakeCache.txt stays at the version number it was first instantiated from. Change-Id: If9ad1549937dad98db9a0f0eda16ef752e7d74aa --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 61effeb2..2e564887 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -9,7 +9,7 @@ ENABLE_TESTING() PROJECT(Fail*) -set(PROJECT_VERSION "1.0.1" CACHE STRING "Fail* version number") +set(PROJECT_VERSION "1.0.1" CACHE INTERNAL "Fail* version number" FORCE) #### Put all resulting library files in /lib #### SET(LIBRARY_OUTPUT_PATH ${CMAKE_BINARY_DIR}/lib) From c4d44aeb0ca46a81ba5b2aefb4530d43fab83b4b Mon Sep 17 00:00:00 2001 From: Horst Schirmeier Date: Tue, 13 Jan 2015 19:09:54 +0100 Subject: [PATCH 2/3] sal: fix watch/breakpoint perf implementation This commit changes the entities in a ResultSet from listener indexes (into ListenerManagers m_BufferList) to BaseListener pointers, as ListenerManager::makeActive(BaseListener*) (called from FastBreakpoints/Watchpoints.ah) may invalidate indexes temporarily stored in a ResultSet. The issue occurs when multiple breakpoints (watchpoints) fire at the same time, and one added more recently than the others occupies the largest index position in m_BufferList. Although this issue is extremely rare and was only observed in a few corner cases up to now, it may have falsified results in the past. Change-Id: I7b788a06d412f15700ca75f56f2be5d3b78465fa --- src/core/sal/perf/BreakpointBuffer.cc | 2 +- src/core/sal/perf/BufferInterface.hpp | 7 ++++--- src/core/sal/perf/FastBreakpoints.ah | 4 ++-- src/core/sal/perf/FastWatchpoints.ah | 2 +- src/core/sal/perf/WatchpointBuffer.cc | 2 +- 5 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/core/sal/perf/BreakpointBuffer.cc b/src/core/sal/perf/BreakpointBuffer.cc index 1d56a55c..90b95b00 100644 --- a/src/core/sal/perf/BreakpointBuffer.cc +++ b/src/core/sal/perf/BreakpointBuffer.cc @@ -16,7 +16,7 @@ ResultSet& PerfVectorBreakpoints::gather(BPEvent* pData) // Update trigger IPtr: pLi->setTriggerInstructionPointer(pData->getTriggerInstructionPointer()); pLi->setTriggerCPU(pData->getTriggerCPU()); - res.add(*it); + res.add(pLi); } } return res; diff --git a/src/core/sal/perf/BufferInterface.hpp b/src/core/sal/perf/BufferInterface.hpp index d612a79b..acc278c6 100644 --- a/src/core/sal/perf/BufferInterface.hpp +++ b/src/core/sal/perf/BufferInterface.hpp @@ -44,14 +44,15 @@ public: * Results (= indices of matching listeners) returned by the "gather"-method, * see below. (This class can be seen as a "temporary fire-list".) */ +class BaseListener; class ResultSet { private: - std::vector m_Res; //!< vector of matching listener indices + std::vector m_Res; //!< vector of pointers to matching listeners public: ResultSet() { } bool hasMore() const { return !m_Res.empty(); } - index_t getNext() { index_t idx = m_Res.back(); m_Res.pop_back(); return idx; } - void add(index_t idx) { m_Res.push_back(idx); } + BaseListener *getNext() { BaseListener *l = m_Res.back(); m_Res.pop_back(); return l; } + void add(BaseListener *l) { m_Res.push_back(l); } size_t size() const { return m_Res.size(); } void clear() { m_Res.clear(); } }; diff --git a/src/core/sal/perf/FastBreakpoints.ah b/src/core/sal/perf/FastBreakpoints.ah index 0abdbdea..27e25f5a 100644 --- a/src/core/sal/perf/FastBreakpoints.ah +++ b/src/core/sal/perf/FastBreakpoints.ah @@ -65,12 +65,12 @@ aspect FastBreakpoints { // Check for matching BPSingleListeners: fail::ResultSet& res1 = ref.getSingleListeners().gather(&tmp); while (res1.hasMore()) - ref.makeActive(ref.dereference(res1.getNext())); + ref.makeActive(res1.getNext()); // Check for matching BPRangeListeners: fail::ResultSet& res2 = ref.getRangeListeners().gather(&tmp); while (res2.hasMore()) - ref.makeActive(ref.dereference(res2.getNext())); + ref.makeActive(res2.getNext()); ref.triggerActiveListeners(); } diff --git a/src/core/sal/perf/FastWatchpoints.ah b/src/core/sal/perf/FastWatchpoints.ah index 59279eee..ce2a2876 100644 --- a/src/core/sal/perf/FastWatchpoints.ah +++ b/src/core/sal/perf/FastWatchpoints.ah @@ -64,7 +64,7 @@ aspect FastWatchpoints { // Check for matching MemAccessListener: fail::ResultSet& res = ref.getMemoryListeners().gather(&tmp); while (res.hasMore()) - ref.makeActive(ref.dereference(res.getNext())); + ref.makeActive(res.getNext()); ref.triggerActiveListeners(); } diff --git a/src/core/sal/perf/WatchpointBuffer.cc b/src/core/sal/perf/WatchpointBuffer.cc index 1a66ff7e..ad2e27b4 100644 --- a/src/core/sal/perf/WatchpointBuffer.cc +++ b/src/core/sal/perf/WatchpointBuffer.cc @@ -20,7 +20,7 @@ ResultSet& PerfVectorWatchpoints::gather(MemAccessEvent* pData) pmal->setTriggerWidth(pData->getTriggerWidth()); pmal->setTriggerAccessType(pData->getTriggerAccessType()); pmal->setTriggerCPU(pData->getTriggerCPU()); - res.add(*it); + res.add(pmal); } } return res; From d68ea990ca38b6222ded8e97c55d466a7402ebfc Mon Sep 17 00:00:00 2001 From: Horst Schirmeier Date: Tue, 13 Jan 2015 19:19:30 +0100 Subject: [PATCH 3/3] sal: catch listener removal corner case This change makes sure an active but not-yet fired listener does not fire anymore if it gets removed. In this case, the listener already has an invalid index, but still needs to be copied to the delete list. This issue has not been observed in the wild, and is unlikely to have caused real problems in the past. Change-Id: I8be8a5b1d4cdc783a092b93f34f33b969894e5da --- src/core/sal/ListenerManager.cc | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/core/sal/ListenerManager.cc b/src/core/sal/ListenerManager.cc index dbe7808e..8e804c78 100644 --- a/src/core/sal/ListenerManager.cc +++ b/src/core/sal/ListenerManager.cc @@ -50,16 +50,22 @@ void ListenerManager::remove(BaseListener* li) // called onDeletion for these listeners): m_DeleteList.insert(m_DeleteList.end(), m_FireList.begin(), m_FireList.end()); - // - li != 0 -> remove single listener (if added previously) - // * Inform the listeners (call onDeletion) - // * Remove the index in the perf. buffer-list (if existing) - // * Find/remove 'li' in 'm_BufferList' + // - li != 0 -> remove single listener + // * If added / not removed before, + // -> inform the listeners (call onDeletion) + // -> Remove the index in the perf. buffer-list (if existing) + // -> Find/remove 'li' in 'm_BufferList' // * If 'li' in 'm_FireList', copy to 'm_DeleteList' - } else if (li->getLocation() != INVALID_INDEX) { // Check if li hasn't been added previously (Q&D) - li->onDeletion(); - if (li->getPerformanceBuffer() != NULL) - li->getPerformanceBuffer()->remove(li->getLocation()); - m_remove(li->getLocation()); + } else { + // has li been removed previously? + if (li->getLocation() != INVALID_INDEX) { + li->onDeletion(); + if (li->getPerformanceBuffer() != NULL) { + li->getPerformanceBuffer()->remove(li->getLocation()); + } + m_remove(li->getLocation()); + } + // if li hasn't fired yet, make sure it doesn't firelist_t::const_iterator it = std::find(m_FireList.begin(), m_FireList.end(), li); if (it != m_FireList.end()) {