cynthia / gperftools

Automatically exported from code.google.com/p/gperftools
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

Windows compilation errors for revision 109 #340

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Here are the problems when compiling current trunk on Windows with Visual 
Studio 2010:

- Two projects fail to convert since they aren't valid XML (I'm guessing that 
you use some kind of tool to generate them, see my patch)

- src/base/atomicops.h includes atomicops-internals-x86-windows.h but this file 
was renamed to atomicops-internals-windows.h

- src/windows/port.cc is missing an include for TCMalloc_Printer (or a forward 
declaration)

- 6 errors in 6 different files (error C2371: 'int8_t' : redefinition; 
different basic types). They all include central_freelist.h (directly or 
indirectly) and #include <stdint.h> was added in this file. This collides with 
definitions from src/windows/port.h

- tcmalloc.cc includes <google/tcmalloc.h> but this file is in the same dir so 
this should probably be #include "tcmalloc.h" (also why the angle brackets?). 
This leaves us with two problems:
    1. tc_malloc and tc_free aren't defined because their definition is guarded by #if !defined(WIN32_DO_PATCHING) 
    2. TC_VERSION macros are undefined. They are inside tcmalloc.h.in but I have no idea how this file is used :)

Attached patches for the first three issues.

Original issue reported on code.google.com by popiz...@gmail.com on 1 Jun 2011 at 6:47

Attachments:

GoogleCodeExporter commented 9 years ago
Thanks!  I've fixed the first three issues as you suggested.

For issue #4, I'm surprised it worked! -- I didn't realize windows had stdint.h 
(maybe only some versions do?)  Anyway, I have protected the central_freelist.h 
#include by the appropriate #ifdef.

For issue #5, we use the angle brackets just as documentation that this file 
will be installed (on unix) as part of the installation process.  Angle vs 
quotes is fine -- they're both the same here.

The tcmalloc.h in src is actually different from the one in src/google (or, for 
windows, src/windows/google).  (I know, it's confusing, that was a bad naming 
choice I should probably fix.)  We want to be #including the latter, so the 
code as it is is correct.  That should also resolve the two problems that you 
were seeing earlier.

What problem were you seeing with the original #include of <google/tcmalloc.h>, 
that prompted you to change it?

Original comment by csilv...@gmail.com on 1 Jun 2011 at 6:21

GoogleCodeExporter commented 9 years ago
Well there is no such file inside google directory, only tcmalloc.h.in is in 
there?

Original comment by popiz...@gmail.com on 1 Jun 2011 at 9:28

GoogleCodeExporter commented 9 years ago
Yes, that's right, the file is created when 'configure' is run.  (The same as 
with config.h.)

In src/windows, there is a google/tcmalloc.h, because windows users don't run 
'configure'.

Original comment by csilv...@gmail.com on 1 Jun 2011 at 9:31

GoogleCodeExporter commented 9 years ago
Have you thought about using CMake?

Original comment by popiz...@gmail.com on 1 Jun 2011 at 9:34

GoogleCodeExporter commented 9 years ago
Yes.  I'm pretty happy with autotools in unix-land, and am not particularly 
interested in moving off that.  So cmake would just be a replacement for using 
visual studio directly, for windows users.  I don't know enough about windows 
to know if that would be a win or not; I assume not since it adds another 
dependency.

Original comment by csilv...@gmail.com on 1 Jun 2011 at 10:34

GoogleCodeExporter commented 9 years ago
By the way, r109 introduced a static |sys_alloc| member into src/system-alloc.h 
(http://code.google.com/p/google-perftools/source/diff?spec=svn109&r=109&format=
side&path=/trunk/src/system-alloc.cc), which is then declared in 
src/system-alloc.cc and used by tcmalloc.cc
IIUC on Windows src/windows/port.cc replaces src/system-alloc.cc
How comes then that |sys_alloc| is not defined in src/windows/port.cc?

Original comment by gli...@google.com on 16 Jun 2011 at 10:32

GoogleCodeExporter commented 9 years ago
You're right, we need to define a sys_alloc in port.cc or similar.

Original comment by csilv...@gmail.com on 8 Jul 2011 at 4:10

GoogleCodeExporter commented 9 years ago
(I'll make that fix for the next release.)

Original comment by csilv...@gmail.com on 8 Jul 2011 at 5:03

GoogleCodeExporter commented 9 years ago
This should be fixed in perftools 1.8, just released.

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