TVector4Variable<IBaseInterface>::SetFloatVectorArray

Feb 3, 2015 at 1:56 PM
Edited Feb 3, 2015 at 2:03 PM
Hello.
I have my doubts about SetFloatVectorArray when offset is bigger than 0.
There is a line inside this method (EffectVariable.inl - line 2536)
memcpy(Data.pVector+Offset, pData, 
      std::min<size_t>((Offset + Count) * sizeof(CEffectVector4), pType->TotalSize)); 
For example if we have in hlsl float4[2] array and we want to fill second vector we will do this:
//c++ code
float someVals[4]={0};
pVar->SetFloatVectorArray( someVals, 1, 1);
//we want to fill only second vector, thats why offset and count are 1
after that inside SetFloatVectorArray we will have memcpy like this:
memcpy(Data.pVector + 1, pData, 32); so we copy 32 bytes to last vector which is 16 bytes long and what is worse we copy it from 16 bytes buffer. So we write second 16 bytes over memory we don't own and from memory we don't own;
In my opinion this line in EffectVariable.inl should look like this:
memcpy(Data.pVector + Offset, pData, std::min<size_t>(Count * sizeof(CEffectVector4),      pType->TotalSize-Offset * sizeof(CEffectVector4)));
Am I missing something, or no one were ever using offsets with this method?
Thanks in advance for any response.
Coordinator
Feb 3, 2015 at 8:33 PM
Edited Feb 3, 2015 at 8:49 PM
The code was originally in the legacy DirectX SDK:
dwordMemcpy(Data.pVector + Offset, pData, min((Offset + Count) * sizeof(CEffectVector4), pType->TotalSize));
dwordMemcpy was just a custom version of the standard memcpy.

The current code is a faithful restatement of that original code. Now, that code could have been wrong as you note.

I think it's relying on the min to ensure that you don't overwrite the end of the buffer but as you indicate it fails to take into account the offset you passed in originally. Does your fix resolve it? It looks like it would be just that Set/Get function.
Feb 4, 2015 at 10:19 AM
Yes, my change resolves the problem.
As you can see there are actually two changes in this one line. First is to copy only wished count of vectors (not count+offset vectors) and second is to ensure to not write after end of the buffer.
memcpy(Data.pVector + Offset, pData, std::min<size_t>(Count * sizeof(CEffectVector4),pType->TotalSize -Offset * sizeof(CEffectVector4)));

And as you noticed the same mistake is in TVector4Variable<IBaseInterface>::GetFloatVectorArray
it should be
memcpy(pData, Data.pVector + Offset, std::min<size_t>(Count * sizeof(CEffectVector4), pType->TotalSize - Offset * sizeof(CEffectVector4)));

Cheers
Coordinator
Feb 4, 2015 at 4:54 PM
Edited Feb 4, 2015 at 4:56 PM
And to confirm this surfaces to the public API via SetFloatVectorArray and GetFloatVectorArray on ID3DX11EffectVectorVariable, correct?

I would guess that Set/GetBoolVectorArray and Set/GetIntVectorArray have the same problem.
Coordinator
Feb 4, 2015 at 4:57 PM
This discussion has been copied to a work item. Click here to go to the work item and continue the discussion.