Type profiler by intercepting 'new' and 'delete' expressions.

build/common.gypi

Index: build/common.gypi
diff --git a/build/common.gypi b/build/common.gypi
index 3f64fa96e10e8cb83e6fac866efed49dcd81935c..5a427a090a0d820264784c3ca0311c7cf051bded 100644
--- a/build/common.gypi
+++ b/build/common.gypi
@@ -253,6 +253,13 @@
       # See http://clang.llvm.org/docs/ThreadSanitizer.html
       'tsan%': 0,
 
+      # Use a modified version of Clang to intercept allocated types and sizes
+      # for allocated objects. -fintercept-allocated-type only works with the
+      # modified clang, but clang_profiling_allocated_type=1 implies clang=1.

[Ryan Sleevi, 2012/08/10 04:16:09]

I'd remove the entire second sentence - it does not seem to add any help here

[Dai Mikurube (NOT FULLTIME), 2012/08/10 07:30:03]

Removed "-fintercept-allocated-type only works with the modified clang, but".

+      # TODO(dmikurube): Add a link to a description page.
+      # TODO(dmikurube): Support mac with another AllocatedTypeMap store.

[Ryan Sleevi, 2012/08/10 04:16:09]

TODOs like this without an http://crbug.com/ link are quickly lost into the
nether. Please consider filing a bug to track this effort

[Dai Mikurube (NOT FULLTIME), 2012/08/10 07:30:03]

Added a link to description page, and crbug.com.

+      'clang_profiling_allocated_type%': 0,
+
       # Set to true to instrument the code with function call logger.
       # See src/third_party/cygprofile/cyg-profile.cc for details.
       'order_profiling%': 0,
@@ -588,6 +595,7 @@
     'clang_use_chrome_plugins%': '<(clang_use_chrome_plugins)',
     'asan%': '<(asan)',
     'tsan%': '<(tsan)',
+    'clang_profiling_allocated_type%': '<(clang_profiling_allocated_type)',
     'order_profiling%': '<(order_profiling)',
     'order_text_section%': '<(order_text_section)',
     'enable_extensions%': '<(enable_extensions)',
@@ -1188,6 +1196,12 @@
         'clang%': 1,
       }],
 
+      ['clang_profiling_allocated_type==1', {

[Ryan Sleevi, 2012/08/10 04:16:09]

nit: Since you don't support all Clang-supporting OSes yet, please add an
appropriate OS guard

[Dai Mikurube (NOT FULLTIME), 2012/08/10 07:30:03]

Done.

+        'clang%': 1,
+        'clang_use_chrome_plugins%': 0,
+        'make_clang_dir%': 'third_party/llvm-allocated-type/Linux_x64',
+      }],
+
       # On valgrind bots, override the optimizer settings so we don't inline too
       # much and make the stacks harder to figure out.
       #
@@ -1593,6 +1607,31 @@
       }],
     ],  # conditions for 'target_defaults'
     'target_conditions': [
+      ['_type=="executable" or _type=="loadable_module"', {
+        'target_conditions': [
+          # Some binaries doesn't use tcmalloc.  We use empty interceptors
+          # for these binaries since the allocated type mapping is recorded
+          # in a tcmalloc-dependent data structure.
+          ['_target_name=="mksnapshot" or _target_name=="protoc" or _target_name=="ppGoogleNaClPluginChrome"', {

[Ryan Sleevi, 2012/08/10 04:16:09]

This is a nasty layering violation that we try very much to avoid.

Please use a target_defaults: target_conditions to appropriately gate this and
allow individual targets to override this behaviour as appropriate, and then use
a target-specific variable to gate this behaviour.

An example of this is the chromium_code variable.

More importantly, rather than injecting the ignore stub, you should instead not
even be passing whatever command-line args you're passing *for these targets*.

[Dai Mikurube (NOT FULLTIME), 2012/08/10 07:30:03]

Thank you for the information.  Yes, I knew it is a bad hacky way, but I didn't
know how to fix it.  It was a result of tries-and-errors with reading gyp code. 
GYP is complex and undocumented...

What I would like is to inject a dependency to a different linked library by
target binaries as replied at
http://codereview.chromium.org/10411047/diff/69004/base/allocator/allocated_t....
 But, modifying each gyp file was not a good idea since we would like to control
this profiling feature for a whole build in one place -- common.gypi.

But, I still don't understand target_defaults well.  How can I do that?

[Ryan Sleevi, 2012/08/10 09:38:50]

I'm sorry, I'm having trouble understanding your response and the connection
between these two statements. It seems that for these targets, you shouldn't be
intercepting at all then?
I don't think these two things are in opposition. My point of highlighting how
chromium_code is that you can, on a per-target basis, in the target's gyp file,
include or exclude specific behaviours as appropriate.

For example, you might have some:
'variables': {
  'interpose_new%': 1,
},
'target_defaults': {
  'variables': {
    # Dai, see the comment on
http://code.google.com/searchframe#OAMlx_jo-ck/src/build/common.gypi&exact_pa...
    # to understand why this is this way
    'interpose_new%': '<(interpose_new)',
  },
  'target_conditions': [
    ['clang_profiling_allocated_type==1 and interpose_new==1', {
      'cflags_cc': [
      ],
      'include_dirs': [
      ],
    }],
  ],
},

[Dai Mikurube (NOT FULLTIME), 2012/08/13 08:32:14]

I looks requiring all things done in compiling-phase.  It is hard for the reason
written in the top "summary".

+            # Ignore allocated types.
+            'conditions': [
+              ['clang_profiling_allocated_type==1', {
+                'dependencies': [
+                  '<!(realpath <(DEPTH)/base/allocator/allocator.gyp)' + ':allocated_type_profiler_ignore#target',

[Ryan Sleevi, 2012/08/10 04:16:09]

1) The use of realpath here is troubling. It definitely seems that GYP's
existing path handling should be appropriate here.
2) The specification of #target is incorrect here - this is specifically for
cross-compiling, and is an internal GYP detail not (generally) meant to be
exposed.

[Dai Mikurube (NOT FULLTIME), 2012/08/10 07:30:03]

realpath got required from http://crrev.com/146172.  From this change, realpath
is a key of a hashtable in gyp.

Both look like because this part is evaluated lazily after some path and target
operations.  It's a large part of this change's hacky points.  But, I have no
idea to fix them. :(

[Ryan Sleevi, 2012/08/10 09:38:50]

Oh, right, that's because GYP dependency resolution happens *between* the pre-
and the post- phase, but you're decidedly  in the post- phase here
(target_conditions is post phase)

See
http://code.google.com/p/gyp/wiki/InputFormatReference#Timing_of_Variable_Exp...
to understand what I mean by pre- and post- phases

This is controlled by 

http://code.google.com/searchframe#OAMlx_jo-ck/tools/gyp/pylib/gyp/input.py&e...

And it's "intentional" to make sure dependencies are properly specified and not
willy-nilly cross-injected.

Can you not just specify the correct dependencies in allocator.gyp:allocator,
which will cause all linkable types (eg: executables, shared_libraries, loadable
modules) to link in these targets as appropriate?

[Dai Mikurube (NOT FULLTIME), 2012/08/13 08:32:14]

For the reason written in the "top" summary, I have to do something for
non-tcmalloc binaries.  How to do it?

+                ]},
+              ],
+            ]}, {
+            # Store allocated types in TCMalloc's allocated_type_map (extended).
+            'conditions': [
+              ['clang_profiling_allocated_type==1', {

[Ryan Sleevi, 2012/08/10 09:38:50]

BUG? This double nested condition seems wrong. Shouldn't it be?

'conditions': [
  ['clang_profiling_allocated_type == 1', {
    'conditions': [
      ['linux_use_tcmalloc == 1', {
        'dependencies': [
'<(DEPTH)/base/allocator/allocator.gyp:allocated_type_profiler_tcmalloc',
        ],
      }, {
        'dependencies': [
'<(DEPTH)/base/allocator/allocator.gyp:allocated_type_profiler_ignore',
        ],
      }],
    ],
  }],
],

[Dai Mikurube (NOT FULLTIME), 2012/08/10 15:51:43]

That's the point I wanted to discuss.  tcmalloc is not used for mksnapshot and
protoc even if linux_use_tcmalloc==1.  mksnapshot doesn't run with
allocated_type_profiler_tcmalloc.

+                'dependencies': [
+                  '<!(realpath <(DEPTH)/base/allocator/allocator.gyp)' + ':allocated_type_profiler_tcmalloc#target',
+                ]},
+              ],
+            ]},
+          ]
+        ]},
+      ],
       ['enable_wexit_time_destructors==1', {
         'conditions': [
           [ 'clang==1', {
@@ -2339,6 +2378,22 @@
               '-fcolor-diagnostics',
             ],
           }],
+          ['clang_profiling_allocated_type==1', {
+            'cflags_cc!': ['-fno-rtti'],
+            'cflags_cc+': ['-gline-tables-only'],
+            'target_conditions': [
+                ['_target_name!="allocator" and _target_name!="allocator_extension_thunks" and _target_name!="libcmt" and _target_name!="allocator_extension_thunks_win64" and _target_name!="allocated_type_profiler_log" and _target_name!="allocated_type_profiler_ignore" and _target_name!="allocated_type_profiler_tcmalloc"', {

[Ryan Sleevi, 2012/08/10 04:16:09]

See comments above about layering violation. These should be refactored.

[Dai Mikurube (NOT FULLTIME), 2012/08/10 07:30:03]

I agree with that, but I have no idea to refactor it...  Here, I would like to
change cflags_cc for almost all the .cc files except for some files related to
specific targets.  Can I do that?

[Ryan Sleevi, 2012/08/10 09:38:50]

In those specific targets that you wish to exclude, manually set the variable
(eg: override the default being inherited from this file) to exclude them.

This is what I meant by comparing with the 'chromium_code' usage, which
selectively enables or disables certain flags on a per-target basis, with a
well-defined default for all targets, and then certain targets opting in.

[Dai Mikurube (NOT FULLTIME), 2012/08/13 08:32:14]

I tried it for the compiler flags, and it looks working.  Could you take a look
at this?

+                    'cflags_cc+': [

[Ryan Sleevi, 2012/08/10 04:16:09]

style nit: The indents here should all be GYP indents, which are two-space
indents.

[Dai Mikurube (NOT FULLTIME), 2012/08/10 07:30:03]

Done.

+                        '-frtti',
+                        '-fintercept-allocation-functions',
+                        '-I../../base/allocator',

[Ryan Sleevi, 2012/08/10 04:16:09]

BUG: You should not be specifying relative paths via -I, as they'll ignore GYP
pathname canonicalization and thus be generator dependent.

Instead, you should consider using the "include_dirs" GYP variable, which is the
correct way to pass such variables.

[Dai Mikurube (NOT FULLTIME), 2012/08/10 07:30:03]

Trying below.

+                        '-include',
+                        'allocated_type_profiler.h',

[Ryan Sleevi, 2012/08/10 04:16:09]

However, more generally, ISTM that instead of forcing base/allocator into all
targets' include_dirs, you should instead be specifying the canonical path for
allocated_type_profiler and using -include with the full path.

'variables': {
  'some_var_name_path': '<(DEPTH)/base/allocator/allocated_type_profiler.h'
},
'cflags_cc+': [
  '-include', '<(some_var_name_path)',
]

Note the suffix of _path, which ensures the proper path relative to where/how
the files are compiled is yielded ( see
http://code.google.com/p/gyp/wiki/InputFormatReference#Pathname_Relativization )

[Dai Mikurube (NOT FULLTIME), 2012/08/10 07:30:03]

It was impossible maybe because it is lazily evaluated (in target_conditions)
like above.  The defined variable was not found here.

"KeyError: 'Undefined variable clang_profiling_allocated_type_header_path in
.../build/all.gyp while trying to load .../build/all.gyp'"

We may have to stop target_conditions before fix it.

[Ryan Sleevi, 2012/08/10 09:38:50]

Oh, that's probably just due to GYP order-of-evaluation. Use
>(some_var_name_path) rather than <(some_var_name_path)

See
http://code.google.com/p/gyp/wiki/InputFormatReference#Timing_of_Variable_Exp...
for more about the ordering of evaluations

[Dai Mikurube (NOT FULLTIME), 2012/08/13 08:32:14]

It doesn't work, too.
KeyError: 'Undefined variable clang_profiling_allocated_type_header_path in
.../src/third_party/WebKit/Source/WebCore/WebCore.gyp/WebCore.gyp'

Actually, I added copying the variable at line 601, and tried '<' again.  It
worked on gyp_chromium, but the result was wrong.  I added a comment here.

+                    ],
+                }],
+            ],
+            'defines': ['PROFILING_ALLOCATED_TYPE'],
+          }],
           ['asan==1', {
             'target_conditions': [
               ['_toolset=="target"', {

[Ryan Sleevi, 2012/08/10 04:16:09]

note: Like ASAN, ISTM like you may need to gate on _toolset - please confirm.

[Dai Mikurube (NOT FULLTIME), 2012/08/10 07:30:03]

Ah... sorry, I have no idea about "_toolset".  Do you know any documents about
that?