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

Pavel Iliin via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Dec 22 04:33:15 PST 2022


ilinpv added a comment.

In D127812#4012283 <https://reviews.llvm.org/D127812#4012283>, @hctim wrote:

> I'm not sure "MSan didn't handle correctly SmallVector" is the case. Given your diagnosis of 3-elements-vs-2, I'm guessing the root cause is that `clang/lib/Sema/SemaDecl.cpp:11369` is wrong:
>
>   !std::equal(CurClones->featuresStrs_begin(),
>               CurClones->featuresStrs_end(),
>               NewClones->featuresStrs_begin()))) {
>
> This construction of `std::equal` is very error-prone, as if `NewClones.size() < CurClones.size()`, then this invariable leads to buffer-overflow. I'm wondering if that's the underlying cause, it would seem entirely possible that expanding the in-place elements are always "initialized" from MSan's perspective and so the current code has a false-negative, and your new code made it so that the vector is now heap-based, which is revealing the underlying issue. Maybe worth trying one more thing and adding an `assert(CurClones->size() <= NewClones->size());` to double check?

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127812



More information about the cfe-commits mailing list