c-index: CXString: fix MSAN read-past-end bug

[elsteveogrande]

Summary

See discussion here:
http://clang-developers.42468.n3.nabble.com/RFC-proper-string-handling-in-CXString-td4059287.html

Bugzilla URL:
https://bugs.llvm.org/show_bug.cgi?id=35896

Previous impl would read the byte past the end of a string (a llvm::StringRef), possibly exceeding the allocation for that memory and raising an MSAN issue.

This will instead save the character range specified by the StringRef and copy it on-demand to a new C string.

Passes existing tests, and now passes with MSAN as well.

[elsteveogrande]

Change string-copy-on-demand logic; do only if not IsNullTerminated.

[elsteveogrande]

Add a needed null-check on input string's data ptr.

[vsk]

tools/c-index-test/c-index-test.c

3581

This looks like a separate bug fix. Is it possible to separate the main_filename changes from this patch?

tools/libclang/CXString.cpp

59–60

Why shouldn't this be defined as createRef("")?

204

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?

tools/libclang/CXString.h

108 ↗

(On Diff #130225)

Generally unrelated whitespace changes should be left out of patches.

[elsteveogrande]

Thanks very much for looking at this @vsk ! I actually found an ASAN bug in my new code, mixing and matching malloc/free and operators new/delete.

tools/c-index-test/c-index-test.c

3581

Will do!

tools/libclang/CXString.cpp

59–60

Hmm, good question. createRef below actually calls back to createEmpty and createNull in the nonnull-but-empty and null cases.

I think I'll do this the other way around, and let createRef have the responsibility of dealing with these fields and nulls and whatnot.

204

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 :) )

[vsk]

Thanks for working on this :).

tools/libclang/CXString.cpp

204

Sgtm, it doesn't look like this is altering the API contract.

[elsteveogrande]

tools/c-index-test/c-index-test.c

3581

Done: https://reviews.llvm.org/D42259

[vsk]

Mind rebasing this when you have a chance?

[elsteveogrande]

Fixes, but first, a question for reviewers:

Looking at the description of clang_disposeString:

/**
 * \brief Free the given string.
 */
CINDEX_LINKAGE void clang_disposeString(CXString string);

Does it seem incorrect to acquire some const char * pointer into this string, dispose the string, and then access the characters?

I've seen this happen a couple of times now. As I make changes to my code I run into this pattern. (Since now this triggers a use-after-free abort.)

I wanted to ask because, though it seemed obvious to me that this is incorrect usage, I'm now wondering if the expectation is that it's ok. Or maybe wasn't technically ok, and we just "got away with it" before. :)

Anyway, assuming it's only correct to use the string before disposing it, then the fixes this time around are:

• Fix an ASAN use-after-free in c-index-test mentioned above. Get the CXString, pass it around, then dispose when we're done with it.
• Change createEmpty and createNull to delegate through createRef
• createRef tolerates nullptr correctly.
• I previously ran into ASAN aborts due to mixing malloc/free/operator new/delete. I had changed everything to use operator new/delete. However from C-land I can't new char[length], only malloc (or strdup or whatever). Change everything to malloc/free instead.

[elsteveogrande]

remove unneeded changes

[vsk]

In D42043#981409, @elsteveogrande wrote:

Fixes, but first, a question for reviewers:

Looking at the description of clang_disposeString:

/**
 * \brief Free the given string.
 */
CINDEX_LINKAGE void clang_disposeString(CXString string);

Does it seem incorrect to acquire some const char * pointer into this string, dispose the string, and then access the characters?

I'm inclined to say that this is incorrect, but skimming through the codebase it seems like we "get away" with this behavior all over the place.

I've seen this happen a couple of times now. As I make changes to my code I run into this pattern. (Since now this triggers a use-after-free abort.)

I wanted to ask because, though it seemed obvious to me that this is incorrect usage, I'm now wondering if the expectation is that it's ok.

I've CC'd some Apple folks who can say more definitively how difficult it'd be to move away from this behavior. IMO it should be doable as long as there's some way to audit the codebase (e.g with ASan).

Or maybe wasn't technically ok, and we just "got away with it" before. :)

Anyway, assuming it's only correct to use the string before disposing it, then the fixes this time around are:

• Fix an ASAN use-after-free in c-index-test mentioned above. Get the CXString, pass it around, then dispose when we're done with it.
• Change createEmpty and createNull to delegate through createRef
• createRef tolerates nullptr correctly.
• I previously ran into ASAN aborts due to mixing malloc/free/operator new/delete. I had changed everything to use operator new/delete. However from C-land I can't new char[length], only malloc (or strdup or whatever). Change everything to malloc/free instead.

[elsteveogrande]

Hi @vsk + @arphaman : I did find such problems with either MSAN (with poison-in-dtor) and ASAN, so we should be able to discover these instances pretty easily :)

[vitalybuka]

Is this stale?

[elsteveogrande]

Rebase + fix conflicts for very old diff. Works again.

ninja check-clang with MSAN-enabled build:

Before:

Failing Tests (2):
    Clang :: CodeGen/signed_metadata.cpp
    Clang :: Index/comment-to-html-xml-conversion.cpp

  Expected Passes    : 12777
  Expected Failures  : 19
  Unsupported Tests  : 291
  Unexpected Failures: 2
FAILED: tools/clang/test/CMakeFiles/check-clang
cd /data/users/steveo/llvm-ct/build/msan/clang/tools/clang/test && /bin/python2.7 /data/users/steveo/llvm-ct/build/msan/clang/./bin/llvm-lit -sv --param clang_site_config=/data/users/steveo/llvm-ct/build/msan/clang/tools/clang/test/lit.site.cfg --param USE_Z3_SOLVER=0 /data/users/steveo/llvm-ct/build/msan/clang/tools/clang/test
ninja: build stopped: subcommand failed.

After:

Failing Tests (1):
    Clang :: CodeGen/signed_metadata.cpp

  Expected Passes    : 12778
  Expected Failures  : 19
  Unsupported Tests  : 291
  Unexpected Failures: 1
FAILED: tools/clang/test/CMakeFiles/check-clang
cd /data/users/steveo/llvm-ct/build/msan/clang/tools/clang/test && /bin/python2.7 /data/users/steveo/llvm-ct/build/msan/clang/./bin/llvm-lit -sv --param clang_site_config=/data/users/steveo/llvm-ct/build/msan/clang/tools/clang/test/lit.site.cfg --param USE_Z3_SOLVER=0 /data/users/steveo/llvm-ct/build/msan/clang/tools/clang/test
ninja: build stopped: subcommand failed.

[elsteveogrande]

hi @vsk + @arphaman , it's been a while -- I brought this old diff up to date, wondering if you could take a look again. Thanks!

[vsk]

+ Jan and Volodymyr. This seemed to be in good shape the last time I looked at it. Not having touched libclang for a while I don't think I can give an official lgtm.

[riccibruno]

Is anyone working on this ? This is the only persistent msan failure when doing check-clang on my machine.

include/clang-c/CXString.h

 |* This header provides the interface to C Index strings.                     *|
 |*                                                                            *|
 \*===----------------------------------------------------------------------===*/
 
 #ifndef LLVM_CLANG_C_CXSTRING_H
 #define LLVM_CLANG_C_CXSTRING_H
 
 #include "clang-c/Platform.h"
+#include <stdbool.h>
 
 #ifdef __cplusplus
 extern "C" {
 #endif
 
 /**
  * \defgroup CINDEX_STRING String manipulation routines
  * \ingroup CINDEX
  *
  * @{
  */
 
 /**
  * A character string.
  *
  * The \c CXString type is used to return strings from the interface when
  * the ownership of that string might differ from one call to the next.
  * Use \c clang_getCString() to retrieve the string data and, once finished
  * with the string data, call \c clang_disposeString() to free the string.
  */
 typedef struct {
-  const void *data;
-  unsigned private_flags;
+  const void *Contents;
+  unsigned Size;
+  bool IsNullTerminated;
+  bool IsOwned;
+  bool IsPooled;
 } CXString;
 
 typedef struct {
   CXString *Strings;
   unsigned Count;
 } CXStringSet;
 
 /**
  * Retrieve the character data associated with the given string.
+ *
+ * This returns a pointer to a C string even if this \b CXString was built
+ * from e.g. a StringRef reference (which is not null-terminated).  In those
+ * cases a string copy is performed on-demand.
+ *
+ * See also \b clang_getStringData and \b clang_getStringSize which could
+ * provide a range or "window" of data, if needed, and a C string is not
+ * strictly needed; this could save a C string allocation, deallocation, and
+ * string copy.
  */
 CINDEX_LINKAGE const char *clang_getCString(CXString string);
 
 /**
+ * Retrieve a pointer to the start of the character data for this string.
+ *
+ * Note that the string is not necessarily null-terminated, so it might not be
+ * suitable for use as a C string.  See \b clang_getCString which guarantees
+ * a C string result.
+ */
+CINDEX_LINKAGE const char *clang_getStringData(CXString string);
+
+/**
+ * Retrieve the string length (not including null-terminator, if any).
+ */
+CINDEX_LINKAGE unsigned clang_getStringSize(CXString string);
+
+/**
  * Free the given string.
  */
 CINDEX_LINKAGE void clang_disposeString(CXString string);
 
 /**
  * Free the given string set.
  */
 CINDEX_LINKAGE void clang_disposeStringSet(CXStringSet *set);
 
 /**
  * @}
  */
 
 #ifdef __cplusplus
 }
 #endif
 #endif
-

tools/c-index-test/c-index-test.c

if (argc == 0)
     return CommentSchemaFile;
 
   if (!strncmp(argv[0], CommentsSchemaArg, strlen(CommentsSchemaArg)))
     CommentSchemaFile = argv[0] + strlen(CommentsSchemaArg);
 
   return CommentSchemaFile;
 }
 
+static CXString createCXString(const char *CS) {
+  CXString Str;
+
+  Str.Contents = (const void *) CS;
+  if (CS) {
+    Str.Size = strlen(CS);
+    Str.IsNullTerminated = true;
+  } else {
+    Str.Size = 0;
+    Str.IsNullTerminated = false;
+  }
+  Str.IsOwned = false;
+  Str.IsPooled = false;
+  return Str;
+}
+
+static CXString copyToCXString(const char *OrigCS) {
+  CXString Str;
+  const char *CS;
+
+  CS = strdup(OrigCS);
+  assert(CS);
+  Str = createCXString(CS);
+  Str.IsOwned = true;
+  return Str;
+}
+
 /******************************************************************************/
 /* Pretty-printing.                                                           */
 /******************************************************************************/
 
 static const char *FileCheckPrefix = "CHECK";
 
 static void PrintCString(const char *CStr) {
   if (CStr != NULL && CStr[0] != '\0') {
static enum DisplayType {
     DisplayType_DisplayName,
     DisplayType_Pretty
 } wanted_display_type = DisplayType_Spelling;
 
 static void printVersion(const char *Prefix, CXVersion Version) {
   if (Version.Major < 0)
     return;
   printf("%s%d", Prefix, Version.Major);
-  
   if (Version.Minor < 0)
     return;
   printf(".%d", Version.Minor);
 
   if (Version.Subminor < 0)
     return;
   printf(".%d", Version.Subminor);
 }
case DisplayType_Pretty: {
     ModifyPrintingPolicyAccordingToEnv(Policy);
     text = clang_getCursorPrettyPrinted(Cursor, Policy);
     clang_PrintingPolicy_dispose(Policy);
     return text;
   }
   }
   assert(0 && "unknown display type"); /* no llvm_unreachable in C. */
   /* Set to NULL to prevent uninitialized variable warnings. */
-  text.data = NULL;
-  text.private_flags = 0;
-  return text;
+  return createCXString(NULL);
 }
 
 static void PrintCursor(CXCursor Cursor, const char *CommentSchemaFile) {
   CXTranslationUnit TU = clang_Cursor_getTranslationUnit(Cursor);
   if (clang_isInvalid(Cursor.kind)) {
     CXString ks = clang_getCursorKindSpelling(Cursor.kind);
     printf("Invalid Cursor => %s", clang_getCString(ks));
     clang_disposeString(ks);
if (!clang_equalCursors(Referenced, clang_getNullCursor())) {
       if (clang_getCursorKind(Referenced) == CXCursor_OverloadedDeclRef) {
         unsigned I, N = clang_getNumOverloadedDecls(Referenced);
         printf("[");
         for (I = 0; I != N; ++I) {
           CXCursor Ovl = clang_getOverloadedDecl(Referenced, I);
           CXSourceLocation Loc;
           if (I)
             printf(", ");
-          
           Loc = clang_getCursorLocation(Ovl);
           clang_getSpellingLocation(Loc, 0, &line, &column, 0);
-          printf("%d:%d", line, column);          
+          printf("%d:%d", line, column);
         }
         printf("]");
       } else {
         CXSourceLocation Loc = clang_getCursorLocation(Referenced);
         clang_getSpellingLocation(Loc, 0, &line, &column, 0);
         printf(":%d:%d", line, column);
       }
 
       if (clang_getCursorKind(Referenced) == CXCursor_TypedefDecl) {
         CXType T = clang_getCursorType(Referenced);
         if (clang_Type_isTransparentTagTypedef(T)) {
           CXType Underlying = clang_getTypedefDeclUnderlyingType(Referenced);
           CXString S = clang_getTypeSpelling(Underlying);
           printf(" (Transparent: %s)", clang_getCString(S));
           clang_disposeString(S);
         }
       }
     }
 
     if (clang_isCursorDefinition(Cursor))
       printf(" (Definition)");
-    
     switch (clang_getCursorAvailability(Cursor)) {
       case CXAvailability_Available:
         break;
-        
       case CXAvailability_Deprecated:
         printf(" (deprecated)");
         break;
-        
       case CXAvailability_NotAvailable:
         printf(" (unavailable)");
         break;
 
       case CXAvailability_NotAccessible:
         printf(" (inaccessible)");
         break;
     }
-    
     NumPlatformAvailability
       = clang_getCursorPlatformAvailability(Cursor,
                                             &AlwaysDeprecated,
                                             &DeprecatedMessage,
                                             &AlwaysUnavailable,
                                             &UnavailableMessage,
                                             PlatformAvailability, 2);
     if (AlwaysUnavailable) {
       printf("  (always unavailable: \"%s\")",
              clang_getCString(UnavailableMessage));
     } else if (AlwaysDeprecated) {
       printf("  (always deprecated: \"%s\")",
              clang_getCString(DeprecatedMessage));
     } else {
       for (I = 0; I != NumPlatformAvailability; ++I) {
         if (I >= 2)
           break;
-        
         printf("  (%s", clang_getCString(PlatformAvailability[I].Platform));
         if (PlatformAvailability[I].Unavailable)
           printf(", unavailable");
         else {
           printVersion(", introduced=", PlatformAvailability[I].Introduced);
           printVersion(", deprecated=", PlatformAvailability[I].Deprecated);
           printVersion(", obsoleted=", PlatformAvailability[I].Obsoleted);
         }
         if (clang_getCString(PlatformAvailability[I].Message)[0])
           printf(", message=\"%s\"",
                  clang_getCString(PlatformAvailability[I].Message));
         printf(")");
       }
     }
     for (I = 0; I != NumPlatformAvailability; ++I) {
       if (I >= 2)
         break;
       clang_disposeCXPlatformAvailability(PlatformAvailability + I);
     }
-    
     clang_disposeString(DeprecatedMessage);
     clang_disposeString(UnavailableMessage);
 
     if (clang_CXXConstructor_isDefaultConstructor(Cursor))
       printf(" (default constructor)");
 
     if (clang_CXXConstructor_isMoveConstructor(Cursor))
       printf(" (move constructor)");
else {
 
     if (Cursor.kind == CXCursor_IBOutletCollectionAttr) {
       CXType T =
         clang_getCanonicalType(clang_getIBOutletCollectionType(Cursor));
       CXString S = clang_getTypeKindSpelling(T.kind);
       printf(" [IBOutletCollection=%s]", clang_getCString(S));
       clang_disposeString(S);
     }
-    
     if (Cursor.kind == CXCursor_CXXBaseSpecifier) {
       enum CX_CXXAccessSpecifier access = clang_getCXXAccessSpecifier(Cursor);
       unsigned isVirtual = clang_isVirtualBase(Cursor);
       const char *accessStr = 0;
 
       switch (access) {
         case CX_CXXInvalidAccessSpecifier:
           accessStr = "invalid"; break;
         case CX_CXXPublic:
           accessStr = "public"; break;
         case CX_CXXProtected:
           accessStr = "protected"; break;
         case CX_CXXPrivate:
           accessStr = "private"; break;
-      }      
-      
+      }
       printf(" [access=%s isVirtual=%s]", accessStr,
              isVirtual ? "true" : "false");
     }
 
     SpecializationOf = clang_getSpecializedCursorTemplate(Cursor);
     if (!clang_equalCursors(SpecializationOf, clang_getNullCursor())) {
       CXSourceLocation Loc = clang_getCursorLocation(SpecializationOf);
       CXString Name = clang_getCursorSpelling(SpecializationOf);
if (!clang_equalCursors(SpecializationOf, clang_getNullCursor())) {
             default:
               printf(" [Template arg %d: kind: %d]\n", I, TAK);
           }
         }
       }
     }
 
     clang_getOverriddenCursors(Cursor, &overridden, &num_overridden);
-    if (num_overridden) {      
+    if (num_overridden) {
       unsigned I;
       LineCol lineCols[50];
       assert(num_overridden <= 50);
       printf(" [Overrides ");
       for (I = 0; I != num_overridden; ++I) {
         CXSourceLocation Loc = clang_getCursorLocation(overridden[I]);
         clang_getSpellingLocation(Loc, 0, &line, &column, 0);
         lineCols[I].line = line;
         lineCols[I].col = column;
       }
       /* Make the order of the override list deterministic. */
       qsort(lineCols, num_overridden, sizeof(LineCol), lineCol_cmp);
       for (I = 0; I != num_overridden; ++I) {
         if (I)
           printf(", ");
         printf("@%d:%d", lineCols[I].line, lineCols[I].col);
       }
       printf("]");
       clang_disposeOverriddenCursors(overridden);
     }
-    
     if (Cursor.kind == CXCursor_InclusionDirective) {
       CXFile File = clang_getIncludedFile(Cursor);
       CXString Included = clang_getFileName(File);
       printf(" (%s)", clang_getCString(Included));
       clang_disposeString(Included);
-      
       if (clang_isFileMultipleIncludeGuarded(TU, File))
         printf("  [multi-include guarded]");
     }
-    
     CursorExtent = clang_getCursorExtent(Cursor);
-    RefNameRange = clang_getCursorReferenceNameRange(Cursor, 
+    RefNameRange = clang_getCursorReferenceNameRange(Cursor,
                                                    CXNameRange_WantQualifier
                                                  | CXNameRange_WantSinglePiece
                                                  | CXNameRange_WantTemplateArgs,
                                                      0);
     if (!clang_equalRanges(CursorExtent, RefNameRange))
       PrintRange(RefNameRange, "SingleRefName");
-    
     for (RefNameRangeNr = 0; 1; RefNameRangeNr++) {
-      RefNameRange = clang_getCursorReferenceNameRange(Cursor, 
+      RefNameRange = clang_getCursorReferenceNameRange(Cursor,
                                                    CXNameRange_WantQualifier
                                                  | CXNameRange_WantTemplateArgs,
                                                        RefNameRangeNr);
       if (clang_equalRanges(clang_getNullRange(), RefNameRange))
         break;
       if (!clang_equalRanges(CursorExtent, RefNameRange))
         PrintRange(RefNameRange, "RefName");
     }
if (Cursor.kind == CXCursor_ObjCPropertyDecl) {
         PRINT_OBJC_QUAL(Byref);
         PRINT_OBJC_QUAL(Oneway);
         printf("]");
       }
     }
   }
 }
 
-static const char* GetCursorSource(CXCursor Cursor) {
+static CXString GetCursorSource(CXCursor Cursor) {
   CXSourceLocation Loc = clang_getCursorLocation(Cursor);
   CXString source;
   CXFile file;
+  const char *sourceCStr;
+  CXString result;
+
   clang_getExpansionLocation(Loc, &file, 0, 0, 0);
   source = clang_getFileName(file);
-  if (!clang_getCString(source)) {
-    clang_disposeString(source);
-    return "<invalid loc>";
-  }
-  else {
-    const char *b = basename(clang_getCString(source));
-    clang_disposeString(source);
-    return b;
-  }
+  sourceCStr = clang_getCString(source);
+  if (!sourceCStr) {
+    result = createCXString("<invalid loc>");
+  } else {
+    result = copyToCXString(basename(sourceCStr));
   }
 
-static CXString createCXString(const char *CS) {
-  CXString Str;
-  Str.data = (const void *) CS;
-  Str.private_flags = 0;
-  return Str;
+  clang_disposeString(source);
+  return result;
 }
 
 /******************************************************************************/
 /* Callbacks.                                                                 */
 /******************************************************************************/
 
 typedef void (*PostVisitTU)(CXTranslationUnit);
 

 void PrintDiagnosticSet(CXDiagnosticSet Set) {
   int i = 0, n = clang_getNumDiagnosticsInSet(Set);
   for ( ; i != n ; ++i) {
     CXDiagnostic Diag = clang_getDiagnosticInSet(Set, i);
     CXDiagnosticSet ChildDiags = clang_getChildDiagnostics(Diag);
     PrintDiagnostic(Diag);
     if (ChildDiags)
       PrintDiagnosticSet(ChildDiags);
-  }  
+  }
 }
 
 void PrintDiagnostics(CXTranslationUnit TU) {
   CXDiagnosticSet TUSet = clang_getDiagnosticSetFromTU(TU);
   PrintDiagnosticSet(TUSet);
   clang_disposeDiagnosticSet(TUSet);
 }
 
 void PrintMemoryUsage(CXTranslationUnit TU) {
   unsigned long total = 0;
   unsigned i = 0;
   CXTUResourceUsage usage = clang_getCXTUResourceUsage(TU);
   fprintf(stderr, "Memory usage:\n");
   for (i = 0 ; i != usage.numEntries; ++i) {
     const char *name = clang_getTUResourceUsageName(usage.entries[i].kind);
     unsigned long amount = usage.entries[i].amount;
     total += amount;
     fprintf(stderr, "  %s : %ld bytes (%f MBytes)\n", name, amount,
             ((double) amount)/(1024*1024));
   }
   fprintf(stderr, "  TOTAL = %ld bytes (%f MBytes)\n", total,
           ((double) total)/(1024*1024));
-  clang_disposeCXTUResourceUsage(usage);  
+  clang_disposeCXTUResourceUsage(usage);
 }
 
 /******************************************************************************/
 /* Logic for testing traversal.                                               */
 /******************************************************************************/
 
 static void PrintCursorExtent(CXCursor C) {
   CXSourceRange extent = clang_getCursorExtent(C);
   PrintRange(extent, "Extent");
 }
 
 /* Data used by the visitors. */
 typedef struct {
   CXTranslationUnit TU;
   enum CXCursorKind *Filter;
   const char *CommentSchemaFile;
 } VisitorData;
 
 
 enum CXChildVisitResult FilteredPrintingVisitor(CXCursor Cursor,
                                                 CXCursor Parent,
                                                 CXClientData ClientData) {
+  CXString Source;
+  const char *SourceCStr;
   VisitorData *Data = (VisitorData *)ClientData;
   if (!Data->Filter || (Cursor.kind == *(enum CXCursorKind *)Data->Filter)) {
     CXSourceLocation Loc = clang_getCursorLocation(Cursor);
     unsigned line, column;
     clang_getSpellingLocation(Loc, 0, &line, &column, 0);
-    printf("// %s: %s:%d:%d: ", FileCheckPrefix,
-           GetCursorSource(Cursor), line, column);
+    Source = GetCursorSource(Cursor);
+    SourceCStr = clang_getCString(Source);
+    printf("// %s: %s:%d:%d: ", FileCheckPrefix, SourceCStr, line, column);
+    clang_disposeString(Source);
     PrintCursor(Cursor, Data->CommentSchemaFile);
     PrintCursorExtent(Cursor);
     if (clang_isDeclaration(Cursor.kind)) {
       enum CX_CXXAccessSpecifier access = clang_getCXXAccessSpecifier(Cursor);
       const char *accessStr = 0;
 
       switch (access) {
         case CX_CXXInvalidAccessSpecifier: break;
static enum CXChildVisitResult FunctionScanVisitor(CXCursor Cursor,
   /* Probe the entire body, looking for both decls and refs. */
   curLine = startLine;
   curColumn = startColumn;
 
   while (startBuf < endBuf) {
     CXSourceLocation Loc;
     CXFile file;
     CXString source;
+    CXString curSource;
+    const char *curSourceCStr;
 
     if (*startBuf == '\n') {
       startBuf++;
       curLine++;
       curColumn = 1;
     } else if (*startBuf != '\t')
       curColumn++;
 
     Loc = clang_getCursorLocation(Cursor);
     clang_getSpellingLocation(Loc, &file, 0, 0, 0);
 
     source = clang_getFileName(file);
     if (clang_getCString(source)) {
       CXSourceLocation RefLoc
         = clang_getLocation(Data->TU, file, curLine, curColumn);
       Ref = clang_getCursor(Data->TU, RefLoc);
       if (Ref.kind == CXCursor_NoDeclFound) {
         /* Nothing found here; that's fine. */
       } else if (Ref.kind != CXCursor_FunctionDecl) {
-        printf("// %s: %s:%d:%d: ", FileCheckPrefix, GetCursorSource(Ref),
+        curSource = GetCursorSource(Ref);
+        curSourceCStr = clang_getCString(curSource);
+        printf("// %s: %s:%d:%d: ", FileCheckPrefix, curSourceCStr,
                curLine, curColumn);
+        clang_disposeString(curSource);
         PrintCursor(Ref, Data->CommentSchemaFile);
         printf("\n");
       }
     }
     clang_disposeString(source);
     startBuf++;
   }
 
   return CXChildVisit_Continue;
 }
 
 /******************************************************************************/
 /* USR testing.                                                               */
 /******************************************************************************/
 
 enum CXChildVisitResult USRVisitor(CXCursor C, CXCursor parent,
                                    CXClientData ClientData) {
+  CXString Source;
+  const char *SourceCStr;
   VisitorData *Data = (VisitorData *)ClientData;
+
   if (!Data->Filter || (C.kind == *(enum CXCursorKind *)Data->Filter)) {
     CXString USR = clang_getCursorUSR(C);
     const char *cstr = clang_getCString(USR);
     if (!cstr || cstr[0] == '\0') {
       clang_disposeString(USR);
       return CXChildVisit_Recurse;
     }
-    printf("// %s: %s %s", FileCheckPrefix, GetCursorSource(C), cstr);
+    Source = GetCursorSource(C);
+    SourceCStr = clang_getCString(Source);
+    printf("// %s: %s %s", FileCheckPrefix, SourceCStr, cstr);
+    clang_disposeString(Source);
 
     PrintCursorExtent(C);
     printf("\n");
     clang_disposeString(USR);
 
     return CXChildVisit_Recurse;
   }
 
static int perform_test_load(CXIndex Idx, CXTranslationUnit TU,
 
   if (Visitor) {
     enum CXCursorKind K = CXCursor_NotImplemented;
     enum CXCursorKind *ck = &K;
     VisitorData Data;
 
     /* Perform some simple filtering. */
     if (!strcmp(filter, "all") || !strcmp(filter, "local")) ck = NULL;
-    else if (!strcmp(filter, "all-display") || 
+    else if (!strcmp(filter, "all-display") ||
              !strcmp(filter, "local-display")) {
       ck = NULL;
       wanted_display_type = DisplayType_DisplayName;
     }
     else if (!strcmp(filter, "all-pretty") ||
              !strcmp(filter, "local-pretty")) {
       ck = NULL;
       wanted_display_type = DisplayType_Pretty;
int perform_test_reparse_source(int argc, const char **argv, int trials,
   struct CXUnsavedFile *unsaved_files = 0;
   int num_unsaved_files = 0;
   int compiler_arg_idx = 0;
   enum CXErrorCode Err;
   int result, i;
   int trial;
   int remap_after_trial = 0;
   char *endptr = 0;
-  
   Idx = clang_createIndex(/* excludeDeclsFromPCH */
                           !strcmp(filter, "local") ? 1 : 0,
                           /* displayDiagnostics=*/1);
-  
   if (parse_remapped_files(argc, argv, 0, &unsaved_files, &num_unsaved_files)) {
     clang_disposeIndex(Idx);
     return -1;
   }
 
   for (i = 0; i < argc; ++i) {
     if (strcmp(argv[i], "--") == 0)
       break;
   }
   if (i < argc)
     compiler_arg_idx = i+1;
   if (num_unsaved_files > compiler_arg_idx)
     compiler_arg_idx = num_unsaved_files;
-  
   /* Load the initial translation unit -- we do this without honoring remapped
    * files, so that we have a way to test results after changing the source. */
   Err = clang_parseTranslationUnit2(Idx, 0,
                                     argv + compiler_arg_idx,
                                     argc - compiler_arg_idx,
                                     0, 0, getDefaultParsingOptions(), &TU);
   if (Err != CXError_Success) {
     fprintf(stderr, "Unable to load translation unit!\n");
     describeLibclangFailure(Err);
     free_remapped_files(unsaved_files, num_unsaved_files);
     clang_disposeIndex(Idx);
     return 1;
   }
-  
   if (checkForErrors(TU) != 0)
     return -1;
 
   if (getenv("CINDEXTEST_REMAP_AFTER_TRIAL")) {
     remap_after_trial =
         strtol(getenv("CINDEXTEST_REMAP_AFTER_TRIAL"), &endptr, 10);
   }
 
Err = clang_reparseTranslationUnit(
         trial >= remap_after_trial ? unsaved_files : 0,
         clang_defaultReparseOptions(TU));
     if (Err != CXError_Success) {
       fprintf(stderr, "Unable to reparse translation unit!\n");
       describeLibclangFailure(Err);
       clang_disposeTranslationUnit(TU);
       free_remapped_files(unsaved_files, num_unsaved_files);
       clang_disposeIndex(Idx);
-      return -1;      
+      return -1;
     }
 
     if (checkForErrors(TU) != 0)
       return -1;
   }
-  
   result = perform_test_load(Idx, TU, filter, NULL, Visitor, PV, NULL);
 
   free_remapped_files(unsaved_files, num_unsaved_files);
   clang_disposeIndex(Idx);
   return result;
 }
 
 static int perform_single_file_parse(const char *filename) {
for (I = 0; I != N; ++I) {
 
     if (Kind == CXCompletionChunk_Optional) {
       fprintf(file, "{Optional ");
       print_completion_string(
                 clang_getCompletionChunkCompletionString(completion_string, I),
                               file);
       fprintf(file, "}");
       continue;
-    } 
+    }
 
     if (Kind == CXCompletionChunk_VerticalSpace) {
       fprintf(file, "{VerticalSpace  }");
       continue;
     }
 
     text = clang_getCompletionChunkText(completion_string, I);
     cstr = clang_getCString(text);
int perform_code_completion(int argc, const char **argv, int timing_only) {
   }
 
   if (results) {
     unsigned i, n = results->NumResults, containerIsIncomplete = 0;
     unsigned long long contexts;
     enum CXCursorKind containerKind;
     CXString objCSelector;
     const char *selectorString;
-    if (!timing_only) {      
+    if (!timing_only) {
       /* Sort the code-completion results based on the typed text. */
       clang_sortCodeCompletionResults(results->Results, results->NumResults);
 
       for (i = 0; i != n; ++i)
         print_completion_result(TU, results, i, stdout);
     }
     n = clang_codeCompleteGetNumDiagnostics(results);
     for (i = 0; i != n; ++i) {
       CXDiagnostic diag = clang_codeCompleteGetDiagnostic(results, i);
       PrintDiagnostic(diag);
       clang_disposeDiagnostic(diag);
     }
-    
     contexts = clang_codeCompleteGetContexts(results);
     print_completion_contexts(contexts, stdout);
-    
     containerKind = clang_codeCompleteGetContainerKind(results,
                                                        &containerIsIncomplete);
-    
     if (containerKind != CXCursor_InvalidCode) {
       /* We have found a container */
       CXString containerUSR, containerKindSpelling;
       containerKindSpelling = clang_getCursorKindSpelling(containerKind);
       printf("Container Kind: %s\n", clang_getCString(containerKindSpelling));
       clang_disposeString(containerKindSpelling);
-      
       if (containerIsIncomplete) {
         printf("Container is incomplete\n");
       }
       else {
         printf("Container is complete\n");
       }
-      
       containerUSR = clang_codeCompleteGetContainerUSR(results);
       printf("Container USR: %s\n", clang_getCString(containerUSR));
       clang_disposeString(containerUSR);
     }
-    
     objCSelector = clang_codeCompleteGetObjCSelector(results);
     selectorString = clang_getCString(objCSelector);
     if (selectorString && strlen(selectorString) > 0) {
       printf("Objective-C selector: %s\n", selectorString);
     }
     clang_disposeString(objCSelector);
-    
     clang_disposeCodeCompleteResults(results);
   }
   clang_disposeTranslationUnit(TU);
   clang_disposeIndex(CIdx);
   free(filename);
 
   free_remapped_files(unsaved_files, num_unsaved_files);
 
static int inspect_cursor_at(int argc, const char **argv,
   int num_unsaved_files = 0;
   enum CXErrorCode Err;
   CXTranslationUnit TU;
   CXCursor Cursor;
   CursorSourceLocation *Locations = 0;
   unsigned NumLocations = 0, Loc;
   unsigned Repeats = 1;
   unsigned I;
-  
   /* Count the number of locations. */
   while (strstr(argv[NumLocations+1], locations_flag) == argv[NumLocations+1])
     ++NumLocations;
 
   /* Parse the locations. */
   assert(NumLocations > 0 && "Unable to count locations?");
   Locations = (CursorSourceLocation *)malloc(
                                   NumLocations * sizeof(CursorSourceLocation));
if (Repeats > 1) {
         describeLibclangFailure(Err);
         clang_disposeTranslationUnit(TU);
         return 1;
       }
     }
 
     if (checkForErrors(TU) != 0)
       return -1;
-    
     for (Loc = 0; Loc < NumLocations; ++Loc) {
       CXFile file = clang_getFile(TU, Locations[Loc].filename);
       if (!file)
         continue;
 
       Cursor = clang_getCursor(TU,
                                clang_getLocation(TU, file, Locations[Loc].line,
                                                  Locations[Loc].column));
 
       if (checkForErrors(TU) != 0)
         return -1;
 
       if (I + 1 == Repeats) {
         handler(Cursor);
         free(Locations[Loc].filename);
       }
     }
   }
-  
   PrintDiagnostics(TU);
   clang_disposeTranslationUnit(TU);
   clang_disposeIndex(CIdx);
   free(Locations);
   free_remapped_files(unsaved_files, num_unsaved_files);
   return 0;
 }
 
static int find_file_refs_at(int argc, const char **argv) {
   int num_unsaved_files = 0;
   enum CXErrorCode Err;
   CXTranslationUnit TU;
   CXCursor Cursor;
   CursorSourceLocation *Locations = 0;
   unsigned NumLocations = 0, Loc;
   unsigned Repeats = 1;
   unsigned I;
-  
   /* Count the number of locations. */
   while (strstr(argv[NumLocations+1], "-file-refs-at=") == argv[NumLocations+1])
     ++NumLocations;
 
   /* Parse the locations. */
   assert(NumLocations > 0 && "Unable to count locations?");
   Locations = (CursorSourceLocation *)malloc(
                                   NumLocations * sizeof(CursorSourceLocation));
if (Repeats > 1) {
         describeLibclangFailure(Err);
         clang_disposeTranslationUnit(TU);
         return 1;
       }
     }
 
     if (checkForErrors(TU) != 0)
       return -1;
-    
     for (Loc = 0; Loc < NumLocations; ++Loc) {
       CXFile file = clang_getFile(TU, Locations[Loc].filename);
       if (!file)
         continue;
 
       Cursor = clang_getCursor(TU,
                                clang_getLocation(TU, file, Locations[Loc].line,
                                                  Locations[Loc].column));
 
       if (checkForErrors(TU) != 0)
         return -1;
 
       if (I + 1 == Repeats) {
         CXCursorAndRangeVisitor visitor = { 0, findFileRefsVisit };
         PrintCursor(Cursor, NULL);
         printf("\n");
         clang_findReferencesInFile(Cursor, file, visitor);
         free(Locations[Loc].filename);
 
         if (checkForErrors(TU) != 0)
           return -1;
       }
     }
   }
-  
   PrintDiagnostics(TU);
   clang_disposeTranslationUnit(TU);
   clang_disposeIndex(CIdx);
   free(Locations);
   free_remapped_files(unsaved_files, num_unsaved_files);
   return 0;
 }
 

 static void printCXIndexLoc(CXIdxLoc loc, CXClientData client_data) {
   IndexData *index_data;
   CXString filename;
   const char *cname;
   CXIdxClientFile file;
   unsigned line, column;
   const char *main_filename;
   int isMainFile;
-  
   index_data = (IndexData *)client_data;
   clang_indexLoc_getFileLocation(loc, &file, 0, &line, &column, 0);
   if (line == 0) {
     printf("<invalid>");
     return;
   }
   if (!file) {
     printf("<no idxfile>");
static CXIdxClientContainer makeClientContainer(CXClientData *client_data,
                                                 const CXIdxEntityInfo *info,
                                                 CXIdxLoc loc) {
   IndexData *index_data;
   IndexDataStringList *node;
   const char *name;
   char *newStr;
   CXIdxClientFile file;
   unsigned line, column;
-  
   name = info->name;
   if (!name)
     name = "<anon-tag>";
 
   clang_indexLoc_getFileLocation(loc, &file, 0, &line, &column, 0);
 
   node =
       (IndexDataStringList *)malloc(sizeof(IndexDataStringList) + strlen(name) +
static void index_diagnostic(CXClientData client_data,
   printCheck(index_data);
 
   numDiags = clang_getNumDiagnosticsInSet(diagSet);
   for (i = 0; i != numDiags; ++i) {
     diag = clang_getDiagnosticInSet(diagSet, i);
     str = clang_formatDiagnostic(diag, clang_defaultDiagnosticDisplayOptions());
     cstr = clang_getCString(str);
     printf("[diagnostic]: %s\n", cstr);
-    clang_disposeString(str);  
-  
+    clang_disposeString(str);
     if (getenv("CINDEXTEST_FAILONERROR") &&
         clang_getDiagnosticSeverity(diag) >= CXDiagnostic_Error) {
       index_data->fail_for_error = 1;
     }
   }
 }
 
 static CXIdxClientFile index_enteredMainFile(CXClientData client_data,
                                        CXFile file, void *reserved) {
   IndexData *index_data;
 
   index_data = (IndexData *)client_data;
   printCheck(index_data);
 
   index_data->main_filename = clang_getFileName(file);
 
   printf("[enteredMainFile]: ");
   printCXIndexFile((CXIdxClientFile)file);
   printf("\n");
 
   return (CXIdxClientFile)file;
 }
 
 static CXIdxClientFile index_ppIncludedFile(CXClientData client_data,
                                             const CXIdxIncludedFileInfo *info) {
   IndexData *index_data;
   CXModule Mod;
   index_data = (IndexData *)client_data;
   printCheck(index_data);
 
   printf("[ppIncludedFile]: ");
   printCXIndexFile((CXIdxClientFile)info->file);
   printf(" | name: \"%s\"", info->filename);
   printf(" | hash loc: ");
   printCXIndexLoc(info->hashLoc, client_data);
   printf(" | isImport: %d | isAngled: %d | isModule: %d",
          info->isImport, info->isAngled, info->isModuleImport);
-  
   Mod = clang_getModuleForFile(index_data->TU, (CXFile)info->file);
   if (Mod) {
     CXString str = clang_Module_getFullName(Mod);
     const char *cstr = clang_getCString(str);
     printf(" | module: %s", cstr);
     clang_disposeString(str);
   }
 
   printf("\n");
 
   return (CXIdxClientFile)info->file;
 }
 
 static CXIdxClientFile index_importedASTFile(CXClientData client_data,
                                          const CXIdxImportedASTFileInfo *info) {
   IndexData *index_data;
   index_data = (IndexData *)client_data;
   printCheck(index_data);
 
   if (index_data->importedASTs) {
     CXString filename = clang_getFileName(info->file);
     importedASTS_insert(index_data->importedASTs, clang_getCString(filename));
     clang_disposeString(filename);
   }
-  
   printf("[importedASTFile]: ");
   printCXIndexFile((CXIdxClientFile)info->file);
   if (info->module) {
     CXString name = clang_Module_getFullName(info->module);
     printf(" | loc: ");
     printCXIndexLoc(info->loc, client_data);
     printf(" | name: \"%s\"", clang_getCString(name));
     printf(" | isImplicit: %d\n", info->isImplicit);

 /******************************************************************************/
 int write_pch_file(const char *filename, int argc, const char *argv[]) {
   CXIndex Idx;
   CXTranslationUnit TU;
   struct CXUnsavedFile *unsaved_files = 0;
   int num_unsaved_files = 0;
   enum CXErrorCode Err;
   int result = 0;
-  
   Idx = clang_createIndex(/* excludeDeclsFromPCH */1, /* displayDiagnostics=*/1);
-  
   if (parse_remapped_files(argc, argv, 0, &unsaved_files, &num_unsaved_files)) {
     clang_disposeIndex(Idx);
     return -1;
   }
 
   Err = clang_parseTranslationUnit2(
       Idx, 0, argv + num_unsaved_files, argc - num_unsaved_files,
       unsaved_files, num_unsaved_files,
       CXTranslationUnit_Incomplete |
           CXTranslationUnit_DetailedPreprocessingRecord |
           CXTranslationUnit_ForSerialization,
       &TU);
   if (Err != CXError_Success) {
     fprintf(stderr, "Unable to load translation unit!\n");
     describeLibclangFailure(Err);
     free_remapped_files(unsaved_files, num_unsaved_files);
     clang_disposeTranslationUnit(TU);
     clang_disposeIndex(Idx);
     return 1;
   }
 
-  switch (clang_saveTranslationUnit(TU, filename, 
+  switch (clang_saveTranslationUnit(TU, filename,
                                     clang_defaultSaveOptions(TU))) {
   case CXSaveError_None:
     break;
 
   case CXSaveError_TranslationErrors:
-    fprintf(stderr, "Unable to write PCH file %s: translation errors\n", 
+    fprintf(stderr, "Unable to write PCH file %s: translation errors\n",
             filename);
-    result = 2;    
+    result = 2;
     break;
 
   case CXSaveError_InvalidTU:
-    fprintf(stderr, "Unable to write PCH file %s: invalid translation unit\n", 
+    fprintf(stderr, "Unable to write PCH file %s: invalid translation unit\n",
             filename);
-    result = 3;    
+    result = 3;
     break;
 
   case CXSaveError_Unknown:
   default:
     fprintf(stderr, "Unable to write PCH file %s: unknown error \n", filename);
     result = 1;
     break;
   }
-  
   clang_disposeTranslationUnit(TU);
   free_remapped_files(unsaved_files, num_unsaved_files);
   clang_disposeIndex(Idx);
   return result;
 }
 
 /******************************************************************************/
 /* Serialized diagnostics.                                                    */

 
 static void printFixIts(CXDiagnostic D, unsigned indent) {
   unsigned i, n = clang_getDiagnosticNumFixIts(D);
   fprintf(stderr, "Number FIXITs = %d\n", n);
   for (i = 0 ; i < n; ++i) {
     CXSourceRange ReplacementRange;
     CXString text;
     text = clang_getDiagnosticFixIt(D, i, &ReplacementRange);
-    
     printIndent(indent);
     fprintf(stderr, "FIXIT: (");
     printLocation(clang_getRangeStart(ReplacementRange));
     fprintf(stderr, " - ");
     printLocation(clang_getRangeEnd(ReplacementRange));
     fprintf(stderr, "): \"%s\"\n", clang_getCString(text));
     clang_disposeString(text);
-  }  
+  }
 }
 
 static void printDiagnosticSet(CXDiagnosticSet Diags, unsigned indent) {
   unsigned i, n;
 
   if (!Diags)
     return;
-  
   n = clang_getNumDiagnosticsInSet(Diags);
   for (i = 0; i < n; ++i) {
     CXSourceLocation DiagLoc;
     CXDiagnostic D;
     CXFile File;
     CXString FileName, DiagSpelling, DiagOption, DiagCat;
     unsigned line, column, offset;
     const char *DiagOptionStr = 0, *DiagCatStr = 0;
-    
     D = clang_getDiagnosticInSet(Diags, i);
     DiagLoc = clang_getDiagnosticLocation(D);
     clang_getExpansionLocation(DiagLoc, &File, &line, &column, &offset);
     FileName = clang_getFileName(File);
     DiagSpelling = clang_getDiagnosticSpelling(D);
-    
     printIndent(indent);
-    
     fprintf(stderr, "%s:%d:%d: %s: %s",
             clang_getCString(FileName),
             line,
             column,
             getSeverityString(clang_getDiagnosticSeverity(D)),
             clang_getCString(DiagSpelling));
 
     DiagOption = clang_getDiagnosticOption(D, 0);
     DiagOptionStr = clang_getCString(DiagOption);
     if (DiagOptionStr) {
       fprintf(stderr, " [%s]", DiagOptionStr);
     }
-    
     DiagCat = clang_getDiagnosticCategoryText(D);
     DiagCatStr = clang_getCString(DiagCat);
     if (DiagCatStr) {
       fprintf(stderr, " [%s]", DiagCatStr);
     }
-    
     fprintf(stderr, "\n");
-    
     printRanges(D, indent);
     printFixIts(D, indent);
-    
     /* Print subdiagnostics. */
     printDiagnosticSet(clang_getChildDiagnostics(D), indent+2);
 
     clang_disposeString(FileName);
     clang_disposeString(DiagSpelling);
     clang_disposeString(DiagOption);
     clang_disposeString(DiagCat);
-  }  
+  }
 }
 
 static int read_diagnostics(const char *filename) {
   enum CXLoadDiag_Error error;
   CXString errorString;
   CXDiagnosticSet Diags = 0;
-  
   Diags = clang_loadDiagnostics(filename, &error, &errorString);
   if (!Diags) {
     fprintf(stderr, "Trouble deserializing file (%s): %s\n",
             getDiagnosticCodeStr(error),
             clang_getCString(errorString));
     clang_disposeString(errorString);
     return 1;
   }
-  
   printDiagnosticSet(Diags, 0);
   fprintf(stderr, "Number of diagnostics: %d\n",
           clang_getNumDiagnosticsInSet(Diags));
   clang_disposeDiagnosticSet(Diags);
   return 0;
 }
 
 static int perform_print_build_session_timestamp(void) {
else if (argc >= 4 && strncmp(argv[1], "-test-load-tu", 13) == 0) {
     if (I)
       return perform_test_load_tu(argv[2], argv[3], argc >= 5 ? argv[4] : 0, I,
                                   NULL);
   }
   else if (argc >= 5 && strncmp(argv[1], "-test-load-source-reparse", 25) == 0){
     CXCursorVisitor I = GetVisitor(argv[1] + 25);
     if (I) {
       int trials = atoi(argv[2]);
-      return perform_test_reparse_source(argc - 4, argv + 4, trials, argv[3], I, 
+      return perform_test_reparse_source(argc - 4, argv + 4, trials, argv[3], I,
                                          NULL);
     }
   }
   else if (argc >= 4 && strncmp(argv[1], "-test-load-source", 17) == 0) {
     CXCursorVisitor I = GetVisitor(argv[1] + 17);
-    
     PostVisitTU postVisit = 0;
     if (strstr(argv[1], "-memory-usage"))
       postVisit = PrintMemoryUsage;
-    
     if (I)
       return perform_test_load_source(argc - 3, argv + 3, argv[2], I,
                                       postVisit);
   }
   else if (argc >= 3 && strcmp(argv[1], "-single-file-parse") == 0)
     return perform_single_file_parse(argv[2]);
   else if (argc >= 4 && strcmp(argv[1], "-test-file-scan") == 0)
     return perform_file_scan(argv[2], argv[3],

tools/libclang/CXString.cpp

 // This file defines routines for manipulating CXStrings. It should be the
 // only file that has internal knowledge of the encoding of the data in
 // CXStrings.
 //
 //===----------------------------------------------------------------------===//
 
 #include "CXString.h"
 #include "CXTranslationUnit.h"
+#include "clang-c/CXString.h"
 #include "clang-c/Index.h"
 #include "clang/Frontend/ASTUnit.h"
 #include "llvm/Support/ErrorHandling.h"
+#include <cstring>
 
 using namespace clang;
 
-/// Describes the kind of underlying data in CXString.
-enum CXStringFlag {
-  /// CXString contains a 'const char *' that it doesn't own.
-  CXS_Unmanaged,
-
-  /// CXString contains a 'const char *' that it allocated with malloc().
-  CXS_Malloc,
-
-  /// CXString contains a CXStringBuf that needs to be returned to the
-  /// CXStringPool.
-  CXS_StringBuf
-};
+static_assert(sizeof(CXString) <= 16, "");
 
 namespace clang {
 namespace cxstring {
 
+/**
+ * This is for \b CXString 's which are created with \b CreateRef(StringRef).
+ * We'll store the info from the input \b StringRef: char ptr and size.
+ *
+ * We don't know for sure whether this is null-terminated so, when and if
+ * \b clang_getCString is called for this \b CXString, we'll allocate C string
+ * storage and copy data into the storage.  We'll memo-ize that in the
+ * \b CString member.
+ *
+ * This is refcounted; the \b Count member is initially 1.  When a \b CXString
+ * instance using this object is disposed via \b clang_disposeString, \b Count
+ * is decremented.  When this string is duplicated the \b Count increases.
+ *
+ * When \b Count finally drops to zero, the ptr at \b CString, and this object,
+ * should be deleted.
+ */
+struct RefCountedCharRange {
+  const char *Data;
+  const char *CString;
+  unsigned Size;
+  unsigned Count;
+};
+
 //===----------------------------------------------------------------------===//
 // Basic generation of CXStrings.
 //===----------------------------------------------------------------------===//
 
-CXString createEmpty() {
+CXString createRef(const char *String) {
   CXString Str;
-  Str.data = "";
-  Str.private_flags = CXS_Unmanaged;
+  Str.Contents = (const void *) String;
+  if (String) {
+    Str.Size = strlen(String);
+    Str.IsNullTerminated = true;
+  } else {
+    Str.Size = 0;
+    Str.IsNullTerminated = false;
+  }
+  Str.IsOwned = false;
+  Str.IsPooled = false;
   return Str;
 }
 
-CXString createNull() {
-  CXString Str;
-  Str.data = nullptr;
-  Str.private_flags = CXS_Unmanaged;
-  return Str;
+CXString createEmpty() {
+  return createRef("");
 }
 
-CXString createRef(const char *String) {
-  if (String && String[0] == '\0')
-    return createEmpty();
+CXString createNull() {
+  return createRef(nullptr);
+}
 
-  CXString Str;
-  Str.data = String;
-  Str.private_flags = CXS_Unmanaged;
-  return Str;
+inline static const char *copyCharRange(const char *CS, unsigned Size) {
+  char *Spelling = (char *) malloc(Size + 1);
+  assert(Spelling);
+  if (CS) {
+    memcpy(Spelling, CS, Size);
+  }
+  Spelling[Size] = 0;
+  return Spelling;
 }
 
  CXString createDup(const char *String) {
-  if (!String)
+  if (!String) {
     return createNull();
-
-  if (String[0] == '\0')
+  }
+  if (String[0] == '\0') {
     return createEmpty();
+  }
 
   CXString Str;
-  Str.data = strdup(String);
-  Str.private_flags = CXS_Malloc;
+  Str.Size = strlen(String);
+  Str.Contents = (const void *) copyCharRange(String, Str.Size);
+  Str.IsNullTerminated = true;
+  Str.IsOwned = true;
+  Str.IsPooled = false;
   return Str;
 }
 
 CXString createRef(StringRef String) {
-  // If the string is not nul-terminated, we have to make a copy.
-
-  // FIXME: This is doing a one past end read, and should be removed! For memory
-  // we don't manage, the API string can become unterminated at any time outside
-  // our control.
-
-  if (!String.empty() && String.data()[String.size()] != 0)
-    return createDup(String);
-
-  CXString Result;
-  Result.data = String.data();
-  Result.private_flags = (unsigned) CXS_Unmanaged;
-  return Result;
+  assert (String.size() <= std::numeric_limits<unsigned>::max());
+  CXString Str;
+  Str.Size = unsigned(String.size());
+  Str.IsNullTerminated = false;
+  Str.IsOwned = false;
+  Str.IsPooled = false;
+  auto *RC = new RefCountedCharRange {
+    /* Data */ String.data(),
+    /* CString */ nullptr,
+    /* Size */ Str.Size,
+    /* Count */ 1,
+  };
+  Str.Contents = (const void *) RC;
+  return Str;
 }
 
 CXString createDup(StringRef String) {
-  CXString Result;
-  char *Spelling = static_cast<char *>(llvm::safe_malloc(String.size() + 1));
-  memmove(Spelling, String.data(), String.size());
-  Spelling[String.size()] = 0;
-  Result.data = Spelling;
-  Result.private_flags = (unsigned) CXS_Malloc;
-  return Result;
+  CXString Str;
+  Str.Size = String.size();
+  Str.Contents = (const void *) copyCharRange(String.data(), Str.Size);
+  Str.IsNullTerminated = true;
+  Str.IsOwned = true;
+  Str.IsPooled = false;
+  return Str;
 }
 
 CXString createCXString(CXStringBuf *buf) {
   CXString Str;
-  Str.data = buf;
-  Str.private_flags = (unsigned) CXS_StringBuf;
+  Str.Contents = buf->Data.data();
+  Str.Size = buf->Data.size();
+  Str.IsNullTerminated = true;
+  Str.IsOwned = false;
+  Str.IsPooled = true;
   return Str;
 }
 
 CXStringSet *createSet(const std::vector<std::string> &Strings) {
   CXStringSet *Set = new CXStringSet;
   Set->Count = Strings.size();
   Set->Strings = new CXString[Set->Count];
   for (unsigned SI = 0, SE = Set->Count; SI < SE; ++SI)
CXStringBuf *getCXStringBuf(CXTranslationUnit TU) {
   return TU->StringPool->getCXStringBuf(TU);
 }
 
 void CXStringBuf::dispose() {
   TU->StringPool->Pool.push_back(this);
 }
 
 bool isManagedByPool(CXString str) {
-  return ((CXStringFlag) str.private_flags) == CXS_StringBuf;
+  return str.IsPooled;
 }
 
 } // end namespace cxstring
 } // end namespace clang
 
 //===----------------------------------------------------------------------===//
 // libClang public APIs.
 //===----------------------------------------------------------------------===//
 
+using namespace clang::cxstring;
+
 const char *clang_getCString(CXString string) {
-  if (string.private_flags == (unsigned) CXS_StringBuf) {
-    return static_cast<const cxstring::CXStringBuf *>(string.data)->Data.data();
+  const char *CString = nullptr;
+
+  // If not null-terminated, then this is a reference to a range of characters
+  // owned elsewhere.  We'll need to copy this to a new C string to ensure it
+  // is null-terminated, if we haven't done so already.
+  if (string.IsNullTerminated) {
+    CString = (const char *) string.Contents;
+  } else {
+    auto *V = const_cast<void *>(string.Contents);
+    if (V) {
+      auto *R = (RefCountedCharRange *) V;
+      if (R) {
+        if (!R->CString) {
+          R->CString = copyCharRange(R->Data, R->Size);
+        }
+        CString = R->CString;
+      }
+    }
   }
-  return static_cast<const char *>(string.data);
+
+  return CString;
+}
+
+const char *clang_getStringData(CXString string) {
+  if (!(string.IsOwned || string.IsNullTerminated)) {
+    auto *R = (const RefCountedCharRange *) string.Contents;
+    return R ? R->Data : nullptr;
+  }
+  return (const char *) string.Contents;
+}
+
+unsigned clang_getStringSize(CXString string) {
+  return string.Size;
 }
 
 void clang_disposeString(CXString string) {
-  switch ((CXStringFlag) string.private_flags) {
-    case CXS_Unmanaged:
-      break;
-    case CXS_Malloc:
-      if (string.data)
-        free(const_cast<void *>(string.data));
-      break;
-    case CXS_StringBuf:
-      static_cast<cxstring::CXStringBuf *>(
-          const_cast<void *>(string.data))->dispose();
-      break;
+  if (string.IsOwned) {
+    // Is a C string that we own; need to free it.
+    const void *Chars = (const void *) string.Contents;
+    free(const_cast<void*>(Chars));
+  } else {
+    if (!string.IsNullTerminated) {
+      // Neither null-terminated nor owned.  Is a char range for which we might
+      // have created a C string.
+      auto *CR = (const RefCountedCharRange *) string.Contents;
+      auto *R = const_cast<RefCountedCharRange *>(CR);
+      if (R) {
+        -- R->Count;
+        if (!R->Count) {
+          const void *CStr = (const void *) R->CString;
+          free(const_cast<void*>(CStr));
+          delete R;
+        }
+      }
+    }
   }
 }
 
 void clang_disposeStringSet(CXStringSet *set) {
   for (unsigned SI = 0, SE = set->Count; SI < SE; ++SI)
     clang_disposeString(set->Strings[SI]);
   delete[] set->Strings;
   delete set;
 }
-