<div style="font-family: arial, helvetica, sans-serif; font-size: 10pt">Assuming the implemented flag naming is ok with everyone, the patch is good. <div>Few questions:</div><div> </div><div>+ unsigned Kind; </div>
<div> Do we only allow 32 sanitizers? Why not 64? </div><div><br></div><div>Do we want to change -faddress-sanitizer in asan/tsan lit tests? </div><div> projects/compiler-rt/lib/asan/lit_tests</div><div> projects/compiler-rt/lib/tsan/lit_tests</div>
<div><br></div><div>How long do we keep the aliases (-faddress-sanitizer => fsanitizer=address)? </div><div>(I like the old name more than the new name, but having two names is confusing)</div><div><br></div><div>Will this allow to easily add *san-specific subflags? </div>
<div>E.g. --sanitizer=address,global-init-order,use-after-return,use-after-scope or --sanitizer=memory,memory-origins</div><div>(The first one indicates 3 asan sub passes that are curretly off by default, the second one indicates that msan should be run with "track origins" feature)</div>
<div><br></div><div>--kcc </div><div><br></div><div><br><br><div class="gmail_quote">On Fri, Nov 2, 2012 at 10:03 AM, Kostya Serebryany <span dir="ltr"><<a href="mailto:kcc@google.com" target="_blank">kcc@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="font-family:arial,helvetica,sans-serif;font-size:10pt"><br><br><div class="gmail_quote"><div class="im">On Fri, Nov 2, 2012 at 9:41 AM, Alexander Potapenko <span dir="ltr"><<a href="mailto:glider@google.com" target="_blank">glider@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><p dir="ltr">I haven't seen the patch yet, but here are two thoughts:<br>
- some tools may be incompatible with each other (e.g. ASan and TSan), so we shouldn't allow to use them together;<br>
- there are many users of TSan and ASan, thus we can't easily rename the corresponding command line options. The right thing to do is to make -fsanitize fully functional, announce that and then remove the existing flags.<br>
I'm also unsure whether having such an umbrella flag for such different tools won't confuse the users.</p></blockquote></div><div>As Richard says, the old flags remain (they become synonyms of the new flags). That sounds fine. </div>
<div>I'll check the patches later today. </div><div><br></div><div>--kcc </div><div><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>
<div>
<div class="gmail_quote">On Nov 2, 2012 4:51 AM, "Richard Smith" <<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hi,<br>
<br>
The attached patch series adds a<br>
-fsanitize=sanitizer1,sanitizer2,sanitizer3 command-line option to<br>
Clang (and a corresponding -fno-sanitize=...). This works as follows<br>
for the driver options:<br>
<br>
-faddress-sanitizer becomes a synonym for -fsanitize=address<br>
-fno-address-sanitizer becomes a synonym for -fno-sanitize=address<br>
-fthread-sanitizer becomes a synonym for -fsanitize=thread<br>
-fno-thread-sanitizer becomes a synonym for -fno-sanitize=thread<br>
-fcatch-undefined-behavior becomes a synonym for -fsanitize=undefined<br>
-fsanitize=undefined is expanded by the driver into a list of<br>
individual undefined behavior sanitizers<br>
<br>
The -cc1 interface is deliberately very simplistic: it loses support<br>
for the existing flags, and instead only supports -fsanitize=..., and<br>
only supports naming the individual checks (and not groups like<br>
-fsanitize=undefined).<br>
<br>
Patch 1 just renames the existing ASan and TSan LangOptions to follow<br>
the naming convention used by the later patches.<br>
Patch 2 adds support to the driver and frontend for parsing<br>
-fsanitize, converting the other options to it, and filling in<br>
LangOptions.<br>
Patch 3 uses the -fsanitize=... values to enable/disable the existing<br>
-fcatch-undefined-behavior checks, and removes frontend support for<br>
the other options.<br>
<br>
Please review! I'd like to get this in for the 3.2 release.<br>
<br>
Thanks!<br>
Richard<br>
</blockquote></div>
</div></div></blockquote></div></div></div><br></div>
</blockquote></div><br></div></div>