[LLVMdev] [cfe-dev] UB in TypeLoc casting

[David Blaikie]

Moving to LLVM dev to discuss the possibility of extending the cast
infrastructure to handle this.

On Tue, Nov 20, 2012 at 5:51 PM, John McCall <rjmccall at apple.com> wrote:
> On Nov 18, 2012, at 5:05 PM, David Blaikie <dblaikie at gmail.com> wrote:
>> TypeLoc casting looks bogus.
>>
>> TypeLoc derived types return true from classof when the dynamic type
>> is not actually an instance of the derived type. The TypeLoc is then
>> (erroneously) cast to the derived type and the derived type's implicit
>> copy ctor is executed (so long as you remember to assign the result of
>> cast<SpecificTypeLoc>), if you copy, otherwise the SpecificTypeLoc's
>> member functions are actually invoked on a TypeLoc object - bogas, but
>> it works (because there's no other members in the SpecificTypeLoc
>> types).
>
> Yep.  See also LLVM's IntrinsicInst.  This kind of UB is very common and
> essentially harmless, but if you want to stamp it out, feel free.
>
>> Richard / Kostya: what's the word on catching this kind of UB
>> (essentially: calling member functions of one type on an instance not
>> of that type)? (in the specific case mentioned below, as discussed in
>> the original thread, ASan or MSan might be able to help)
>>
>> Clang Dev: what should we do to fix this? I don't think it's helpful
>> to add machinery to llvm::cast to handle concrete type conversion, so
>> I think we should consider a non-llvm::cast solution for this
>> hierarchy. Replace the llvm::isa calls with getTypeLocClass calls (or
>> a separate wrapper for that) & add a SpecificTypeLoc(const TypeLoc&)
>> ctor for every specific TypeLoc that has an appropriate assert & calls
>> up through to the base copy ctor. It'll be a fair bit of code to add
>> to TypeLoc, but nothing complicated.
>>
>> Any other ideas?
>
> I don't see why isa<> and cast<> aren't the right code interface for this,
> rather than reinventing something else.  You just need to teach cast<>
> to construct and return a proper temporary.

LLVM-dev folks: what do you reckon? Should we extend the cast
infrastructure to be able to handle value type conversions?

It doesn't really feel like the right fit to me, but I'll defer to the
community's wisdom if it's overwhelming. I'd just like to see a
solution wherein we can't write this particular kind of bug again,
ideally.

> You should construct that temporary from a QualType and void*.
> Please do not add a constructor which basically breaks the static
> type safety of the entire hierarchy.

[fair point - bad suggestion on my part, but the key point was: some
solution that doesn't use the cast infrastructure because these are
value type conversions not pointer/reference conversions for objects
that are already of the intended destination type]