[PATCH] D127812: [AArch64] FMV support and necessary target features dependencies.

Thu Dec 22 14:43:28 PST 2022

hctim added a comment.

In D127812#4012854 <https://reviews.llvm.org/D127812#4012854>, @ilinpv wrote:

> I don't think `std::equal` is underlying cause here. We have featuresStrs_size() comparison before calling it:
>
>   if (CurClones && NewClones &&
>             (CurClones->featuresStrs_size() != NewClones->featuresStrs_size() ||
>              !std::equal(CurClones->featuresStrs_begin(),
>                          CurClones->featuresStrs_end(),
>                          NewClones->featuresStrs_begin()))) {
>
> Also even if we completely remove std::equal the use-of-uninitialized-value error still persist.

Sorry for the red herring. MSan is detecting use-after-destructor here.

`Strings` holds `StringRef`-references to the memory allocated in `StringsBuffer`.

When StringsBuffer is template-constructed with two inline elements (i.e. `SmallVector<SmallString<64>, 2> StringsBuffer;`), the third element that gets added (in `clang::Sema::checkTargetClonesAttrString`, `SemaDeclAttr.cpp:3568`) causes the `SmallVector` to move the three strings to the heap (the two already existing inline and the new addition).

`SmallVector` cleans up the inline memory by calling the destructors on it. MSan dutifully marks the destroyed memory as uninitialized, to detect use-after-destruction. The reason why ASan doesn't generate a report here is that the memory is still technically "reachable", it's possible to have destroyed memory that's still part of a live stack frame or hasn't had its heap allocation cleaned up yet.

Because `Strings` captured the reference of the two string objects when they were inline-allocated, as soon as this move-to-heap happens, these two references are dangling. Then, when any caller attempts to iterate over `Strings`, it finally explodes as MSan correctly detects the use of destroyed memory.

Allocating 3 objects inline solves the issue for now, as there's no uses that end up needing more than 3 elements, but this leaves a footgun for anyone who would add another element later.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127812/new/

https://reviews.llvm.org/D127812