This is an archive of the discontinued LLVM Phabricator instance.

[clang][msan] Turn on -fsanitize-memory-param-retval by default
ClosedPublic

Authored by aeubanks on Sep 26 2022, 12:56 PM.

Details

Summary

This eagerly reports use of undef values when passed to noundef
parameters or returned from noundef functions.

This also decreases binary sizes under msan.

To go back to the previous behavior, pass -fno-sanitize-memory-param-retval.

Diff Detail

Event Timeline

aeubanks created this revision.Sep 26 2022, 12:56 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 26 2022, 12:56 PM
aeubanks requested review of this revision.Sep 26 2022, 12:56 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 26 2022, 12:56 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript

hmm, looks like this breaks a bunch of msan tests with -DLLVM_ENABLE_RUNTIMES=compiler-rt and ninja && ninja check-runtimes

MemorySanitizer-X86_64 :: Linux/swapcontext_annotation_reset.cpp
MemorySanitizer-X86_64 :: bsearch.cpp
MemorySanitizer-X86_64 :: chained_origin.cpp
MemorySanitizer-X86_64 :: chained_origin_empty_stack.cpp
MemorySanitizer-X86_64 :: chained_origin_memcpy.cpp
MemorySanitizer-X86_64 :: chained_origin_memmove.cpp
MemorySanitizer-X86_64 :: cxa_atexit.cpp
MemorySanitizer-X86_64 :: insertvalue_origin.cpp
MemorySanitizer-X86_64 :: no_sanitize_memory_prop.cpp
MemorySanitizer-X86_64 :: noundef_analysis.cpp
MemorySanitizer-X86_64 :: param_tls_limit.cpp
MemorySanitizer-X86_64 :: qsort.cpp
MemorySanitizer-X86_64 :: signal_stress_test.cpp
MemorySanitizer-X86_64 :: unpoison_param.cpp
MemorySanitizer-X86_64 :: vector_cvt.cpp
SanitizerCommon-msan-x86_64-Linux :: get_module_and_offset_for_pc.cpp
UBSan-MemorySanitizer-x86_64 :: TestCases/Misc/monitor.cpp
UBSan-MemorySanitizer-x86_64 :: TestCases/TypeCheck/vptr.cpp

before looking into those, is it ok to proceed with this?

vitalybuka added reviewers: thakis, hans, glider.EditedSep 26 2022, 4:27 PM

LGTM. I was thinking about this recently as well.
Main inconvenience is users like Chromium. They have precompiled libraries. They will need to use -fno-sanitize-memory-param-retval, if recompiling is not an option.

do you want the tests to all properly work with -fsanitize-memory-param-retval, or just pin them to -fno-sanitize-memory-param-retval if they're failing?

do you want the tests to all properly work with -fsanitize-memory-param-retval, or just pin them to -fno-sanitize-memory-param-retval if they're failing?

I have some draft patch for D134683.

vitalybuka added inline comments.Sep 26 2022, 5:18 PM
clang/include/clang/Driver/Options.td
1767

you need to update SanitizerArgs::MsanParamRetval

SanitizerArgs.cpp:1187

if (!MsanParamRetval)
    CmdArgs.push_back("-fno-sanitize-memory-param-retval");

without that option.td is no-op

Then D134683 works with and without your patch

vitalybuka updated this revision to Diff 463058.EditedSep 26 2022, 5:27 PM

Update MsanParamRetval

aeubanks updated this revision to Diff 463061.Sep 26 2022, 5:45 PM

add release notes

diff --git a/clang/test/Driver/fsanitize-memory-param-retval.c b/clang/test/Driver/fsanitize-memory-param-retval.c
index d82d20812186..79ade32178b6 100644
--- a/clang/test/Driver/fsanitize-memory-param-retval.c
+++ b/clang/test/Driver/fsanitize-memory-param-retval.c
@@ -1,14 +1,14 @@
-// RUN: %clang -target i386-gnu-linux %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
-// RUN: %clang -target x86_64-linux-gnu %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
-// RUN: %clang -target aarch64-linux-gnu %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-linux-gnu %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-linux-gnu %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
-// RUN: %clang -target x86_64-linux-gnu %s -fsanitize=kernel-memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target i386-gnu-linux %s -fsanitize=memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target x86_64-linux-gnu %s -fsanitize=memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target aarch64-linux-gnu %s -fsanitize=memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-linux-gnu %s -fsanitize=memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target riscv64-linux-gnu %s -fsanitize=memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target x86_64-linux-gnu %s -fsanitize=kernel-memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
 
-// CHECK: "-fsanitize-memory-param-retval"
+// CHECK: "-fno-sanitize-memory-param-retval"
 
-// RUN: %clang -target aarch64-linux-gnu -fsyntax-only %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck --check-prefix=11 %s
-// 11: "-fsanitize-memory-param-retval"
+// RUN: %clang -target aarch64-linux-gnu -fsyntax-only %s -fsanitize=memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck --check-prefix=11 %s
+// 11: "-fno-sanitize-memory-param-retval"
 
-// RUN: not %clang -target x86_64-linux-gnu -fsyntax-only %s -fsanitize=memory -fsanitize-memory-param-retval=1 2>&1 | FileCheck --check-prefix=EXCESS %s
-// EXCESS: error: unknown argument: '-fsanitize-memory-param-retval=
+// RUN: not %clang -target x86_64-linux-gnu -fsyntax-only %s -fsanitize=memory -fno-sanitize-memory-param-retval=1 2>&1 | FileCheck --check-prefix=EXCESS %s
+// EXCESS: error: unknown argument: '-fno-sanitize-memory-param-retval=
aeubanks updated this revision to Diff 463096.Sep 26 2022, 10:14 PM

update test

aeubanks edited the summary of this revision. (Show Details)Sep 27 2022, 1:11 PM
aeubanks updated this revision to Diff 463322.Sep 27 2022, 1:12 PM

rebase (to run through presubmits with test fixes)

xbolva00 added inline comments.Sep 27 2022, 1:23 PM
clang/docs/ReleaseNotes.rst
450

Probably I would mention that there is -fno-sanitize-memory-param-retval to disable it.

So one can use “-fsanitize=memory -fno-sanitize-memory-param-retval“ to unblock failed builds.

aeubanks updated this revision to Diff 463327.Sep 27 2022, 1:31 PM

mention -fno-sanitize-memory-param-retval in release notes

vitalybuka accepted this revision.Sep 27 2022, 1:32 PM
This revision is now accepted and ready to land.Sep 27 2022, 1:32 PM
MaskRay accepted this revision.Sep 27 2022, 1:33 PM