[clang-tools-extra] 555eacf - [clangd] Fix undefined behavior when generating error message at rename with an invalid name

[Aleksandr Platonov via cfe-commits]

Author: Aleksandr Platonov
Date: 2021-12-19T22:28:26+03:00
New Revision: 555eacf75f21cd1dfc6363d73ad187b730349543

URL: https://github.com/llvm/llvm-project/commit/555eacf75f21cd1dfc6363d73ad187b730349543
DIFF: https://github.com/llvm/llvm-project/commit/555eacf75f21cd1dfc6363d73ad187b730349543.diff

LOG: [clangd] Fix undefined behavior when generating error message at rename with an invalid name

`Message()` lambda uses `Reason.Details` as an input parameter for `llvm::formatv()`, but `Reason` in `Message()` is a local object.
Return value of `llvm::formatv()` contains references to its input arguments, thus `Message()` returns an object which contains a reference to `Details` field of the local object `Reason`.
This patch fixes this behavior by passing `Reason` as a reference to `Message()` to ensure that return value of `Message()` contains references to alive object and also prevents copying of `InvalidName` structure at passing it to `makeError()`.

Provided test passes on Linux+GCC with or without this patch, but fails on Windows+VisualStudio without this patch.

Reviewed By: sammccall

Differential Revision: https://reviews.llvm.org/D115959

Added: 
    

Modified: 
    clang-tools-extra/clangd/refactor/Rename.cpp
    clang-tools-extra/clangd/unittests/RenameTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/refactor/Rename.cpp b/clang-tools-extra/clangd/refactor/Rename.cpp
index 76182375ea170..5e157db5900af 100644
--- a/clang-tools-extra/clangd/refactor/Rename.cpp
+++ b/clang-tools-extra/clangd/refactor/Rename.cpp
@@ -455,7 +455,7 @@ std::string toString(InvalidName::Kind K) {
 }
 
 llvm::Error makeError(InvalidName Reason) {
-  auto Message = [](InvalidName Reason) {
+  auto Message = [](const InvalidName &Reason) {
     switch (Reason.K) {
     case InvalidName::Keywords:
       return llvm::formatv("the chosen name \"{0}\" is a keyword",
@@ -733,7 +733,7 @@ llvm::Expected<RenameResult> rename(const RenameInputs &RInputs) {
     return makeError(ReasonToReject::SameName);
   auto Invalid = checkName(RenameDecl, RInputs.NewName);
   if (Invalid)
-    return makeError(*Invalid);
+    return makeError(std::move(*Invalid));
 
   auto Reject = renameable(RenameDecl, RInputs.MainFilePath, RInputs.Index);
   if (Reject)

diff  --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp b/clang-tools-extra/clangd/unittests/RenameTests.cpp
index f062f91c94378..26b3d7ad1c6c0 100644
--- a/clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -1060,6 +1060,11 @@ TEST(RenameTest, Renameable) {
       )cpp",
        "conflict", !HeaderFile, "Conflict"},
 
+      {R"cpp(
+        int V^ar;
+      )cpp",
+       "\"const\" is a keyword", !HeaderFile, "const"},
+
       {R"cpp(// Trying to rename into the same name, SameName == SameName.
         void func() {
           int S^ameName;