[PATCH] D71172: [clang][Tooling] Fix potential UB in ExpandResponseFilesCompilationDatabase

2019-12-09 Thread Sam McCall via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG966fac1941ea: [clang][Tooling] Fix potential UB in 
ExpandResponseFilesCompilationDatabase (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71172/new/

https://reviews.llvm.org/D71172

Files:
  clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp


Index: clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
===
--- clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
+++ clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
@@ -61,7 +61,9 @@
   llvm::StringSaver Saver(Alloc);
   llvm::cl::ExpandResponseFiles(Saver, Tokenizer, Argv, false, false, *FS,
 llvm::StringRef(Cmd.Directory));
-  Cmd.CommandLine.assign(Argv.begin(), Argv.end());
+  // Don't assign directly, Argv aliases CommandLine.
+  std::vector ExpandedArgv(Argv.begin(), Argv.end());
+  Cmd.CommandLine = std::move(ExpandedArgv);
 }
 return Cmds;
   }


Index: clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
===
--- clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
+++ clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
@@ -61,7 +61,9 @@
   llvm::StringSaver Saver(Alloc);
   llvm::cl::ExpandResponseFiles(Saver, Tokenizer, Argv, false, false, *FS,
 llvm::StringRef(Cmd.Directory));
-  Cmd.CommandLine.assign(Argv.begin(), Argv.end());
+  // Don't assign directly, Argv aliases CommandLine.
+  std::vector ExpandedArgv(Argv.begin(), Argv.end());
+  Cmd.CommandLine = std::move(ExpandedArgv);
 }
 return Cmds;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71172: [clang][Tooling] Fix potential UB in ExpandResponseFilesCompilationDatabase

2019-12-09 Thread pre-merge checks [bot] via Phabricator via cfe-commits 
merge_guards_bot added a comment.

Build result: pass - 60606 tests passed, 0 failed and 726 were skipped.

Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71172/new/

https://reviews.llvm.org/D71172



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71172: [clang][Tooling] Fix potential UB in ExpandResponseFilesCompilationDatabase

2019-12-09 Thread pre-merge checks [bot] via Phabricator via cfe-commits 
merge_guards_bot added a comment.

Build result: pass - 60606 tests passed, 0 failed and 726 were skipped.

Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71172/new/

https://reviews.llvm.org/D71172



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71172: [clang][Tooling] Fix potential UB in ExpandResponseFilesCompilationDatabase

2019-12-09 Thread liu hui via Phabricator via cfe-commits 
lh123 updated this revision to Diff 232771.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71172/new/

https://reviews.llvm.org/D71172

Files:
  clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp


Index: clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
===
--- clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
+++ clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
@@ -61,7 +61,9 @@
   llvm::StringSaver Saver(Alloc);
   llvm::cl::ExpandResponseFiles(Saver, Tokenizer, Argv, false, false, *FS,
 llvm::StringRef(Cmd.Directory));
-  Cmd.CommandLine.assign(Argv.begin(), Argv.end());
+  // Don't assign directly, Argv aliases CommandLine.
+  std::vector ExpandedArgv(Argv.begin(), Argv.end());
+  Cmd.CommandLine = std::move(ExpandedArgv);
 }
 return Cmds;
   }


Index: clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
===
--- clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
+++ clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
@@ -61,7 +61,9 @@
   llvm::StringSaver Saver(Alloc);
   llvm::cl::ExpandResponseFiles(Saver, Tokenizer, Argv, false, false, *FS,
 llvm::StringRef(Cmd.Directory));
-  Cmd.CommandLine.assign(Argv.begin(), Argv.end());
+  // Don't assign directly, Argv aliases CommandLine.
+  std::vector ExpandedArgv(Argv.begin(), Argv.end());
+  Cmd.CommandLine = std::move(ExpandedArgv);
 }
 return Cmds;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71172: [clang][Tooling] Fix potential UB in ExpandResponseFilesCompilationDatabase

2019-12-09 Thread liu hui via Phabricator via cfe-commits 
lh123 updated this revision to Diff 232770.
lh123 added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71172/new/

https://reviews.llvm.org/D71172

Files:
  clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp


Index: clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
===
--- clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
+++ clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
@@ -61,7 +61,9 @@
   llvm::StringSaver Saver(Alloc);
   llvm::cl::ExpandResponseFiles(Saver, Tokenizer, Argv, false, false, *FS,
 llvm::StringRef(Cmd.Directory));
-  Cmd.CommandLine.assign(Argv.begin(), Argv.end());
+  // Don't assign directly, Argv aliases CommandLine
+  std::vector ExpandedArgv(Argv.begin(), Argv.end());
+  Cmd.CommandLine = std::move(ExpandedArgv);
 }
 return Cmds;
   }


Index: clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
===
--- clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
+++ clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
@@ -61,7 +61,9 @@
   llvm::StringSaver Saver(Alloc);
   llvm::cl::ExpandResponseFiles(Saver, Tokenizer, Argv, false, false, *FS,
 llvm::StringRef(Cmd.Directory));
-  Cmd.CommandLine.assign(Argv.begin(), Argv.end());
+  // Don't assign directly, Argv aliases CommandLine
+  std::vector ExpandedArgv(Argv.begin(), Argv.end());
+  Cmd.CommandLine = std::move(ExpandedArgv);
 }
 return Cmds;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71172: [clang][Tooling] Fix potential UB in ExpandResponseFilesCompilationDatabase

2019-12-09 Thread liu hui via Phabricator via cfe-commits 
lh123 added a comment.

@sammccall Could you commit this, I don't have commit access.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71172/new/

https://reviews.llvm.org/D71172



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71172: [clang][Tooling] Fix potential UB in ExpandResponseFilesCompilationDatabase

2019-12-09 Thread Sam McCall via Phabricator via cfe-commits 
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Thanks!
Might be worth a comment like "don't assign directly, Argv aliases CommandLine" 
or so


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71172/new/

https://reviews.llvm.org/D71172



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71172: [clang][Tooling] Fix potential UB in ExpandResponseFilesCompilationDatabase

2019-12-08 Thread pre-merge checks [bot] via Phabricator via cfe-commits 
merge_guards_bot added a comment.

Build result: pass - 60602 tests passed, 0 failed and 726 were skipped.

Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71172/new/

https://reviews.llvm.org/D71172



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71172: [clang][Tooling] Fix potential UB in ExpandResponseFilesCompilationDatabase

2019-12-08 Thread liu hui via Phabricator via cfe-commits 
lh123 created this revision.
lh123 added reviewers: kadircet, sammccall, hokein.
Herald added subscribers: cfe-commits, usaxena95, ilya-biryukov.
Herald added a project: clang.

`vector::assign` will cause UB at here.

fixes: https://github.com/clangd/clangd/issues/223


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71172

Files:
  clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp


Index: clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
===
--- clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
+++ clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
@@ -61,7 +61,8 @@
   llvm::StringSaver Saver(Alloc);
   llvm::cl::ExpandResponseFiles(Saver, Tokenizer, Argv, false, false, *FS,
 llvm::StringRef(Cmd.Directory));
-  Cmd.CommandLine.assign(Argv.begin(), Argv.end());
+  std::vector ExpandedArgv(Argv.begin(), Argv.end());
+  Cmd.CommandLine = std::move(ExpandedArgv);
 }
 return Cmds;
   }


Index: clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
===
--- clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
+++ clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
@@ -61,7 +61,8 @@
   llvm::StringSaver Saver(Alloc);
   llvm::cl::ExpandResponseFiles(Saver, Tokenizer, Argv, false, false, *FS,
 llvm::StringRef(Cmd.Directory));
-  Cmd.CommandLine.assign(Argv.begin(), Argv.end());
+  std::vector ExpandedArgv(Argv.begin(), Argv.end());
+  Cmd.CommandLine = std::move(ExpandedArgv);
 }
 return Cmds;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits