#[Bug] Bad sort length in EntityQueryManager.cs

1 messages · Page 1 of 1 (latest)

pulsar totem
#

Was browsing source code because of https://discord.com/channels/489222168727519232/1122460128491343932 and I think I've spotted a random bug in the source.

EntityQueryManager.cs
line 461. in CalculateRequiredComponentsFromQuery()

NativeSortExtension.Sort(intersection, maxIntersectionCount);

I think maxIntersectionCount should be replaced by
queries[0].AllCount + queries[0].DisabledCount
Otherwise if the second query has a larger combined All+Disable count (basically the maxIntersectionCount) you are sorting in unallocated memory

#

code in question

        private ComponentType* CalculateRequiredComponentsFromQuery(ref UnsafeScratchAllocator allocator, ArchetypeQuery* queries, int queryCount, out int outRequiredComponentsCount)
        {
            var maxIntersectionCount = 0;
            for (int queryIndex = 0; queryIndex < queryCount; ++queryIndex)
                maxIntersectionCount = math.max(maxIntersectionCount, queries[queryIndex].AllCount + queries[queryIndex].DisabledCount);

            // Populate and sort a combined array of all+disabled component types and their access modes from the first ArchetypeQuery
            var outRequiredComponents = (ComponentType*)allocator.Allocate<ComponentType>(maxIntersectionCount+1);
            // The first required component is always Entity.
            outRequiredComponents[0] = ComponentType.ReadWrite<Entity>();
            var intersection = outRequiredComponents + 1;
            for (int j = 0; j < queries[0].AllCount; ++j)
            {
                intersection[j] = new ComponentType
                {
                    TypeIndex = queries[0].All[j],
                    AccessModeType = (ComponentType.AccessMode)queries[0].AllAccessMode[j],
                };
            }
            for (int j = 0; j < queries[0].DisabledCount; ++j)
            {
                intersection[j+queries[0].AllCount] = new ComponentType
                {
                    TypeIndex = queries[0].Disabled[j],
                    AccessModeType = (ComponentType.AccessMode)queries[0].DisabledAccessMode[j],
                };
            }
            NativeSortExtension.Sort(intersection, maxIntersectionCount); // THIS HERE 

Only the size of queries[0].AllCount + queries[0].DisabledCount is being written to intersection therefore if maxIntersectionCount is greater than this there is a very very very small chance the memory at the location would match a component

#

You'll notice this is done correctly in the second loop on 486

int queryRequiredCount = queries[i].AllCount + queries[i].DisabledCount;
// ...
NativeSortExtension.Sort(queryRequiredTypes, queryRequiredCount);```
using the query in question count not the max count
#

@obsidian quiver sorry for ping but you're pretty active and I'm not going to report this to QA as I always run into a wall when reporting code specific issues that I can't create a repo for...

#

[Bug] Bad sort length in EntityQueryManager.cs

pulsar totem
#

After a sleep, does this loop even need to exist?

            var maxIntersectionCount = 0;
            for (int queryIndex = 0; queryIndex < queryCount; ++queryIndex)
                maxIntersectionCount = math.max(maxIntersectionCount, queries[queryIndex].AllCount + queries[queryIndex].DisabledCount);```
it's doing intersection checks and based on the following code in the method, maybe all this needs to be is
`maxIntersectionCount = queries[0].AllCount + queries[0].DisabledCount;`
 as intersection can never be larger than the smallest count anyway right