[PATCH] D42043: c-index: CXString: fix MSAN read-past-end bug

Vedant Kumar via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 17 15:32:12 PST 2018


vsk added reviewers: akyrtzi, benlangmuir.
vsk added inline comments.


================
Comment at: tools/c-index-test/c-index-test.c:3268
 
-  filename = clang_getFileName(file);
-  index_data->main_filename = clang_getCString(filename);
-  clang_disposeString(filename);
+  index_data->main_filename = clang_getFileName(file);
 
----------------
This looks like a separate bug fix. Is it possible to separate the main_filename changes from this patch?


================
Comment at: tools/libclang/CXString.cpp:59
 CXString createEmpty() {
   CXString Str;
+  Str.Contents = (const void *) "";
----------------
Why shouldn't this be defined as createRef("")?


================
Comment at: tools/libclang/CXString.cpp:213
+  if (string.IsNullTerminated) {
+    CString = (const char *) string.Contents;
+  } else {
----------------
Basic question: If a non-owning CXString is null-terminated, what provides the guarantee that the string is in fact valid when getCString() is called? Is the user of the C API responsible for ensuring the lifetime of the string is valid?


================
Comment at: tools/libclang/CXString.h:108
 
 #endif
----------------
Generally unrelated whitespace changes should be left out of patches.


Repository:
  rC Clang

https://reviews.llvm.org/D42043





More information about the cfe-commits mailing list