enable noundef analysis with -fsanitize-memory-param-retval

[kda]

Summary

Enable noundef analysis (-enable-noundef-analysis) via the -fsanitize-memory-param-retval clang flag.
This completes the work found in:

• https://reviews.llvm.org/D116855
• https://reviews.llvm.org/D116633

Depends on D116633

[kda]

add missing files.

[vitalybuka]

Nice!

This completes the work found in:

It's too late for this set of patches, but phabricator supports "Depends on" comments (I guess only of the first patch upload), and manual editing of "parent/child" revisions. Check "Edit Related Revisions..." in the top right corner.
So you can create convenient "Stack" tab like in liihttps://reviews.llvm.org/D117286

clang/include/clang/Driver/Options.td

1678

[vitalybuka]

Just noticed, -enable-noundef-analysis is missing tests undef llvm/llvm-project/clang/test/Driver/
Can you add few near maybe before llvm-project/clang/test/Driver/clang_f_opts.c:531 (just before discard-value-names tests)

[vitalybuka]

In D117293#3243772, @vitalybuka wrote:

Just noticed, -enable-noundef-analysis is missing tests undef llvm/llvm-project/clang/test/Driver/
Can you add few near maybe before llvm-project/clang/test/Driver/clang_f_opts.c:531 (just before discard-value-names tests)

Actually noundef is not f_opt, and I am not sure how or do we need to test ImpliedByAnyOf in Driver.

[kda]

In D117293#3243735, @vitalybuka wrote:

Nice!

This completes the work found in:

It's too late for this set of patches, but phabricator supports "Depends on" comments (I guess only of the first patch upload), and manual editing of "parent/child" revisions. Check "Edit Related Revisions..." in the top right corner.
So you can create convenient "Stack" tab like in liihttps://reviews.llvm.org/D117286

I added "Depends On: D116633" (but I am confused because it appears crossed out.)

[vitalybuka]

In D117293#3244668, @kda wrote:

In D117293#3243735, @vitalybuka wrote:

Nice!

This completes the work found in:

It's too late for this set of patches, but phabricator supports "Depends on" comments (I guess only of the first patch upload), and manual editing of "parent/child" revisions. Check "Edit Related Revisions..." in the top right corner.
So you can create convenient "Stack" tab like in liihttps://reviews.llvm.org/D117286

I added "Depends On: D116633" (but I am confused because it appears crossed out.)

Crossed out means submitted. It's OK.
It should be "Depends On D116633", no ":" (at least that's how I do and it works)
I guess it will work only on the first patch upload, After then you need to "Edit Related Revisions..." manually.

[kda]

In D117293#3243986, @vitalybuka wrote:

In D117293#3243772, @vitalybuka wrote:

Just noticed, -enable-noundef-analysis is missing tests undef llvm/llvm-project/clang/test/Driver/
Can you add few near maybe before llvm-project/clang/test/Driver/clang_f_opts.c:531 (just before discard-value-names tests)

Actually noundef is not f_opt, and I am not sure how or do we need to test ImpliedByAnyOf in Driver.

The CodeGen tests cover these interactions.
When I was building this, I added the tests first (they failed), then I added the ImpliedByAnyOf one-liner, and the tests passed.

[vitalybuka]

To clarify: LGTM as-is, I don't expect any changes.

clang/include/clang/Driver/Options.td

: Joined<["-"], "fsanitize-address-destructor=">,
       Flags<[CC1Option]>,
       HelpText<"Set destructor type used in ASan instrumentation">,
       Group<f_clang_Group>,
       Values<"none,global">,
       NormalizedValuesScope<"llvm::AsanDtorKind">,
       NormalizedValues<["None", "Global"]>,
       MarshallingInfoEnum<CodeGenOpts<"SanitizeAddressDtor">, "Global">;
 defm sanitize_memory_param_retval
-  : BoolOption<"f", "sanitize-memory-param-retval",
+    : BoolFOption<"sanitize-memory-param-retval",
         CodeGenOpts<"SanitizeMemoryParamRetval">,
         DefaultFalse,
         PosFlag<SetTrue, [CC1Option], "Enable">, NegFlag<SetFalse, [], "Disable">,
-      BothFlags<[], " detection of uninitialized parameters and return values">>,
-    Group<f_clang_Group>;
-// Note: This flag was introduced when it was necessary to distinguish between
+        BothFlags<[], " detection of uninitialized parameters and return values">>;
+//// Note: This flag was introduced when it was necessary to distinguish between
 //       ABI for correct codegen.  This is no longer needed, but the flag is
 //       not removed since targeting either ABI will behave the same.
 //       This way we cause no disturbance to existing scripts & code, and if we
 //       want to use this flag in the future we will cause no disturbance then
 //       either.
 def fsanitize_hwaddress_abi_EQ
     : Joined<["-"], "fsanitize-hwaddress-abi=">,
       Group<f_clang_Group>,
defm clear_ast_before_backend : BoolOption<"",
   "clear-ast-before-backend",
   CodeGenOpts<"ClearASTBeforeBackend">,
   DefaultFalse,
   PosFlag<SetTrue, [], "Clear">,
   NegFlag<SetFalse, [], "Don't clear">,
   BothFlags<[], " the Clang AST before running backend code generation">>;
 def enable_noundef_analysis : Flag<["-"], "enable-noundef-analysis">, Group<f_Group>,
   HelpText<"Enable analyzing function argument and return types for mandatory definedness">,
-  MarshallingInfoFlag<CodeGenOpts<"EnableNoundefAttrs">>;
+  MarshallingInfoFlag<CodeGenOpts<"EnableNoundefAttrs">>,
+  ImpliedByAnyOf<[fsanitize_memory_param_retval.KeyPath]>;
 def discard_value_names : Flag<["-"], "discard-value-names">,
   HelpText<"Discard value names in LLVM IR">,
   MarshallingInfoFlag<CodeGenOpts<"DiscardValueNames">>;
 def plugin_arg : JoinedAndSeparate<["-"], "plugin-arg-">,
     MetaVarName<"<name> <arg>">,
     HelpText<"Pass <arg> to plugin <name>">;
 def add_plugin : Separate<["-"], "add-plugin">, MetaVarName<"<name>">,
   HelpText<"Use the named plugin action in addition to the default action">,

clang/test/CodeGen/attr-noundef.cpp

 // RUN: %clang -cc1 -triple x86_64-gnu-linux -x c++ -S -emit-llvm -enable-noundef-analysis %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-INTEL
 // RUN: %clang -cc1 -triple aarch64-gnu-linux -x c++ -S -emit-llvm -enable-noundef-analysis %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-AARCH
+// RUN: %clang -cc1 -triple x86_64-gnu-linux -x c++ -S -emit-llvm -fsanitize-memory-param-retval %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-INTEL
+// RUN: %clang -cc1 -triple aarch64-gnu-linux -x c++ -S -emit-llvm -fsanitize-memory-param-retval %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-AARCH
+
+// no-sanitize-memory-param-retval does NOT conflict with enable-noundef-analysis
+// RUN: %clang -cc1 -triple x86_64-gnu-linux -x c++ -S -emit-llvm -enable-noundef-analysis -fno-sanitize-memory-param-retval %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-INTEL
+// RUN: %clang -cc1 -triple x86_64-gnu-linux -x c++ -S -emit-llvm -fno-sanitize-memory-param-retval -enable-noundef-analysis %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-INTEL
 
 //************ Passing structs by value
 // TODO: No structs may currently be marked noundef
 
 namespace check_structs {
 struct Trivial {
   int a;
 };

clang/test/CodeGen/indirect-noundef.cpp

 // RUN: %clang -cc1 -x c++ -triple x86_64-unknown-unknown -O0 -emit-llvm -enable-noundef-analysis -o - %s | FileCheck %s
+// RUN: %clang -cc1 -x c++ -triple x86_64-unknown-unknown -O0 -emit-llvm -fsanitize-memory-param-retval -o - %s | FileCheck %s
+
+// no-sanitize-memory-param-retval does NOT conflict with enable-noundef-analysis
+// RUN: %clang -cc1 -x c++ -triple x86_64-unknown-unknown -O0 -emit-llvm -fno-sanitize-memory-param-retval -enable-noundef-analysis -o - %s | FileCheck %s
+// RUN: %clang -cc1 -x c++ -triple x86_64-unknown-unknown -O0 -emit-llvm -enable-noundef-analysis -fno-sanitize-memory-param-retval -o - %s | FileCheck %s
 
 union u1 {
   int val;
 };
 
 // CHECK: @indirect_callee_int_ptr = [[GLOBAL:(dso_local )?global]] i32 (i32)*
 int (*indirect_callee_int_ptr)(int);
 // CHECK: @indirect_callee_union_ptr = [[GLOBAL]] i32 (i32)*

clang/test/CodeGen/msan-param-retval.c

 // RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -o - %s | \
 // RUN:     FileCheck %s --check-prefix=CLEAN
 // RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -Xclang -enable-noundef-analysis -o - %s | \
 // RUN:     FileCheck %s --check-prefixes=NOUNDEF,NOUNDEF_ONLY
 // RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -Xclang -enable-noundef-analysis -mllvm -msan-eager-checks -o - %s | \
 // RUN:     FileCheck %s --check-prefixes=NOUNDEF,EAGER
 // RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -fsanitize-memory-param-retval -o - %s | \
-// RUN:     FileCheck %s --check-prefixes=CLEAN
+// RUN:     FileCheck %s --check-prefixes=NOUNDEF,EAGER
 // RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -Xclang -enable-noundef-analysis -fsanitize-memory-param-retval -o - %s | \
 // RUN:     FileCheck %s --check-prefixes=NOUNDEF,EAGER
 
 void bar(int x) {
 }
 
 // CLEAN:   define dso_local void @bar(i32 %x) #0 {
 // NOUNDEF: define dso_local void @bar(i32 noundef %x) #0 {
 // CLEAN:        @__msan_param_tls
 // NOUNDEF_ONLY: @__msan_param_tls
 // EAGER-NOT:    @__msan_param_tls

compiler-rt/test/msan/noundef_analysis.cpp

 // RUN: %clangxx_msan %s -o %t && %run %t >%t.out 2>&1
 // RUN: FileCheck %s --check-prefix=MISSED --allow-empty < %t.out
 // RUN: %clangxx_msan %s -Xclang -enable-noundef-analysis -mllvm -msan-eager-checks=1 -o %t && not %run %t >%t.out 2>&1
 // RUN: FileCheck %s < %t.out
+// RUN: %clangxx_msan %s -fsanitize-memory-param-retval -o %t && not %run %t >%t.out 2>&1
+// RUN: FileCheck %s < %t.out
 
 struct SimpleStruct {
   int md1;
 };
 
 static int sink;
 
 static void examineValue(int x) { sink = x; }