staticanalysis / memory-sanitizer

Automatically exported from code.google.com/p/memory-sanitizer
0 stars 0 forks source link

Issues in origin handling #6

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
StoreInst stores origin unconditionally. This is wrong, because a short store 
of fully initialized value will reset origin for the entire 4-byte location.

StoreInst only stores origin for the first 4 bytes of the destination.

InsertElementInst cleans result shadow. It should Select one, instead.

Original issue reported on code.google.com by euge...@google.com on 9 Nov 2012 at 8:19

GoogleCodeExporter commented 9 years ago
In fact, StoreInst behaviour is also a performance problem. We could save a lot 
of memory bandwidth by not storing origins for fully initialized values. An 
extra branch and one more BB sounds like a good tradeoff for that. I'll do a 
benchmark.

Original comment by euge...@google.com on 9 Nov 2012 at 8:47

GoogleCodeExporter commented 9 years ago
Some of points above are fixed:
- StoreInst now stores origin only if corresponding shadow is non-zero.
- InsertElementInst in properly handled.
- 5-10% speed improvement in track-origins mode.

Possible future optimizations:
- branch weights for the origin store test
- avoid loading origin for clean values. This can have even more effect that 
the store optimization.

The following is NOT yet fixed:
- StoreInst only stores origin for the first 4 bytes of the destination.

Original comment by euge...@google.com on 19 Nov 2012 at 12:30

GoogleCodeExporter commented 9 years ago
Another optimization idea: delay origin loads. Instead of loading origin along 
with shadow, build an origin calculation tree associated with every app temp, 
and emit the code for it at the store time, entirely in the cold branch 
(store_shadow != 0).

This won't work for function arguments (TLS may get overwritten by other calls).

Original comment by euge...@google.com on 22 Nov 2012 at 11:19

GoogleCodeExporter commented 9 years ago
Fixed "StoreInst only stores origin for the first 4 bytes of the destination.":
http://llvm.org/viewvc/llvm-project?view=revision&revision=226658

Original comment by euge...@google.com on 21 Jan 2015 at 1:23