From f6a023d156a38c69b9cc10a55e991366d6a6465c Mon Sep 17 00:00:00 2001 From: ykiko Date: Fri, 29 Nov 2024 20:11:16 +0800 Subject: [PATCH] Fix some memory issues in the `Server`. --- .vscode/launch.json | 13 ++++- CMakeLists.txt | 9 ++-- include/Server/Async.h | 93 ++++++++++++++++++++--------------- include/Server/Scheduler.h | 16 +++--- include/Server/Trace.h | 8 +++ scripts/build-dev-test.sh | 10 +++- scripts/build-llvm-dev.py | 1 + scripts/build-llvm-release.py | 32 ++++++++++++ scripts/build-release.sh | 8 +++ src/Compiler/Compiler.cpp | 48 +++++++++++++----- src/Server/Config.cpp | 2 +- src/Server/Scheduler.cpp | 58 ++++++++++++++++------ tests/ASTVisitor/test.cpp | 14 +++--- tests/ASTVisitor/test.h | 13 ----- unittests/Server/Async.cpp | 25 ++++++++++ 15 files changed, 247 insertions(+), 103 deletions(-) create mode 100644 include/Server/Trace.h create mode 100644 scripts/build-llvm-release.py create mode 100755 scripts/build-release.sh delete mode 100644 tests/ASTVisitor/test.h create mode 100644 unittests/Server/Async.cpp diff --git a/.vscode/launch.json b/.vscode/launch.json index e9c38937..2852125b 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -7,7 +7,7 @@ { "type": "lldb", "request": "launch", - "name": "clice_socket", + "name": "Debug", "program": "${workspaceFolder}/build/bin/clice", "args": [ "--config=${workspaceFolder}/docs/clice.toml" @@ -16,7 +16,16 @@ { "type": "lldb", "request": "launch", - "name": "clice_test", + "name": "Release", + "program": "${workspaceFolder}/build-release/bin/clice", + "args": [ + "--config=${workspaceFolder}/docs/clice.toml" + ] + }, + { + "type": "lldb", + "request": "launch", + "name": "Test", "program": "${workspaceFolder}/build/bin/clice-tests", "args": [ "--test-dir=${workspaceFolder}/tests", diff --git a/CMakeLists.txt b/CMakeLists.txt index 2b63586e..cd88c542 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,13 +1,10 @@ cmake_minimum_required(VERSION 3.22) project(CLICE_PROJECT) -set(CLICE_LIB_TYPE SHARED) set(CMAKE_CXX_STANDARD 20) set(CMAKE_EXPORT_COMPILE_COMMANDS ON) set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin) -set(LLVM_INSTALL_PATH "${CMAKE_SOURCE_DIR}/deps/llvm/build-install") - set(CMAKE_PREFIX_PATH "${LLVM_INSTALL_PATH}") find_package(LLVM REQUIRED CONFIG) find_package(Clang REQUIRED CONFIG) @@ -69,10 +66,11 @@ file(GLOB_RECURSE SRC_FILES "${CMAKE_SOURCE_DIR}/src/Server/*.cpp") list(APPEND CLICE_SERVER_SOURCES ${SRC_FILES}) add_executable(clice ${CLICE_SERVER_SOURCES}) -target_include_directories(clice PRIVATE +include_directories( "${CMAKE_SOURCE_DIR}/deps/toml/include" "${CMAKE_SOURCE_DIR}/deps/libuv/include" ) + target_link_libraries(clice PRIVATE clice-core uv) @@ -83,13 +81,14 @@ if(CLICE_ENABLE_TEST) file(GLOB_RECURSE TEST_SRC_FILES "${CMAKE_SOURCE_DIR}/unittests/*/*.cpp") list(APPEND CLICE_TEST_SOURCES ${TEST_SRC_FILES}) - add_executable(clice-tests ${CLICE_TEST_SOURCES}) + add_executable(clice-tests ${CLICE_TEST_SOURCES} ${SRC_FILES}) target_include_directories(clice-tests PRIVATE "${CMAKE_SOURCE_DIR}/deps/googletest/googletest/include" ) target_link_libraries(clice-tests PRIVATE clice-core gtest_main + uv ) endif() diff --git a/include/Server/Async.h b/include/Server/Async.h index 4c2179fa..c21b5356 100644 --- a/include/Server/Async.h +++ b/include/Server/Async.h @@ -48,9 +48,7 @@ void write(json::Value id, json::Value result); template struct result { - union { - Value value; - }; + std::optional value; result() {} @@ -61,12 +59,14 @@ struct result { } decltype(auto) await_resume() noexcept { - return std::move(value); + assert(value.has_value() && "await_resume: no value"); + return std::move(*value); } template void return_value(T&& val) noexcept { - new (&value) Value(std::forward(val)); + assert(!value.has_value() && "return_value: value already set"); + value.emplace(std::forward(val)); } }; @@ -90,7 +90,8 @@ inline void schedule(std::coroutine_handle<> handle) { auto callback = [](uv_async_t* async) { auto handle = std::coroutine_handle<>::from_address(async->data); handle.resume(); - uv_close((uv_handle_t*)async, [](uv_handle_t* handle) { delete handle; }); + uv_close(reinterpret_cast(async), nullptr); + delete async; }; uv_check_call(uv_async_init, loop, async, callback); @@ -98,24 +99,25 @@ inline void schedule(std::coroutine_handle<> handle) { } template -void schedule(promise promise) { +void schedule(const promise& promise) { schedule(promise.handle()); } template > struct task_awaiter : result { std::remove_cvref_t task; + uv_work_t work; std::coroutine_handle<> caller; void await_suspend(std::coroutine_handle<> caller) noexcept { static_assert(!std::is_reference_v, "return type must not be a reference"); this->caller = caller; - uv_work_t* work = new uv_work_t{.data = this}; + work.data = this; auto work_cb = [](uv_work_t* work) { auto& awaiter = uv_cast(work); if constexpr(!std::is_void_v) { - new (&awaiter.value) Ret(awaiter.task()); + awaiter.value.emplace(awaiter.task()); } else { awaiter.task(); } @@ -124,10 +126,9 @@ struct task_awaiter : result { auto after_work_cb = [](uv_work_t* work, int status) { auto& awaiter = uv_cast(work); awaiter.caller.resume(); - delete work; }; - uv_check_call(uv_queue_work, loop, work, work_cb, after_work_cb); + uv_check_call(uv_queue_work, loop, &work, work_cb, after_work_cb); } }; @@ -229,15 +230,37 @@ struct awaiter { async::schedule(h); } - decltype(auto) await_resume() noexcept - requires (!std::is_void_v) - { - return std::move(h.promise().value); + decltype(auto) await_resume() noexcept { + if constexpr(!std::is_void_v) { + auto value = std::move(*h.promise().value); + h.destroy(); + return value; + } else { + h.destroy(); + } + } +}; + +template +struct final_awaiter { + std::coroutine_handle<> caller; + + bool await_ready() noexcept { + return false; } - void await_resume() noexcept - requires (std::is_void_v) - {} + template + void await_suspend(std::coroutine_handle self) noexcept { + /// If this coroutine is a top-level coroutine, its caller is empty. + if(!caller) { + return; + } + + /// Schedule the caller to run in the event loop. + async::schedule(caller); + } + + void await_resume() noexcept {} }; template @@ -257,28 +280,7 @@ struct promise_type : result { } auto final_suspend() noexcept { - struct FinalAwaiter { - std::coroutine_handle<> caller; - - bool await_ready() noexcept { - return false; - } - - void await_suspend(std::coroutine_handle<> self) noexcept { - self.destroy(); - /// If this coroutine is a top-level coroutine, its caller is empty. - if(!caller) { - return; - } - - /// Schedule the caller to run in the event loop. - async::schedule(caller); - } - - void await_resume() noexcept {} - }; - - return FinalAwaiter{.caller = caller}; + return final_awaiter{caller}; } }; @@ -305,12 +307,21 @@ public: return h; } + bool done() const noexcept { + return h.done(); + } + + void destroy() noexcept { + assert(h && h.done() && "destroy: invalid coroutine handle"); + h.destroy(); + } + private: coroutine_handle h; }; /// Suspend current coroutine and invoke the callback with its handle. -/// Note the callback invoked before the coroutine is suspended. So it is +/// Note the callback invoked before the coroutine is suspended. So it is /// template auto suspend(Callback&& callback) { diff --git a/include/Server/Scheduler.h b/include/Server/Scheduler.h index a5725253..3efb7f7e 100644 --- a/include/Server/Scheduler.h +++ b/include/Server/Scheduler.h @@ -22,19 +22,23 @@ struct PCH { /// All files involved in building this PCH(excluding the source file). std::vector deps; + uint32_t size() const { + return preamble.size() - preamble.ends_with('@'); + } + /// FIXME: use asyncronous file system API. bool needUpdate(llvm::StringRef sourceContent) { /// Check whether the header part changed. - if(sourceContent.substr(0, preamble.size()) != preamble) { + if(sourceContent.substr(0, size()) != preamble.substr(0, size())) { return true; } /// Check timestamp of all files involved in building this PCH. - fs::file_status build; - if(auto error = fs::status(path, build)) { - llvm::errs() << "Error: " << error.message() << "\n"; - std::terminate(); - } + // fs::file_status build; + // if(auto error = fs::status(path, build)) { + // llvm::errs() << "Error: " << error.message() << "\n"; + // std::terminate(); + // } /// TODO: check whether deps changed through comparing timestamps. return false; diff --git a/include/Server/Trace.h b/include/Server/Trace.h new file mode 100644 index 00000000..58710e48 --- /dev/null +++ b/include/Server/Trace.h @@ -0,0 +1,8 @@ +#pragma once + +namespace clice { + + + + +} \ No newline at end of file diff --git a/scripts/build-dev-test.sh b/scripts/build-dev-test.sh index 853a3147..be4c8c11 100755 --- a/scripts/build-dev-test.sh +++ b/scripts/build-dev-test.sh @@ -1 +1,9 @@ -cmake -B build -G Ninja -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_C_COMPILER=clang -DCMAKE_BUILD_TYPE=Debug -DCLICE_ENABLE_TEST=ON -DCMAKE_CXX_FLAGS="-DSPDLOG_NO_EXCEPTIONS -fno-rtti -fno-exceptions -g -O0" \ No newline at end of file +cmake -B build -G Ninja \ +-DCMAKE_CXX_COMPILER=clang++ \ +-DCMAKE_C_COMPILER=clang \ +-DCMAKE_BUILD_TYPE=Debug \ +-DCLICE_ENABLE_TEST=ON \ +-DCMAKE_CXX_FLAGS="${CMAKE_CXX_FLAGS} -fno-rtti -fno-exceptions -g -O0 -fsanitize=address" \ +-DCMAKE_LINKER_FLAGS="${CMAKE_LINKER_FLAGS} -fsanitize=address" \ +-DLLVM_INSTALL_PATH="./deps/llvm/build-install" \ +-DCLICE_LIB_TYPE=SHARED \ No newline at end of file diff --git a/scripts/build-llvm-dev.py b/scripts/build-llvm-dev.py index 1bb2bab7..861c7cde 100644 --- a/scripts/build-llvm-dev.py +++ b/scripts/build-llvm-dev.py @@ -16,6 +16,7 @@ args = [ '-DLLVM_TARGETS_TO_BUILD=X86', '-DLLVM_ENABLE_PROJECTS=clang', '-DCMAKE_INSTALL_PREFIX=./build-install', + '-DLLVM_USE_SANITIZER=Address', ] subprocess.run(['cmake'] + args) diff --git a/scripts/build-llvm-release.py b/scripts/build-llvm-release.py new file mode 100644 index 00000000..91219386 --- /dev/null +++ b/scripts/build-llvm-release.py @@ -0,0 +1,32 @@ +import os +import shutil +import subprocess + +os.chdir('deps/llvm') + +args = [ + '-B=./build-release', + '-S=./llvm', + '-G=Ninja', + '-DLLVM_USE_LINKER=lld', + '-DCMAKE_C_COMPILER=clang', + '-DCMAKE_CXX_COMPILER=clang++', + '-DCMAKE_BUILD_TYPE=Release', + '-DLLVM_TARGETS_TO_BUILD=X86', + '-DLLVM_ENABLE_PROJECTS=clang', + '-DCMAKE_INSTALL_PREFIX=./build-release-install', +] + +subprocess.run(['cmake'] + args) +subprocess.run(['cmake', '--build', 'build-release', '--target', 'clang']) +subprocess.run(['cmake', '--build', 'build-release', '--target', 'install']) + +src = "./clang/lib/Sema/" +dst = "./build-release-install/include/clang/Sema/" + +for file in ["CoroutineStmtBuilder.h", "TypeLocBuilder.h", "TreeTransform.h"]: + shutil.copyfile(src + file, dst + file) + print(f"Copying {src + file} to {dst + file}") + + + diff --git a/scripts/build-release.sh b/scripts/build-release.sh new file mode 100755 index 00000000..adffd3a7 --- /dev/null +++ b/scripts/build-release.sh @@ -0,0 +1,8 @@ +cmake -B build-release -G Ninja \ +-DCMAKE_CXX_COMPILER=clang++ \ +-DCMAKE_C_COMPILER=clang \ +-DCLICE_ENABLE_TEST=ON \ +-DCMAKE_BUILD_TYPE=Release \ +-DCMAKE_CXX_FLAGS="-fno-rtti -fno-exceptions -O3 -g" \ +-DLLVM_INSTALL_PATH="./deps/llvm/build-release-install" \ +-DCLICE_LIB_TYPE=STATIC \ No newline at end of file diff --git a/src/Compiler/Compiler.cpp b/src/Compiler/Compiler.cpp index 581e595d..c7826cfe 100644 --- a/src/Compiler/Compiler.cpp +++ b/src/Compiler/Compiler.cpp @@ -4,7 +4,10 @@ namespace clice { -static void setInvocation(clang::CompilerInvocation& invocation) { +static void adjustInvocation(clang::CompilerInvocation& invocation) { + auto& frontOpts = invocation.getFrontendOpts(); + frontOpts.DisableFree = false; + clang::LangOptions& langOpts = invocation.getLangOpts(); langOpts.CommentOpts.ParseAllComments = true; langOpts.RetainCommentsFromSystemHeaders = true; @@ -22,24 +25,22 @@ Compiler::Compiler(llvm::StringRef filepath, clang::CreateInvocationOptions options; auto invocation = clang::createInvocation(args, options); - instance = std::make_unique(); + /// FIXME: use a thread safe for every thread. + instance = std::make_unique( + std::make_shared()); + adjustInvocation(*invocation); instance->setInvocation(std::move(invocation)); // FIXME: customize DiagnosticConsumer if(consumer) { - instance->createDiagnostics(*llvm::vfs::getRealFileSystem(), consumer, true); + instance->createDiagnostics(*vfs, consumer, true); } else { instance->createDiagnostics( - *llvm::vfs::getRealFileSystem(), + *vfs, new clang::TextDiagnosticPrinter(llvm::outs(), new clang::DiagnosticOptions()), true); } - - if(!instance->createTarget()) { - llvm::errs() << "Failed to create target\n"; - std::terminate(); - } } bool Compiler::applyPCH(llvm::StringRef filepath, std::uint32_t bound, bool endAtStart) { @@ -47,7 +48,7 @@ bool Compiler::applyPCH(llvm::StringRef filepath, std::uint32_t bound, bool endA auto& preproc = instance->getPreprocessorOpts(); preproc.UsePredefines = false; preproc.ImplicitPCHInclude = filepath; - preproc.PrecompiledPreambleBytes.first = {}; + preproc.PrecompiledPreambleBytes.first = bound; preproc.PrecompiledPreambleBytes.second = endAtStart; preproc.DisablePCHOrModuleValidation = clang::DisableValidationForModuleKind::PCH; return true; @@ -61,6 +62,7 @@ bool Compiler::applyPCM(llvm::StringRef filepath, llvm::StringRef name) { void Compiler::buildAST() { action = std::make_unique(); + instance->getFrontendOpts().DisableFree = false; ExecuteAction(); m_Resolver = std::make_unique(instance->getSema()); } @@ -68,6 +70,10 @@ void Compiler::buildAST() { void Compiler::generatePCH(llvm::StringRef outpath, std::uint32_t bound, bool endAtStart) { content = content.substr(0, bound); instance->getFrontendOpts().OutputFile = outpath; + instance->getFrontendOpts().ProgramAction = clang::frontend::GeneratePCH; + instance->getPreprocessorOpts().PrecompiledPreambleBytes = {0, false}; + instance->getPreprocessorOpts().GeneratePreamble = true; + instance->getLangOpts().CompilingPCH = true; action = std::make_unique(); ExecuteAction(); } @@ -99,6 +105,8 @@ void Compiler::codeCompletion(llvm::StringRef filepath, std::terminate(); } + /// instance->getASTContext().setExternalSource(nullptr); + if(auto error = action->Execute()) { llvm::errs() << "Failed to execute action: " << error << "\n"; std::terminate(); @@ -106,16 +114,34 @@ void Compiler::codeCompletion(llvm::StringRef filepath, } void Compiler::ExecuteAction() { - if(content != "") { + { auto buffer = llvm::MemoryBuffer::getMemBufferCopy(content); instance->getPreprocessorOpts().addRemappedFile(filepath, buffer.release()); } + if(auto VFSWithRemapping = createVFSFromCompilerInvocation(instance->getInvocation(), + instance->getDiagnostics(), + llvm::vfs::getRealFileSystem())) { + instance->createFileManager(VFSWithRemapping); + } + + if(!instance->createTarget()) { + llvm::errs() << "Failed to create target\n"; + std::terminate(); + } + + llvm::outs() << instance->getLangOpts().Modules << "\n"; + llvm::outs() << instance->getLangOpts().CPlusPlusModules << "\n"; + if(!action->BeginSourceFile(*instance, instance->getFrontendOpts().Inputs[0])) { llvm::errs() << "Failed to begin source file\n"; std::terminate(); } + /// llvm::outs() << instance->getPreprocessorOpts().ImplicitPCHInclude << "\n"; + + /// instance->getASTContext().setExternalSource(nullptr); + auto& preproc = instance->getPreprocessor(); // FIXME: add PPCallbacks to collect information. diff --git a/src/Server/Config.cpp b/src/Server/Config.cpp index 0571ee7e..afab1c28 100644 --- a/src/Server/Config.cpp +++ b/src/Server/Config.cpp @@ -1,4 +1,4 @@ - +#define TOML_EXCEPTIONS 0 #include #include diff --git a/src/Server/Scheduler.cpp b/src/Server/Scheduler.cpp index a518e7b3..8c5c3bdf 100644 --- a/src/Server/Scheduler.cpp +++ b/src/Server/Scheduler.cpp @@ -12,29 +12,52 @@ void PCH::apply(Compiler& compiler) const { } } +struct Tracer { + std::chrono::system_clock::time_point start = std::chrono::system_clock::now(); + + auto duration() { + return std::chrono::duration_cast( + std::chrono::system_clock::now() - start); + } +}; + async::promise Scheduler::updatePCH(llvm::StringRef filepath, llvm::StringRef content, llvm::ArrayRef args) { - log::info("Start building PCH for {0}", filepath.str()); + std::string outpath = "/home/ykiko/C++/clice2/build/cache/xxx.pch"; - clang::PreambleBounds bounds = {0, 0}; - co_await async::schedule_task([&] { - Compiler compiler(filepath, content, args); - bounds = clang::Lexer::ComputePreamble(content, {}, false); - if(bounds.Size != 0) { - compiler.generatePCH(outpath, bounds.Size, bounds.PreambleEndsAtStartOfLine); + auto [iter, success] = pchs.try_emplace(filepath); + if(success || iter->second.needUpdate(content)) { + log::info("Start building PCH for {0}", filepath.str()); + + Tracer tracer; + clang::PreambleBounds bounds = {0, 0}; + co_await async::schedule_task([&] { + Compiler compiler(filepath, content, args); + bounds = clang::Lexer::ComputePreamble(content, {}, false); + if(bounds.Size != 0) { + compiler.generatePCH(outpath, bounds.Size, bounds.PreambleEndsAtStartOfLine); + } + }); + + auto preamble = content.substr(0, bounds.Size).str(); + if(bounds.PreambleEndsAtStartOfLine) { + preamble.append("@"); } - }); - log::info("Build PCH success"); + pchs[filepath] = PCH{ + .path = outpath, + .preamble = preamble, + .deps = {}, + }; - auto preamble = content.substr(0, bounds.Size).str(); - if(bounds.PreambleEndsAtStartOfLine) { - preamble.append("@"); + log::info("PCH for {0} is up-to-date, elapsed {1}ms", + filepath.str(), + tracer.duration().count()); + } else { + log::info("Reuse PCH from {0}", filepath.str()); } - - pchs.try_emplace(filepath, PCH{.path = outpath, .preamble = std::move(preamble)}); co_return; } @@ -67,6 +90,7 @@ async::promise Scheduler::buildAST(llvm::StringRef filepath, llvm::StringR bool isModule = false; co_await (isModule ? updatePCM() : updatePCH(filepath, content, args)); + Tracer tracer; log::info("Start building AST for {0}", filepath.str()); auto task = [&path, &content, &args, pch = pchs.at(filepath)] { @@ -86,13 +110,17 @@ async::promise Scheduler::buildAST(llvm::StringRef filepath, llvm::StringR auto& file = files[path]; file.compiler = std::move(compiler); - log::info("Build AST success"); + log::info("Build AST successfully for {0}, elapsed {1}ms", + filepath.str(), + tracer.duration().count()); if(!file.waitings.empty()) { auto task = std::move(file.waitings.front()); async::schedule(task.waiting); file.waitings.pop_front(); } + + file.isIdle = true; } async::promise Scheduler::add(llvm::StringRef path, llvm::StringRef content) { diff --git a/tests/ASTVisitor/test.cpp b/tests/ASTVisitor/test.cpp index 434d78d6..62cf64aa 100644 --- a/tests/ASTVisitor/test.cpp +++ b/tests/ASTVisitor/test.cpp @@ -1,9 +1,7 @@ -void test(int and x); +#include +#include -// void foo [[gnu::format(printf, 1, 3)]] (const char* s, char* buf, ...); -// -// void __attribute__((__format__(printf, 1, 3))) bar(const char* s, char* buf, ...); -// -// void foo2(int x) { -// if(x < 3) [[unlikely]] {} -// } +int main() { + printf("Hello world"); + return 0; +} diff --git a/tests/ASTVisitor/test.h b/tests/ASTVisitor/test.h deleted file mode 100644 index 96bdce4a..00000000 --- a/tests/ASTVisitor/test.h +++ /dev/null @@ -1,13 +0,0 @@ -template -struct X; - -using Z = X; - -template <> -struct X { - using type = int; -}; - -int main() { - Z::type x = 1; -} diff --git a/unittests/Server/Async.cpp b/unittests/Server/Async.cpp new file mode 100644 index 00000000..12aa8dad --- /dev/null +++ b/unittests/Server/Async.cpp @@ -0,0 +1,25 @@ +#include "../Test.h" +#include "Server/Async.h" + +namespace { + +using namespace clice; + +async::promise add(int a, int b) { + co_return a + b; +} + +async::promise add2(int a, int b) { + auto result = co_await add(a, b); + co_return result; +} + +TEST(clice, coroutine) { + auto p = add2(1, 2); + async::run(p); + ASSERT_TRUE(p.done()); + ASSERT_EQ(p.handle().promise().value, 3); + p.destroy(); +} + +} // namespace