[sanitizer][msan] VarArgHelper for loongarch64

[Ami-zhang]

Summary

This patch adds support for variadic argument for loongarch64,
which is based on MIPS64. And check-msan all pass.

[Ami-zhang]

Ping

[vitalybuka]

llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp

5840

It looks like exact clone of Mips64 helper.
Only if (TargetTriple.getArch() == Triple::mips64) { is missing

Why not just use Mips64 then?

[Ami-zhang]

llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp

5840

Yes, the VarArg convention in LoongArch's ABI is similar to that of Mips, so VarArgLoongArch64Helper is here based on Mips.

If use Mips64 helper, like this:

+  else if (TargetTriple.isMIPS64() || TargetTriple.isLoongArch64())
     return new VarArgMIPS64Helper(Func, Msan, Visitor);

VarArgMIPS64Helper is a MIPS64-specific implementation. Adding loongarch64 here, does it seem somewhat ambiguous, suggesting LoongArch as a variant of MIPS architecture? I don't know if it's good to use it directly like mentioned above. So is it okay like above or have any better suggestions?

Thanks for your time.

[vitalybuka]

llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp

5840

How about something line this:

// Loongarch64 is not a MIPS, but the current vargs calling convention matches the MIPS.
using VarArgLoongArch64Helper = VarArgMIPS64Helper;

Another alternative is to rename VarArgMIPS64Helper into something more general, maybe there is a name for this calling convention already?

llvm/test/Instrumentation/MemorySanitizer/LoongArch/vararg-loongarch64.ll

4

even if we use MIPS helper, please keep this new test, in case someone modify Mips part ignoring loongarch64

[xen0n]

I'm not looking at this in more detail right now (it's 3 am here), but isn't the LoongArch psABI mostly resembling RISCV, and not Mips?

[Ami-zhang]

In D158587#4639972, @xen0n wrote:

I'm not looking at this in more detail right now (it's 3 am here), but isn't the LoongArch psABI mostly resembling RISCV, and not Mips?

I have read varargs part in the RISCV psABI at https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-cc.adoc#va_list-va_start-and-va_arg and in Mips "elf64-2.4.pdf". That of LoongArch psABI is indeed very similar to that of RISCV, and also similar to MIPS.

RISCV is not yet supported in the MSan vararg instrumentation, so i referred to the implementation based on Mips here. After testing and verifying, the Mips-based vararg implementation in LoongArch works fine.

The ABI similarities between architectures could be coincidental, therefore we can't ignore the potential risks of sharing code. I should pay attention and update it accordingly if there are any changes in the future.

llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp

5840

Thanks for your suggestions. I'm inclined towards the first alternative.

As for renaming VarArgMIPS64Helper, we did consider renaming it to something more generic, but we haven't found a better name yet, and it's worth taking some time to research and find a suitable name. So for now, I will go with the first option.

[Ami-zhang]

Use alias approach to implement VarArgLoongArch64Helper

[fmayer]

llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp

5840

Please add a comment to the VarArgMIPS64Helper definition that explains that this is also used for Loongarch.

[vitalybuka]

LGTM with @fmayer request

[Ami-zhang]

Add a comment

compiler-rt/test/msan/signal_stress_test.cpp

 // RUN: %clangxx_msan -fno-sanitize-memory-param-retval -std=c++11 -O0 %s -o %t && %run %t
 //
 // Test that va_arg shadow from a signal handler does not leak outside.
 
 // Reported deadly signal due to stack-overflow
 // XFAIL: target={{.*netbsd.*}}
 
-// VarArg implementation on LoongArch isn't supported yet.
-// UNSUPPORTED: target=loongarch{{.*}}
-
 #include <signal.h>
 #include <stdarg.h>
 #include <sanitizer/msan_interface.h>
 #include <assert.h>
 #include <sys/time.h>
 #include <stdio.h>
 
 const int kSigCnt = 200;

llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp

for (size_t i = 0, n = VAStartInstrumentationList.size(); i < n; i++) {
         IRB.CreateMemCpy(OverflowArgAreaOriginPtr, Alignment, SrcPtr, Alignment,
                          VAArgOverflowSize);
       }
     }
   }
 };
 
 /// MIPS64-specific implementation of VarArgHelper.
+/// NOTE: This is also used for LoongArch64.
 struct VarArgMIPS64Helper : public VarArgHelper {
   Function &F;
   MemorySanitizer &MS;
   MemorySanitizerVisitor &MSV;
   AllocaInst *VAArgTLSCopy = nullptr;
   Value *VAArgSize = nullptr;
 
   SmallVector<CallInst *, 16> VAStartInstrumentationList;
for (size_t VaStartNo = 0, VaStartNum = VAStartInstrumentationList.size();
       NextNodeIRBuilder IRB(OrigInst);
       Value *VAListTag = OrigInst->getArgOperand(0);
       copyRegSaveArea(IRB, VAListTag);
       copyOverflowArea(IRB, VAListTag);
     }
   }
 };
 
+// Loongarch64 is not a MIPS, but the current vargs calling convention matches
+// the MIPS.
+using VarArgLoongArch64Helper = VarArgMIPS64Helper;
+
 /// A no-op implementation of VarArgHelper.
 struct VarArgNoOpHelper : public VarArgHelper {
   VarArgNoOpHelper(Function &F, MemorySanitizer &MS,
                    MemorySanitizerVisitor &MSV) {}
 
   void visitCallBase(CallBase &CB, IRBuilder<> &IRB) override {}
 
   void visitVAStartInst(VAStartInst &I) override {}
else if (TargetTriple.isMIPS64())
     return new VarArgMIPS64Helper(Func, Msan, Visitor);
   else if (TargetTriple.getArch() == Triple::aarch64)
     return new VarArgAArch64Helper(Func, Msan, Visitor);
   else if (TargetTriple.getArch() == Triple::ppc64 ||
            TargetTriple.getArch() == Triple::ppc64le)
     return new VarArgPowerPC64Helper(Func, Msan, Visitor);
   else if (TargetTriple.getArch() == Triple::systemz)
     return new VarArgSystemZHelper(Func, Msan, Visitor);
+  else if (TargetTriple.isLoongArch64())
+    return new VarArgLoongArch64Helper(Func, Msan, Visitor);
   else
     return new VarArgNoOpHelper(Func, Msan, Visitor);
 }
 
 bool MemorySanitizer::sanitizeFunction(Function &F, TargetLibraryInfo &TLI) {
   if (!CompileKernel && F.getName() == kMsanModuleCtorName)
     return false;
 

llvm/test/Instrumentation/MemorySanitizer/LoongArch/vararg-loongarch64.ll

+; RUN: opt < %s -S -passes=msan 2>&1 | FileCheck %s
+
+target datalayout = "e-m:e-p:64:64-i64:64-i128:128-n64-S128"
+target triple = "loongarch64-unknown-linux-gnu"
+
+;; First, check allocation of the save area.
+declare void @llvm.lifetime.start.p0(i64, ptr nocapture) #1
+declare void @llvm.va_start(ptr) #2
+declare void @llvm.va_end(ptr) #2
+declare void @llvm.lifetime.end.p0(i64, ptr nocapture) #1
+define i32 @foo(i32 %guard, ...) {
+; CHECK-LABEL: @foo
+; CHECK:    [[TMP1:%.*]] = load {{.*}} @__msan_va_arg_overflow_size_tls
+; CHECK:    [[TMP2:%.*]] = add i64 0, [[TMP1]]
+; CHECK:    [[TMP3:%.*]] = alloca {{.*}} [[TMP2]]
+; CHECK:    call void @llvm.memset.p0.i64(ptr align 8 [[TMP3]], i8 0, i64 [[TMP2]], i1 false)
+; CHECK:    [[TMP4:%.*]] = call i64 @llvm.umin.i64(i64 [[TMP2]], i64 800)
+; CHECK:    call void @llvm.memcpy.p0.p0.i64(ptr align 8 [[TMP3]], ptr align 8 @__msan_va_arg_tls, i64 [[TMP4]], i1 false)
+;
+  %vl = alloca ptr, align 8
+  call void @llvm.lifetime.start.p0(i64 32, ptr %vl)
+  call void @llvm.va_start(ptr %vl)
+  call void @llvm.va_end(ptr %vl)
+  call void @llvm.lifetime.end.p0(i64 32, ptr %vl)
+  ret i32 0
+}
+
+;; Save the incoming shadow value from the arguments in the __msan_va_arg_tls
+;; array.
+define i32 @bar() {
+; CHECK-LABEL: @bar
+; CHECK:    store i32 0, ptr @__msan_va_arg_tls, align 8
+; CHECK:    store i64 0, ptr inttoptr (i64 add (i64 ptrtoint (ptr @__msan_va_arg_tls to i64), i64 8) to ptr), align 8
+; CHECK:    store i64 0, ptr inttoptr (i64 add (i64 ptrtoint (ptr @__msan_va_arg_tls to i64), i64 16) to ptr), align 8
+; CHECK:    store {{.*}} 24, {{.*}} @__msan_va_arg_overflow_size_tls
+;
+  %1 = call i32 (i32, ...) @foo(i32 0, i32 1, i64 2, double 3.000000e+00)
+  ret i32 %1
+}
+
+;; Check multiple fixed arguments.
+declare i32 @foo2(i32 %g1, i32 %g2, ...)
+define i32 @bar2() {
+; CHECK-LABEL: @bar2
+; CHECK:    store i64 0, ptr @__msan_va_arg_tls, align 8
+; CHECK:    store i64 0, ptr inttoptr (i64 add (i64 ptrtoint (ptr @__msan_va_arg_tls to i64), i64 8) to ptr), align 8
+; CHECK:    store {{.*}} 16, {{.*}} @__msan_va_arg_overflow_size_tls
+;
+  %1 = call i32 (i32, i32, ...) @foo2(i32 0, i32 1, i64 2, double 3.000000e+00)
+  ret i32 %1
+}
+
+;; Test that MSan doesn't generate code overflowing __msan_va_arg_tls when too many arguments are
+;; passed to a variadic function.
+declare i64 @sum(i64 %n, ...)
+define dso_local i64 @many_args() {
+;; If the size of __msan_va_arg_tls changes the second argument of `add` must also be changed.
+; CHECK-LABEL: @many_args
+; CHECK:    i64 add (i64 ptrtoint (ptr @__msan_va_arg_tls to i64), i64 792)
+; CHECK-NOT: i64 add (i64 ptrtoint (ptr @__msan_va_arg_tls to i64), i64 800)
+;
+entry:
+  %ret = call i64 (i64, ...) @sum(i64 120,
+  i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1,
+  i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1,
+  i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1,
+  i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1,
+  i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1,
+  i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1,
+  i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1,
+  i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1,
+  i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1,
+  i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1,
+  i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1,
+  i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1, i64 1
+  )
+  ret i64 %ret
+}