From 7f021738ef2d74f78093dbe4fa37cbfa6645e05a Mon Sep 17 00:00:00 2001 From: Nicky Date: Wed, 10 Apr 2024 21:02:40 +0200 Subject: Fix ASAN errors from LLVector4a::memcpyNonAliased16 Found by running with -fsanitze=thread Suggestion to avoid accessing invalid memory: In both cases memory will be allocated by can be accessed beyond bounds. In LLPolyMesh it can be off by at least one (+x%2). Though I am not even sure if even in best case it always will be a multiple of 16. In LLViewerJointMesh::updateFaceData the code tries to account for padding by, but the allocation in LLPolyMeshSharedData::allocateVertexData is done without any padding. Thus the sizes must not match. Replacing the calls with memcpy as a quick fix to see if the error goes away fixed address sanitzer complaining. It is up to debate if memcpy is a good replacement. LLVector4a::memcpyNonAliased16 was invented for performance. But on the other hand one could argue that nowadays every stdlib maintainer will very heavily optmize functions like memcpy themselves and could take advantage of CPU features the old LL implementation does not take into account. AVX comes to mind. In any case did I not measure any of this. --- indra/llappearance/llpolymesh.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra/llappearance') diff --git a/indra/llappearance/llpolymesh.cpp b/indra/llappearance/llpolymesh.cpp index dab14851c8..79b879b487 100644 --- a/indra/llappearance/llpolymesh.cpp +++ b/indra/llappearance/llpolymesh.cpp @@ -981,7 +981,7 @@ void LLPolyMesh::initializeForMorph() LLVector4a::memcpyNonAliased16((F32*) mScaledNormals, (F32*) mSharedData->mBaseNormals, sizeof(LLVector4a) * mSharedData->mNumVertices); LLVector4a::memcpyNonAliased16((F32*) mBinormals, (F32*) mSharedData->mBaseNormals, sizeof(LLVector4a) * mSharedData->mNumVertices); LLVector4a::memcpyNonAliased16((F32*) mScaledBinormals, (F32*) mSharedData->mBaseNormals, sizeof(LLVector4a) * mSharedData->mNumVertices); - LLVector4a::memcpyNonAliased16((F32*) mTexCoords, (F32*) mSharedData->mTexCoords, sizeof(LLVector2) * (mSharedData->mNumVertices + mSharedData->mNumVertices%2)); + memcpy((F32*) mTexCoords, (F32*) mSharedData->mTexCoords, sizeof(LLVector2) * (mSharedData->mNumVertices)); // allocated in LLPolyMeshSharedData::allocateVertexData for (U32 i = 0; i < mSharedData->mNumVertices; ++i) { -- cgit v1.2.3 From 3747dd9a085e4d75ec21c8048f1269bc3f29e582 Mon Sep 17 00:00:00 2001 From: Andrey Kleshchev Date: Mon, 23 Sep 2024 17:27:17 +0300 Subject: viewer#2631 Optimize LLWearable::writeToAvatar --- indra/llappearance/llwearable.cpp | 14 ++++++++------ indra/llappearance/llwearabledata.cpp | 34 ++++++++++++++++++---------------- 2 files changed, 26 insertions(+), 22 deletions(-) (limited to 'indra/llappearance') diff --git a/indra/llappearance/llwearable.cpp b/indra/llappearance/llwearable.cpp index a7e5292fed..ae038e09cc 100644 --- a/indra/llappearance/llwearable.cpp +++ b/indra/llappearance/llwearable.cpp @@ -645,9 +645,10 @@ void LLWearable::addVisualParam(LLVisualParam *param) void LLWearable::setVisualParamWeight(S32 param_index, F32 value) { - if( is_in_map(mVisualParamIndexMap, param_index ) ) + visual_param_index_map_t::iterator found = mVisualParamIndexMap.find(param_index); + if (found != mVisualParamIndexMap.end()) { - LLVisualParam *wearable_param = mVisualParamIndexMap[param_index]; + LLVisualParam *wearable_param = found->second; wearable_param->setWeight(value); } else @@ -658,9 +659,10 @@ void LLWearable::setVisualParamWeight(S32 param_index, F32 value) F32 LLWearable::getVisualParamWeight(S32 param_index) const { - if( is_in_map(mVisualParamIndexMap, param_index ) ) + visual_param_index_map_t::const_iterator found = mVisualParamIndexMap.find(param_index); + if(found != mVisualParamIndexMap.end()) { - const LLVisualParam *wearable_param = mVisualParamIndexMap.find(param_index)->second; + const LLVisualParam *wearable_param = found->second; return wearable_param->getWeight(); } else @@ -733,9 +735,9 @@ void LLWearable::writeToAvatar(LLAvatarAppearance* avatarp) if( (((LLViewerVisualParam*)param)->getWearableType() == mType) && (!((LLViewerVisualParam*)param)->getCrossWearable()) ) { S32 param_id = param->getID(); + // get weight from wearable and write back into character F32 weight = getVisualParamWeight(param_id); - - avatarp->setVisualParamWeight( param_id, weight); + param->setWeight(weight); } } } diff --git a/indra/llappearance/llwearabledata.cpp b/indra/llappearance/llwearabledata.cpp index 7598ed67f3..f3b76da224 100644 --- a/indra/llappearance/llwearabledata.cpp +++ b/indra/llappearance/llwearabledata.cpp @@ -286,43 +286,45 @@ const LLWearable* LLWearableData::getWearable(const LLWearableType::EType type, LLWearable* LLWearableData::getTopWearable(const LLWearableType::EType type) { - U32 count = getWearableCount(type); - if ( count == 0) + wearableentry_map_t::const_iterator wearable_iter = mWearableDatas.find(type); + if (wearable_iter == mWearableDatas.end()) { return NULL; } + const wearableentry_vec_t& wearable_vec = wearable_iter->second; - return getWearable(type, count-1); + size_t size = wearable_vec.size(); + if (size == 0) + { + return NULL; + } + return wearable_vec[size - 1]; } const LLWearable* LLWearableData::getTopWearable(const LLWearableType::EType type) const { - U32 count = getWearableCount(type); - if ( count == 0) + wearableentry_map_t::const_iterator wearable_iter = mWearableDatas.find(type); + if (wearable_iter == mWearableDatas.end()) { return NULL; } + const wearableentry_vec_t& wearable_vec = wearable_iter->second; - return getWearable(type, count-1); -} - -LLWearable* LLWearableData::getBottomWearable(const LLWearableType::EType type) -{ - if (getWearableCount(type) == 0) + size_t size = wearable_vec.size(); + if (size == 0) { return NULL; } + return wearable_vec[size - 1]; +} +LLWearable* LLWearableData::getBottomWearable(const LLWearableType::EType type) +{ return getWearable(type, 0); } const LLWearable* LLWearableData::getBottomWearable(const LLWearableType::EType type) const { - if (getWearableCount(type) == 0) - { - return NULL; - } - return getWearable(type, 0); } -- cgit v1.2.3