[cfe-dev] tracking CXString disposal via CXString.private_flags

[Ben Coman via cfe-dev]

greetings all,

This is a request to help clang_disposeString(CXString string) [1]
avoid double-freeing of memory.
(TL;DR, refer last bullet point)

I am interfacing to libclang via FFI from Pharo Smalltalk.  This is a
garbage collected environment with a "workspace/playground" interface
(like a REPL) where an operator can create Smalltalk objects within
the environment - for example a CSSString returned from
clang_getClangVersion().  When an object is garbage collected a
finalization mechanism can be invoked for external resources - for
example calling clang_disposeString().  For testing and interactive
development this finalization mechanism can also be invoked directly
by the user.

The problem is over a long interactive period, a user can lose track
of which CXStrings have been clang_disposeString()'d. Accidently
invoking it on an object twice crashes the environment due to
double-free()'ing the memory. The same happens if the user manually
invokes it, and later the garbage collector finalization invokes it
again. The following demonstrates an analogy of operating within the
Pharo environment...


#include <clang-c/Index.h>
#include <stdio.h>
int main( int argc, const char *const * argv )
{
        CXString version = clang_getClangVersion();
        printf("%s\n", clang_getCString(version));
        printf("returned=\t%x\t%x\n", (unsigned int) version.data,
version.private_flags);

        clang_disposeString(version);
        printf("disposed=\t%x\t%x\n", (unsigned int) version.data,
version.private_flags);

//Some time later, after hours or days of other interactive tasks
        clang_disposeString(version);
}


$ clang-3.5 -I/usr/lib/llvm-3.5/include -lclang -o version version.c
$ ./version
new= 80499e0 80487a2
Debian clang version 3.5.0-10 (tags/RELEASE_350/final) (based on LLVM 3.5.0)
returned= 921ea80 1
disposed= 921ea80 1
*** Error in `./version': corrupted double-linked list: 0x0921ead0 ***
Aborted


I would like the Pharo code to guard against calling
clang_disposeString() twice.  The possibilities I see for tracking the
disposal-state are:

* Adding an instance variable to the CXString definition within Pharo,
but this crashes the system, presumably due to writing outside the
memory allocated by libclang

* Wrapping CXStrings returned by the libclang api in MyCXString having
the trackign variables, but this adds friction to the interface

* From Pharo after calling clang_disposeString() write to
private_flags, but this is not good for a library-user to write to a
variable tagged private

* Request that clang_disposeString() in libclang [1] alter the state
of private_flags to prevent double-free()ing. Something like this...

    case CXS_Malloc:
      if (string.data)
        free(const_cast<void *>(string.data));
      break;

==>

    case CXS_Malloc:
      if (string.data) {
        string.private_flags = CXS_Disposed;
        free(const_cast<void *>(string.data));
      }
      break;
    case CSX_Disposed:
      break;

[1] https://github.com/llvm-mirror/clang/blob/google/stable/tools/libclang/CXString.cpp

As well as preventing string.data being double-free()'d, I'll also be
able to peek at the state from within Pharo for debugging and unit
testing, or raising a Pharo exception if its finalization runs twice.
For me this is the simplest thing that would work, thus my hopeful
request.   What is the best path to promoting this proposal?
I've read http://clang.llvm.org/hacking.html , but firstly interested
in how such a change will be perceived.

cheers -ben


P.S. As a secondary issue, I was a little surprised that calling
clang_getCString() after clang_disposeString() worked without
complaint.  Of course memory is not reallocated in my little example
above, but seems a possible failure mode in amore complex program.  I
wonder if it might also be useful to have soemthing like...

    const char *clang_getCString(CXString string) {
      if string.private_flags = (unsigned) CSX_Disposed
        return NULL;

or...
    case CXS_Malloc:
      if (string.data) {
        string.private_flags = CXS_Disposed;
        free(const_cast<void *>(string.data));
        string.data = NULL;
      }