[PATCH] D139296: [msan][CodeGen] Set noundef for C return value

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka updated this revision to Diff 480335.
vitalybuka added a comment.

remove redundant check


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139296

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CodeGen/cleanup-destslot-simple.c
  clang/test/CodeGen/msan-param-retval.c


Index: clang/test/CodeGen/msan-param-retval.c
===================================================================
--- clang/test/CodeGen/msan-param-retval.c
+++ clang/test/CodeGen/msan-param-retval.c
@@ -23,13 +23,20 @@
   return 1;
 }
 
-// CHECK: define dso_local i32 @foo() #0 {
-// CHECK:   @__msan_retval_tls
+// CLEAN:   define dso_local i32 @foo() #0 {
+// NOUNDEF: define dso_local noundef i32 @foo() #0 {
+// CLEAN:        @__msan_retval_tls
+// NOUNDEF_ONLY: @__msan_retval_tls
+// EAGER-NOT:    @__msan_retval_tls
 // CHECK: }
 
 int noret() {
 }
 
-// CHECK: define dso_local i32 @noret() #0 {
+// CLEAN:   define dso_local i32 @noret() #0 {   
+// NOUNDEF: define dso_local noundef i32 @noret() #0 {
 // CHECK:   %retval = alloca
-// CHECK: }
\ No newline at end of file
+// CLEAN:        @__msan_retval_tls
+// NOUNDEF_ONLY: @__msan_retval_tls
+// EAGER-NOT:    @__msan_retval_tls
+// CHECK: }
Index: clang/test/CodeGen/cleanup-destslot-simple.c
===================================================================
--- clang/test/CodeGen/cleanup-destslot-simple.c
+++ clang/test/CodeGen/cleanup-destslot-simple.c
@@ -64,7 +64,12 @@
 // CHECK-MSAN-NEXT:    [[_MSLD1:%.*]] = load i32, ptr [[TMP11]], align 4, !dbg [[DBG20]]
 // CHECK-MSAN-NEXT:    call void @llvm.lifetime.end.p0(i64 8, ptr nonnull [[P]]), !dbg [[DBG22:![0-9]+]]
 // CHECK-MSAN-NEXT:    call void @llvm.lifetime.end.p0(i64 4, ptr nonnull [[X]]) #[[ATTR2]], !dbg [[DBG22]]
-// CHECK-MSAN-NEXT:    store i32 [[_MSLD1]], ptr @__msan_retval_tls, align 8, !dbg [[DBG23:![0-9]+]]
+// CHECK-MSAN-NEXT:    [[_MSCMP2_NOT:%.*]] = icmp eq i32 [[_MSLD1]], 0, !dbg [[DBG23:![0-9]+]]
+// CHECK-MSAN-NEXT:    br i1 [[_MSCMP2_NOT]], label [[TMP13:%.*]], label [[TMP12:%.*]], !dbg [[DBG23]], !prof [[PROF21]]
+// CHECK-MSAN:       12:
+// CHECK-MSAN-NEXT:    call void @__msan_warning_noreturn() #[[ATTR3]], !dbg [[DBG23]]
+// CHECK-MSAN-NEXT:    unreachable, !dbg [[DBG23]]
+// CHECK-MSAN:       13:
 // CHECK-MSAN-NEXT:    ret i32 [[TMP8]], !dbg [[DBG23]]
 //
 // CHECK-KMSAN-LABEL: @test(
Index: clang/lib/CodeGen/CGCall.cpp
===================================================================
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -1798,6 +1798,13 @@
 
 static bool HasStrictReturn(const CodeGenModule &Module, QualType RetTy,
                             const Decl *TargetDecl) {
+  // As-is msan can not tolerate noundef mismatch between caller and
+  // implementation. Mismatch is possible for e.g. indirect calls from C-caller
+  // into C++. Such mismatches lead to confusing false reports. To avoid
+  // expensive workaround on msan we enforce initialization event in uncommon
+  // cases where it's allowed.
+  if (Module.getLangOpts().Sanitize.has(SanitizerKind::Memory))
+    return true;
   // C++ explicitly makes returning undefined values UB. C's rule only applies
   // to used values, so we never mark them noundef for now.
   if (!Module.getLangOpts().CPlusPlus)
@@ -1818,7 +1825,6 @@
   // Try to respect what the programmer intended.
   return Module.getCodeGenOpts().StrictReturn ||
          !Module.MayDropFunctionReturn(Module.getContext(), RetTy) ||
-         Module.getLangOpts().Sanitize.has(SanitizerKind::Memory) ||
          Module.getLangOpts().Sanitize.has(SanitizerKind::Return);
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D139296.480335.patch
Type: text/x-patch
Size: 3314 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20221206/bd8fffca/attachment.bin>