64-bit Windows issue

[GoogleCodeExporter]

I am compiling on Windows Server 2008 64-bit using Microsoft Visual Studio 
Express 2008.  

Compilation Issue 1:

In Atomicops.h it, there is not support for 64 bit windows. I made ignored 
macro _M_IX86 as I saw Atomic ops for 64 bit is written in base/atomicops-
internals-x86-msvc.h

Compilation Issue 2:
__tls_used should be changed to _tls_used.
It says that __tls_used is undefined.
trunk/src/windows/port.cc

Linking Issue 3:
new and deletes are not linked. Windows definition file is changed as per 
mingled name in lin generated for me.

src/windows/vc7and8.def

Runtime Issue 4:

With above changes, does it can compile but it does not run. frag_unittest 
crashes at JUMP instruction at in file 
src/windows/preamble_patcher_with_stub.cc

  if (IT_JUMP == instruction_type) {
      ASSERT(false, "Unable to patch because there is a jump instruction "
                     "in the first 5 bytes.");
      return SIDESTEP_JUMP_INSTRUCTION;

What steps will reproduce the problem?
1. Compile and run the frag test on 64-bit windows 
2.
3.

What is the expected output? What do you see instead?
frag_unittest should pass

What version of the product are you using? On what operating system?
windows server 2008 64 bit

Please provide any additional information below.

         ntdll.dll!00000000777e4ea0()   
          [Frames below may be incorrect and/or missing, no symbols loaded 
for ntdll.dll]  
        ntdll.dll!00000000778455e7()    
        ntdll.dll!000000007785790c()    
    ntdll.dll!000000007780c865()    
    ntdll.dll!00000000777decad()    
    ntdll.dll!00000000777de13d()    
    ntdll.dll!00000000777dea57()    
    ntdll.dll!00000000777e59f8()    
    000000008001fc60()  
    ntdll.dll!00000000777cd24e()    
    kernel32.dll!0000000077699f5a()     
    kernel32.dll!000000007769df73()     
    msvcr90d.dll!_write_nolock(int fh=2, const void * 
buf=0x0000000180062810, unsigned int cnt=95)  Line 335 + 0x5b bytes C
    msvcr90d.dll!_write(int fh=2, const void * buf=0x0000000180062810, 
unsigned int cnt=95)  Line 75 + 0x13 bytes  C
>   libtcmalloc_minimal-debug.dll!
sidestep::PreamblePatcher::RawPatchWithStub(void * 
target_function=0x000000007769cea0, void * 
replacement_function=0x000000018001fcd0, unsigned char * 
preamble_stub=0x00000000024203c0, unsigned long stub_size=32, unsigned 
long * bytes_needed=0x0000000000000000)  Line 111 + 0x1f bytes  C++
    libtcmalloc_minimal-debug.dll!
sidestep::PreamblePatcher::RawPatchWithStubAndProtections(void * 
target_function=0x000000007769cea0, void * 
replacement_function=0x000000018001fcd0, unsigned char * 
preamble_stub=0x00000000024203c0, unsigned long stub_size=32, unsigned 
long * bytes_needed=0x0000000000000000)  Line 69 + 0x26 bytes   C++
    libtcmalloc_minimal-debug.dll!sidestep::PreamblePatcher::RawPatch
(void * target_function=0x000000007769cea0, void * 
replacement_function=0x000000018001fcd0, void * * 
original_function_stub=0x0000000180076360)  Line 142 + 0x29 bytes   C++
    libtcmalloc_minimal-debug.dll!sidestep::PreamblePatcher::Patch<int 
(__cdecl*)(void * __ptr64,unsigned long,void * __ptr64)>(int (void *, 
unsigned long, void *)* target_function=0x000000007769cea0, int (void *, 
unsigned long, void *)* replacement_function=0x000000018001fcd0, int (void 
*, unsigned long, void *)* * original_function_stub=0x0000000180076360)  
Line 154    C++
    libtcmalloc_minimal-debug.dll!PatchWindowsFunctions()  Line 254 + 
0x69 bytes  C++
    libtcmalloc_minimal-debug.dll!TCMallocGuard::TCMallocGuard()  Line 
2615    C++
    libtcmalloc_minimal-debug.dll!`dynamic initializer 
for 'module_enter_exit_hook''()  Line 2633 + 0x26 bytes C++
    msvcr90d.dll!_initterm(void (void)* * pfbegin=0x000000018003b1a8, 
void (void)* * pfend=0x000000018003b2d8)  Line 903  C
    libtcmalloc_minimal-debug.dll!_CRT_INIT(void * 
hDllHandle=0x0000000180000000, unsigned long dwReason=1, void * 
lpreserved=0x000000000012fb00)  Line 320    C
    libtcmalloc_minimal-debug.dll!__DllMainCRTStartup(void * 
hDllHandle=0x0000000180000000, unsigned long dwReason=1, void * 
lpreserved=0x000000000012fb00)  Line 537 + 0x13 bytes   C
    libtcmalloc_minimal-debug.dll!_DllMainCRTStartup(void * 
hDllHandle=0x0000000180000000, unsigned long dwReason=1, void * 
lpreserved=0x000000000012fb00)  Line 508    C
    ntdll.dll!00000000777cfd5a()    
    ntdll.dll!00000000777d548f()    

I have explained all steps above.

Thanks,
Ram

Original issue reported on code.google.com by shiningram@gmail.com on 13 Nov 2008 at 11:59

[GoogleCodeExporter]

The preamble-patcher uses a disassembler to figure out how to patch.  This
disassembler is restricted to x86 right now -- it doesn't support other
architectures.  So it looks like perftools won't work on 64-bit windows any 
time soon.

If someone wants to contribute an IA64 or x86_64 disassembler, we'd be glad to 
put it
into use!  But unfortunately, supporting non-32-bit windows seems like a 
difficult task.

Original comment by csilv...@gmail.com on 14 Nov 2008 at 2:03

[GoogleCodeExporter]

Original comment by csilv...@gmail.com on 14 Nov 2008 at 2:04

• Added labels: Type-Enhancement

[GoogleCodeExporter]

Original comment by csilv...@gmail.com on 14 Nov 2008 at 2:04

[GoogleCodeExporter]

Original comment by csilv...@gmail.com on 14 Nov 2008 at 2:04

• Added labels: OpSys-Windows, Priority-Medium

[GoogleCodeExporter]

Original comment by csilv...@gmail.com on 25 Feb 2009 at 11:13

• Added labels: Priority-Low
• Removed labels: Priority-Medium

[GoogleCodeExporter]

Original comment by csilv...@gmail.com on 11 Mar 2009 at 9:23

• Added labels: Type-Porting
• Removed labels: Type-Enhancement

[GoogleCodeExporter]

Issue 233 has been merged into this issue.

Original comment by csilv...@gmail.com on 9 Apr 2010 at 3:05

[GoogleCodeExporter]

Do I get it right: Using static linking it might be possible to use tcmalloc in 
a 64-bit environment?

Original comment by weidenri...@gmx.de on 29 Oct 2010 at 7:47

[GoogleCodeExporter]

It's not about linking, it's about using static patching rather than runtime 
patching.  If you follow the patching instructions pointed to by the windows 
README, it should be possible to use tcmalloc in 64-bit windows.

Original comment by csilv...@gmail.com on 29 Oct 2010 at 9:00

[GoogleCodeExporter]

And static patching requires static linking of libc. And - if I get it right - 
this requires that the program does not use dlls or at least does not allocate 
memory in one dll that is freed in the other. 
http://stackoverflow.com/questions/787216/should-i-link-to-the-visual-studio-c-r
untime-statically-or-dynamically
Agreed?

Original comment by weidenri...@gmx.de on 1 Nov 2010 at 3:59

[GoogleCodeExporter]

Sorry, I don't know the details of this technique.  You'd have to ask the 
author of the original groups post.

Original comment by csilv...@gmail.com on 1 Nov 2010 at 8:28

[GoogleCodeExporter]

1. atomicops-internals-x86-msvc.h is used instead of falling back to 
atomicops-internals-x86.h
2. atomicops-internals-x86-msvc.h renamed to atomicops-internals-msvc.h since 
the file contains both 32bit and 64bit implementations. Having x86 in the name 
would be misleading.
3. changed inline asm in cycleclock.h to use intrinsic function since this 
works for both x86 and x64
4. Leading underscore removed for symbol names in port.cc since x64  uses 
different name decorations. __tls_used becomes _tls_used. Same thing goes for 
_p_thread_callback_tcmalloc and _p_process_term_tcmalloc.

I didn't want to add x64 configuration to all .vcproj files because current 
files are from the older Visual Studio then the one I'm using. After you add 
the x64 configuration you have to change the symbol name in all projects that 
depend on tcmalloc. Just change __tcmalloc to _tcmalloc in the project 
properties - Linker - Input - Force Symbol References. Without this change all 
the projects that depend on it will fail to link.

Original comment by popiz...@gmail.com on 3 Feb 2011 at 10:24

Attachments:

• windows_x64.patch

[GoogleCodeExporter]

Thank you for this patch, and I'm sorry I've not responded to it in so long.  
I'm swamped on another project and probably will be for at least another two 
months.  Plus, I don't really know anything about windows, so this is slower 
for me. :-}  I'll see if I can recruit someone else to help out reviewing this.

Your patch doesn't show the new file (atomicops-internals-msvc.h) being added.  
Can I assume it's the same as atomicops-internals-x86-msvc.h?  That is, you 
just changed the filename, no content?

It looks like all these changes are pretty straightforward, actually.  I'll try 
to carve out a bit of time to apply them.

Original comment by csilv...@gmail.com on 7 Apr 2011 at 3:12

• Changed state: Accepted

[GoogleCodeExporter]

OK, I've started making changes on 1, 2, and 3.  A note on (3): I saw you 
didn't #include <intrin.h> in the cycleclock file, though it seems like it 
should need it.  Should I be doing that?  Do you know why the code is working 
anyway?

And a note on (4): rather than have to use different decorations, I'd rather 
just add the following lines to src/windows/patch_functions.cc:

// This version is needed for x64, which uses a different symbol-decoration 
scheme.
extern "C" PERFTOOLS_DLL_DECL void __tcmalloc();
void __tcmalloc() { }

If you add those lines, do the existing .vcproj files work under x64?  Or are 
there other .vcproj changes I would need to make for x64?

Original comment by csilv...@gmail.com on 7 Apr 2011 at 3:28

[GoogleCodeExporter]

Original comment by csilv...@gmail.com on 7 Apr 2011 at 3:32

• Changed state: Started

[GoogleCodeExporter]

Yes the changes are very simple.

atomicops-internals-msvc.h is just a renamed atomicops-internals-x86-msvc.h 
because having x86 in the file name is kind of misleading.

I have no idea why the code compiles without intin.h, I forgot to include it. 
The code might not compile on other platforms without it.

I'll try your decoration change on some other branch since I have problems 
compiling trunk, here they are:

1. tcmalloc.h(88) : error C2018: unknown character '0x40'. Same thing with 
TC_VERSION_MAJOR and TC_VERSION_MINOR macros, I have ho idea what @ character 
means in this context or how to fix this?
2. cycleclock.h(53) : fatal error C1083: Cannot open include file: 
'sys/time.h': No such file or directory. Shouldn't this be <time.h>?

Original comment by popiz...@gmail.com on 7 Apr 2011 at 9:44

[GoogleCodeExporter]

I've applied your change against revision 101 and removed my changes from 
port.cc but I get three linker errors (that changes from port.cc tried to 
address) complaining about __tls_used, _p_thread_callback_tcmalloc and 
_p_process_term_tcmalloc. I don't see how is your change related to this 
problem? The linker complains about these specific symbols, which have nothing 
to do with _tcmalloc, or am I wrong?

Original comment by popiz...@gmail.com on 7 Apr 2011 at 10:05

[GoogleCodeExporter]

Sorry top-of-tree breaks under windows.  While I try to be careful, I don't 
test on all architectures before committing every revision, so these things 
slip through. :-(  I'm glad you found a revision that works.  (You could also 
just use the last official release.)

You're right, the changes from port.cc have to stay.  The reason to introduce 
the __tcmalloc is to avoid having to change the .vcproj files, like you 
mentioned in comment 12 (after the 4 points).

Original comment by csilv...@gmail.com on 8 Apr 2011 at 12:00

[GoogleCodeExporter]

Yes your change does the trick, no need to change .vcproj files. I think it's 
best that they stay the way they are (without x64 configuration). Whoever needs 
x64 configuration can easily add it. Having x64 configuration might create 
problems for users that don't have x64 compiler installed, so it's best that we 
leave it out, but I'm not sure about this.

I've also noticed something strange. Runtime library property is set to 
Multi-threaded DLL (/MD) for each .cc file in libtcmalloc_minimal separately. 
The project itself is set to Multi-Threaded (/MT). File configuration overrides 
project configuration and everything works fine until you try to use tcmalloc 
statically, like Chrome does. In this case you have to change Runtime library 
for every .cc file. I'm not sure if this was intentional, but if it wasn't I 
would advise you to change this property for every .cc file in 
libtcmalloc_minimal. You can do this by selecting them all and changing Runtime 
library to <inherit from parent or project defaults>. This way project property 
would be in effect.

Original comment by popiz...@gmail.com on 8 Apr 2011 at 6:56

[GoogleCodeExporter]

Yes, the settings are done this way intentionally, because the vcproj files are 
written to support MSVC 7.1, which does not have project-global settings.  The 
project-global '/MT' is added by the conversion process to whatever version of 
MSVC you're using.

Original comment by csilv...@gmail.com on 9 Apr 2011 at 3:11

[GoogleCodeExporter]

I believe all 4 changes are in perftools 1.8, just released.  If I missed 
something, feel free to reopen the bug.

Original comment by csilv...@gmail.com on 16 Jul 2011 at 1:18

• Changed state: Fixed

[GoogleCodeExporter]

Reopening for the original runtime issue 4, requiring a 64-bit disassembler.  
It looks like such a thing may be coming soon!

Original comment by csilv...@gmail.com on 21 Sep 2011 at 3:26

• Changed state: New

[GoogleCodeExporter]

Never mind, issue 360 is a better place for this.  Re-closing.

Original comment by csilv...@gmail.com on 21 Sep 2011 at 3:27

• Changed state: Fixed