From e9fdd5044328875216959245ab9e0dab6e65eef0 Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Fri, 27 Aug 2021 14:50:07 +0000 Subject: [PATCH] writer/spirv: Fix stack-use-after-return Never create semantic types on the stack. Always use ProgramBuilder::create(). These stack pointers have a tendency of being stored, either by other types as sub-types, or by maps (see associated bug). Also, there's a lot of logic that assumes that semantic types are de-duplicated, and that you can compare pointers. Creating new instances on the stack will break this in exciting ways. Fixed: chromium:1243944 Change-Id: I40a652f8c424030106adad2e6531287af13c8714 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/62943 Auto-Submit: Ben Clayton Commit-Queue: David Neto Kokoro: Kokoro Reviewed-by: David Neto --- src/writer/spirv/builder.cc | 46 +++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 25 deletions(-) diff --git a/src/writer/spirv/builder.cc b/src/writer/spirv/builder.cc index 5f1247f0b7..11942ef669 100644 --- a/src/writer/spirv/builder.cc +++ b/src/writer/spirv/builder.cc @@ -478,9 +478,9 @@ bool Builder::GenerateExecutionModes(ast::Function* func, uint32_t id) { } has_overridable_workgroup_size_ = true; - sem::U32 u32; - sem::Vector vec3_u32(&u32, 3); - uint32_t vec3_u32_type_id = GenerateTypeIfNeeded(&vec3_u32); + auto* vec3_u32 = + builder_.create(builder_.create(), 3); + uint32_t vec3_u32_type_id = GenerateTypeIfNeeded(vec3_u32); if (vec3_u32_type_id == 0) { return 0; } @@ -1675,23 +1675,19 @@ uint32_t Builder::GenerateConstantIfNeeded(const ScalarConstant& constant) { switch (constant.kind) { case ScalarConstant::Kind::kU32: { - sem::U32 u32; - type_id = GenerateTypeIfNeeded(&u32); + type_id = GenerateTypeIfNeeded(builder_.create()); break; } case ScalarConstant::Kind::kI32: { - sem::I32 i32; - type_id = GenerateTypeIfNeeded(&i32); + type_id = GenerateTypeIfNeeded(builder_.create()); break; } case ScalarConstant::Kind::kF32: { - sem::F32 f32; - type_id = GenerateTypeIfNeeded(&f32); + type_id = GenerateTypeIfNeeded(builder_.create()); break; } case ScalarConstant::Kind::kBool: { - sem::Bool bool_; - type_id = GenerateTypeIfNeeded(&bool_); + type_id = GenerateTypeIfNeeded(builder_.create()); break; } } @@ -2457,8 +2453,9 @@ uint32_t Builder::GenerateIntrinsic(ast::CallExpression* call, const uint32_t kMaxNormalExponent = 0x7f00000; auto set_id = GetGLSLstd450Import(); - sem::U32 u32; - auto unsigned_id = GenerateTypeIfNeeded(&u32); + auto* u32 = builder_.create(); + + auto unsigned_id = GenerateTypeIfNeeded(u32); auto exponent_mask_id = GenerateConstantIfNeeded(ScalarConstant::U32(kExponentMask)); auto min_exponent_id = @@ -2470,8 +2467,8 @@ uint32_t Builder::GenerateIntrinsic(ast::CallExpression* call, // same size, and create vector constants by replicating the scalars. // I expect backend compilers to fold these into unique constants, so // there is no loss of efficiency. - sem::Vector uvec_ty(&u32, fvec_ty->Width()); - unsigned_id = GenerateTypeIfNeeded(&uvec_ty); + auto* uvec_ty = builder_.create(u32, fvec_ty->Width()); + unsigned_id = GenerateTypeIfNeeded(uvec_ty); auto splat = [&](uint32_t scalar_id) -> uint32_t { auto splat_result = result_op(); OperandList splat_params{Operand::Int(unsigned_id), splat_result}; @@ -2562,12 +2559,12 @@ uint32_t Builder::GenerateIntrinsic(ast::CallExpression* call, auto* result_vector_type = intrinsic->ReturnType()->As(); if (result_vector_type && intrinsic->Parameters()[2]->Type()->is_scalar()) { - sem::Bool bool_type; - sem::Vector bool_vec_type(&bool_type, result_vector_type->Width()); - if (!GenerateTypeIfNeeded(&bool_vec_type)) { + auto* bool_vec_ty = builder_.create( + builder_.create(), result_vector_type->Width()); + if (!GenerateTypeIfNeeded(bool_vec_ty)) { return 0; } - cond_id = GenerateSplat(cond_id, &bool_vec_type); + cond_id = GenerateSplat(cond_id, bool_vec_ty); if (cond_id == 0) { return 0; } @@ -3231,8 +3228,8 @@ bool Builder::GenerateAtomicIntrinsic(ast::CallExpression* call, return false; } - sem::Bool bool_sem_type; - auto bool_type = GenerateTypeIfNeeded(&bool_sem_type); + auto* bool_sem_ty = builder_.create(); + auto bool_type = GenerateTypeIfNeeded(bool_sem_ty); if (bool_type == 0) { return false; } @@ -3924,8 +3921,7 @@ bool Builder::GenerateTextureType(const sem::Texture* texture, uint32_t type_id = 0u; if (texture->IsAnyOf()) { - sem::F32 f32; - type_id = GenerateTypeIfNeeded(&f32); + type_id = GenerateTypeIfNeeded(builder_.create()); } else if (auto* s = texture->As()) { type_id = GenerateTypeIfNeeded(s->type()); } else if (auto* ms = texture->As()) { @@ -3978,8 +3974,8 @@ bool Builder::GenerateArrayType(const sem::Array* ary, const Operand& result) { bool Builder::GenerateMatrixType(const sem::Matrix* mat, const Operand& result) { - sem::Vector col_type(mat->type(), mat->rows()); - auto col_type_id = GenerateTypeIfNeeded(&col_type); + auto* col_type = builder_.create(mat->type(), mat->rows()); + auto col_type_id = GenerateTypeIfNeeded(col_type); if (has_error()) { return false; }