samdeane
commented
8 years ago It’s a long-shot, and I’m working on the assumption that it’s probably asan getting confused, but if it rings any bells @ccgus (or @logancollins), let me know!
Open samdeane opened 8 years ago
samdeane
commented
8 years ago It’s a long-shot, and I’m working on the assumption that it’s probably asan getting confused, but if it rings any bells @ccgus (or @logancollins), let me know!
samdeane
commented
8 years ago The line it’s actually hitting is: https://github.com/BohemianCoding/CocoaScript/blob/develop/src/framework/mocha/Utilities/MOFunctionArgument.m#L870
ccgus
commented
8 years ago No bells are ringing, sorry.
logancollins
commented
8 years ago Yeah, not ringing any bells for me either. I haven’t run any of my projects using it with the address sanitizer.
Logan
On Sep 6, 2016, at 10:35 AM, ccgus notifications@github.com wrote:
No bells are ringing, sorry.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ccgus/CocoaScript/issues/43#issuecomment-245027775, or mute the thread https://github.com/notifications/unsubscribe-auth/AArmBmpg0dNtRgIVR_DOVN6lC-60eBuPks5qnaRPgaJpZM4J2AXB.
samdeane
commented
8 years ago I've got a small test script which can reproduce this in the coscript tool:
framework("Foundation");
var string = NSMutableString.alloc().initWithString_("❦❦❦❦❦❦")
var range = NSMakeRange(0, 13)
var updatedRange = MOPointer.new()
var ok = string.applyTransform_reverse_range_updatedRange_(NSStringTransformToXMLHex, true, range, updatedRange)
print("result was " + ok)
print(updatedRange.location)Perhaps that's a misunderstanding abut how MOPointer is supposed to be used? If not, I think there's a genuine bug here.
What seems to happen is that storage is allocated for the updatedRange argument, and it's the size of a pointer.
When Mocha is unpacking the results it gets back from the function call, it tries to unpack the NSRange structure from the storage that was allocated for the pointer, as if it was actually the NSRange structure.
ASAN reports a buffer overflow when it tries to read from the second double of the NSRange, since the storage is actually only for a pointer.
samdeane
commented
8 years ago If I instead change updatedRange to have a value passed in:
var updatedRange = MOPointer.alloc().initWithValue_(range)then ASAN instead falls over when the value is being set, before the function invocation.
In this case it's a similar problem in reverse - the code is trying to copy the value into _storage as if it is an NSRange, but in fact it's only the size of a pointer.
samdeane
commented
8 years ago Part of the problem I think seems to be the dereference arguments in setValueAsJSValue and getValueAsJSValueInContext.
When these are true, the type encoding is treated as if the argument is actually the structure, and not a pointer to the structure. But no modification is made to the pointer, which still points at _storage, which is still the size of a pointer.
I'm not sure whether the error is actually that this code is wrong, or that _storage should have somehow have been set up to be the correct size by then.
samdeane
commented
8 years ago Any hints gratefully received @logancollins 😁
samdeane
commented
7 years ago I've been tracing through a similar crash recently, and I think I've satisfied myself that there is a bug in Mocha around this.
When setting up the arguments for a function call, if an argument is supposed to be a pointer to a structure (eg the second argument to CGPathCreateCopyByTransformingPath), and if I pass in an MOPointer initialised with a structure value, what seems to happen is that storage is allocated for the pointer to the structure, but then the value of the structure is written into that storage... likely overwriting the end of the block. As one would expect, sometimes it survives this, sometimes it doesn't.
Hacking the malloc in MOFunctionArgument allocStorage to allocate 1024 bytes more than it needs is enough to fix my crasher. Clearly not a proper fix, and perhaps it's just a coincidence, but it suggests that I'm right about the overwrite.
If I am correct, I'm not quite sure what the fix is. Either the type encoding being used in allocateStorage is wrong in this case, or (I think more likely), it is right, but we need to allocate another block somewhere/somehow to contain the struct value, and then write the pointer to that block into the original storage.
SheffieldKevin
commented
6 years ago This can be closed. The example code that Sam has above we now realise can be fixed with the following.
framework("Foundation");
var string = NSMutableString.alloc().initWithString_("❦❦❦❦❦❦")
var range = NSMakeRange(0, 13)
var updatedRange = NSMakeRange(0, 0)
var updatedRangePtr = MOPointer.new(updatedRange)
var ok = string.applyTransform_reverse_range_updatedRange_(NSStringTransformToXMLHex, true, range, updatedRangePtr)
print("result was " + ok)
print(updatedRange.location)Interesting. So in the example, I was incorrectly using the MOPointer object?
My reading of it was that it was supposed to be able to allocate its own storage for the thing it was pointing to (in the case where you were using it to receive a value). The usage pattern you're using suggests that I got that wrong?
SheffieldKevin
commented
6 years ago Hi Sam 👋 It doesn't seem to. @mathieudutour tried adding some type information to the pointer so that CocoaScript new the type of object that the pointer needed to be pointing to to but the address sanitizer still fired. The above fixed the problem. Perhaps there is a problem in CocoaScript but in this instance the problem is now fixed.
I’ve been debugging a Sketch script, and hit this guy in the debugger:
It’s probably a false positive, but it is in an interesting area.
I’m fairly sure that the function being called is a
glyphRangeForCharacterRange:actualCharacterRangewhich takes a pointer to an NSRange. The script in question is passing a new MOPointer for this.The failure seems to be happening whilst trying to read the second field (
length) of the range structure, presumably after the call has completed.As far as I can see, the code is allocating storage of the correct size, and is calculating the correct offsets, but asan seems to disagree (and I’m not 100% clear how/when the MOPointerValue storage gets allocated in this situation).