Stepping and breakpoints for multiline conditionals are surprising with Clang on Windows.

Makred as blocking 

The particular code I noticed this on was:

  if (!thumbnail_service ||
      !thumbnail_service->ShouldAcquirePageThumbnail(url, page_transition_)) {
    return;
  }

from https://cs.chromium.org/chromium/src/chrome/browser/thumbnails/thumbnail_tab_helper.cc?l=197

I set a breakpoint on the if condition and that breakpoint gets hit twice through the code, which was really confusing!

The assembly looks like this:

  // Skip if we don't need to update the thumbnail.
  if (!thumbnail_service ||
 call        scoped_refptr<thumbnails::ThumbnailService>::operator bool
 test        al,1  
 jne         ThumbnailTabHelper::UpdateThumbnailIfNecessary+237h (193EBC97h)  
 jmp         ThumbnailTabHelper::UpdateThumbnailIfNecessary+269h (193EBCC9h) 
      !thumbnail_service->ShouldAcquirePageThumbnail(url, page_transition_)) {
 lea         ecx,[thumbnail_service]  
 ...
 call        eax  

  // Skip if we don't need to update the thumbnail.
  if (!thumbnail_service ||
 test        al,1  
 jne         ThumbnailTabHelper::UpdateThumbnailIfNecessary+275h (193EBCD5h)  
    return;
 mov         dword ptr [esi+3Ch],1  
 jmp         ThumbnailTabHelper::UpdateThumbnailIfNecessary+2A4h (193EBD04h)  
  }

The symbols are blaming the "test al,1" and the "jne" associated with the condition back to the first line of the if statement. This makes theoretical sense but isn't what you expect when stepping through the program.

Testing with VC, it looks like it groups the entire if condition into one "statement" which you "Step" takes you over (Clang shows the two lines of the condition as separate statements). I'm not sure which of these schemes is better, it may depend on what you're trying to do.


 // Skip if we don't need to update the thumbnail.
  if (!thumbnail_service ||
 lea         ecx,[thumbnail_service]  
 call        scoped_refptr<thumbnails::ThumbnailService>::operator bool
 movzx       edx,al  
 test        edx,edx  
 je          ThumbnailTabHelper::UpdateThumbnailIfNecessary+1B3h
 lea         ecx,[thumbnail_service]  
 ...
 test        ecx,ecx  
 jne         ThumbnailTabHelper::UpdateThumbnailIfNecessary+1BDh
      !thumbnail_service->ShouldAcquirePageThumbnail(url, page_transition_)) {
    return;
 lea         ecx,[thumbnail_service]  
 call        scoped_refptr<thumbnails::ThumbnailService>::~scoped_refptr<thumbnails::ThumbnailService>
 jmp         ThumbnailTabHelper::UpdateThumbnailIfNecessary+1D9h
  }

I think the solution is to always blame the corresponding test/jump lines of a condition to the immediately previous statement that they're testing, rather than the if statement as a whole.
 

Stepping and breakpoints for multiline conditionals are surprising with Clang on Windows.

The particular code I noticed this on was:

  if (!thumbnail_service ||
      !thumbnail_service->ShouldAcquirePageThumbnail(url, page_transition_)) {
    return;
  }

from https://cs.chromium.org/chromium/src/chrome/browser/thumbnails/thumbnail_tab_helper.cc?l=197

I set a breakpoint on the if condition and that breakpoint gets hit twice through the code, which was really confusing!

The assembly looks like this:

  // Skip if we don't need to update the thumbnail.
  if (!thumbnail_service ||
 call        scoped_refptr<thumbnails::ThumbnailService>::operator bool
 test        al,1  
 jne         ThumbnailTabHelper::UpdateThumbnailIfNecessary+237h (193EBC97h)  
 jmp         ThumbnailTabHelper::UpdateThumbnailIfNecessary+269h (193EBCC9h) 
      !thumbnail_service->ShouldAcquirePageThumbnail(url, page_transition_)) {
 lea         ecx,[thumbnail_service]  
 ...
 call        eax  

  // Skip if we don't need to update the thumbnail.
  if (!thumbnail_service ||
 test        al,1  
 jne         ThumbnailTabHelper::UpdateThumbnailIfNecessary+275h (193EBCD5h)  
    return;
 mov         dword ptr [esi+3Ch],1  
 jmp         ThumbnailTabHelper::UpdateThumbnailIfNecessary+2A4h (193EBD04h)  
  }

The symbols are blaming the "test al,1" and the "jne" associated with the condition back to the first line of the if statement. This makes theoretical sense but isn't what you expect when stepping through the program.

Testing with VC, it looks like it groups the entire if condition into one "statement" which you "Step" takes you over (Clang shows the two lines of the condition as separate statements). I'm not sure which of these schemes is better, it may depend on what you're trying to do.


 // Skip if we don't need to update the thumbnail.
  if (!thumbnail_service ||
 lea         ecx,[thumbnail_service]  
 call        scoped_refptr<thumbnails::ThumbnailService>::operator bool
 movzx       edx,al  
 test        edx,edx  
 je          ThumbnailTabHelper::UpdateThumbnailIfNecessary+1B3h
 lea         ecx,[thumbnail_service]  
 ...
 test        ecx,ecx  
 jne         ThumbnailTabHelper::UpdateThumbnailIfNecessary+1BDh
      !thumbnail_service->ShouldAcquirePageThumbnail(url, page_transition_)) {
    return;
 lea         ecx,[thumbnail_service]  
 call        scoped_refptr<thumbnails::ThumbnailService>::~scoped_refptr<thumbnails::ThumbnailService>
 jmp         ThumbnailTabHelper::UpdateThumbnailIfNecessary+1D9h
  }

I think the solution is to always blame the corresponding test/jump lines of a condition to the immediately previous statement that they're testing, rather than the if statement as a whole.
 

Stepping and breakpoints for multiline conditionals are surprising with Clang on Windows.

The particular code I noticed this on was:

  if (!thumbnail_service ||
      !thumbnail_service->ShouldAcquirePageThumbnail(url, page_transition_)) {
    return;
  }

from https://cs.chromium.org/chromium/src/chrome/browser/thumbnails/thumbnail_tab_helper.cc?l=197

I set a breakpoint on the if condition and that breakpoint gets hit twice through the code, which was really confusing!

The assembly looks like this:

      // Skip if we don't need to update the thumbnail.
      if (!thumbnail_service ||
 call        scoped_refptr<thumbnails::ThumbnailService>::operator bool
 test        al,1  
 jne         ThumbnailTabHelper::UpdateThumbnailIfNecessary+237h (193EBC97h)  
 jmp         ThumbnailTabHelper::UpdateThumbnailIfNecessary+269h (193EBCC9h) 
          !thumbnail_service->ShouldAcquirePageThumbnail(url, page_transition_)) {
 lea         ecx,[thumbnail_service]  
 ...
 call        eax  

      // Skip if we don't need to update the thumbnail.
      if (!thumbnail_service ||
 test        al,1  
 jne         ThumbnailTabHelper::UpdateThumbnailIfNecessary+275h (193EBCD5h)  
        return;
 mov         dword ptr [esi+3Ch],1  
 jmp         ThumbnailTabHelper::UpdateThumbnailIfNecessary+2A4h (193EBD04h)  
  }

The symbols are blaming the "test al,1" and the "jne" associated with the condition back to the first line of the if statement. This makes theoretical sense but isn't what you expect when stepping through the program.

Testing with VC, it looks like it groups the entire if condition into one "statement" which you "Step" takes you over (Clang shows the two lines of the condition as separate statements). I'm not sure which of these schemes is better, it may depend on what you're trying to do.


      // Skip if we don't need to update the thumbnail.
      if (!thumbnail_service ||
 lea         ecx,[thumbnail_service]  
 call        scoped_refptr<thumbnails::ThumbnailService>::operator bool
 movzx       edx,al  
 test        edx,edx  
 je          ThumbnailTabHelper::UpdateThumbnailIfNecessary+1B3h
 lea         ecx,[thumbnail_service]  
 ...
 test        ecx,ecx  
 jne         ThumbnailTabHelper::UpdateThumbnailIfNecessary+1BDh
          !thumbnail_service->ShouldAcquirePageThumbnail(url, page_transition_)) {
        return;
 lea         ecx,[thumbnail_service]  
 call        scoped_refptr<thumbnails::ThumbnailService>::~scoped_refptr<thumbnails::ThumbnailService>
 jmp         ThumbnailTabHelper::UpdateThumbnailIfNecessary+1D9h
      }

I think the solution is to always blame the corresponding test/jump lines of a condition to the immediately previous statement that they're testing, rather than the if statement as a whole.
 

Stepping and breakpoints for multiline conditionals are surprising with Clang on Windows.

The particular code I noticed this on was:

  if (!thumbnail_service ||
      !thumbnail_service->ShouldAcquirePageThumbnail(url, page_transition_)) {
    return;
  }

from https://cs.chromium.org/chromium/src/chrome/browser/thumbnails/thumbnail_tab_helper.cc?l=197

I set a breakpoint on the if condition and that breakpoint gets hit twice through the code, which was really confusing!

The assembly looks like this:

      // Skip if we don't need to update the thumbnail.
      if (!thumbnail_service ||
 call        scoped_refptr<thumbnails::ThumbnailService>::operator bool
 test        al,1  
 jne         ThumbnailTabHelper::UpdateThumbnailIfNecessary+237h (193EBC97h)  
 jmp         ThumbnailTabHelper::UpdateThumbnailIfNecessary+269h (193EBCC9h) 
          !thumbnail_service->ShouldAcquirePageThumbnail(url, page_transition_)) {
 lea         ecx,[thumbnail_service]  
 ...
 call        eax  

      // Skip if we don't need to update the thumbnail.
      if (!thumbnail_service ||
 test        al,1  
 jne         ThumbnailTabHelper::UpdateThumbnailIfNecessary+275h (193EBCD5h)  
        return;
 mov         dword ptr [esi+3Ch],1  
 jmp         ThumbnailTabHelper::UpdateThumbnailIfNecessary+2A4h (193EBD04h)  
      }

The symbols are blaming the "test al,1" and the "jne" associated with the condition back to the first line of the if statement. This makes theoretical sense but isn't what you expect when stepping through the program.

Testing with VC, it looks like it groups the entire if condition into one "statement" which you "Step" takes you over (Clang shows the two lines of the condition as separate statements). I'm not sure which of these schemes is better, it may depend on what you're trying to do.


      // Skip if we don't need to update the thumbnail.
      if (!thumbnail_service ||
 lea         ecx,[thumbnail_service]  
 call        scoped_refptr<thumbnails::ThumbnailService>::operator bool
 movzx       edx,al  
 test        edx,edx  
 je          ThumbnailTabHelper::UpdateThumbnailIfNecessary+1B3h
 lea         ecx,[thumbnail_service]  
 ...
 test        ecx,ecx  
 jne         ThumbnailTabHelper::UpdateThumbnailIfNecessary+1BDh
          !thumbnail_service->ShouldAcquirePageThumbnail(url, page_transition_)) {
        return;
 lea         ecx,[thumbnail_service]  
 call        scoped_refptr<thumbnails::ThumbnailService>::~scoped_refptr<thumbnails::ThumbnailService>
 jmp         ThumbnailTabHelper::UpdateThumbnailIfNecessary+1D9h
      }

I think the solution is to always blame the corresponding test/jump lines of a condition to the immediately previous statement that they're testing, rather than the if statement as a whole.