On Thu, Nov 29, 2012 at 7:43 PM, Eli Friedman <span dir="ltr"><<a href="mailto:eli.friedman@gmail.com" target="_blank">eli.friedman@gmail.com</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5">On Thu, Nov 29, 2012 at 3:49 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
> Moving to LLVM dev to discuss the possibility of extending the cast<br>
> infrastructure to handle this.<br>
><br>
> On Tue, Nov 20, 2012 at 5:51 PM, John McCall <<a href="mailto:rjmccall@apple.com">rjmccall@apple.com</a>> wrote:<br>
>> On Nov 18, 2012, at 5:05 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
>>> TypeLoc casting looks bogus.<br>
>>><br>
>>> TypeLoc derived types return true from classof when the dynamic type<br>
>>> is not actually an instance of the derived type. The TypeLoc is then<br>
>>> (erroneously) cast to the derived type and the derived type's implicit<br>
>>> copy ctor is executed (so long as you remember to assign the result of<br>
>>> cast<SpecificTypeLoc>), if you copy, otherwise the SpecificTypeLoc's<br>
>>> member functions are actually invoked on a TypeLoc object - bogas, but<br>
>>> it works (because there's no other members in the SpecificTypeLoc<br>
>>> types).<br>
>><br>
>> Yep.  See also LLVM's IntrinsicInst.  This kind of UB is very common and<br>
>> essentially harmless, but if you want to stamp it out, feel free.<br>
>><br>
>>> Richard / Kostya: what's the word on catching this kind of UB<br>
>>> (essentially: calling member functions of one type on an instance not<br>
>>> of that type)? (in the specific case mentioned below, as discussed in<br>
>>> the original thread, ASan or MSan might be able to help)<br>
>>><br>
>>> Clang Dev: what should we do to fix this? I don't think it's helpful<br>
>>> to add machinery to llvm::cast to handle concrete type conversion, so<br>
>>> I think we should consider a non-llvm::cast solution for this<br>
>>> hierarchy. Replace the llvm::isa calls with getTypeLocClass calls (or<br>
>>> a separate wrapper for that) & add a SpecificTypeLoc(const TypeLoc&)<br>
>>> ctor for every specific TypeLoc that has an appropriate assert & calls<br>
>>> up through to the base copy ctor. It'll be a fair bit of code to add<br>
>>> to TypeLoc, but nothing complicated.<br>
>>><br>
>>> Any other ideas?<br>
>><br>
>> I don't see why isa<> and cast<> aren't the right code interface for this,<br>
>> rather than reinventing something else.  You just need to teach cast<><br>
>> to construct and return a proper temporary.<br>
><br>
> LLVM-dev folks: what do you reckon? Should we extend the cast<br>
> infrastructure to be able to handle value type conversions?<br>
<br>
</div></div>No, I don't think that makes sense; the casting infrastructure is<br>
complicated enough without having to deal with this.  It would be easy<br>
enough to implement a method on TypeLoc to handle this,<br>
<div class="im"><br>
> It doesn't really feel like the right fit to me, but I'll defer to the<br>
> community's wisdom if it's overwhelming. I'd just like to see a<br>
> solution wherein we can't write this particular kind of bug again,<br>
> ideally.<br>
<br>
</div>Attaching proof-of-concept solution which prevents this kind of bug.<br>
(Ugly, and requires C++11 support as written, but potentially<br>
interesting anyway.)</blockquote><div><br></div><div>The C++11 should be easy to remove (move the enable_if to the return type). This approach seems reasonable to me.</div></div>