From 0551bc361823022473a89f9fc3b7f2852070d919 Mon Sep 17 00:00:00 2001 From: Fraser Cormack Date: Tue, 17 Oct 2023 11:58:39 +0100 Subject: [PATCH] [host] Fix move-after-move errors The use of `llvm::consumeError` is just to mark the error as 'handled' - it doesn't actually report the error in any way. If there's a callback we have reported *and* handled the error, so using `llvm::consumeError` is more correctly placed in an 'else' block. This fixes potential move-after-move errors when a compilation error happens and the callback doesn't halt execution at that point. --- modules/compiler/targets/host/source/kernel.cpp | 12 ++++++++---- modules/compiler/targets/host/source/target.cpp | 6 ++++-- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/modules/compiler/targets/host/source/kernel.cpp b/modules/compiler/targets/host/source/kernel.cpp index 1e7fe27b6..2cf904d66 100644 --- a/modules/compiler/targets/host/source/kernel.cpp +++ b/modules/compiler/targets/host/source/kernel.cpp @@ -334,8 +334,9 @@ HostKernel::lookupOrCreateOptimizedKernel(std::array local_size) { if (auto callback = target.getNotifyCallbackFn()) { callback(llvm::toString(std::move(err)).c_str(), /*data*/ nullptr, /*data_size*/ 0); + } else { + llvm::consumeError(std::move(err)); } - llvm::consumeError(std::move(err)); return cargo::make_unexpected(compiler::Result::FINALIZE_PROGRAM_FAILURE); } // Register this JITDylib so we can clear up its resources later. @@ -362,8 +363,9 @@ HostKernel::lookupOrCreateOptimizedKernel(std::array local_size) { if (auto callback = target.getNotifyCallbackFn()) { callback(llvm::toString(std::move(err)).c_str(), /*data*/ nullptr, /*data_size*/ 0); + } else { + llvm::consumeError(std::move(err)); } - llvm::consumeError(std::move(err)); return cargo::make_unexpected(compiler::Result::FINALIZE_PROGRAM_FAILURE); } @@ -374,8 +376,9 @@ HostKernel::lookupOrCreateOptimizedKernel(std::array local_size) { if (auto callback = target.getNotifyCallbackFn()) { callback(llvm::toString(std::move(err)).c_str(), /*data*/ nullptr, /*data_size*/ 0); + } else { + llvm::consumeError(std::move(err)); } - llvm::consumeError(std::move(err)); return cargo::make_unexpected(compiler::Result::FINALIZE_PROGRAM_FAILURE); } @@ -390,8 +393,9 @@ HostKernel::lookupOrCreateOptimizedKernel(std::array local_size) { if (auto callback = target.getNotifyCallbackFn()) { callback(llvm::toString(std::move(err)).c_str(), /*data*/ nullptr, /*data_size*/ 0); + } else { + llvm::consumeError(std::move(err)); } - llvm::consumeError(std::move(err)); return cargo::make_unexpected( compiler::Result::FINALIZE_PROGRAM_FAILURE); } diff --git a/modules/compiler/targets/host/source/target.cpp b/modules/compiler/targets/host/source/target.cpp index 430b1747f..370dabf60 100644 --- a/modules/compiler/targets/host/source/target.cpp +++ b/modules/compiler/targets/host/source/target.cpp @@ -228,8 +228,9 @@ compiler::Result HostTarget::initWithBuiltins( if (auto callback = getNotifyCallbackFn()) { callback(llvm::toString(std::move(err)).c_str(), /*data*/ nullptr, /*data_size*/ 0); + } else { + llvm::consumeError(std::move(err)); } - llvm::consumeError(std::move(err)); return compiler::Result::OUT_OF_MEMORY; } orc_engine = std::move(*JIT); @@ -239,8 +240,9 @@ compiler::Result HostTarget::initWithBuiltins( if (auto callback = getNotifyCallbackFn()) { callback(llvm::toString(std::move(err)).c_str(), /*data*/ nullptr, /*data_size*/ 0); + } else { + llvm::consumeError(std::move(err)); } - llvm::consumeError(std::move(err)); return compiler::Result::FAILURE; } target_machine = std::move(*TM);