This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by kda on Aug 8 2022, 10:18 AM.

Details

Summary

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

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

Diff Detail

Event Timeline

kda created this revision.Aug 8 2022, 10:18 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 8 2022, 10:18 AM
kda requested review of this revision.Aug 8 2022, 10:18 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptAug 8 2022, 10:18 AM
Herald added subscribers: llvm-commits, Restricted Project. · View Herald Transcript
kda updated this revision to Diff 450879.Aug 8 2022, 10:58 AM

collected all updates.

kda updated this revision to Diff 450927.Aug 8 2022, 1:41 PM

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

kda updated this revision to Diff 451317.Aug 9 2022, 4:56 PM

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

kda edited the summary of this revision. (Show Details)Aug 10 2022, 8:57 AM
kda added a reviewer: vitalybuka.
kcc added a subscriber: kcc.Aug 10 2022, 9:04 AM

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 added a comment.Aug 10 2022, 9:10 AM

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 added a comment.Aug 10 2022, 9:16 AM

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 added a comment.Aug 10 2022, 9:28 AM
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 added inline comments.Aug 10 2022, 9:59 AM
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 added inline comments.Aug 10 2022, 10:01 AM
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 updated this revision to Diff 451578.Aug 10 2022, 11:16 AM

restore the variable name to the global and fix up tests

kda updated this revision to Diff 451579.Aug 10 2022, 11:18 AM

remove unnecessary allocation (no longer writing null byte in)

kda marked an inline comment as done.Aug 10 2022, 11:19 AM

Restored variable name. (Still dropping function name.)

kda retitled this revision from Remove variable names from sanitize-memory-track-origins binary. to Remove function name from sanitize-memory-track-origins binary..Aug 10 2022, 11:20 AM
vitalybuka accepted this revision.Aug 10 2022, 11:26 AM
vitalybuka added inline comments.
compiler-rt/lib/msan/msan_report.cpp
51–53

to many %s ?

This revision is now accepted and ready to land.Aug 10 2022, 11:26 AM
vitalybuka added inline comments.Aug 10 2022, 11:30 AM
compiler-rt/lib/msan/msan_report.cpp
51–53

my mistake, it matches.

kcc added a comment.Aug 10 2022, 12:06 PM

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 marked 3 inline comments as done.Aug 10 2022, 3:31 PM
kda updated this revision to Diff 451659.Aug 10 2022, 3:32 PM

sync to head

This revision was landed with ongoing or failed builds.Aug 10 2022, 3:45 PM
This revision was automatically updated to reflect the committed changes.