Remove function name from sanitize-memory-track-origins binary.

[kda]

Summary

This work is being done to reduce the size of MSAN with track origins binary.

Builds upon: https://reviews.llvm.org/D131205

[kda]

collected all updates.

[kda]

removed those changes found in https://reviews.llvm.org/D131205

[kda]

Refactor to address function changes and print generic message regarding location of uninitialized stack variable.

[kcc]

please show an error message on a non-trivial test case
(lots of variables, some defined on the same line) before and after.
Code size is important, but we cannot decrease the report quality

[kda]

In D131415#3713055, @kcc wrote:

please show an error message on a non-trivial test case
(lots of variables, some defined on the same line) before and after.
Code size is important, but we cannot decrease the report quality

Where can I get the name of the variable? (I think it is buried in the debug info some place, but I would need an example to help me extract it.)
An alternative I thought of was putting this behind a flag.

[kcc]

Where can I get the name of the variable? (I think it is buried in the debug info some place, but I would need an example to help me extract it.)
An alternative I thought of was putting this behind a flag.

Not sure I understand the question. MSan gets the variable names today just fine:

% cat stack-uninit.c 
void bar(int *a) {}

int main() {
  int foo;
  bar(&foo);
  if (foo) return 42;
  return 0;
}

% clang -g -fsanitize=memory -fsanitize-memory-track-origins  stack-uninit.c && ./a.out 
==389328==WARNING: MemorySanitizer: use-of-uninitialized-value
...

  Uninitialized value was created by an allocation of 'foo' in the stack frame of function 'main'

[kda]

In D131415#3713065, @kcc wrote:

Uninitialized value was created by an allocation of 'foo' in the stack frame of function 'main'

Absolutely. And this change will remove foo and main from the global.
The new message is:

Uninitialized value was created on the stack (first frame below should reference the variable)

I believe that foo is somewhere else in the binary, but I don't know the code which would be required to draw it out and display it in the message.
I'm looking for a hint.

[vitalybuka]

compiler-rt/lib/msan/msan_report.cpp

52

It technically not the stack, just a frame.
Shorter message like this has the same meaning:
"Uninitialized value was created here:"

[vitalybuka]

compiler-rt/lib/msan/msan_report.cpp

52

actually we still want to note that it's on that stack

"Uninitialized value was created by an allocation in the stack frame here:"

[kda]

restore the variable name to the global and fix up tests

[kda]

remove unnecessary allocation (no longer writing null byte in)

[kda]

Restored variable name. (Still dropping function name.)

[vitalybuka]

compiler-rt/lib/msan/msan_report.cpp

51–53

to many %s ?

[vitalybuka]

compiler-rt/lib/msan/msan_report.cpp

51–53

my mistake, it matches.

[kcc]

dropping the function name is fine, we have it from the stack traces.
the variable name can't be found anywhere else because in most cases
msan is used with -gline-tables-only, i.e. DWARF doesn't have the names of locals.

[kda]

sync to head

compiler-rt/lib/msan/msan.cpp

 int msan_inited = 0;
 bool msan_init_is_running;
 
 int msan_report_count = 0;
 
 // Array of stack origins.
 // FIXME: make it resizable.
 static const uptr kNumStackOriginDescrs = 1024 * 1024;
-#ifdef kda_COMMENTED_OUT
-static const char *StackOriginDescr[kNumStackOriginDescrs];
-#endif  /* kda_COMMENTED_OUT */
 static uptr StackOriginPC[kNumStackOriginDescrs];
 static atomic_uint32_t NumStackOriginDescrs;
 
 void Flags::SetDefaults() {
 #define MSAN_FLAG(Type, Name, DefaultValue, Description) Name = DefaultValue;
 #include "msan_flags.inc"
 #undef MSAN_FLAG
 }

 }
 
 void UnpoisonThreadLocalState() {
 }
 
 const char *GetStackOriginDescr(u32 id, uptr *pc) {
   CHECK_LT(id, kNumStackOriginDescrs);
   if (pc) *pc = StackOriginPC[id];
-#ifdef kda_COMMENTED_OUT
-  return StackOriginDescr[id];
-#endif  /* kda_COMMENTED_OUT */
   return nullptr;
 }
 
 u32 ChainOrigin(u32 id, StackTrace *stack) {
   MsanThread *t = GetCurrentThread();
   if (t && t->InSignalHandler())
     return id;
 
void __msan_set_alloca_origin4(void *a, uptr size, char *descr, uptr pc) {
   static const u32 first_timer =
       dash + (dash << 8) + (dash << 16) + (dash << 24);
   u32 *id_ptr = (u32*)descr;
   bool print = false;  // internal_strstr(descr + 4, "AllocaTOTest") != 0;
   u32 id = *id_ptr;
   if (id == first_timer) {
     u32 idx = atomic_fetch_add(&NumStackOriginDescrs, 1, memory_order_relaxed);
     CHECK_LT(idx, kNumStackOriginDescrs);
-#ifdef kda_COMMENTED_OUT
-    StackOriginDescr[idx] = descr + 4;
-#endif  /* kda_COMMENTED_OUT */
 #if SANITIZER_PPC64V1
     // On PowerPC64 ELFv1, the address of a function actually points to a
     // three-doubleword data structure with the first field containing
     // the address of the function's code.
     if (pc)
       pc = *reinterpret_cast<uptr*>(pc);
 #endif
     StackOriginPC[idx] = pc;

compiler-rt/lib/msan/msan_report.cpp

 };
 
 static void DescribeStackOrigin(const char *so, uptr pc) {
   const char* function_name = "kUNKNOWN_FUNCTIONk";
   if (pc) {
     // For some reason function address in LLVM IR is 1 less then the address
     // of the first instruction.
     pc = StackTrace::GetNextInstructionPc(pc);
-    SymbolizedStack* frame = Symbolizer::GetOrInit()->SymbolizePC(pc);
-#ifdef kda_COMMENTED_OUT
-    function_name = DemangleFunctionName(frame->info.function);
-#endif  /* kda_COMMENTED_OUT */
+    SymbolizedStack *frame = Symbolizer::GetOrInit()->SymbolizePC(pc);
     // TODO: Does not work for SANITIZER_NETBSD
     function_name = frame->info.function;
   }
   Decorator d;
-  //char *s = internal_strdup(so);
-  //char *sep = internal_strchr(s, '@');
-  //CHECK(sep);
-  //*sep = '\0';
   Printf("%s", d.Origin());
   Printf(
       "  %sUninitialized value was created by an allocation of '%s%s%s'"
       " in the stack frame of function '%s%s%s'%s\n",
-#ifdef kda_COMMENTED_OUT
-      d.Origin(), d.Name(), s, d.Origin(), d.Name(), sep + 1, d.Origin(),
-      d.Origin(), d.Name(), "kUNKNOWN_VARIABLEk", d.Origin(), d.Name(), "kUNKNOWN_FUNCTIONk", d.Origin(),
-#endif  /* kda_COMMENTED_OUT */
-      d.Origin(), d.Name(), "kUNKNOWN_VARIABLEk", d.Origin(), d.Name(), function_name, d.Origin(),
-      d.Default());
-#ifdef kda_COMMENTED_OUT
-  InternalFree(s);
-#endif  /* kda_COMMENTED_OUT */
+      d.Origin(), d.Name(), "kUNKNOWN_VARIABLEk", d.Origin(), d.Name(),
+      function_name, d.Origin(), d.Default());
 
   if (pc) {
-#ifdef kda_COMMENTED_OUT
-    // For some reason function address in LLVM IR is 1 less then the address
-    // of the first instruction.
-    pc = StackTrace::GetNextInstructionPc(pc);
-#endif  /* kda_COMMENTED_OUT */
     StackTrace(&pc, 1).Print();
   }
 }
 
 static void DescribeOrigin(u32 id) {
   VPrintf(1, "  raw origin id: %d\n", id);
   Decorator d;
   Origin o = Origin::FromRawId(id);

compiler-rt/test/msan/stack-origin.cpp

-// RUNx: %clangxx_msan -O0 %s -o %t && not %run %t >%t.out 2>&1
-// RUNx: FileCheck %s < %t.out
-// RUNx: %clangxx_msan -O1 %s -o %t && not %run %t >%t.out 2>&1
-// RUNx: FileCheck %s < %t.out
-// RUNx: %clangxx_msan -O2 %s -o %t && not %run %t >%t.out 2>&1
-// RUNx: FileCheck %s < %t.out
-// RUNx: %clangxx_msan -O3 %s -o %t && not %run %t >%t.out 2>&1
-// RUNx: FileCheck %s < %t.out
+// RUN: %clangxx_msan -O0 %s -o %t && not %run %t >%t.out 2>&1
+// RUN: FileCheck %s < %t.out
+// RUN: %clangxx_msan -O1 %s -o %t && not %run %t >%t.out 2>&1
+// RUN: FileCheck %s < %t.out
+// RUN: %clangxx_msan -O2 %s -o %t && not %run %t >%t.out 2>&1
+// RUN: FileCheck %s < %t.out
+// RUN: %clangxx_msan -O3 %s -o %t && not %run %t >%t.out 2>&1
+// RUN: FileCheck %s < %t.out
 
 // RUN: %clangxx_msan -fsanitize-memory-track-origins -O0 %s -o %t && not %run %t >%t.out 2>&1
 // RUN: FileCheck %s < %t.out && FileCheck %s --check-prefix=CHECK-ORIGINS < %t.out
-// RUNx: %clangxx_msan -fsanitize-memory-track-origins -O1 %s -o %t && not %run %t >%t.out 2>&1
-// RUNx: FileCheck %s < %t.out && FileCheck %s --check-prefix=CHECK-ORIGINS < %t.out
-// RUNx: %clangxx_msan -fsanitize-memory-track-origins -O2 %s -o %t && not %run %t >%t.out 2>&1
-// RUNx: FileCheck %s < %t.out && FileCheck %s --check-prefix=CHECK-ORIGINS < %t.out
-// RUNx: %clangxx_msan -fsanitize-memory-track-origins -O3 %s -o %t && not %run %t >%t.out 2>&1
-// RUNx: FileCheck %s < %t.out && FileCheck %s --check-prefix=CHECK-ORIGINS < %t.out
+// RUN: %clangxx_msan -fsanitize-memory-track-origins -O1 %s -o %t && not %run %t >%t.out 2>&1
+// RUN: FileCheck %s < %t.out && FileCheck %s --check-prefix=CHECK-ORIGINS < %t.out
+// RUN: %clangxx_msan -fsanitize-memory-track-origins -O2 %s -o %t && not %run %t >%t.out 2>&1
+// RUN: FileCheck %s < %t.out && FileCheck %s --check-prefix=CHECK-ORIGINS < %t.out
+// RUN: %clangxx_msan -fsanitize-memory-track-origins -O3 %s -o %t && not %run %t >%t.out 2>&1
+// RUN: FileCheck %s < %t.out && FileCheck %s --check-prefix=CHECK-ORIGINS < %t.out
 
 #include <stdlib.h>
 int main(int argc, char **argv) {
-  int xyz;
-  int *volatile p = &xyz;
+  int x;
+  int *volatile p = &x;
   return *p;
   // CHECK: WARNING: MemorySanitizer: use-of-uninitialized-value
   // CHECK: {{#0 0x.* in main .*stack-origin.cpp:}}[[@LINE-2]]
 
   // CHECK-ORIGINS: Uninitialized value was created by an allocation of 'x' in the stack frame of function 'main'
   // CHECK-ORIGINS: {{#0 0x.* in main .*stack-origin.cpp:}}[[@LINE-7]]
 
   // CHECK: SUMMARY: MemorySanitizer: use-of-uninitialized-value {{.*stack-origin.cpp:.* main}}
 }

llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp

   Value *getLocalVarDescription(AllocaInst &I) {
     SmallString<2048> StackDescriptionStorage;
     raw_svector_ostream StackDescription(StackDescriptionStorage);
     // We create a string with a description of the stack allocation and
     // pass it into __msan_set_alloca_origin.
     // It will be printed by the run-time if stack-originated UMR is found.
     // The first 4 bytes of the string are set to '----' and will be replaced
     // by __msan_va_arg_overflow_size_tls at the first call.
-#ifdef kda_COMMENTED_OUT
-    StackDescription << "----" << I.getName() << "@" << F.getName();
-#endif  /* kda_COMMENTED_OUT */
     StackDescription << "----";
     return createPrivateNonConstGlobalForString(*F.getParent(),
                                                 StackDescription.str());
   }
 
   void poisonAllocaUserspace(AllocaInst &I, IRBuilder<> &IRB, Value *Len) {
     if (PoisonStack && ClPoisonStackWithCall) {
       IRB.CreateCall(MS.MsanPoisonStackFn,