[cfe-dev] UB in TypeLoc casting

[John McCall]

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.

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.

John.