Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MSAN reports false positives on interleaved storage on ARM AArch64 #72848

Closed
vrabaud opened this issue Nov 20, 2023 · 7 comments
Closed

MSAN reports false positives on interleaved storage on ARM AArch64 #72848

vrabaud opened this issue Nov 20, 2023 · 7 comments
Assignees
Labels
backend:AArch64 compiler-rt:msan Memory sanitizer false-positive Warning fires when it should not

Comments

@vrabaud
Copy link

vrabaud commented Nov 20, 2023

On clang 18, the following test simply reports: "WARNING: MemorySanitizer: use-of-uninitialized-value".
It seems to be the case for different kinds of input. Even displaying the first element of the outputs fails.

#include <iostream>
#include <arm_neon.h>
TEST(Msan,interlace) {
  int16x8x2_t vec2;
  vec2.val[0] = vdupq_n_s16(1);
  vec2.val[1] = vdupq_n_s16(2);
  int16_t dst2[8*2];
  vst2q_s16(dst2, vec2);
  for(int16_t i: dst2) std::cout << (int)i << ",";
  std::cout << std::endl;

  uint8x16x3_t vec3;
  vec3.val[0] = vdupq_n_u8(3);
  vec3.val[1] = vdupq_n_u8(4);
  vec3.val[2] = vdupq_n_u8(5);
  uint8_t dst3[16*3];  
  vst3q_u8(dst3, vec3);
  for(uint8_t i: dst3) std::cout << (int)i << ",";
  std::cout << std::endl;
}
@EugeneZelenko EugeneZelenko added backend:AArch64 compiler-rt:msan Memory sanitizer false-positive Warning fires when it should not and removed new issue labels Nov 20, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 20, 2023

@llvm/issue-subscribers-backend-aarch64

Author: Vincent Rabaud (vrabaud)

On clang 18, the following test simply reports: "WARNING: MemorySanitizer: use-of-uninitialized-value". It seems to be the case for different kinds of input. Even displaying the first element of the outputs fails. ```c++ #include <iostream> #include <arm_neon.h> TEST(Msan,interlace) { int16x8x2_t vec2; vec2.val[0] = vdupq_n_s16(1); vec2.val[1] = vdupq_n_s16(2); int16_t dst2[8*2]; vst2q_s16(dst2, vec2); for(int16_t i: dst2) std::cout << (int)i << ","; std::cout << std::endl;

uint8x16x3_t vec3;
vec3.val[0] = vdupq_n_u8(3);
vec3.val[1] = vdupq_n_u8(4);
vec3.val[2] = vdupq_n_u8(5);
uint8_t dst3[16*3];
vst3q_u8(dst3, vec3);
for(uint8_t i: dst3) std::cout << (int)i << ",";
std::cout << std::endl;
}

</details>

@brianosman
Copy link

We're seeing similar failures with vst3_lane_u8: https://godbolt.org/z/nWq8EYzTf

Running that on an ARM device with MSAN triggers use-of-uninitialized-value

@ramosian-glider
Copy link
Contributor

First off, this clearly has nothing to do with the vararg support patch, as there are no varargs.

I suspect that MSan is lacking the Neon intrinsics support.
The following code:

  int16_t dst2[8*2];
  vst2q_s16(dst2, vec2);
  std::cout << (int)dst2[0] << ",";

gets compiled into the following IR:

...
  %dst3 = alloca [48 x i8], align 1
  call void @llvm.lifetime.start.p0(i64 32, ptr nonnull %dst2) #9
  %0 = ptrtoint ptr %dst2 to i64
  %1 = xor i64 %0, 193514046488576
  %2 = inttoptr i64 %1 to ptr
  call void @llvm.memset.p0.i64(ptr noundef nonnull align 4 dereferenceable(32) %2, i8 -1, i64 32, i1 false)
  call void @llvm.aarch64.neon.st2.v8i16.p0(<8 x i16> <i16 1, i16 1, i16 1, i16 1, i16 1, i16 1, i16 1, i16 1>, <8 x i16> <i16 2, i16 2, i16 2, i16 2, i16 2, i16 2, i16 2, i16 2>, ptr nonnull %dst2)
  %_msld = load i16, ptr %2, align 4
  %_mscmp175.not = icmp eq i16 %_msld, 0
  br i1 %_mscmp175.not, label %4, label %3

3:
  call void @__msan_warning_noreturn() #10
  unreachable

@ramosian-glider
Copy link
Contributor

CC @eugenis

@ramosian-glider
Copy link
Contributor

Godbolt link for posterity: https://godbolt.org/z/MMz3qvj69

@thurstond
Copy link
Contributor

This particular test case works with mainline LLVM as of last week: "[msan] Implement support for Arm NEON vst{2,3,4} instructions" (#99360)

Currently, it only works for VST with integer operands (since it was a particularly common use case). There is followup work planned to implement support for VST with floating-point operands, non-interleaved vector store (VST1x{2,3,4}), and vector loads.

@thurstond
Copy link
Contributor

thurstond commented Oct 30, 2024

For reference, here are the relevant patches that fixed the reported case (and more), and accompanying tests:

Link Title Date
#98247 Precommit MSan Arm NEON vst tests July 17, 2024
#99360 Implement support for Arm NEON vst{2,3,4} instructions July 19, 2024
#99555 Precommit MSan Arm NEON vst tests with origin-tracking July 18, 2024
#100189 Add more NEON VST tests July 23, 2024
#100210 Add baseline output for neon_vst_float.ll July 23, 2024
#100435 Enable and update neon_vst_float test case July 24, 2024
#100644 Support vst1x_{2,3,4} and vst_{2,3,4} with floating-point parameters July 29, 2024
#100645 Precommit tests for Arm NEON VST with lanes July 29, 2024
#101215 Support vst{2,3,4}_lane instructions August 9, 2024
#101420 Precommit tests for Arm NEON vector shift August 7, 2024
#102507 Support most Arm NEON vector shift instructions August 8, 2024
#114462 Add test for Arm NEON tbl intrinsics October 31, 2024
#114490 Add handleIntrinsicByApplyingToShadow; support NEON tbl/tbx November 1, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:AArch64 compiler-rt:msan Memory sanitizer false-positive Warning fires when it should not
Projects
None yet
Development

No branches or pull requests

6 participants