r195819 - [libclang] Make sure we don't access past the tokens buffer while token annotation.

[Alp Toker]

On 27/11/2013 09:02, Argyrios Kyrtzidis wrote:
>>> @@ -6498,6 +6515,11 @@ bool RunSafely(llvm::CrashRecoveryContex
>>>                  unsigned Size) {
>>>     if (!Size)
>>>       Size = GetSafetyThreadStackSize();
>>> +  if (getenv("LIBCLANG_DISABLE_CRASH_RECOVERY")) {
>>> +    // Don't use crash recovery.
>>> +    llvm::llvm_execute_on_thread(Fn, UserData, Size);
>>> +    return true;
>>> +  }
>>
>> Hi Argyrios,
>>
>> I don't think this change does what you want. It'll force threaded 
>> execution even when not requested.
>>
>> How about just:
>>
>> |--- a/tools/libclang/CIndex.cpp||
>> ||+++ b/tools/libclang/CIndex.cpp||
>> ||@@ -2544,7 +2544,8 @@ CXIndex clang_createIndex(int 
>> excludeDeclarationsFromPCH,||
>> ||                           int displayDiagnostics) {||
>> ||   // We use crash recovery to make some of our APIs more reliable, 
>> implicitly||
>> ||   // enable it.||
>> ||-  llvm::CrashRecoveryContext::Enable();||
>> ||+  if (!getenv("LIBCLANG_DISABLE_CRASH_RECOVERY"))||
>> ||+    llvm::CrashRecoveryContext::Enable();||
>> ||||
>> ||   // Enable support for multithreading in LLVM.||
>> ||   {||
>> |
>> Given that RunSafely() is documented to "execute the given code 
>> safely" I think this would also be more self-documenting.
>>
>
> That's much better; in r195829, thanks!

Great!

I'd like to see CIndex provide an API changing this to opt-in during the 
3.5 iteration the same way we did recently for PrettyStackTrace.

Adding signal global handlers is generally undesirable when embedding 
into larger applications and the users I know of are just patching to 
turn this off anyway.

Alp.


-- 
http://www.nuanti.com
the browser experts