[PATCH] Speed-up use-after-scope (re-writing to SSA) (version 2)

[Martin Liška]

On 01/20/2017 03:27 PM, Jakub Jelinek wrote:
> On Fri, Jan 20, 2017 at 03:08:21PM +0100, Martin Liška wrote:
>> Unfortunately this way would not work as clobber marks content of the memory as uninitialize
>> is different behavior that just marking a memory can be used (and maybe already contains a value).
>>
>> This shows the problem:
>>
>> #include <string.h>
>>
>> char cc;
>> char ptr[] = "sparta2";
>>
>> void get(char **x)
>> {
>>   *x = ptr;
>> }
>>   
>> int main()
>> {
>>   char *here = &cc;
>>
>>   for (;;)
>>     {
>>     next_line:
>> 	if (here == NULL)
>> 	  __builtin_abort();
>> 	get (&here);
>> 	if (strcmp (here, "sparta") == 0)
>> 	    goto next_line;
>> 	else if (strcmp (here, "sparta2") == 0)
>> 	  break;
>>     }
>> }
>>
>> With the patch, DSE would optimize out '*here = &cc;' and thus aborts. The problem is definitely
>> related to goto magic, where we are more defensive in placement of ASAN_MARK(UNPOISON,...).
>> Hope your optimization is still valid for situations w/o artificial ASAN_MARK(UNPOISON,...) placed due
>> to goto magic.
>>
>> Do we still want to do it now, or postponing to GCC 8 would be better option?
> 
> I'd still like to resolve it for GCC 7 if at all possible, I think otherwise
> -fsanitize=address is by default unnecessarily slower (so it is a regression
> anyway).

Good, I hope I have patch that finally works as we want. It add attribute to variables that
are unpoisoned as live switch variables, or are defined in a label which address is taken.

With [1] I can bootstrap-asan and the patch can bootstrap on ppc64le-redhat-linux
and survives regression tests.

I'm sending latest diff, as well as the final pair of patches I would like to install.

Ready to be installed?
Martin

> So, do we always amit ASAN_MARK(UNPOISON, ...) at the start of scope and
> then yet another ASAN_MARK(UNPOISON, ...) at the goto destination?
> At least on the above testcase that is the case, so if we say split
> ASAN_MARK_UNPOISON into something that is used at the start of scope
> (we'd emit the clobber for those) and others (we would not), then perhaps we
> could get around that.  The above is BTW a clear case where shouldn't emit
> UNPOISON on the label, as the goto doesn't cross its initialization.
> But I can see with gotos from outside of some var's scope into it we
> wouldn't handle it properly.  Perhaps for now set some
> flag/attribute/whatever on vars for which we emit the conservative
> UNPOISON and never allow those to be made non-addressable (i.e. for those
> say that POISON/UNPOISON actually makes them always addressable)?
> 
> 	Jakub
> 

[1] https://gcc.gnu.org/ml/gcc-patches/2017-01/msg01446.html

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Speed-up-use-after-scope-v2-rewrite-into-SSA.patch
Type: text/x-patch
Size: 13828 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20170123/339bd941/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-use-after-scope-handle-writes-to-a-poisoned-variable.patch
Type: text/x-patch
Size: 11654 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20170123/339bd941/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: use-after-scope-diff.patch
Type: text/x-patch
Size: 1384 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20170123/339bd941/attachment-0002.bin>