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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to