New issue
Advanced search Search tips
Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug

Blocked on:
issue 782281

Blocking:
issue 636111



Sign in to add a comment

clang/win debug information incorrect when multiple inheritance is used

Project Member Reported by sorin@chromium.org, Nov 14 2017

Issue description

The debug information might be incorrect when multiple inheritance is used.

It appears that the this pointer points to the incorrect object slice when the execution flow under the debugger reaches a virtual function implemented by the 
derived class.

I've run the code under both windbg and the visual studio debuggers and the result is the same. Both debuggers show an incorrect value for the this pointer.

The Chromium code in question is at https://cs.chromium.org/chromium/src/components/update_client/url_fetcher_downloader.h?rcl=866b71163824c6c60ac37de03bda222f08221f44&l=28

class UrlFetcherDownloader : public CrxDownloader,
                             public net::URLFetcherDelegate

The bases for the inheritance above are abstract but not pure abstract classes (not sure if this is relevant or not).

Inside a member function of UrlFetcherDownloader, we instantiate a URL fetcher

url_fetcher_ = net::URLFetcher::Create(0, url, net::URLFetcher::GET, this,
                                         traffic_annotation);

and we expect the fetcher to call us back on its delegate methods. The code works at runtime, however, in the debugger, the "this" value inside of the delegate implementation of UrlFetcherDownloader is wrong. Therefore, the debugger shows garbage for the members of UrlFetcherDownloader.

The value of "this" passed as an argument to URLFetcher::Create	0x00000000403679c0

The value of "this" that the debugger shows inside UrlFetcherDownloader::OnURLFetchDownloadProgress

this	0x00000000403678f8 

If this is cast to UrlFetcherDownloader*, the value of the pointer is the correct "this". 

UrlFetcherDownloader* p = static_cast<UrlFetcherDownloader*>(this);
p	0x00000000403679c0

Inspecting the disassembly code and the value of RCX upon entering the virtual function implementation shows the correct value for "this", as expected.
RCX = 00000000403679C0

It looks like a debug info issue or something related to how the Windows debuggers introspect the program being debugged.
 

Comment 1 by sorin@chromium.org, Nov 14 2017

Blocking: 636111

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

Owner: r...@chromium.org
Status: Assigned (was: Untriaged)
Thanks for the report. This issue seems to happen because clang adjusts 'this' in the prologue and stores the adjusted value. MSVC does not. MSVC remembers the adjustment from net::URLFetcherDelegate to UrlFetcherDownloader and applies it on use.

I think there are two possible fixes:
1. Emit zero for the ThisAdjustment of virtual method overrides, since clang does the adjustment in code
2. Adjust clang's codegen pattern to match VC++

The best option would be if CodeView had a way to express that a variable is equal to a register or memory value plus some constant, but the format doesn't seem to support that.

Option 1 is problematic if users mix and match type information generated by MSVC and clang, because the type information will disagree, and the debugger may find either MSVC or clang's type information for the method.

The only quick fix is option 2, but it has downsides as well.

Separately, I've noticed that optimized debug info for 'this' is very wrong. Fixing the underlying debug build issue might improve optimized builds, but we should check back later.

Here's a reduced example:

#include <stdio.h>
struct A {
  virtual void f() = 0;
  int a = 1;
};
struct B {
  virtual void g() = 0;
  int b = 2;
};
struct C : A, B {
  void f() override;
  void g() override;
  int c = 3;
};
void C::f() {
  printf("f %d %d\n", a, c);
}
void C::g() {
  printf("g %d %d\n", b, c);
  __debugbreak(); // wrong 'this' here
}
int main() {
  C c;
  printf("c %p\n", &c);
  c.f();
  c.g();
}

Comment 3 by r...@chromium.org, Nov 16 2017

I got out a patch for option 2 today: https://reviews.llvm.org/D40109

I confirmed locally that windbg dumps 'this' correctly in C::g in that reduced example.

Comment 4 by r...@chromium.org, Nov 21 2017

Blockedon: 782281
This landed as r318440, and is in the clang roll that just landed. Will close when we let the clang roll bake for a day.

Comment 5 by h...@chromium.org, Nov 27 2017

Status: Fixed (was: Assigned)
The Clang roll appears to be sticking.

Sign in to add a comment