[PATCH] D76624: [MSan] Add instrumentation for SystemZ

[Ilya Leoshkevich via Phabricator via llvm-commits]

iii marked 8 inline comments as done.
iii added inline comments.


================
Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:4662
+    // of what it can be. In particular, single element structs and large
+    // types have already been taken care of.
+    if (T->isFloatingPointTy())
----------------
uweigand wrote:
> Is argument type coercion also taken care of?  This is about the implicit promotion to 64 bits.
> 
> For example, a 32-bit integer is passed as zero- or sign-extended 64-bit value, so all 8 bytes of the slot in either the GPR save area or overflow area are filled by the caller and may be validly accessed by the callee.
> 
> In clang's SystemZABIInfo::EmitVAArg this is done by this call:
>     if (AI.getCoerceToType())
>       ArgTy = AI.getCoerceToType();
> 
No, we get the original (non-coerced) type here. The offset calculation is still correct in presence of type coercion, because the slot size is always 8, but the code currently lacks instrumentation that would fill the shadow for the extensions bits. I'll add it.


================
Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:4664
+    if (T->isFloatingPointTy())
+      return AK_FloatingPoint;
+    if (T->isIntegerTy() && T->getPrimitiveSizeInBits() <= 64)
----------------
uweigand wrote:
> We recently added -msoft-float support in particular to support kernel builds.
> 
> I guess this also needs to be supported here ... (in soft-float mode, floating-point arguments are passed in GPRs).
I couldn't find a way to query floating point ABI, so I'll use the indirect way to detect this and assume that userspace is always hardfloat, and kernel is always softfloat.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76624