New issue
Advanced search Search tips
Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug

Blocked on:
issue 735328

Blocking:
issue 636111



Sign in to add a comment

Incorrect line when debug stepping with clang on Windows

Project Member Reported by scottmg@chromium.org, May 26 2017

Issue description

I just spent a while trying to figure out why 

if (condition())
  return;

was returning early out of a function when I was *sure* condition() should be false. And indeed it turned out it was false.

When stepping, clang binaries step on to the "return;" line, even though it's never executed. 

Given:
https://chromium-review.googlesource.com/c/517425/1/base/test/run_all_base_unittests.cc

Stepping through Test() in with is_clang=true (i.e. F10 repeatedly), is_clang=true stops with the cursor on lines 20, 21, and 22.

With is_clang=false, the cursor stops on lines 20 and 22 as expected.

 

Comment 1 by thakis@chromium.org, May 26 2017

Blocking: 636111

Comment 2 by r...@chromium.org, May 31 2017

Was this with optimizations enabled? This didn't repro for me outside of Chrome, in either windbg or VS. Pushing F10 stopped at 'if', 'printf', then '}'.
Darn, I didn't paste my args.gn or any other relevant version info, sorry. Let me try to repro again.
Here's a complete log:

c:\src\cr\src>git new step-testing
Checking out files: 100% (3160/3160), done.
Branch step-testing set up to track remote branch master from origin.
Switched to a new branch 'step-testing'

[step-testing]c:\src\cr\src>git cl patch 517425
remote: Counting objects: 375671, done
remote: Finding sources: 100% (5/5)
remote: Total 5 (delta 0), reused 3 (delta 0)U
Unpacking objects: 100% (5/5), done.
From https://chromium.googlesource.com/chromium/src
 * branch            refs/changes/25/517425/2 -> FETCH_HEAD
Committed patch for change 517425 patchset 2 locally

[step-testing]c:\src\cr\src>cat out\debug\args.gn
is_debug = true
is_component_build = true
enable_nacl = false
is_chrome_branded = true
symbol_level = 2
target_cpu = "x86"
is_win_fastlink = true
is_clang = true
win_console_app = true
win_linker_timing = true
use_goma = true

[step-testing]c:\src\cr\src>type third_party\llvm-build\cr_build_revision
303910-1

[step-testing]c:\src\cr\src>ninja -C out\Debug base_unittests
ninja: Entering directory `out\Debug'
[0->1/1 ~1] Regenerating ninja files
...
[0->296/296 ~1] LINK base_unittests.exe base_unittests.exe.pdb

[step-testing]c:\src\cr\src>dbg15 out\debug\base_unittests.exe

Then F10 in Test() hits lines 19,20,21,22,23 for me.

Flipping to is_clang=false/use_goma=false hits lines 19,20,22,23.

Possibly also relevant, I'm debugging in VS2017.

Comment 5 by r...@chromium.org, Jun 30 2017

OK, my repro didn't work because this appears to be 32-bit only. This standalone program has the same issue:

#include <stdio.h>
bool Canceled() { return false; }
void Test() {
  if (Canceled())
    return;
  printf("do something else\n");
}
int main(int argc, char **argv) {
  Test();
  return 0;
}

Comment 6 by r...@chromium.org, Jun 30 2017

Owner: r...@chromium.org
The problem is pretty apparent from the assembly:

$ clang -S -m32 -g -gcodeview  -o - t.cpp  | grep -B2 -A2 t.cpp:[56]
LBB0_1:                                 # %if.then
Ltmp2:
        .cv_loc 0 1 5 5                 # t.cpp:5:5
        jmp     LBB0_3
Ltmp3:
LBB0_2:                                 # %if.end
        leal    "??_C@_0BD@PIPOCKPE@do?5something?5else?6?$AA@", %eax
        .cv_loc 0 1 6 3                 # t.cpp:6:3
        movl    %eax, (%esp)
        calll   _printf

$ clang -S -m64 -g -gcodeview  -o - t.cpp  | grep -B2 -A2 t.cpp:[56]
.LBB0_1:                                # %if.then
.Ltmp1:
        .cv_loc 0 1 5 5                 # t.cpp:5:5
        jmp     .LBB0_3
.Ltmp2:
.LBB0_2:                                # %if.end
        .cv_loc 0 1 6 3                 # t.cpp:6:3
        leaq    "??_C@_0BD@PIPOCKPE@do?5something?5else?6?$AA@"(%rip), %rcx
        callq   printf


The .cv_loc directive indicating that we've reached t.cpp:6 is on the wrong side of the LEA instruction in the 32-bit version. The debugger stops on the LEA, and says, I'm still on line 5, so that's what I'll highlight in the debugger.

I know Paul Robinson and Sony were making changes in this area. They're trying to remove location information from these kinds of constant materialization instructions to the debugger "jumps around less". In this case, it may have regressed our line tables at -O0.

Comment 7 by r...@chromium.org, Jun 30 2017

Yeah, this might have been LLVM r289468. Paul only updated the DWARF line tables, not CV line tables.

Anyway, it should be better after r306889. We'll have to do a clang roll. Everyone's on vacation so this will probably happen in a few weeks.

The amusing thing about this bug is that inserting more printfs or LOGs would make the stepping more confusing because of the string literal materialization.
Labels: TE-NeedsTriageHelp
Seems issue is coding related which is out of TE scope.Hence adding TE-NeedsTriagehelp label.

Comment 9 by r...@chromium.org, Jul 7 2017

Blockedon: 735328
Status: Assigned (was: Unconfirmed)
Status: Fixed (was: Assigned)
https://chromium-review.googlesource.com/566280 rolled in r306889, this should be better now.

Sign in to add a comment