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

Vedant Kumar via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 18 11:33:49 PST 2018


vsk added a comment.

Thanks for working on this :).



================
Comment at: tools/libclang/CXString.cpp:213
+  if (string.IsNullTerminated) {
+    CString = (const char *) string.Contents;
+  } else {
----------------
elsteveogrande wrote:
> vsk wrote:
> > 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?
> I believe the API itself is the one building `CXString` instances, and the user of the C API doesn't really create them, only use them.  So the API has to ensure the string stays "good" while there may be references to it.
> 
> (Which feels a little fragile.  But I think that's the tradeoff being made.  You'll get either "fast" strings, or data guaranteed to be sane.  I'd opt for safer data but I don't know who's using this C API and am afraid to introduce a serious perf regression.  So it'll stay this way and I'll try my best to solve *-SAN issues with these constraints :) )
Sgtm, it doesn't look like this is altering the API contract.


Repository:
  rC Clang

https://reviews.llvm.org/D42043





More information about the cfe-commits mailing list