Type change in StackSamplingProfiler from void* to uintptr_t.

base/profiler/stack_sampling_profiler.h

Index: base/profiler/stack_sampling_profiler.h
diff --git a/base/profiler/stack_sampling_profiler.h b/base/profiler/stack_sampling_profiler.h
index 5a6c3d0514d00a337d462f06e79c0e89ced5a249..681edef96968135c39eed41eaec05794ddda7b4f 100644
--- a/base/profiler/stack_sampling_profiler.h
+++ b/base/profiler/stack_sampling_profiler.h
@@ -69,7 +69,7 @@ class BASE_EXPORT StackSamplingProfiler {
     ~Module();
 
     // Points to the base address of the module.
-    const void* base_address;
+    uintptr_t base_address;

[Mike Wittman, 2015/08/31 20:57:57]

I'd prefer to keep the const void* type for this and instruction_pointer. This
makes debugging the profiler a *lot* easier since the Visual Studio debugger
automatically symbolizes these when explicitly represented as pointer values.

I think this should be possible by creating an IPC::ParamTraits template
specialization for Module and Frame.

[erikchen, 2015/08/31 21:15:17]

The type of this member should not be dependent on Visual Studio capabilities.

Back when this class was only ever instantiated/used in a single process, const
void* was the correct type.

Since this class is going to be passed across Chrome IPC, const void* is no
longer the correct type (and is even dangerous, since one might be tempted to
dereference the pointer in the receiving process).

[Mike Wittman, 2015/08/31 21:42:00]

Debuggability of the code is always a relevant concern, and that code runs only
on Windows.
However, I agree that representing as pointers in a different process is
misleading at best.

Unfortunately I don't see a good way to preserve the pointer representation in
the browser process while supporting uintptr_t for IPC, so uintptr_t it is. I'll
provide additional comments accordingly.

[sydli, 2015/09/01 00:29:59]

Okay; I split this into a separate CL:
https://codereview.chromium.org/1319973011/
in case any functionality of the profiler breaks with the uintptr_t change
(Don't think it should, but just in case). I'll run the related tests on a
Windows machine.

[Mike Wittman, 2015/09/01 00:41:57]

The const void* => uintptr_t change should be pretty low risk. If it compiles
and there's no obvious issues in the code review, it should be fine.

Since we don't want pointers in the IPC representation, I think we should keep
that change in this CL.

 
     // An opaque binary string that uniquely identifies a particular program
     // version with high probability. This is parsed from headers of the loaded
@@ -89,11 +89,12 @@ class BASE_EXPORT StackSamplingProfiler {
     // Identifies an unknown module.
     static const size_t kUnknownModuleIndex = static_cast<size_t>(-1);
 
+    Frame();
     Frame(const void* instruction_pointer, size_t module_index);

[Mike Wittman, 2015/08/31 21:42:00]

Similarly, the |instruction_pointer| parameter should use the same type as is
stored in the struct.

[sydli, 2015/09/01 00:29:59]

Done.

     ~Frame();
 
     // The sampled instruction pointer within the function.
-    const void* instruction_pointer;
+    uintptr_t instruction_pointer;
 
     // Index of the module in CallStackProfile::modules. We don't represent
     // module state directly here to save space.