New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 777943 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Nov 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 341941
issue 82385



Sign in to add a comment

Need to update clang-cl/static_initializers to find clang-cl initializers and destructors

Project Member Reported by brucedaw...@chromium.org, Oct 24 2017

Issue description

tools\win\static_initializers generates a report listing all static initializers and destructors (global constructors and destructors). On binaries created with clang-cl this report lists constructors/destructors from f:\binaries\Intermediate\vctools\ (those from the VC++ CRT) but nothing else.

The report for VC++ chrome.dll listed constructors for kDefaultColorNTPBackground, kDefaultColorNTPText, and kDefaultColorNTPLink, all of which are const globals that are initialized through calls to color_utils::GetSysSkColor(COLOR_WINDOW); which calls GetSysColor which is an OS call. Thus, these three globals have static initializers. However they do not show up in the report for clang chrome.dll. Running symbolsort on clang's chrome.dll and searching for kDefaultColorNTPBackground finds nothing.

It appears that no symbol is being generated for these static initializers, so it is possible that clang-cl needs to be updated to emit such a symbol. And, the static_initialiers tool may also need updating.

 
Cc: thakis@chromium.org
Blocking: 82385

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

Cc: r...@chromium.org
Owner: r...@chromium.org
It looks like we demangle the symbol name for the initializer in our debug info. For this simple program, I see "??__Emy_global@YAXXZ" instead of "`dynamic initializer for 'my_global''":

$ cat t.cpp
struct Foo {
  Foo() { x = 32; }
  int x;
};
Foo my_global;
extern "C" __declspec(dllexport) int getit() { return my_global.x; }

$ rm -f t.pdb && cl -Z7 -c t.cpp  && link -debug -nodefaultlib t.obj -noentry -dll && cvdump t.pdb  | grep my_global
...
t.obj : warning LNK4210: .CRT section exists; there may be unhandled static initializers or terminators
        Type = 0x1007           Scope = global  ??__Emy_global@@YAXXZ
S_PUB32: [0003:00000000], Flags: 00000000, ?my_global@@3UFoo@@A
(0000BC) S_LDATA32: [0005:00000000], Type:             0x1008, my_global$initializer$
(000154) S_LPROC32: [0001:00000020], Cb: 00000015, Type:             0x1007, `dynamic initializer for 'my_global''
S_LPROCREF: 0x00000000: (   1, 00000154) `dynamic initializer for 'my_global''
S_GDATA32: [0003:00000000], Type:             0x1006, my_global
S_LDATA32: [0005:00000000], Type:             0x1008, my_global$initializer$

$ rm -f t.pdb && clang-cl -Z7 -c t.cpp  && link -debug -nodefaultlib t.obj -noentry -dll && cvdump t.pdb  | grep my_global
...
t.obj : warning LNK4210: .CRT section exists; there may be unhandled static initializers or terminators
        Type = 0x1001           Scope = global  ??__Emy_global@@YAXXZ
S_PUB32: [0003:00000000], Flags: 00000000, ?my_global@@3UFoo@@A
(000034) S_LPROC32: [0001:00000020], Cb: 0000001A, Type:             0x1001, ??__Emy_global@@YAXXZ
S_LPROCREF: 0x00000000: (   1, 00000034) ??__Emy_global@@YAXXZ
S_GDATA32: [0003:00000000], Type:             0x1007, my_global

Checking if the symbol name also starts with "??__E" in static_initializers.cc should be enough to fix this.

Clang also has -Wglobal-constructors and -Wglobal-destructors which will catch these if we can deploy them more widely.
Thanks for the patch!

Even with the patch I'm still not seeing symbols for kDefaultColorNTPBackground, kDefaultColorNTPText, and kDefaultColorNTPLink which logically *must* have constructors, I think, so I'm still suspicious.

I've attached .zip files containing reports for two 64-bit canary builds, one VC++ and the other clang (created with the patched version of static_initializers from crrev.com/c/752187). The batch files to create them are shown below:

    getdata.bat:
@if "%1" == "" goto justprint
mkdir %~dp0%1
cd /d %~dp0%1
@echo Make sure that you've run regvc.bat or equivalent to get required tools in the path
@rem call getcanarysymbols.bat
c:\src\chromium3\src\tools\win\static_initializers\static_initializers.exe "C:\Users\brucedawson\AppData\Local\Google\Chrome SxS\Application\chrome.exe" | sort > chrome.exe.txt
c:\src\chromium3\src\tools\win\static_initializers\static_initializers.exe "C:\Users\brucedawson\AppData\Local\Google\Chrome SxS\Application\%1\chrome.dll" | sort > chrome.dll.txt
c:\src\chromium3\src\tools\win\static_initializers\static_initializers.exe "C:\Users\brucedawson\AppData\Local\Google\Chrome SxS\Application\%1\chrome_child.dll" | sort > chrome_child.dll.txt
cd /d %~dp0
call summarize.bat %1
@exit /b

:justprint
dir "C:\Users\brucedawson\AppData\Local\Google\Chrome SxS\Application" /ad
@echo Run again with the version directory as a parameter



    summarize.bat:
set inputs=%1\chrome.dll.txt %1\chrome_child.dll.txt %1\chrome.exe.txt
if exist %1\chrome.exe.txt goto full
set inputs=%1\chrome.dll.txt %1\chrome_child.dll.txt
:full

type %inputs% 2>nul | find /v "Static initializers in" | find /v "Intermediate\vctools" | find /c "dynamic initializer"
type %inputs% 2>nul | find /v "Intermediate\vctools" | find /c "dynamic atexit destructor"

type %inputs% 2>nul | find /v "Static initializers in" | find /v "Intermediate\vctools" | find "dynamic initializer" >%1\dynamic_initializers.txt
type %inputs% 2>nul | find /v "Intermediate\vctools" | find "dynamic atexit destructor" >%1\atexit_destructors.txt

64.0.3247.0.zip
11.1 KB Download
64.0.3256.0.zip
10.4 KB Download

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

I think I see what's up with those initializers now. To ensure that they run in the same order that they appeared in the source file, clang emits code that looks like:

__GLOBAL_sub_I_$basename:
  call "??__Eglobal1@YAXXZ"
  call "??__Eglobal2@YAXXZ"
  call "??__Eglobal3@YAXXZ"
  ret

And we inline the "??__E" functions into the __GLOBAL_sub_I* function.

I guess we just need to look for __GLOBAL_sub_I like we do on Linux.
I tested with this change and it works nicely. I had to add an include of Windows.h, adjust the name to search for from __GLOBAL_sub_I to _GLOBAL__sub_I (note the movement of one underscore). I also printed the phrase "dynamic initializer" on those for consistent grepping.

With this change my results (on 64.0.3260.0) show 33 static initializers and 158 dynamic atexit destructors, which is consistent with the 48/140 results from VC++ builds, and gives me enough information to file a separate clang bug (constructors generated for __PURE_APPDOMAIN_GLOBAL static locale::id id;). In particular, the initializers for kDefaultColorNTPBackground, kDefaultColorNTPText, and kDefaultColorNTPLink are visible:

    obj/chrome/browser/browser_3/theme_properties.obj: _GLOBAL__sub_I_theme_properties.cc (dynamic initializer)

My full patch (on top of your previous patch) is shown below. I think it's ready now. You can land it or I can.

diff --git a/tools/win/static_initializers/static_initializers.cc b/tools/win/static_initializers/static_initializers.cc
index 305500e95448..2800ac385aad 100644
--- a/tools/win/static_initializers/static_initializers.cc
+++ b/tools/win/static_initializers/static_initializers.cc
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.

+#include <Windows.h>
 #include <dbghelp.h>
 #include <dia2.h>
 #include <stdio.h>
@@ -105,6 +106,15 @@ static void PrintIfDynamicInitializer(const std::wstring& module,
         wcsstr(bstr_name, L"`dynamic atexit destructor for '")) {
       wprintf(L"%s: %s\n", module.c_str(), bstr_name);
     }
+    // If there are multiple dynamic initializers in one translation unit then
+    // a shared function is created and the individual initializers may be
+    // inlined into it. These functions start with a characteristic name that
+    // includes the source file. Finding the actual objects can be done through
+    // source inspection or by setting a breakpoint on the printed name. The
+    // "dynamic initializer" string is printed for consistent grepping.
+    if (wcsstr(bstr_name, L"_GLOBAL__sub_I")) {
+      wprintf(L"%s: %s (dynamic initializer)\n", module.c_str(), bstr_name);
+    }
     // As of this writing, Clang does not undecorate the symbol names for
     // dynamic initializers, so the debug info contains the decorated name,
     // which starts with "??__E" or "??__F" for atexit destructors. Check for

Comment 7 by r...@chromium.org, Nov 7 2017

Thanks, I patched that into https://chromium-review.googlesource.com/c/chromium/src/+/752187 and it's winding through the CQ, although I don't know what it will do since that code isn't built as part of the build system.
Project Member

Comment 8 by bugdroid1@chromium.org, Nov 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b711f2361e5bfd3a42bf7f382eb24779383285af

commit b711f2361e5bfd3a42bf7f382eb24779383285af
Author: Reid Kleckner <rnk@google.com>
Date: Tue Nov 07 03:23:04 2017

Update static_initializers tool to detect clang's initializers

Clang currently puts the undecorated symbol name in the debug info for
initializer functions. Fixing that is not simply a matter of calling
UnDecorateSymbolName in Clang because we want our output to be the same
when we're cross-compiling, and we don't have our own MS ABI demangler.
Do the undecoration in this tool in the meantime.

R=brucedawson@chromium.org

Bug:  777943 
Change-Id: I3087e4fdbde849762d446ffcc9aa2ea60e9cb21e
Reviewed-on: https://chromium-review.googlesource.com/752187
Commit-Queue: Reid Kleckner <rnk@chromium.org>
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514373}
[modify] https://crrev.com/b711f2361e5bfd3a42bf7f382eb24779383285af/tools/win/static_initializers/build.bat
[modify] https://crrev.com/b711f2361e5bfd3a42bf7f382eb24779383285af/tools/win/static_initializers/static_initializers.cc

Status: Verified (was: Untriaged)
I built the tool after your CL landed and confirmed that it is working. Closing as verified.

Sign in to add a comment