New issue
Advanced search Search tips
Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug

Blocked on:
issue 760615

Blocking:
issue 636111
issue 709690



Sign in to add a comment

(debug build) Cannot view NRVOed local variables with clang on Windows in MSVS's debuggers

Project Member Reported by jam@chromium.org, Aug 14 2017 Back to list

Issue description

See this simple page which just does document.cookie: https://jsfiddle.net/xe3m3yf6/ and add a breakpoint in std::string CookieStore::BuildCookieLine(const std::vector<CanonicalCookie*>& cookies) {

I can't see the local variable |cookie_line|'s value in MSVS. This is a normal debug build with the following gn args:
is_debug = true
enable_nacl = false
target_cpu = "x86"
is_win_fastlink = true
use_goma = true
goma_dir = "E:\src\goma"
symbol_level = 2
 

Comment 1 by thakis@chromium.org, Aug 14 2017

Blocking: 636111
(Kind of similar to issue 753934 / discussion inhttps://bugs.llvm.org/show_bug.cgi?id=34136 , but issue 753934 is about base::debug::Alias(). Also, it's about debug builds, where we expect this to currently work FWIU.)

Comment 2 by r...@chromium.org, Aug 14 2017

This is a debug build, so I doubt those are related. The fix for issue 753934 is in the instcombine optimization pass, so it won't fix this, unless we always compile this file with optimizations.

There's nothing obviously special about the code in question. I wonder if this is /debug:fastlink related.

Comment 3 by thakis@chromium.org, Aug 16 2017

Summary: (debug build) Local variable not shown with clang on Windows in MSVS's debuggers (was: Local variable not shown with clang on Windows in MSVS's debuggers)

Comment 4 by jam@chromium.org, Aug 16 2017

Also more examples: I put a breakpoint in NavigationRequest::CreateBrowserInitiated and can't see many variables like browser_initiated, request_body, is_form_submission, browser_initiated. VS just says "Error reading register value".

Comment 5 Deleted

I think there are multiple issues here, but here's a reduced repro for the first one |cookie_line|.

// cookies.cpp
// clang-cl.exe -m32 /Od /Z7 /MDd /GR- /c cookies.cpp /Focookies.obj
// link /debug cookies.obj /out:cookies.exe
#include <string>

std::string foo() {
  std::string bar;
  std::string baz;
  bar = "hello";
  bar += baz;
  return bar;
}

int main(int argc, char **argv) {
  std::string line = foo();
  return 0;
}

Note that /debug:fastlink is not required.

In this case, MSVC debugger says that |bar| is optimized away, but it happily displays |baz|.  This is interesting since |bar| would have NRVO applied to it if optimizations were on, but looking at the generated code you can see that we correctly don't apply NRVO (since we specified /Od).

Comment 7 by r...@chromium.org, Aug 16 2017

I'm pretty sure we do apply NRVO in that example, and that's what's up. That happens in clang regardless of mid-level optimization level.
I was looking at a similar case:

#include <iostream>
#include <string>

std::string debugfail() {
  std::string x;
  std::string input;
  for (;;) {
    std::cin >> input;
    if (input == "done") break;
    // if (input == "last") return input;
    x += input;
  }
  return x;
}

int main(int argc, char *argv[]) {
  std::cout << debugfail() << std::endl;
  return 0;
}

As rnk suspected, if I defeat NRVO by removing the comment markers from the alternative return path, both x and input can be viewed in the debugger. For the code as given above (with the alternate return commented out), this is what the relevant part of the PDB looks like:

      52 | S_GPROC32 [size = 52] `debugfail`
           parent = 0, end = 172, addr = 0001:40960, code size = 209
           type = `0x1002 (std::basic_string<char, std::cha...)`, debug start = 0, debug end = 0, flags = none
     104 | S_LOCAL [size = 16] `input`
           type=0x1000 (std::basic_string<char, std::cha...), flags = none
     120 | S_DEFRANGE_REGISTER_REL [size = 20]
           register = 335, base ptr = 96, offset in parent = 0, has spilled udt = false
           range = [0001:40985,+184), gaps =
     140 | S_LOCAL [size = 12] `x`
           type=0x1000 (std::basic_string<char, std::cha...), flags = none
     152 | S_DEFRANGE_REGISTER_REL [size = 20]
           register = 330, base ptr = 0, offset in parent = 0, has spilled udt = false
           range = [0001:40990,+5), gaps =

This encodes input relative to rsp, and x relative to rcx. I suspect the reason we can't view x in the debugger is that the range is really narrow.

Looking at the disassembly, there are a couple of different ways we could describe the location of x. Here, we have chosen rcx+0, but we could have chosen rsp+0x50, which is valid for a much larger range.
Cc: inglorion@chromium.org
Owner: inglorion@chromium.org
Status: Started
I got x to display in the debugger by hex-editing the pdb so that it now reads

     140 | S_LOCAL [size = 12] `x`
           type=0x1000 (std::basic_string<char, std::cha...), flags = none
     152 | S_DEFRANGE_REGISTER_REL [size = 20]
           register = 335, base ptr = 80, offset in parent = 0, has spilled udt = false
           range = [0001:40990,+179), gaps =

We really should be outtputing S_BPREL32 instead of S_LOCAL + S_DEFRANGE_REGISTER_REL.

Comment 11 by r...@chromium.org, Aug 21 2017

This is the NRVO bug. S_BPREPL32 won't fix this. There is no way in CodeView to express where clang puts the variable. We'll have to pretend it's a 'std::string&' that refers to the return value instead.

Comment 12 by r...@chromium.org, Aug 23 2017

Summary: (debug build) Cannot view NRVOed local variables with clang on Windows in MSVS's debuggers (was: (debug build) Local variable not shown with clang on Windows in MSVS's debuggers)

Comment 13 by r...@chromium.org, Aug 23 2017

Blocking: 709690
I landed a fix for this in LLVM https://reviews.llvm.org/rL312034. Once we do a Clang roll with that, this should be fixed for Chromium.
Blockedon: 760615

Comment 16 by h...@chromium.org, Sep 8 2017

The roll has landed.

jam & inglorion: can you verify whether this is good now?

Comment 17 by h...@chromium.org, Sep 14 2017

Ping?

Comment 18 by h...@chromium.org, Sep 20 2017

Status: Fixed
Using Chromium #503270 with https://chromium-review.googlesource.com/c/chromium/src/+/675867 patched in (though that one shouldn't affect this bug), and building with:

is_clang = true
is_debug = true
target_cpu = "x86"
use_goma = true
enable_nacl = false

breaking in and stepping through CookieStore::BuildCookieLine(const std::vector<CanonicalCookie*>& cookies) with Visual Studio, all locals look good including cookie_line.
Thanks, I've verified this.

Sign in to add a comment