gallium/swr: Fix various asserts and security issues
authorJan Zielinski <jan.zielinski@intel.com>
Wed, 5 Feb 2020 12:57:55 +0000 (13:57 +0100)
committerMarge Bot <eric+marge@anholt.net>
Wed, 5 Feb 2020 16:08:44 +0000 (16:08 +0000)
To improve the robustness of the code, we want to better
detect issues in testing (using asserts) and use more
secure techniques.

Reviewed-by: Krzysztof Raszkowski <krzysztof.raszkowski@intel.com>
Tested-by: Marge Bot <https://gitlab.freedesktop.org/mesa/mesa/merge_requests/3710>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/merge_requests/3710>

14 files changed:
src/gallium/auxiliary/gallivm/lp_bld_init.c
src/gallium/drivers/swr/rasterizer/common/rdtsc_buckets.cpp
src/gallium/drivers/swr/rasterizer/common/swr_assert.h
src/gallium/drivers/swr/rasterizer/core/api.cpp
src/gallium/drivers/swr/rasterizer/core/arena.h
src/gallium/drivers/swr/rasterizer/core/tessellator.h
src/gallium/drivers/swr/rasterizer/core/threads.cpp
src/gallium/drivers/swr/rasterizer/jitter/builder_mem.cpp
src/gallium/drivers/swr/rasterizer/jitter/builder_misc.cpp
src/gallium/drivers/swr/rasterizer/jitter/fetch_jit.cpp
src/gallium/drivers/swr/rasterizer/jitter/functionpasses/lower_x86.cpp
src/gallium/drivers/swr/swr_query.cpp
src/gallium/drivers/swr/swr_shader.cpp
src/gallium/drivers/swr/swr_state.cpp

index efc0096a744be34bc89154058c3bc1674f29bae6..6206d79cc54c7bbe8db62af66d0a20356e085416 100644 (file)
@@ -450,7 +450,7 @@ lp_build_init(void)
        */
       lp_native_vector_width = 128;
    }
+
    lp_native_vector_width = debug_get_num_option("LP_NATIVE_VECTOR_WIDTH",
                                                  lp_native_vector_width);
 
@@ -513,6 +513,7 @@ gallivm_create(const char *name, LLVMContextRef context)
       }
    }
 
+   assert(gallivm != NULL);
    return gallivm;
 }
 
index e19a2d110458a5e0757f2b0b6d459ddcf4ac5310..2404ae7590d1ab82b733c6a0b2c4b81672e85f9d 100644 (file)
@@ -103,9 +103,10 @@ void BucketManager::PrintBucket(
     BUCKET_DESC& desc = mBuckets[bucket.id];
 
     // construct hierarchy visualization
+    std::string str = arrows[level];
+    str += desc.name;
     char hier[80];
-    strcpy(hier, arrows[level]);
-    strcat(hier, desc.name.c_str());
+    strcpy_s(hier, sizeof(hier), str.c_str());
 
     // print out
     fprintf(f,
@@ -156,6 +157,7 @@ void BucketManager::PrintReport(const std::string& filename)
 {
     {
         FILE* f = fopen(filename.c_str(), "w");
+        assert(f);
 
         mThreadMutex.lock();
         for (const BUCKET_THREAD& thread : mThreads)
index 7e90e21e12f4959b56c4b94c02fa0173d04e9186..f6bf83ea5a8ca139eb429ae3f8285aa4ffb0ebeb 100644 (file)
@@ -164,12 +164,6 @@ void SwrTrace(
 #define SWR_ASSERT(e, ...) _SWR_ASSERT(true, e, ##__VA_ARGS__)
 #define SWR_ASSUME_ASSERT(e, ...) SWR_ASSERT(e, ##__VA_ARGS__)
 #define SWR_TRACE(_fmtstr, ...) _SWR_TRACE(_fmtstr, ##__VA_ARGS__)
-
-#if defined(assert)
-#undef assert
-#endif
-#define assert(exp) SWR_ASSERT(exp)
-
 #endif // SWR_ENABLE_ASSERTS
 
 #if SWR_ENABLE_REL_ASSERTS
index 5405bf2d8ba476bdc39e47d572b8144544722135..04f03c78f423ecc57109039a32147c7178c43354 100644 (file)
@@ -478,7 +478,7 @@ void SWR_API SwrSaveState(HANDLE hContext, void* pOutputStateBlock, size_t memSi
 {
     SWR_CONTEXT* pContext = GetContext(hContext);
     auto         pSrc     = GetDrawState(pContext);
-    SWR_ASSERT(pOutputStateBlock && memSize >= sizeof(*pSrc));
+    assert(pOutputStateBlock && memSize >= sizeof(*pSrc));
 
     memcpy(pOutputStateBlock, pSrc, sizeof(*pSrc));
 }
@@ -487,7 +487,7 @@ void SWR_API SwrRestoreState(HANDLE hContext, const void* pStateBlock, size_t me
 {
     SWR_CONTEXT* pContext = GetContext(hContext);
     auto         pDst     = GetDrawState(pContext);
-    SWR_ASSERT(pStateBlock && memSize >= sizeof(*pDst));
+    assert(pStateBlock && memSize >= sizeof(*pDst));
 
     memcpy(pDst, pStateBlock, sizeof(*pDst));
 }
index a3cfdb47818fe0528291d1dd5738e8cd12c32a9f..831617c213f60079b320c07b1477b2f630f03158 100644 (file)
@@ -110,7 +110,7 @@ struct CachingAllocatorT : DefaultAllocator
 
             if (pBlock)
             {
-                SWR_ASSUME_ASSERT(pPrevBlock && pPrevBlock->pNext == pBlock);
+                assert(pPrevBlock && pPrevBlock->pNext == pBlock);
                 pPrevBlock->pNext = pBlock->pNext;
                 pBlock->pNext     = nullptr;
 
index 66b6d5e102abe408b87359aa8306f4629f533127..26e6d542b28da1a293a523dd96d02d489d713d62 100644 (file)
@@ -113,6 +113,7 @@ namespace Tessellator
 
             default:
                 SWR_INVALID("Invalid Tessellation Domain: %d", Domain);
+                assert(false);
             }
 
             NumDomainPoints = (uint32_t)SUPER::GetPointCount();
index 113a31ee0c63e6242313c3911cbfc7700a75889f..c75fb568431e720c6cdd579e423d777ebd07c510 100644 (file)
@@ -1006,6 +1006,7 @@ void CreateThreadPool(SWR_CONTEXT* pContext, THREAD_POOL* pPool)
     CPUNumaNodes nodes;
     uint32_t     numThreadsPerProcGroup = 0;
     CalculateProcessorTopology(nodes, numThreadsPerProcGroup);
+    assert(numThreadsPerProcGroup > 0);
 
     // Assumption, for asymmetric topologies, multi-threaded cores will appear
     // in the list before single-threaded cores.  This appears to be true for
@@ -1191,7 +1192,7 @@ void CreateThreadPool(SWR_CONTEXT* pContext, THREAD_POOL* pPool)
     pContext->NumWorkerThreads = pPool->numThreads;
 
     pPool->pThreadData = new (std::nothrow) THREAD_DATA[pPool->numThreads];
-    SWR_ASSERT(pPool->pThreadData);
+    assert(pPool->pThreadData);
     memset(pPool->pThreadData, 0, sizeof(THREAD_DATA) * pPool->numThreads);
     pPool->numaMask = 0;
 
@@ -1203,7 +1204,7 @@ void CreateThreadPool(SWR_CONTEXT* pContext, THREAD_POOL* pPool)
         pContext->workerPrivateState.pfnInitWorkerData = nullptr;
         pContext->workerPrivateState.pfnFinishWorkerData = nullptr;
     }
+
     // initialize contents of SWR_WORKER_DATA
     size_t perWorkerSize =
         AlignUpPow2(pContext->workerPrivateState.perWorkerPrivateStateSize, 64);
@@ -1231,7 +1232,7 @@ void CreateThreadPool(SWR_CONTEXT* pContext, THREAD_POOL* pPool)
     }
 
     pPool->pThreads = new (std::nothrow) THREAD_PTR[pPool->numThreads];
-    SWR_ASSERT(pPool->pThreads);
+    assert(pPool->pThreads);
 
     if (pContext->threadInfo.MAX_WORKER_THREADS)
     {
@@ -1300,7 +1301,7 @@ void CreateThreadPool(SWR_CONTEXT* pContext, THREAD_POOL* pPool)
                     if (numRemovedThreads)
                     {
                         --numRemovedThreads;
-                        SWR_REL_ASSERT(numReservedThreads);
+                        assert(numReservedThreads);
                         --numReservedThreads;
                         pPool->pApiThreadData[numReservedThreads].workerId    = 0xFFFFFFFFU;
                         pPool->pApiThreadData[numReservedThreads].procGroupId = core.procGroup;
index 2d8240187c5535f4a0dde41c4a3712e7dfa49da1..1418d65e2d0addbe3683e316350fdafcc2af9ae5 100644 (file)
@@ -632,6 +632,7 @@ namespace SwrJit
                     break;
                 }
 
+                assert(vConstMask && "Invalid info.numComps value");
                 vGatherOutput[swizzleIndex] =
                     BITCAST(PSHUFB(BITCAST(vGatherInput, v32x8Ty), vConstMask), vGatherTy);
                 // after pshufb for x channel
@@ -654,7 +655,7 @@ namespace SwrJit
 //        if (vSrc->getType() != mSimdFP32Ty)
 //        {
 //            vSrc = BITCAST(vSrc, mSimdFP32Ty);
-//        }                                               
+//        }
         SWR_ASSERT(vSrc->getType()->getVectorElementType()->isFloatTy());
         VSCATTERPS(pDst, vMask, vOffsets, vSrc, C(1));
         return;
index 183ba880c0fb70bdf607bf6c073ba657520d3960..4f6b0b46a603eb112e6554f295180ece65553ead 100644 (file)
@@ -553,6 +553,7 @@ namespace SwrJit
         else
         {
             Constant* cB = dyn_cast<Constant>(b);
+            assert(cB != nullptr);
             // number of 8 bit elements in b
             uint32_t numElms = cast<VectorType>(cB->getType())->getNumElements();
             // output vector
index 72704e94e4ccc47a87e98f5120b89056fac715ae..dea509b0e7eb83963920724c3f5f7eb8f1889885 100644 (file)
@@ -209,8 +209,8 @@ Function* FetchJit::Create(const FETCH_COMPILE_STATE& fetchState)
             : vIndices = GetSimdValid32bitIndices(indices, pLastIndex);
         break; // incoming type is already 32bit int
     default:
-        SWR_INVALID("Unsupported index type");
         vIndices = nullptr;
+        assert(false && "Unsupported index type");
         break;
     }
 
@@ -1392,11 +1392,11 @@ void FetchJit::Shuffle8bpcGatherd16(Shuffle8bpcArgs& args)
             conversionFactor = VIMMED1((float)(1.0));
             break;
         case CONVERT_USCALED:
-            SWR_INVALID("Type should not be sign extended!");
+            assert(false && "Type should not be sign extended!");
             conversionFactor = nullptr;
             break;
         default:
-            SWR_ASSERT(conversionType == CONVERT_NONE);
+            assert(conversionType == CONVERT_NONE);
             conversionFactor = nullptr;
             break;
         }
@@ -1466,11 +1466,11 @@ void FetchJit::Shuffle8bpcGatherd16(Shuffle8bpcArgs& args)
             conversionFactor = VIMMED1((float)(1.0));
             break;
         case CONVERT_SSCALED:
-            SWR_INVALID("Type should not be zero extended!");
+            assert(false && "Type should not be zero extended!");
             conversionFactor = nullptr;
             break;
         default:
-            SWR_ASSERT(conversionType == CONVERT_NONE);
+            assert(conversionType == CONVERT_NONE);
             conversionFactor = nullptr;
             break;
         }
@@ -1511,6 +1511,7 @@ void FetchJit::Shuffle8bpcGatherd16(Shuffle8bpcArgs& args)
                                      3, -1, -1, -1, 7, -1, -1, -1, 11, -1, -1, -1, 15, -1, -1, -1});
                         break;
                     default:
+                        assert(false && "Invalid component");
                         vConstMask = nullptr;
                         break;
                     }
@@ -1762,11 +1763,11 @@ void FetchJit::Shuffle16bpcGather16(Shuffle16bpcArgs& args)
             conversionFactor = VIMMED1((float)(1.0));
             break;
         case CONVERT_USCALED:
-            SWR_INVALID("Type should not be sign extended!");
+            assert(false && "Type should not be sign extended!");
             conversionFactor = nullptr;
             break;
         default:
-            SWR_ASSERT(conversionType == CONVERT_NONE);
+            assert(conversionType == CONVERT_NONE);
             conversionFactor = nullptr;
             break;
         }
index 2fadca340dbe06aa9c67b31eb5eb61183190e6b3..fc5157e89c98f09e60fe6da172ee2c693b4f5c3d 100644 (file)
@@ -224,13 +224,16 @@ namespace SwrJit
                                       TargetWidth*    pWidth,
                                       Type**          pTy)
         {
+            assert(pCallInst);
             Type* pVecTy = pCallInst->getType();
 
             // Check for intrinsic specific types
             // VCVTPD2PS type comes from src, not dst
             if (intrinName.equals("meta.intrinsic.VCVTPD2PS"))
             {
-                pVecTy = pCallInst->getOperand(0)->getType();
+                Value* pOp = pCallInst->getOperand(0);
+                assert(pOp);
+                pVecTy = pOp->getType();
             }
 
             if (!pVecTy->isVectorTy())
@@ -307,7 +310,9 @@ namespace SwrJit
 
         Instruction* ProcessIntrinsicAdvanced(CallInst* pCallInst)
         {
-            Function*   pFunc     = pCallInst->getCalledFunction();
+            Function*   pFunc = pCallInst->getCalledFunction();
+            assert(pFunc);
+
             auto&       intrinsic = intrinsicMap2[mTarget][pFunc->getName()];
             TargetWidth vecWidth;
             Type*       pElemTy;
@@ -367,6 +372,7 @@ namespace SwrJit
         Instruction* ProcessIntrinsic(CallInst* pCallInst)
         {
             Function* pFunc = pCallInst->getCalledFunction();
+            assert(pFunc);
 
             // Forward to the advanced support if found
             if (intrinsicMap2[mTarget].find(pFunc->getName()) != intrinsicMap2[mTarget].end())
@@ -731,7 +737,9 @@ namespace SwrJit
 
         auto B       = pThis->B;
         auto vf32Src = pCallInst->getOperand(0);
+        assert(vf32Src);
         auto i8Round = pCallInst->getOperand(1);
+        assert(i8Round);
         auto pfnFunc =
             Intrinsic::getDeclaration(B->JM()->mpCurrentModule, Intrinsic::x86_avx_round_ps_256);
 
index 8c49a2ebb2952bb19f1b130449766e7d111b4c81..005b64fb090507c2bc19c9f14c4004682582c234 100644 (file)
@@ -46,9 +46,9 @@ swr_create_query(struct pipe_context *pipe, unsigned type, unsigned index)
    assert(index < MAX_SO_STREAMS);
 
    pq = (struct swr_query *) AlignedMalloc(sizeof(struct swr_query), 64);
-   memset(pq, 0, sizeof(*pq));
 
    if (pq) {
+      memset(pq, 0, sizeof(*pq));
       pq->type = type;
       pq->index = index;
    }
index b8f2a10e04d93edbf725b9210df3671ce92e9a48..fcbdcd54369aaf5fc3fd9a4d30c1b5aed2e6b368 100644 (file)
@@ -1510,6 +1510,7 @@ BuilderSWR::CompileGS(struct swr_context *ctx, swr_jit_gs_key &key)
 
    struct lp_build_sampler_soa *sampler =
       swr_sampler_soa_create(key.sampler, PIPE_SHADER_GEOMETRY);
+   assert(sampler != nullptr);
 
    struct lp_bld_tgsi_system_values system_values;
    memset(&system_values, 0, sizeof(system_values));
@@ -1523,6 +1524,7 @@ BuilderSWR::CompileGS(struct swr_context *ctx, swr_jit_gs_key &key)
       ubyte semantic_idx = info->input_semantic_index[slot];
 
       unsigned vs_slot = locate_linkage(semantic_name, semantic_idx, &ctx->vs->info.base);
+      assert(vs_slot < PIPE_MAX_SHADER_OUTPUTS);
 
       vs_slot += VERTEX_ATTRIB_START_SLOT;
 
@@ -1736,6 +1738,7 @@ BuilderSWR::CompileTES(struct swr_context *ctx, swr_jit_tes_key &key)
 
    struct lp_build_sampler_soa *sampler =
       swr_sampler_soa_create(key.sampler, PIPE_SHADER_TESS_EVAL);
+   assert(sampler != nullptr);
 
    struct lp_bld_tgsi_system_values system_values;
    memset(&system_values, 0, sizeof(system_values));
@@ -1830,6 +1833,7 @@ BuilderSWR::CompileTES(struct swr_context *ctx, swr_jit_tes_key &key)
       // Where in TCS output is my attribute?
       // TESS_TODO: revisit after implement pass-through TCS
       unsigned tcs_slot = locate_linkage(semantic_name, semantic_idx, pPrevShader);
+      assert(tcs_slot < PIPE_MAX_SHADER_OUTPUTS);
 
       // Skip tessellation levels - these go to the tessellator, not TES
       switch (semantic_name) {
@@ -2035,6 +2039,7 @@ BuilderSWR::CompileTCS(struct swr_context *ctx, swr_jit_tcs_key &key)
 
    struct lp_build_sampler_soa *sampler =
       swr_sampler_soa_create(key.sampler, PIPE_SHADER_TESS_CTRL);
+   assert(sampler != nullptr);
 
    struct lp_bld_tgsi_system_values system_values;
    memset(&system_values, 0, sizeof(system_values));
@@ -2069,6 +2074,7 @@ BuilderSWR::CompileTCS(struct swr_context *ctx, swr_jit_tcs_key &key)
 
       unsigned vs_slot =
          locate_linkage(semantic_name, semantic_idx, &ctx->vs->info.base);
+      assert(vs_slot < PIPE_MAX_SHADER_OUTPUTS);
 
       vs_slot += VERTEX_ATTRIB_START_SLOT;
 
@@ -2311,6 +2317,7 @@ BuilderSWR::CompileVS(struct swr_context *ctx, swr_jit_vs_key &key)
 
    struct lp_build_sampler_soa *sampler =
       swr_sampler_soa_create(key.sampler, PIPE_SHADER_VERTEX);
+   assert(sampler != nullptr);
 
    struct lp_bld_tgsi_system_values system_values;
    memset(&system_values, 0, sizeof(system_values));
@@ -2397,6 +2404,7 @@ BuilderSWR::CompileVS(struct swr_context *ctx, swr_jit_vs_key &key)
             }
          }
       }
+      assert(cv < PIPE_MAX_SHADER_OUTPUTS);
       LLVMValueRef cx = LLVMBuildLoad(gallivm->builder, outputs[cv][0], "");
       LLVMValueRef cy = LLVMBuildLoad(gallivm->builder, outputs[cv][1], "");
       LLVMValueRef cz = LLVMBuildLoad(gallivm->builder, outputs[cv][2], "");
@@ -2419,6 +2427,7 @@ BuilderSWR::CompileVS(struct swr_context *ctx, swr_jit_vs_key &key)
          if ((pLastFE->clipdist_writemask & clip_mask & (1 << val)) ||
              ((pLastFE->culldist_writemask << pLastFE->num_written_clipdistance) & (1 << val))) {
             unsigned cv = locate_linkage(TGSI_SEMANTIC_CLIPDIST, val < 4 ? 0 : 1, pLastFE);
+            assert(cv < PIPE_MAX_SHADER_OUTPUTS);
             if (val < 4) {
                LLVMValueRef dist = LLVMBuildLoad(gallivm->builder, outputs[cv][val], "");
                WriteVS(unwrap(dist), pVsCtx, vtxOutput, VERTEX_CLIPCULL_DIST_LO_SLOT, val);
@@ -2711,7 +2720,7 @@ BuilderSWR::CompileFS(struct swr_context *ctx, swr_jit_fs_key &key)
          linkedAttrib = pPrevShader->num_outputs + extraAttribs - 1;
          swr_fs->pointSpriteMask |= (1 << linkedAttrib);
          extraAttribs++;
-      } else if (linkedAttrib == 0xFFFFFFFF) {
+      } else if (linkedAttrib + 1 == 0xFFFFFFFF) {
          inputs[attrib][0] = wrap(VIMMED1(0.0f));
          inputs[attrib][1] = wrap(VIMMED1(0.0f));
          inputs[attrib][2] = wrap(VIMMED1(0.0f));
@@ -2733,15 +2742,16 @@ BuilderSWR::CompileFS(struct swr_context *ctx, swr_jit_fs_key &key)
       Value *offset = NULL;
       if (semantic_name == TGSI_SEMANTIC_COLOR && key.light_twoside) {
          bcolorAttrib = locate_linkage(
-               TGSI_SEMANTIC_BCOLOR, semantic_idx, pPrevShader) - 1;
+               TGSI_SEMANTIC_BCOLOR, semantic_idx, pPrevShader);
          /* Neither front nor back colors were available. Nothing to load. */
          if (bcolorAttrib == 0xFFFFFFFF && linkedAttrib == 0xFFFFFFFF)
             continue;
          /* If there is no front color, just always use the back color. */
-         if (linkedAttrib == 0xFFFFFFFF)
+         if (linkedAttrib + 1 == 0xFFFFFFFF)
             linkedAttrib = bcolorAttrib;
 
          if (bcolorAttrib != 0xFFFFFFFF) {
+            bcolorAttrib -= 1;
             if (interpMode == TGSI_INTERPOLATE_CONSTANT) {
                swr_fs->constantMask |= 1 << bcolorAttrib;
             } else if (interpMode == TGSI_INTERPOLATE_COLOR) {
@@ -2797,6 +2807,7 @@ BuilderSWR::CompileFS(struct swr_context *ctx, swr_jit_fs_key &key)
    }
 
    sampler = swr_sampler_soa_create(key.sampler, PIPE_SHADER_FRAGMENT);
+   assert(sampler != nullptr);
 
    struct lp_bld_tgsi_system_values system_values;
    memset(&system_values, 0, sizeof(system_values));
index 34e36a8025a0688e6b7a2e03b92020ca64402c93..625f0050ad59b0951a8c4337686c94fdaf2e3778 100644 (file)
@@ -71,6 +71,7 @@ swr_create_blend_state(struct pipe_context *pipe,
                        const struct pipe_blend_state *blend)
 {
    struct swr_blend_state *state = CALLOC_STRUCT(swr_blend_state);
+   assert(state != nullptr);
 
    memcpy(&state->pipe, blend, sizeof(*blend));