<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">I think the 0003 patch makes a lot of sense. I wish I'd done that in the first place! (You'll need a corresponding change to compiler-rt of course.)</div><div class="gmail_quote"><br></div><div class="gmail_quote">For the 0002 patch, could we use a different representation instead, to deduplicate the common suffixes? Something like:</div><div class="gmail_quote"><br></div><div class="gmail_quote">  struct Path { Path *parent; const char *content; };</div><div class="gmail_quote"><br></div><div class="gmail_quote">Split each path on each '/' (or some smarter heuristic), and produce:</div><div class="gmail_quote"><br></div><div class="gmail_quote">  Path p0 { nullptr, "/path/to/build/" };</div><div class="gmail_quote">  Path p1 { &p0, "foo/" };</div><div class="gmail_quote">  Path p2 { &p1, "bar/" };</div><div class="gmail_quote">  Path foo_bar_baz_h { &p2, "baz.h" };</div><div class="gmail_quote"><div class="gmail_quote">  Path foo_bar_quux_h { &p2, "quux.h" };</div><div class="gmail_quote"><br></div><div class="gmail_quote">The downside of this is increased relocation size, which is already a significant problem for ubsan binaries. I'm not sure how much of that we can fix without teaching the linker about ubsan data.</div><div class="gmail_quote"><br></div><div class="gmail_quote">The 0001 patch makes me wonder if we can move (most of) the ubsan data into a section that's not mapped into the program's address space on startup, and somehow lazily read it from the binary if a check fails. This will need some work to cope with our duplicate suppression mechanism (which writes back to the ubsan data), but if you're having address space issues, this could help substantially.</div><div><br></div></div><div class="gmail_quote">On Tue, Apr 26, 2016 at 9:49 AM, Filipe Cabecinhas via cfe-dev <span dir="ltr"><<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Hi all,<br>
<br>
I’m proposing two ways to make ubsan instrumented code (really data…) smaller.<br>
<br>
Why?<br>
Our platform has limited resources and game teams have to play within<br>
those to create their games. During debugging, they might have some<br>
more relaxed limits, but they still have a budget they need to stick<br>
to. And they tend to use everything they can!<br>
The size of code+data counts towards that budget, and ubsan is very<br>
happy to make code+data a lot bigger when enabled. This can end up<br>
posing some problems and making ubsan harder to use than we would<br>
like.<br>
This might not be as useful for the usual operating systems people<br>
use, but it might help other people with space constraints enable<br>
ubsan more easily.<br>
<br>
<br>
Biggest space “wasters”:<br>
A quick objdump + grep found that not all check handlers are equal<br>
(checks inserting >10k calls to handler):<br>
<br>
clang (OS X):<br>
793539 ___ubsan_handle_type_mismatch_abort<br>
24204 ___ubsan_handle_load_invalid_value_abort<br>
(4127 ___ubsan_handle_builtin_unreachable)<br>
…<br>
<br>
Game:<br>
837785 __ubsan_handle_type_mismatch<br>
20391 __ubsan_handle_load_invalid_value<br>
16037 __ubsan_handle_out_of_bounds<br>
11357 __ubsan_handle_add_overflow<br>
…<br>
<br>
<br>
Savings:<br>
First, I made clang emit all the ubsan check data to a different<br>
section (and the type_mismatch data, specifically, to another), to<br>
ease accounting (patch attached: 0001-*.patch, provided so anyone can<br>
check it out. I don’t think it’s useful for it to be in clang).<br>
<br>
With an estimate of 48B per type_mismatch (SourceLocation (char* + 2x<br>
u32) + TypeDescriptor ref (uptr) + Alignment (uptr) + unsigned char ==<br>
8+2*4+8+8+1 == 33, which will be upped to 48 (!!) with padding (on<br>
x86-64 we’re aligning all structures to 16B)), we end up with (for<br>
type_mismatch checks’ static data):<br>
<br>
Before:<br>
clang: ~47.73 MiB<br>
game:  ~59.24 MiB<br>
<br>
After 0003-*.patch:<br>
clang: ~31.82 MiB (~67%)<br>
game:  ~39.49 MiB (~67%)<br>
<br>
<br>
(These numbers don’t match 793539*48 bytes because we end up eliding<br>
some checks. The relative savings in the data section will be the<br>
same, though)<br>
<br>
<br>
I also have a patch for minimizing ubsan source location data<br>
(parametrizable), which would apply to all checks, but with string<br>
merging, we end up not saving that much, in the size of cstring/rodata<br>
sections. This patch allows the user to drop the first N path<br>
components or only keep the last N patch components from the source<br>
locations emitted by clang.<br>
<br>
Before:<br>
clang: ~1.28 MiB<br>
game:  ~10.75 MiB<br>
<br>
After 0002-*.patch:<br>
clang: ~1.19 MiB (~93%)<br>
game:  ~8.46 MiB (~79%)<br>
<br>
<br>
<br>
Summing up:<br>
I’m proposing two patches, one saves a lot more than the other in the<br>
cases I’ve seen:<br>
  - Make static check data for type_mismatch 7 bytes smaller. This<br>
check is *by far* the most emitted one. And its static data (per check<br>
handler call) is 48B (with padding). Minimised version is 32B (with<br>
padding). (0003-*.patch)<br>
  - Add -fstrip-path-components-from-checks=N (do bike-shed…), which<br>
tells ubsan to strip the first N path components when emitting<br>
filename information (or only emit the last N, if N is negative).<br>
(0002-*.patch)<br>
<br>
The data minimizing patch might not be implementable while keeping<br>
library compatibility (as in: being able to use objects compiled with<br>
an older version of clang on a more recent ubsan library. We might be<br>
able to come up with a heuristic, though), but yields 1/3 savings in<br>
type_mismatch static data (not accounting for file names nor type<br>
descriptors).<br>
The file name minimizing patch is simple enough, and wouldn’t imply<br>
any changes to the sanitizer libraries.<br>
<br>
I know these patches are missing tests, this email is an RFC, to know<br>
if there is interest in having this upstream. Proper patches (updated<br>
with comments and tests) will follow depending on the response.<br>
<br>
Thank you,<br>
<br>
  Filipe<br>
<br>_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
<br></blockquote></div><br></div></div>