From a72a33388f0e8acc20adf96372691886753e0adc Mon Sep 17 00:00:00 2001 From: Keisuke Kuroyanagi Date: Wed, 21 Aug 2013 18:57:58 +0900 Subject: [PATCH] Fix native crash by caused wrong munmap. Bug: 10402083 Change-Id: I5aa2cecd20cd3202c2326b1cbb6758257afd6d5d --- ...y_structure_with_buffer_policy_factory.cpp | 4 +- .../dictionary/dynamic_patricia_trie_policy.h | 6 +- .../dictionary/patricia_trie_policy.h | 6 +- .../{mmaped_buffer.h => mmapped_buffer.h} | 55 ++++++++++--------- 4 files changed, 37 insertions(+), 34 deletions(-) rename native/jni/src/suggest/policyimpl/dictionary/utils/{mmaped_buffer.h => mmapped_buffer.h} (56%) diff --git a/native/jni/src/suggest/policyimpl/dictionary/dictionary_structure_with_buffer_policy_factory.cpp b/native/jni/src/suggest/policyimpl/dictionary/dictionary_structure_with_buffer_policy_factory.cpp index cc5252c43..434858d67 100644 --- a/native/jni/src/suggest/policyimpl/dictionary/dictionary_structure_with_buffer_policy_factory.cpp +++ b/native/jni/src/suggest/policyimpl/dictionary/dictionary_structure_with_buffer_policy_factory.cpp @@ -22,7 +22,7 @@ #include "suggest/policyimpl/dictionary/dynamic_patricia_trie_policy.h" #include "suggest/policyimpl/dictionary/patricia_trie_policy.h" #include "suggest/policyimpl/dictionary/utils/format_utils.h" -#include "suggest/policyimpl/dictionary/utils/mmaped_buffer.h" +#include "suggest/policyimpl/dictionary/utils/mmapped_buffer.h" namespace latinime { @@ -31,7 +31,7 @@ namespace latinime { const int bufOffset, const int size, const bool isUpdatable) { // Allocated buffer in MmapedBuffer::openBuffer() will be freed in the destructor of // impl classes of DictionaryStructureWithBufferPolicy. - const MmapedBuffer *const mmapedBuffer = MmapedBuffer::openBuffer(path, pathLength, bufOffset, + const MmappedBuffer *const mmapedBuffer = MmappedBuffer::openBuffer(path, pathLength, bufOffset, size, isUpdatable); if (!mmapedBuffer) { return 0; diff --git a/native/jni/src/suggest/policyimpl/dictionary/dynamic_patricia_trie_policy.h b/native/jni/src/suggest/policyimpl/dictionary/dynamic_patricia_trie_policy.h index 708967d7b..8ba057b20 100644 --- a/native/jni/src/suggest/policyimpl/dictionary/dynamic_patricia_trie_policy.h +++ b/native/jni/src/suggest/policyimpl/dictionary/dynamic_patricia_trie_policy.h @@ -24,7 +24,7 @@ #include "suggest/policyimpl/dictionary/bigram/bigram_list_policy.h" #include "suggest/policyimpl/dictionary/header/header_policy.h" #include "suggest/policyimpl/dictionary/shortcut/shortcut_list_policy.h" -#include "suggest/policyimpl/dictionary/utils/mmaped_buffer.h" +#include "suggest/policyimpl/dictionary/utils/mmapped_buffer.h" namespace latinime { @@ -33,7 +33,7 @@ class DicNodeVector; class DynamicPatriciaTriePolicy : public DictionaryStructureWithBufferPolicy { public: - DynamicPatriciaTriePolicy(const MmapedBuffer *const buffer) + DynamicPatriciaTriePolicy(const MmappedBuffer *const buffer) : mBuffer(buffer), mHeaderPolicy(mBuffer->getBuffer()), mDictRoot(mBuffer->getBuffer() + mHeaderPolicy.getSize()), mBigramListPolicy(mDictRoot), mShortcutListPolicy(mDictRoot) {} @@ -86,7 +86,7 @@ class DynamicPatriciaTriePolicy : public DictionaryStructureWithBufferPolicy { DISALLOW_IMPLICIT_CONSTRUCTORS(DynamicPatriciaTriePolicy); static const int MAX_CHILD_COUNT_TO_AVOID_INFINITE_LOOP; - const MmapedBuffer *const mBuffer; + const MmappedBuffer *const mBuffer; const HeaderPolicy mHeaderPolicy; // TODO: Consolidate mDictRoot. const uint8_t *const mDictRoot; diff --git a/native/jni/src/suggest/policyimpl/dictionary/patricia_trie_policy.h b/native/jni/src/suggest/policyimpl/dictionary/patricia_trie_policy.h index 0d85050f3..d0567fd85 100644 --- a/native/jni/src/suggest/policyimpl/dictionary/patricia_trie_policy.h +++ b/native/jni/src/suggest/policyimpl/dictionary/patricia_trie_policy.h @@ -24,7 +24,7 @@ #include "suggest/policyimpl/dictionary/bigram/bigram_list_policy.h" #include "suggest/policyimpl/dictionary/header/header_policy.h" #include "suggest/policyimpl/dictionary/shortcut/shortcut_list_policy.h" -#include "suggest/policyimpl/dictionary/utils/mmaped_buffer.h" +#include "suggest/policyimpl/dictionary/utils/mmapped_buffer.h" namespace latinime { @@ -33,7 +33,7 @@ class DicNodeVector; class PatriciaTriePolicy : public DictionaryStructureWithBufferPolicy { public: - PatriciaTriePolicy(const MmapedBuffer *const buffer) + PatriciaTriePolicy(const MmappedBuffer *const buffer) : mBuffer(buffer), mHeaderPolicy(mBuffer->getBuffer()), mDictRoot(mBuffer->getBuffer() + mHeaderPolicy.getSize()), mBigramListPolicy(mDictRoot), mShortcutListPolicy(mDictRoot) {} @@ -97,7 +97,7 @@ class PatriciaTriePolicy : public DictionaryStructureWithBufferPolicy { private: DISALLOW_IMPLICIT_CONSTRUCTORS(PatriciaTriePolicy); - const MmapedBuffer *const mBuffer; + const MmappedBuffer *const mBuffer; const HeaderPolicy mHeaderPolicy; const uint8_t *const mDictRoot; const BigramListPolicy mBigramListPolicy; diff --git a/native/jni/src/suggest/policyimpl/dictionary/utils/mmaped_buffer.h b/native/jni/src/suggest/policyimpl/dictionary/utils/mmapped_buffer.h similarity index 56% rename from native/jni/src/suggest/policyimpl/dictionary/utils/mmaped_buffer.h rename to native/jni/src/suggest/policyimpl/dictionary/utils/mmapped_buffer.h index 08add03c6..6febd7832 100644 --- a/native/jni/src/suggest/policyimpl/dictionary/utils/mmaped_buffer.h +++ b/native/jni/src/suggest/policyimpl/dictionary/utils/mmapped_buffer.h @@ -14,8 +14,8 @@ * limitations under the License. */ -#ifndef LATINIME_MMAPED_BUFFER_H -#define LATINIME_MMAPED_BUFFER_H +#ifndef LATINIME_MMAPPED_BUFFER_H +#define LATINIME_MMAPPED_BUFFER_H #include #include @@ -27,39 +27,40 @@ namespace latinime { -class MmapedBuffer { +class MmappedBuffer { public: - static MmapedBuffer* openBuffer(const char *const path, const int pathLength, - const int bufOffset, const int size, const bool isUpdatable) { + static MmappedBuffer* openBuffer(const char *const path, const int pathLength, + const int bufferOffset, const int bufferSize, const bool isUpdatable) { const int openMode = isUpdatable ? O_RDWR : O_RDONLY; - const int fd = open(path, openMode); - if (fd < 0) { + const int mmapFd = open(path, openMode); + if (mmapFd < 0) { AKLOGE("DICT: Can't open the source. path=%s errno=%d", path, errno); return 0; } const int pagesize = getpagesize(); - const int offset = bufOffset % pagesize; - int adjOffset = bufOffset - offset; - int adjSize = size + offset; + const int offset = bufferOffset % pagesize; + int alignedOffset = bufferOffset - offset; + int alignedSize = bufferSize + offset; const int protMode = isUpdatable ? PROT_READ | PROT_WRITE : PROT_READ; - void *const mmapedBuffer = mmap(0, adjSize, protMode, MAP_PRIVATE, fd, adjOffset); - if (mmapedBuffer == MAP_FAILED) { + void *const mmappedBuffer = mmap(0, alignedSize, protMode, MAP_PRIVATE, mmapFd, + alignedOffset); + if (mmappedBuffer == MAP_FAILED) { AKLOGE("DICT: Can't mmap dictionary. errno=%d", errno); - close(fd); + close(mmapFd); return 0; } - uint8_t *const buffer = static_cast(mmapedBuffer) + offset; + uint8_t *const buffer = static_cast(mmappedBuffer) + offset; if (!buffer) { AKLOGE("DICT: buffer is null"); - close(fd); + close(mmapFd); return 0; } - return new MmapedBuffer(buffer, adjSize, fd, offset, isUpdatable); + return new MmappedBuffer(buffer, bufferSize, mmappedBuffer, alignedSize, mmapFd, + isUpdatable); } - ~MmapedBuffer() { - int ret = munmap(static_cast(mBuffer - mBufferOffset), - mBufferSize + mBufferOffset); + ~MmappedBuffer() { + int ret = munmap(mMmappedBuffer, mAlignedSize); if (ret != 0) { AKLOGE("DICT: Failure in munmap. ret=%d errno=%d", ret, errno); } @@ -82,18 +83,20 @@ class MmapedBuffer { } private: - AK_FORCE_INLINE MmapedBuffer(uint8_t *const buffer, const int bufferSize, const int mmapFd, - const int bufferOffset, const bool isUpdatable) - : mBuffer(buffer), mBufferSize(bufferSize), mMmapFd(mmapFd), - mBufferOffset(bufferOffset), mIsUpdatable(isUpdatable) {} + AK_FORCE_INLINE MmappedBuffer(uint8_t *const buffer, const int bufferSize, + void *const mmappedBuffer, const int alignedSize, const int mmapFd, + const bool isUpdatable) + : mBuffer(buffer), mBufferSize(bufferSize), mMmappedBuffer(mmappedBuffer), + mAlignedSize(alignedSize), mMmapFd(mmapFd), mIsUpdatable(isUpdatable) {} - DISALLOW_IMPLICIT_CONSTRUCTORS(MmapedBuffer); + DISALLOW_IMPLICIT_CONSTRUCTORS(MmappedBuffer); uint8_t *const mBuffer; const int mBufferSize; + void *const mMmappedBuffer; + const int mAlignedSize; const int mMmapFd; - const int mBufferOffset; const bool mIsUpdatable; }; } -#endif /* LATINIME_MMAPED_BUFFER_H */ +#endif /* LATINIME_MMAPPED_BUFFER_H */