New issue
Advanced search Search tips
Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 17
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocked on:
issue 769761

Blocking:
issue 636111
issue 709690



Sign in to add a comment

(debug build) Local variable shows null in some stack frames in MSVS

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

Issue description

This is the same gn args as in  bug 755213 .

In content_shell, load https://httpbin.org/forms/post and hit submit. Might need to reload as well. Run with --enable-browser-side-navigation.

Putting a breakpoint in NavigationEntryImpl::ConstructCommonNavigationParams, I see |post_body| has data. However the really strange thing is that one method down in the stack (NavigationRequest::CreateBrowserInitiated), request_body just shows "null" in MSVS's debugger.
 

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

Blocking: 636111
Owner: h...@chromium.org
Status: Assigned
If we can repro this in chrome, see if we can make a minimal repro outside of chrome so we understand the problem.

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

Blocking: 709690

Comment 4 by wfh@google.com, Aug 24 2017

Cc: wfh@chromium.org
This seems to now be working.

Here is what I see after I built chrome with the provided gn args and clang r312049:

NavigationRequest::CreateBrowserInitiated and NavigationEntryImpl::ConstructCommonNavigationParams are entered before I actually submit the form. I don't look at those invocations. I submit the form. This doesn't invoke either method. Then I use Ctl+R to re-submit the form.

The breakpoint in CreateBrowserInitiated is hit. post_body is shown as  {ptr_=0x00000000 <NULL> }. I step through the method, and, consistent with that value, request_body is initialized by calling frame_entry.GetPostData(). At that point, request_body shows up as a non-null value {ptr_=0x36c4c118 {elements_={ size=1 }, contains_sensitive_info_=false, ... }

@jam, does that look like it's working now?
Cc: inglorion@chromium.org

Comment 7 by thakis@chromium.org, Aug 30 2017

"Now" meaning with currently-pinned clang? If so, that has rnk's local patch for optimized debug info. Does it work without that too?
Sorry, I should have been clearer. I tested after I built with clang r312049, not with pinned clang. My clang build does not have rnk's D36596 applied.

Comment 9 by thakis@chromium.org, Aug 30 2017

In that case, jam can't verify this yet since we're not rolled to that revision yet :-) we should make a "roll again" bug and Mark the bugs we believe to be fixed with the roll as blocked on that roll bug.

Comment 10 by h...@chromium.org, Aug 30 2017

Blockedon: 760615

Comment 11 by h...@chromium.org, Aug 30 2017

Filed  crbug.com/760615  for the roll.

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

Owner: inglorion@chromium.org
Clang rolled this morning.

jam & inglorion, can you verify whether this is fixed?
I grabbed a pre-built Chrome and tried a number of things, but couldn't get it to break in CreateBrowserInitiated. Hopefully, someone else will be able to educate me on how to do this, and/or confirm that the problem is fixed now.

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

Owner: r...@chromium.org
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

"c:\Program Files (x86)\Microsoft Visual Studio 14.0\Common7\IDE\devenv.exe" /debugexe out\release\chrome.exe --enable-browser-side-navigation


Breaking with VS in NavigationEntryImpl::ConstructCommonNavigationParams (ctrl-r to re-submit the form and force this code path), looking at the parent stack frame (NavigationRequest::CreateBrowserInitiated) shows request_body as "Error reading register value".

The line in NavigationRequest::CreateBrowserInitiated looks like:

    CommonNavigationParams common_params = entry.ConstructCommonNavigationParams(
      frame_entry, request_body, dest_url, dest_referrer, navigation_type,
      previews_state, navigation_start);

And the call sequence:


  CommonNavigationParams common_params = entry.ConstructCommonNavigationParams(
097DAE55  mov         ecx,dword ptr [entry]  
097DAE58  mov         edx,dword ptr [navigation_start]  
097DAE5B  mov         edi,dword ptr [previews_state]  
097DAE5E  mov         ebx,dword ptr [navigation_type]  
097DAE61  mov         eax,dword ptr [dest_referrer]  
097DAE64  mov         dword ptr [esi+70h],eax  
097DAE67  mov         eax,dword ptr [dest_url]  
097DAE6A  mov         dword ptr [esi+6Ch],eax  
097DAE6D  mov         eax,dword ptr [frame_entry]  
097DAE70  sub         esp,20h  
097DAE73  mov         dword ptr [esi+68h],eax  
097DAE76  mov         eax,esp  
097DAE78  mov         dword ptr [eax+1Ch],edx  
097DAE7B  mov         dword ptr [eax+18h],edi  
097DAE7E  mov         dword ptr [eax+14h],ebx  
097DAE81  mov         edx,dword ptr [esi+70h]  
097DAE84  mov         dword ptr [eax+10h],edx  
097DAE87  mov         edx,dword ptr [esi+6Ch]  
097DAE8A  mov         dword ptr [eax+0Ch],edx  
097DAE8D  lea         edx,[request_body]            <----- We're passing the address..
097DAE93  mov         dword ptr [eax+8],edx  
097DAE96  mov         edx,dword ptr [esi+68h]  
097DAE99  mov         dword ptr [eax+4],edx  
097DAE9C  lea         edx,[common_params]  
097DAEA2  mov         dword ptr [eax],edx  
097DAEA4  mov         dword ptr [esi+64h],edx  
097DAEA7  call        content::NavigationEntryImpl::ConstructCommonNavigationParams (071D1230h)  
      frame_entry, request_body, dest_url, dest_referrer, navigation_type,
      previews_state, navigation_start);


request_body is a scoped_refptr so it's passed inalloca. rnk, do you think we get the debug info wrong here?

Comment 15 by h...@chromium.org, Sep 21 2017

My bad, everything's passed by reference so there's no inalloca.

I haven't found reduced test case that fails yet.

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

Owner: h...@chromium.org

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

I still haven't been able to create a stand-alone repro.

Attaching the source and build command to make it easier to work with the original source.

..\..\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe  "-cc1" "-triple" "i386-pc-windows-msvc19.0.0" "-emit-obj" "-mrelax-all" "-mincremental-linker-compatible" "-mrelocation-model" "static" "-mthread-model" "posix" "-mdisable-fp-elim" "-relaxed-aliasing" "-fmath-errno" "-masm-verbose" "-mconstructor-aliases" "-target-cpu" "pentium4" "--dependent-lib=msvcrtd" "--dependent-lib=oldnames" "-fno-rtti-data" "-stack-protector" "2" "-gcodeview" "-fms-volatile" "-gcodeview" "-debug-info-kind=limited" "-debugger-tuning=gdb" "-ffunction-sections"  "-O0" "-fdeprecated-macro" "-fdebug-compilation-dir" "C:\\src\\chromium\\src\\out\\release" "-fno-use-cxa-atexit" "-fms-extensions" "-fms-compatibility" "-fms-compatibility-version=19.0" "-std=c++14" "-fdelayed-template-parsing" "-fno-inline" "-mllvm" "-instcombine-lower-dbg-declare=1" -w "-o" "obj/content/browser/browser/navigation_entry_impl.obj" "-x" "c++" \src\tmp\a.ii
a.zip
1.2 MB Download

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

Sigh, wrong file.. it's navigation_request.cc that has the parent frame.
a.zip
1.3 MB Download

Comment 19 by h...@chromium.org, Sep 22 2017

Annotated asm for the call:

LBB3496_8:                              # %cond.end
  #DEBUG_VALUE: CreateBrowserInitiated:request_body <- [DW_OP_plus_uconst 168, DW_OP_deref] [%ESI+0]
  #DEBUG_VALUE: CreateBrowserInitiated:navigation_request <- [DW_OP_plus_uconst 204, DW_OP_deref] [%ESI+0]
  .cv_loc 3496 175 240 0          # ../../content/browser/frame_host/navigation_request.cc:240:0
  movl  28(%ebp), %ecx
  calll "?is_renderer_initiated@NavigationEntryImpl@content@@QBE_NXZ"
  xorb  $1, %al
  movb  %al, 212(%esi)
  .cv_loc 3496 175 242 0          # ../../content/browser/frame_host/navigation_request.cc:242:0
  movl  28(%ebp), %ecx
  movl  52(%ebp), %edx
  movl  36(%ebp), %edi
  movl  32(%ebp), %ebx
  movl  20(%ebp), %eax
  movl  %eax, 112(%esi)         # 4-byte Spill
  movl  16(%ebp), %eax
  movl  %eax, 108(%esi)         # 4-byte Spill
  movl  24(%ebp), %eax
  subl  $32, %esp
  movl  %eax, 104(%esi)         # 4-byte Spill
  movl  %esp, %eax
  movl  %edx, 28(%eax)
  movl  %edi, 24(%eax)
  movl  %ebx, 20(%eax)
  movl  112(%esi), %edx         # 4-byte Reload
  movl  %edx, 16(%eax)
  movl  108(%esi), %edx         # 4-byte Reload
  movl  %edx, 12(%eax)
  leal  248(%esi), %edx
  movl  %edx, 8(%eax)
  movl  104(%esi), %edx         # 4-byte Reload
  movl  %edx, 4(%eax)
  leal  968(%esi), %edx
  movl  %edx, (%eax)
  movl  %edx, 100(%esi)         # 4-byte Spill
  calll "?ConstructCommonNavigationParams@NavigationEntryImpl@content@@QBE?AUCommonNavigationParams@2@ABVFrameNavigationEntry@2@ABV?$scoped_refptr@VResourceRequestBody@content@@@@ABVGURL@@ABUReferrer@2@W4Value@FrameMsg_Navigate_Type@@HABVTimeTicks@base@@@Z"


We're using %esi to point to locals because we're realigned the stack. And I think that contributes to the problem :-) Smaller repro coming up..

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

Small repro:

struct S {
  ~S() {}
  void *p;
};

int g(S s, double*) {
  return 42;
}

int main() {
  double d = 0;
  S s = {0};

  g(s, &d);

  return 0;
}

Build with:

clang-cl -m32 /O0 /Z7 \src\tmp\a.cc


Open in VS debugger, break on entry to g() and try to inspect the locals of the parent stack frame (main). d shows "Error reading register value."

We're in business :-)
So, the repro in #20 doesn't work if I actually step into g(). I must have not done that when I last tried (but I was sure I did).

Well, that's good I suppose.


What's not so good is that the original issue in #0 still repros. And it doesn't report "Error reading register value" that I ran into, but post_body being null in the parent frame. And it still is.

This was all using the same clang version as in #20. I'll sync up and see if it still happens with the latest.
Argh, it's request_body, not post_body, that was reported to be null in the parent frame according to #0. And it's not when I look at it now; it looks entirely correct.
I went back to Chromium #494316 (Aug 14, so around the time jam filed this bug) and did a build with

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

request_body does indeed show as null in the parent of NavigationEntryImpl::ConstructCommonNavigationParams.


Using the same Chromium revision, but with the current Clang (313786) (need to build with -Werror disabled), and it still reproduces.

(I'm guessing the reason it doesn't seem to repro with trunk Chromium is due to the frame layouts having changed.)


Looking at the parent frame, other values seem off too. browser_initiated shows true (4) and some of the pointers seem off.

I think the "Error reading register value" was a red herring from me not stepping into the callee function (it might still be something we can do better at), and the real issue is that some frame offsets are not right.
This is what the locals look right before the call:

		content::NavigationEntryImpl::is_renderer_initiated returned	false	bool
		browser_initiated	true	bool
+		common_params	{url="enable-browser-side-navigation" referrer={url="àäÁV0äÆVP®\x19" policy=1455875120 } transition=...}	content::CommonNavigationParams
+		controller	0x31ee71b8 {browser_context_=0x2f7220f8 {pref_change_registrar_={observers_={...} service_=0x2f7708a8 {...} } ...} ...}	content::NavigationControllerImpl *
+		dest_referrer	{url="https://httpbin.org/forms/post" policy=kWebReferrerPolicyNoReferrerWhenDowngrade (2) }	const content::Referrer &
+		dest_url	"https://httpbin.org/post"	const GURL &
+		entry	{frame_tree_=unique_ptr {parent=0x00000000 <NULL> frame_entry=[1] 0x32119cd0 RefCount: 1 children={ size=0 } } ...}	const content::NavigationEntryImpl &
+		frame_entry	RefCount: 1	const content::FrameNavigationEntry &
+		frame_tree_node	0x2f787b50 {frame_tree_=0x31ee7290 {render_frame_delegate_=0x31ee7194 {delegate_=0x311654ac {interstitial_observers_=...} ...} ...} ...}	content::FrameTreeNode *
+		initiator	(null)	base::Optional<url::Origin>
		is_form_submission	true	bool
		is_history_navigation_in_new_child	false	bool
		is_same_document_history_load	false	bool
+		navigation_request	unique_ptr {frame_tree_node_=0x352ef000 {frame_tree_=0x6d6d6f43 {render_frame_delegate_=??? render_view_delegate_=...} ...} ...}	std::unique_ptr<content::NavigationRequest,std::default_delete<content::NavigationRequest> > &
+		navigation_start	{...}	const base::TimeTicks &
		navigation_type	RELOAD (0)	FrameMsg_Navigate_Type::Value
+		post_body	null	const scoped_refptr<content::ResourceRequestBody> &
		previews_state	0	int
+		request_body	[1] 0x34d8d880 {elements_={ size=1 } identifier_=1506994820549887 contains_sensitive_info_=false }	scoped_refptr<content::ResourceRequestBody>


This is what it looks like after we've entered the callee:

browser_initiated	true (4)	bool
+		common_params	{url="X¯\x19" referrer={url="" policy=224897797 } transition=30 ...}	content::CommonNavigationParams
+		controller	0x31ee71b8 {browser_context_=0x2f7220f8 {pref_change_registrar_={observers_={...} service_=0x2f7708a8 {...} } ...} ...}	content::NavigationControllerImpl *
+		dest_referrer	{url="https://httpbin.org/forms/post" policy=kWebReferrerPolicyNoReferrerWhenDowngrade (2) }	const content::Referrer &
+		dest_url	"https://httpbin.org/post"	const GURL &
+		entry	{frame_tree_=unique_ptr {parent=0x00000000 <NULL> frame_entry=[1] 0x32119cd0 RefCount: 1 children={ size=0 } } ...}	const content::NavigationEntryImpl &
+		frame_entry	RefCount: 1	const content::FrameNavigationEntry &
+		frame_tree_node	0x2f787b50 {frame_tree_=0x31ee7290 {render_frame_delegate_=0x31ee7194 {delegate_=0x311654ac {interstitial_observers_=...} ...} ...} ...}	content::FrameTreeNode *
+		initiator	(null)	base::Optional<url::Origin>
is_form_submission	false	bool
		is_history_navigation_in_new_child	false	bool
		is_same_document_history_load	false	bool
+		navigation_request	unique_ptr {frame_tree_node_=??? common_params_={url={...} referrer={url={...} policy=??? } transition=??? ...} ...}	std::unique_ptr<content::NavigationRequest,std::default_delete<content::NavigationRequest> > &
+		navigation_start	{...}	const base::TimeTicks &
		navigation_type	RELOAD (0)	FrameMsg_Navigate_Type::Value
+		post_body	null	const scoped_refptr<content::ResourceRequestBody> &
		previews_state	0	int
+		request_body	null	scoped_refptr<content::ResourceRequestBody>


common_params, is_form_submission, navigation_request, request_body have values that look bad.

common_params is a local, getting initialized by the call.
is_form_submission is a local, already defined, so 4 is unexpected
navigation_request is a local, initialized after the call, so garbage is OK i suppose
request_body is a local, and it's been initialized, so null is unexpected.
navigation_request.obj.zip
1.9 MB Download
> is_form_submission is a local, already defined, so 4 is unexpected
That should be *false* is unexpected.

The unexpected 4 is for browser_initiated
The caller:

// static
std::unique_ptr<NavigationRequest> NavigationRequest::CreateBrowserInitiated(
    FrameTreeNode* frame_tree_node,
    const GURL& dest_url,
    const Referrer& dest_referrer,
    const FrameNavigationEntry& frame_entry,
    const NavigationEntryImpl& entry,
    FrameMsg_Navigate_Type::Value navigation_type,
    PreviewsState previews_state,
    bool is_same_document_history_load,
    bool is_history_navigation_in_new_child,
    const scoped_refptr<ResourceRequestBody>& post_body,
    const base::TimeTicks& navigation_start,
    NavigationControllerImpl* controller) {
096A6431  mov         dword ptr [esi+00000090h],ecx  
096A6437  mov         ecx,dword ptr [ebp+0Ch]  
096A643A  mov         dword ptr [esi+0000008Ch],ecx  
096A6440  lea         ecx,[esi+000000C8h]  
096A6446  mov         dword ptr [esi+00000088h],ecx  
096A644C  mov         ecx,dword ptr ds:[0DE9A564h]  
096A6452  mov         dword ptr [esi+00000608h],ecx  
096A6458  and         al,1  
096A645A  mov         byte ptr [esi+000000B7h],al  
096A6460  and         ah,1  
096A6463  mov         byte ptr [esi+000000B6h],ah  
  // A form submission happens either because the navigation is a
  // renderer-initiated form submission that took the OpenURL path or a
  // back/forward/reload navigation the does a form resubmission.
  scoped_refptr<ResourceRequestBody> request_body;
096A6469  mov         ecx,dword ptr [esi+00000088h]  
096A646F  mov         dword ptr [esi+00000084h],ebx  
096A6475  mov         dword ptr [esi+00000080h],edi  
096A647B  mov         dword ptr [esi+7Ch],edx  
096A647E  call        071D23B0  
  if (post_body) {
096A6483  mov         ecx,dword ptr [ebp+30h]  
096A6486  mov         dword ptr [esi+78h],eax  
096A6489  call        0736CBEE  
096A648E  test        al,1  
096A6490  jne         096A649B  
096A6496  jmp         096A64B7  
    // Standard form submission from the renderer.
    request_body = post_body;
096A649B  lea         ecx,[esi+000000C8h]  
096A64A1  mov         eax,dword ptr [ebp+30h]  
096A64A4  sub         esp,4  
096A64A7  mov         dword ptr [esp],eax  
096A64AA  call        0720E20C  
  } else if (frame_entry.method() == "POST") {
096A64AF  mov         dword ptr [esi+74h],eax  
096A64B2  jmp         096A6522  
096A64B7  mov         ecx,dword ptr [ebp+18h]  
096A64BA  call        073581DF  
096A64BF  lea         ecx,ds:[0D69D899h]  
096A64C5  sub         esp,8  
096A64C8  mov         dword ptr [esp],eax  
096A64CB  mov         dword ptr [esp+4],ecx  
096A64CF  call        07235D3E  
096A64D4  add         esp,8  
  } else if (frame_entry.method() == "POST") {
096A64D7  test        al,1  
096A64D9  jne         096A64E4  
096A64DF  jmp         096A651D  
    // Form resubmission during a back/forward/reload navigation.
    request_body = frame_entry.GetPostData();
096A64E4  lea         eax,[esi+000000C4h]  
096A64EA  mov         ecx,dword ptr [ebp+18h]  
096A64ED  sub         esp,4  
096A64F0  mov         dword ptr [esp],eax  
096A64F3  call        072292FF  
096A64F8  lea         ecx,[esi+000000C8h]  
096A64FE  lea         eax,[esi+000000C4h]  
096A6504  sub         esp,4  
096A6507  mov         dword ptr [esp],eax  
096A650A  call        073C35F7  
096A650F  lea         ecx,[esi+000000C4h]  
096A6515  mov         dword ptr [esi+70h],eax  
096A6518  call        07393F5A  
  } else if (frame_entry.method() == "POST") {
096A651D  jmp         096A6522  
  }
  // TODO(arthursonzogni): Form submission with the "GET" method is possible.
  // This is not currently handled here.
  bool is_form_submission = !!request_body;
096A6522  lea         ecx,[esi+000000C8h]  
096A6528  call        0736CBEE  
096A652D  xor         al,0FFh  
096A652F  xor         al,0FFh  
096A6531  and         al,1  
096A6533  mov         byte ptr [esi+000000B5h],al  

  base::Optional<url::Origin> initiator =
096A6539  mov         ecx,dword ptr [ebp+0Ch]  
096A653C  call        073DBCD3  
096A6541  test        al,1  
096A6543  jne         096A654E  
096A6549  jmp         096A6561  
096A654E  lea         ecx,[esi+000005A8h]  
096A6554  call        0735B9A7  
096A6559  mov         dword ptr [esi+6Ch],eax  
096A655C  jmp         096A658B  
096A6561  mov         ecx,dword ptr [ebp+0Ch]  
096A6564  call        072CD58A  
096A6569  mov         ecx,eax  
096A656B  call        073C35B1  
096A6570  mov         ecx,eax  
096A6572  call        07158632  
096A6577  lea         ecx,[esi+000005A8h]  
096A657D  sub         esp,4  
096A6580  mov         dword ptr [esp],eax  
096A6583  call        072B30CC  
096A6588  mov         dword ptr [esi+68h],eax  
      frame_tree_node->IsMainFrame()
          ? base::Optional<url::Origin>()
          : base::Optional<url::Origin>(
                frame_tree_node->frame_tree()->root()->current_origin());

  // While the navigation was started via the LoadURL path it may have come from
  // the renderer in the first place as part of OpenURL.
  bool browser_initiated = !entry.is_renderer_initiated();
096A658B  mov         ecx,dword ptr [ebp+1Ch]  
096A658E  call        07139A66  
096A6593  xor         al,1  
096A6595  mov         byte ptr [esi+000000B4h],al  

  CommonNavigationParams common_params = entry.ConstructCommonNavigationParams(
096A659B  mov         ecx,dword ptr [ebp+1Ch]  
096A659E  mov         edx,dword ptr [ebp+34h]  
096A65A1  mov         edi,dword ptr [ebp+24h]  
096A65A4  mov         ebx,dword ptr [ebp+20h]  
096A65A7  mov         eax,dword ptr [ebp+14h]  
096A65AA  mov         dword ptr [esi+64h],eax  
096A65AD  mov         eax,dword ptr [ebp+10h]  
096A65B0  mov         dword ptr [esi+60h],eax  
096A65B3  mov         eax,dword ptr [ebp+18h]  
096A65B6  sub         esp,20h  
096A65B9  mov         dword ptr [esi+5Ch],eax  
096A65BC  mov         eax,esp  
096A65BE  mov         dword ptr [eax+1Ch],edx  
096A65C1  mov         dword ptr [eax+18h],edi  
096A65C4  mov         dword ptr [eax+14h],ebx  
096A65C7  mov         edx,dword ptr [esi+64h]  
096A65CA  mov         dword ptr [eax+10h],edx  
096A65CD  mov         edx,dword ptr [esi+60h]  
096A65D0  mov         dword ptr [eax+0Ch],edx  
096A65D3  lea         edx,[esi+000000C8h]  
096A65D9  mov         dword ptr [eax+8],edx  
096A65DC  mov         edx,dword ptr [esi+5Ch]  
096A65DF  mov         dword ptr [eax+4],edx  
096A65E2  lea         edx,[esi+00000380h]  
096A65E8  mov         dword ptr [eax],edx  
096A65EA  mov         dword ptr [esi+58h],edx  
096A65ED  call        071CB9CA  
      frame_entry, request_body, dest_url, dest_referrer, navigation_type,
      previews_state, navigation_start);




The callee:

--- C:\src\chromium\src\content\browser\frame_host\navigation_entry_impl.cc ----

CommonNavigationParams NavigationEntryImpl::ConstructCommonNavigationParams(
    const FrameNavigationEntry& frame_entry,
    const scoped_refptr<ResourceRequestBody>& post_body,
    const GURL& dest_url,
    const Referrer& dest_referrer,
    FrameMsg_Navigate_Type::Value navigation_type,
    PreviewsState previews_state,
    const base::TimeTicks& navigation_start) const {
09682570  push        ebp  
09682571  mov         ebp,esp  
09682573  push        ebx  
09682574  push        edi  
09682575  push        esi  
09682576  and         esp,0FFFFFFF8h  
09682579  sub         esp,128h  
0968257F  mov         esi,esp  
09682581  mov         eax,dword ptr [ebp+8]  
09682584  mov         edx,eax  
09682586  mov         edi,dword ptr [ebp+24h]  
09682589  mov         ebx,dword ptr [ebp+20h]  
0968258C  mov         dword ptr [esi+00000084h],eax  
09682592  mov         eax,dword ptr [ebp+1Ch]  
09682595  mov         dword ptr [esi+00000080h],eax  
0968259B  mov         eax,dword ptr [ebp+18h]  
0968259E  mov         dword ptr [esi+7Ch],eax  
096825A1  mov         eax,dword ptr [ebp+14h]  
096825A4  mov         dword ptr [esi+78h],eax  
096825A7  mov         eax,dword ptr [ebp+10h]  
096825AA  mov         dword ptr [esi+74h],eax  
096825AD  mov         eax,dword ptr [ebp+0Ch]  
096825B0  mov         dword ptr [esi+70h],eax  
096825B3  lea         eax,[esi+00000090h]  
096825B9  mov         dword ptr [esi+6Ch],eax  
096825BC  mov         eax,dword ptr ds:[0DE9A564h]  
096825C1  mov         dword ptr [esi+00000120h],eax  
096825C7  mov         dword ptr [esi+0000008Ch],ecx  
096825CD  mov         eax,dword ptr [esi+0000008Ch]  
  FrameMsg_UILoadMetricsReportType::Value report_type =
096825D3  mov         dword ptr [esi+00000088h],0  
...
I think I have a repro:

struct S {
  ~S() {}
  void *p;
};

extern "C" void h(S s, double*) {
  return;
}

extern "C" int g(S s, double*) {
  double d = 0;
  h(s, &d);
  return 42;
} 

int main() {
  double d = 0;
  S s = {0}; 

  int one = 1, two = 2, three = 3;
  g(s, &d);
  
  return (int)d;
}

Build with:

bin\clang-cl /c -m32 /O0 /Z7 \src\tmp\a.cc

Break in g(), observe garbage locals in main().
> Build with:
>
> bin\clang-cl /c -m32 /O0 /Z7 \src\tmp\a.cc

Drop the /c to actually get an .exe of course.
So, I might be holding it wrong, but it seems that changing the prologue of g from:

_g:                                     # @g
Lfunc_begin2:
	.cv_func_id 2
	.cv_loc	2 1 10 0                # \src\tmp\a.cc:10:0
# BB#0:                                 # %entry
	pushl	%ebp
	movl	%esp, %ebp
	pushl	%ebx
	pushl	%edi
	pushl	%esi
	andl	$-8, %esp
	subl	$24, %esp
To:

_g:                                     # @g
Lfunc_begin2:
	.cv_func_id 2
	.cv_loc	2 1 10 0                # \src\tmp\a.cc:10:0
# BB#0:                                 # %entry
	pushl	%ebp
	movl	%esp, %ebp
	pushl	%ebx
	pushl	%esi     <---- Push %esi first.
	pushl	%edi
	andl	$-8, %esp
	subl	$24, %esp

Makes it work. Maybe they're expecting csrs to be pushed in a fixed order..
Hmm, no...
> Hmm, no....
Yeah, I hadn't stepped past when we clobber %esi in g.
MSVC prologue for a function that clobbers csrs:

g	PROC
; File c:\src\tmp\a.cc
; Line 10
	push	ebp
	mov	ebp, esp
	sub	esp, 12					; 0000000cH
	push	ebx
	push	esi
	push	edi

I tried making our prolog look similar, but that didn't seem to help.
Cc: h...@chromium.org
Owner: r...@chromium.org
rnk is looking into emitting FPO data to fix this.
This initial patch for FPO data handles the small test case in c#28, hopefully it does the larger Chrome one as well: https://reviews.llvm.org/D38776
Blockedon: -760615 769761
I tried a build with the latest roll (https://chromium-review.googlesource.com/c/chromium/src/+/714337) patched in and the same revision and build options as in #23, and all the variables on the parent frame of NavigationEntryImpl::ConstructCommonNavigationParams now show up correctly :-)
Status: Fixed
The roll is sticking.
Thanks, I've verified this.

Sign in to add a comment