On 11/02/2016 03:20 PM, Marek Polacek wrote:
> On Wed, Nov 02, 2016 at 11:10:53AM +0100, Jakub Jelinek wrote:
>> On Wed, Nov 02, 2016 at 10:59:26AM +0100, Jakub Jelinek wrote:
>>>> Which is gimplified as:
>>>>
>>>>     int * ptr;
>>>>
>>>>     switch (argc) <default: <D.2575>, case 1: <D.2573>>
>>>>     {
>>>>       int a;
>>>>
>>>>       try
>>>>         {
>>>>           ASAN_MARK (2, &a, 4);
>>>>           <D.2573>:
>>>>           goto <D.2574>;
>>>>           <D.2575>:
>>>>           ptr = &a;
>>>>           goto <D.2574>;
>>>>         }
>>>>       finally
>>>>         {
>>>>           ASAN_MARK (1, &a, 4);
>>>>         }
>>
>>> Shouldn't there be also ASAN_MARK on the implicit gotos from the switch
>>> statement?  Otherwise, consider this being done in a loop, after the first
>>> iteration you ASAN_MARK (1, &a, 4) (i.e. poison), then you iterate say with
>>> args 1 and have in case 1: a = 0;, won't that trigger runtime error?
>>
>> Wonder if for the variables declared inside of switch body, because we don't
>> care about uses before scope, it wouldn't be more efficient to arrange for
>> int *p, *q, *r;
>> switch (x)
>>   {
>>     int a;
>>   case 1:
>>     p = &a;
>>     a = 5;
>>     break;
>>     int b;
>>   case 2:
>>     int c;
>>     q = &b;
>>     r = &c;
>>     b = 3;
>>     c = 4;
>>     break;
>>   }
>> to effectively ASAN_MARK (2, &a, 4); ASAN_MARK (2, &b, 4); ASAN_MARK (2, &c, 
>> 4);
>> before the GIMPLE_SWITCH, instead of unpoisoning them on every case label
>> where they might be in scope.  Though, of course, at least until lower pass
>> that is quite ugly, because it would refer to "not yet declared" variables.
>> Perhaps we'd need to move the ASAN_MARK and GIMPLE_SWITCH (but of course not
>> the expression evaluation of the switch control expression) inside of the
>> switches' GIMPLE_BIND.
> 
> So is there anything I should do wrt -Wswitch-unreachable?
> 
>       Marek
> 

Probably not. I'm having a patch puts GIMPLE_SWITCH statement to a proper place
in GIMPLE_BIND. Let's see whether such patch can bootstrap and survive 
regression
tests.

Thanks,
Martin

Reply via email to