Heap checking currently does not support FreeBSD6 (requesting feedback on porting)

[alk]

Originally reported on Google Code with ID 311

What steps will reproduce the problem?

1. build the tools with --enable-heap-check

2. write a small program. Compile and link it with libtcmalloc.

#include <stdio.h>
#include <stdlib.h>

int main(int argc, char** argv)
{
    printf("********** start **********\n");
    int* i1 = new int(1000);
    int* i2 = new int(1000);
    int* i3 = new int(1000);
    int* i4 = new int(1000);
    int* i5 = new int(1000);
    int* i6 = new int(1000);
    printf("*********** end ***********\n");

    return 0;
}

$ g++ -L/usr/local/lib main.cpp -o main -ltcmalloc

3. Run the heap checker as follows: 

$ export PERFTOOLS_VERBOSE=10
$ HEAPCHECK=normal ./main
Unable to open /proc/self/environ, falling back on getenv("HEAPCHECK"), which may not
work
MemoryRegionMap Init
MemoryRegionMap Init done
Starting tracking the heap
Found hooked allocator at 3: 0x680a2a69 <- 0x682a5b8f
Found hooked allocator at 2: 0x680a2a69 <- 0x682a3114
Found hooked allocator at 2: 0x680a2a69 <- 0x6808713a
Found hooked allocator at 2: 0x680a2a69 <- 0x682a1b51
Found hooked allocator at 2: 0x680a2a69 <- 0x68087154
Found hooked allocator at 2: 0x680a0e03 <- 0x68087172
Found hooked allocator at 2: 0x680a2a69 <- 0x682a3114
Found hooked allocator at 2: 0x680a2a69 <- 0x682a7a04
Found hooked allocator at 2: 0x680a0e03 <- 0x68097b4a
Going to ignore live object at 0x80cd000 of 4 bytes
Found hooked allocator at 2: 0x680a2a69 <- 0x682a698a
Found hooked allocator at 2: 0x680a0e03 <- 0x681330ff
Found hooked allocator at 2: 0x680a0e03 <- 0x68095fae
Found hooked allocator at 2: 0x680a0e03 <- 0x681330ff
Found hooked allocator at 2: 0x680a0e03 <- 0x681330ff
Found hooked allocator at 2: 0x680a0e03 <- 0x681330ff
No shared libs detected. Will likely report false leak positives for statically linked
executables.
Turning perftools heap leak checking off
MemoryRegionMap Shutdown
MemoryRegionMap Shutdown done
********** start **********
*********** end ***********
$ 

What is the expected output? What do you see instead?

As you can see, we hit a condition in the heap check startup/init that causes heap
checking to be disabled as indicated by the message:

No shared libs detected. Will likely report false leak positives for statically linked
executables.
Turning perftools heap leak checking off

What version of the product are you using? On what operating system?

google-perftools-1.7/FreeBSD6

Please provide any additional information below.

Here is the code snippet where the heap check is bailing out:

src/heap-checker.cc

HeapLeakChecker::ProcMapsResult HeapLeakChecker::UseProcMapsLocked(
                                  ProcMapsTask proc_maps_task) {
  RAW_DCHECK(heap_checker_lock.IsHeld(), "");
  // Need to provide own scratch memory to ProcMapsIterator:
  ProcMapsIterator::Buffer buffer; 
  ProcMapsIterator it(0, &buffer);
  if (!it.Valid()) {
    int errsv = errno;
    RAW_LOG(ERROR, "Could not open /proc/self/maps: errno=%d. "
                   "Libraries will not be handled correctly.", errsv); 
    return CANT_OPEN_PROC_MAPS;
  }
  uint64 start_address, end_address, file_offset;
  int64 inode;
  char *permissions, *filename;
  bool saw_shared_lib = false;
  while (it.Next(&start_address, &end_address, &permissions,
                 &file_offset, &inode, &filename)) {
    if (start_address >= end_address) {
      // Warn if a line we can be interested in is ill-formed:
      if (inode != 0) { 
        RAW_LOG(ERROR, "Errors reading /proc/self/maps. "
                       "Some global memory regions will not "
                       "be handled correctly.");
      }
      // Silently skip other ill-formed lines: some are possible
      // probably due to the interplay of how /proc/self/maps is updated
      // while we read it in chunks in ProcMapsIterator and
      // do things in this loop.
      continue;
    }
    // Determine if any shared libraries are present.
    if (inode != 0 && strstr(filename, "lib") && strstr(filename, ".so")) {
      saw_shared_lib = true; 
    }
    switch (proc_maps_task) {
      case DISABLE_LIBRARY_ALLOCS:
        // All lines starting like
        // "401dc000-4030f000 r??p 00132000 03:01 13991972  lib/bin"
        // identify a data and code sections of a shared library or our binary
        if (inode != 0 && strncmp(permissions, "r-xp", 4) == 0) { 
          DisableLibraryAllocsLocked(filename, start_address, end_address);
        }       
        break;  
      case RECORD_GLOBAL_DATA:
        RecordGlobalDataLocked(start_address, end_address,
                               permissions, filename);
        break;  
      default:
        RAW_CHECK(0, "");
    }
  }
******************************* HERE *********************************  
  if (!saw_shared_lib) {
    RAW_LOG(ERROR, "No shared libs detected. Will likely report false leak "
                   "positives for statically linked executables.");
    return NO_SHARED_LIBS_IN_PROC_MAPS;
  }
  return PROC_MAPS_USED;
}

The documentation seems to suggest that the heap check is only currently supported
on Linux. I suspect that whatever linux specific code could be made platform independent
or easily ported to work with FreeBSD. I would like to add support for FreeBSD but
need a bit of a background overview on the Linux specific bits that would need to change.
Is someone willing to get me up to speed here to get this work underway? 

Regards,

Dave

Reported by chappedm on 2011-02-11 22:01:19

[alk]

The biggest linux-specific part is the code that freezes all threads while the heap-checker
does its data gathering.  It also has to be careful to only call library routines that
never allocate memory; in practice, this means making mostly syscalls, which it does
via a syscall-wrapper file (in src/base) which is linux-specific.  There may also be
other linux-isms, like the code-block that you identified.  I believe FreeBSD has support
for /proc but it's not on by default; you may need to run with /proc to get any useful
data out of the heap-profiler in freebsd.

These are the linux-isms I can think of offhand.  There may be others I don't know
about.

Reported by csilvers on 2011-02-11 23:25:23

• Labels added: Type-Enhancement, Priority-Medium

[alk]

FreeBSD-6 is long obsolete. Current version is 8.1. You should upgrade and use the current
port devel/google-perftools there.

I am getting the output on 8.1:
********** start **********
*********** end ***********

If you don't have /proc mounted -- add this line to /etc/fstab:
proc /proc procfs rw 0 0

Reported by yuri@tsoft.com on 2011-02-20 12:25:50

[alk]

Yes. We are moving to freebsd8 in the fall. I work in telecom/cable/wireless networking
equipment industry so platform stability is crucial. In other words, once things are
stable keep it that way. Unfortunately that means falling behind the times as well.

I have /proc mounted but still no berries. Its not a huge deal since the heap profiler
works. The others were just nice to haves.

This can now be closed off as it doesn't seem like its worth the effort to address.

Reported by chappedm on 2011-03-03 04:36:03

[alk]

On a side note it looks like it could just be that the layout of my /procs is just different
than expected. I have /proc/curproc/map instead of /proc/self/maps. Easy enough to
patch locally. I'll give it a shot.

$ cat /proc/curproc/map 
0x8048000 0x804a000 2 0 0xa90e9210 r-x 1 0 0x0 COW NC vnode /bin/cat
0x804a000 0x804b000 1 0 0xa8ec36b4 rw- 2 0 0x2180 NCOW NNC default -
0x804b000 0x804d000 2 0 0xa8ec36b4 rwx 2 0 0x2180 NCOW NNC default -
0x6804a000 0x68069000 24 0 0xa10649cc r-x 187 64 0x4 COW NC vnode /libexec/ld-elf.so.1
0x68069000 0x6806a000 1 0 0xa9b1a294 rw- 1 0 0x2180 COW NNC vnode /libexec/ld-elf.so.1
0x6806a000 0x6806f000 5 0 0xa9ae7000 rw- 2 0 0x2180 NCOW NNC default -
0x6806f000 0x68080000 11 0 0xa9ae7000 rwx 2 0 0x2180 NCOW NNC default -
0x68080000 0x6814c000 79 0 0xa9afe840 r-x 1 0 0x2180 COW NNC vnode /lib/libc.so.6
0x6814c000 0x6814d000 1 0 0xa9b5f8c4 r-x 1 0 0x2180 COW NNC vnode /lib/libc.so.6
0x6814d000 0x68152000 5 0 0xa7607000 rwx 1 0 0x2180 COW NNC vnode /lib/libc.so.6
0x68152000 0x68167000 4 0 0xa9b28948 rwx 1 0 0x2180 NCOW NNC default -
0x9fbe0000 0x9fc00000 3 0 0xa9b28e70 rwx 1 0 0x2180 NCOW NNC default -

Reported by chappedm on 2011-03-03 04:56:46

[alk]

Right -- this may just require some tweaks to src/base/sysinfo.cc (it looks like the
format of the procs file is different too, which may require a few more changes, though
we do try to have freebsd support in sysinfo.cc already).  Let me know how it turns
out!

Reported by csilvers on 2011-03-03 05:46:30

[alk]

Ok. So I did some late night hackery and found some interesting tidbits.

1. /proc/curproc/map does not list any inode information. In the freebsd specific bits
of src/base/sysinfo.cc we explicitly set inode = 0 because it does not exist in the
proc map.

    693 #elif defined(__FreeBSD__)
    694     // For the format, see http://www.freebsd.org/cgi/cvsweb.cgi/src/sys/fs/procfs/procfs_map.c?rev=1.31&content-type=text/
       x-cvsweb-markup
    695     tmpstart = tmpend = tmpoffset = 0;
    696     tmpinode = 0;
    697     major = minor = 0;   // can't get this info in freebsd
    698     if (inode)
    699       *inode = 0;        // nor this
    700     if (offset)
    701       *offset = 0;       // seems like this should be in there, but maybe not

2. src/heap-checker.cc will always fail if it doesn't find at least one proc map entry
with a non-zero inode number.

    882     if (inode != 0 && strstr(filename, "lib") && strstr(filename, ".so")) {
    883       saw_shared_lib = true;
    884     }

So I think that it is a valid observation that this currently does not work for freebsd6
or freebsd8 for that matter :(. I will see if I can test this and confirm under freebsd8
over the next few days.

Further to this, I decided to see what happens if I force it to ignore the maps checking.
It seems that there are other issues unrelated to the proc maps stuff. The lines begining
with DC: are printfs that I added:

TPC-G6-30$ export HEAPCHECK=normal 
TPC-G6-30$ ./main
Unable to open /proc/self/environ, falling back on getenv("HEAPCHECK"), which may not
work
MemoryRegionMap Init
MemoryRegionMap Init done
Starting tracking the heap
Found hooked allocator at 3: 0x680ad939 <- 0x682c6b8f
Found hooked allocator at 2: 0x680ad939 <- 0x682c4114
Found hooked allocator at 2: 0x680ad939 <- 0x6808fd9e
Found hooked allocator at 2: 0x680ad939 <- 0x682c2b51
Found hooked allocator at 2: 0x680ad939 <- 0x6808fdb8
Found hooked allocator at 2: 0x680abceb <- 0x6808fdd6
Found hooked allocator at 2: 0x680ad939 <- 0x682c4114
Found hooked allocator at 2: 0x680ad939 <- 0x682c8a04
Found hooked allocator at 2: 0x680abceb <- 0x6809f97a
Found hooked allocator at 2: 0x680ad939 <- 0x682c798a
Found hooked allocator at 2: 0x680abceb <- 0x68169184
Found hooked allocator at 2: 0x680abceb <- 0x6809ddf2
Found hooked allocator at 2: 0x680abceb <- 0x68169184
Found hooked allocator at 2: 0x680abceb <- 0x68169184
Found hooked allocator at 2: 0x680abceb <- 0x68169184
Found hooked allocator at 2: 0x680ad939 <- 0x682936a2
DC: Heap checker init
DC: Constructing file with name '/proc/curproc/map'
inode=0 filename=/d2/ports/devel/google-perftools/work/google-perftools-1.6/debug/main
inode=0 filename=-
inode=0 filename=-
inode=0 filename=/libexec/ld-elf.so.1
inode=0 filename=/libexec/ld-elf.so.1
inode=0 filename=-
inode=0 filename=-
inode=0 filename=/d2/local/lib/libtcmalloc.so.0
inode=0 filename=/d2/local/lib/libtcmalloc.so.0
inode=0 filename=/d2/local/lib/libtcmalloc.so.0
inode=0 filename=-
inode=0 filename=/usr/lib/libstdc++.so.6
inode=0 filename=/usr/lib/libstdc++.so.6
inode=0 filename=/usr/lib/libstdc++.so.6
inode=0 filename=-
inode=0 filename=/lib/libm.so.4
inode=0 filename=/lib/libm.so.4
inode=0 filename=/lib/libm.so.4
inode=0 filename=/lib/libc.so.6
inode=0 filename=/lib/libc.so.6
inode=0 filename=/lib/libc.so.6
inode=0 filename=-
inode=0 filename=/usr/lib/libthr.so.2
inode=0 filename=/usr/lib/libthr.so.2
inode=0 filename=/usr/lib/libthr.so.2
inode=0 filename=-
inode=0 filename=/lib/libgcc_s.so.1
inode=0 filename=/lib/libgcc_s.so.1
inode=0 filename=/lib/libgcc_s.so.1
inode=0 filename=-
inode=0 filename=-
inode=0 filename=-
Found hooked allocator at 2: 0x680abceb <- 0x68169184
Found hooked allocator at 2: 0x680abceb <- 0x68095e3e
Found hooked allocator at 2: 0x680abceb <- 0x68169184
Found hooked allocator at 2: 0x680abc4b <- 0x68095f9d
WARNING: Perftools heap leak checker is active -- Performance may suffer
DC: FLAGS_heap_check=normal
Found hooked allocator at 2: 0x680abceb <- 0x680966dd
Found hooked allocator at 2: 0x680abceb <- 0x68093b16
Found hooked allocator at 2: 0x680abc4b <- 0x68093849
Going to ignore live object at 0x80cd040 of 7 bytes
Start check "_main_" profile: 6683 bytes in 18 objects
DC: setting do_main_heap_check=true
********** start **********
Found hooked allocator at 2: 0x680abceb <- 0x8048626
Found hooked allocator at 2: 0x680abceb <- 0x804863c
Found hooked allocator at 2: 0x680abceb <- 0x8048652
Found hooked allocator at 2: 0x680abceb <- 0x8048668
Found hooked allocator at 2: 0x680abceb <- 0x804867e
Found hooked allocator at 2: 0x680abceb <- 0x8048694
*********** end ***********
DC: +HeapLeakChecker_AfterDestructors
DC: FLAGS_heap_check_after_destructors=0
Check failed: !do_main_heap_check: should have done it
DC: +RunHeapCleanups

I didn't get a chance to look too much into this failure but it seems to happen in
all heap check modes. I suspect that it has something to do with the global initialization/destruction
order of objects. Anyone else want to venture a hypothesis?

Reported by chappedm on 2011-03-03 15:41:43

[alk]

Looks like the heap checker doesn't work on freebsd8 either:

(wheel)# export HEAP_CHECK_DUMP_DIRECTORY=/d2/tmp
(wheel)# export HEAPCHECK=normal
(wheel)# LD_PRELOAD=/usr/local/lib/libtcmalloc.so ./main
No shared libs detected. Will likely report false leak positives for statically linked
executables.
Turning perftools heap leak checking off
********** start **********
*********** end ***********
(wheel)#

So this is an issue that will need to be fixed. I am up for the task and will keep
the updates rolling as I make progress.

Reported by chappedm on 2011-03-03 17:33:25

[alk]

Alright, so I have made some progress on this. I have been running against linux and
freebsd6/8 to compare and contrast. These are my findings:

- linux is rock solid, everything works as expected (wish I worked with linux every
day)

- freebsd6/8 both have the same fundamental issues
    - they will only run in draconian mode and the resulting output is a bit strange
    - they both crash in <= strict mode which I have a fix for however when they run
to completion they claim that there is no leak detected which is incorrect
    -mmap, munmap, and sbrk hooks do not get honored and therefore we do not record
any regions
    - mremap does not exist in freebsd which isn't a big deal since it can be and is
better off implemented using mmap/munmap/etc which we will have wrappers for anyways.

Here is the output of one of the unit tests showing the mmap hook failure. I just tweaked
it slightly to also run the mmap portion for freebsd.

TPC-G6-30$ ./tcmalloc_unittest 
Testing empty allocation
Testing STL use
Sanity-testing all the memory allocation functions
Check failed: g_MmapHook_calls > 0

So the hook isn't getting called at all. After a bit more digging in src/malloc_hook.cc
I discovered that there is no support for wrapping mmap, munmap, sbrk, etc for freebsd.
It doesn't look too challenging to add it in.

So to summarize I have the following items to address:

1. Fix the /proc parsing for freebsd
2. Fix the incorrect leak reporting when in any mode other than draconian
3. Fix the crash that occurs when no regions are created as a result of mmap, munmap,
and sbrk not being correctly replaced.
4. Add support for mmap, munmap, sbrk to src/malloc_hook.cc
5. Fix up unit tests to also start verifying freebsd centric components
6. Test against linux and freebsd 6/8 (I don't easily have access to 7)
7. Submit patches for all of the above work.

Let me know if there is anything I missed or perhaps other freebsd-ish type things
that need to be fixed :)

-Dave

Reported by chappedm on 2011-03-04 07:17:36

[alk]

Wow, I had forgotten that we only override mmap for linux systems.  Fixing that would
be great.  In fact, your whole agenda looks great.  I wholeheartedly approve!

I've taken a stab at (1), at least in the context of the heap-checker (making it so
it deals ok with inode being 0).  I'm currently having the heap-checker author look
it over to make sure it's ok, but assuming he gives, the ok, I'll submit it to svn
maybe tomorrow.

Also, feel free to intersperse (7) with the other steps. :-)  Also, if you haven't
already, can you please sign the CLA at http://code.google.com/legal/individual-cla-v1.0.html?

Reported by csilvers on 2011-03-04 08:46:02

[alk]

Ok, I just checked in code to the svn that should help with (1).

Reported by csilvers on 2011-03-04 23:53:03

[alk]

Great! I have a fix ready for (3). I am hoping to find some cycles this weekend to tackle
(4).

Reported by chappedm on 2011-03-05 02:41:50

[alk]

Quick patch for (3). It was either this way or add a new method for checking if the
regions_ were empty/null and protect attempted iterations of regions_. Your call :)
It seems that we can really only end up here if mmap/mremap/sbrk hooks are not correctly
overidden for the taget platform. Maybe a log message would also be useful to warn
the user that their platform is not fully supported.

~/development/google-perftools-read-only/src$ diff ./.svn/text-base/memory_region_map.h.svn-base
memory_region_map.h
282a283,291
>   // A special iterator returned when attempting to retrieve
>   // begin and end iterators for iterating over the memory
>   // regions. Under normal operating conditions there should
>   // always be at least one region. If there isn't, then the
>   // regions_ can be NULL and therefore we need a meaningful
>   // sentinel iterator to return. This can occur on platforms
>   // that do not have mmap/sbrk/mremap hooks correctly overridden.
>   static RegionSet::iterator empty_regions_itr_;
> 

~/development/google-perftools-read-only/src$ diff ./.svn/text-base/memory_region_map.cc.svn-base
memory_region_map.cc
140a141
> MemoryRegionMap::RegionSet::iterator MemoryRegionMap::empty_regions_itr_ = MemoryRegionMap::RegionSet::iterator();
352,353c353
<   RAW_CHECK(regions_ != NULL, "");
<   return regions_->begin();
---
>   return regions_ ? regions_->begin() : empty_regions_itr_;
358,359c358
<   RAW_CHECK(regions_ != NULL, "");
<   return regions_->end();
---
>   return regions_ ? regions_->end() : empty_regions_itr_;

Reported by chappedm on 2011-03-09 03:58:59

[alk]

Hmm, looking at this, it doesn't seem actually all that helpful.  What happens if you
skipped (3) and went straight to (4)?  Then (3) would be unnecessary, right?

Looking at the heap-checker, it looks like fixing (3) won't actually do much to make
the heap-checker actually work better.  So maybe it's better to focus on (4).

Reported by csilvers on 2011-03-10 07:47:47

[alk]

Fair enough :) As far as (4) goes I have things working under freebsd now. I just need
to do some cleanup and additional testing.

Reported by chappedm on 2011-03-11 20:37:06

[alk]

I don't understand much of what is going on above, but just for the record: I have the
same problem ("No shared libraries detected") in linux.

My OS: Debian squeeze/sid amd64 in VirtualBox on Windows 7

I have both google-perftools7 and libgoogle-perftools-7 installed.

Reported by hanulova on 2011-03-15 14:54:05

[alk]

This sounds like a new bug, since it's not related to freebsd.  It could have something
to do with VirtualBox; I'm not clear on what that is or how well it emulates linux.
 What does 'cat /proc/self/maps' say?

Reported by csilvers on 2011-03-15 19:32:38

[alk]

@Dave

Can you share the patches for (2) and (4) - I'm stuck with the same problem too and
I feel your fix might help.

Reported by rajtermite on 2011-03-30 11:57:16

[alk]

Patch is almost ready. This coming Monday at the latest.

Reported by chappedm on 2011-03-30 12:54:36

[alk]

@Dave

Is the patch ready yet?

Reported by rajtermite on 2011-04-07 06:01:39

[alk]

I have attached the patch. Everything appears to be running fine except for 4 failing
unit tests in the heap-checker_unittest suite. As far as I can tell, the failures are
related to the background busy thread in these tests as well as the use of thread specific
storage and some underlying pthread api calls. There are possibly some freebsd-isms
that remain to be worked out here. I attached a copy of the output from 'make check'.
Unfortuantely, I am out of cycles to investigate further right now. As it is, I have
written a few sample processes to test functionality of the heap checker and it is
operating correctly running those isolated tests.

Regards,

Dave

Reported by chappedm on 2011-04-20 16:40:24

---

• Attachment: gpt-freebsd-patch.tgz

[alk]

Thank you for the patch!  I'm currently swamped with work, but will definitely take
a look at this before the next perftools release (which may not be for a month or two
:-( ).

I briefly looked this over now and was surprised by the destructor ordering issue you
saw.  The language guarantees that destructors are called in reverse order of construction.
 This suggests that these things were constructed in a different order on linux and
freebsd.  Can you confirm that that's what you are seeing, in gdb or the like?

If so, it has to do with what the linker does -- it's implementation-defined what order
constructors are run in, but I think the linker typically decides this.  Maybe there's
a way of reordering the files in libtcmalloc.la, in Makefile.am, to get things to work
properly under freebsd 6.

Reported by csilvers on 2011-04-21 02:26:03

[alk]

Just curious if this is available yet in the latest development snapshot.

Regards,

Dave

Reported by chappedm on 2011-06-08 14:36:59

[alk]

I've not had a chance to try to apply this yet.  Did you ever have a chance to look
at the cause of the destructor ordering issues, as I mentioned in my previous comment?
 I'd like to make sure I understand what the problems are that this patch is fixing,
before applying it.

(As I mentioned before, (1) should already be in the svn-root.  But (4) definitely
isn't yet.  I apologize for it taking so long; I've got a big backlog of perftools
changes I'm slowly making my way through.)

Reported by csilvers on 2011-06-08 21:34:18

[alk]

OK, I had a few free moments to look at this today.  I've made the changes to malloc_hook.cc
to add mmap/etc overriding for freebsd, and currently have that out for review.  I
still don't understand the issue with destructor order, so I haven't tried to apply
the patch to heap-checker.cc yet.  I am also waiting to patch the tests (tcmalloc_unittest.cc,
and malloc_hook_test.cc) to enable freebsd until the other changes are in.  Likewise
with the change to configure.ac to enable heap-checking on freebsd by default.  I guess
I'll also need to update the INSTALL file.

I think once this is in then you've done all of (1)-(7)? Except perhaps (2).  I'm hoping
to do a new release shortly, and it would be great to have freebsd fixes be in it.

Reported by csilvers on 2011-07-08 01:50:10

• Status changed: Started

[alk]

Here is the feedback I got on the review of the malloc_hook changes:

------------------------------------
>> MallocHook::InvokeMunmapHook(start, length);
Did you intentionally leave out the MMap and Munmap replacements here?
------------------------------------
>> return mmap(start, length, prot, flags, fd, offset);
Shouldn't this be do_mmap?
------------------------------------
>> return munmap(start, length);
Won't this be hooked?
------------------------------------

These last two comments refer to the UnhookedMmap/unmmap calls, and seem right to me
-- these are actually calling the hooked versions.  I'm not sure what the first comment
is referring to; perhaps it will be clearer to you.

Reported by csilvers on 2011-07-08 22:16:18

[alk]

I tried compiling this on our own freebsd 8.1 machine, and it said:
---
/var/tmp//ccT3ufoe.s: Assembler messages:
/var/tmp//ccT3ufoe.s:3891: Error: suffix or operands invalid for `mov'
---
I have a 64-bit machine; would that explain it?

I'd like to get this patch into the next release, but I feel it's not ready yet.

Reported by csilvers on 2011-07-12 01:00:15

[alk]

OK, I added the relevant code to perftools 1.8, just released, but commented it out
because of the compile errors I was seeing.  Feel free to hack on the new version!
-- hopefully it's pretty easy to work with.

Reported by csilvers on 2011-07-16 01:22:52

• Status changed: Accepted

[alk]

Any more word on this?  It would be great to get it working, but it's not there yet
(at least in my tests).

Reported by csilvers on 2011-09-02 01:15:41

[alk]

    Apologies for letting this go so long. Unfortunately I have left the company I was
working with and no longer have access to a machine running freebsd. However, I see
the fundamental issue here in malloc_hook_mmap_freebsd.h.

1. movl .curbrk, %%eax

    movl is a 32-bit move instruction. So effectively here we are saying, take the
address of the symbol .curbrk and place it in register eax. However, since addresses
on a 64-bit machine will be 64-bit, the operands do not agree with the move instruction.

2. "movl %%eax, %0

    Again we have an issue here because the output operand curbrk is a void*. So we
are trying to take a 32-bit value from eax and place that in a 64-bit memory location.

3. The register eax remains a 32 bit register in the amd64 register set. So the register
used needs to change between 32-bit and 64-bit modes.

The fix here for (1) and (2) is to just use the 'mov' instruction instead of the 'movl'
instruction. The 'mov' instruction will select the correct move instruction based on
the operands. To fix (3) we need to use a preprocessor macro to determine whether we
are compiling for a 32-bit or 64-bit architecture. So the correct replacement for this
inline assembly is:

 #if defined(__x86_64__) || defined(__amd64__)
    __asm__ __volatile__(
        "mov .curbrk, %%rax;"
        "mov %%rax, %0"
        : "=r" (curbrk)
        :: "%rax");
#else
    __asm__ __volatile__(
        "mov .curbrk, %%eax;"
        "mov %%eax, %0"
        : "=r" (curbrk)
        :: "%eax");
#endif

Regards,

Dave

Reported by chappedm on 2011-10-11 21:18:36

[alk]

Thanks for the suggested fix! -- I appreciate your looking after this, even though your
life must be in flux these days.  I'll try it out as I prepare the next release, and
let you know how it works.

Reported by csilvers on 2011-10-11 21:43:38

[alk]

A colleague just pointed out that AT&T assembly format requires the suffix so in the
patch above:

64-bit
    change the mov to movq
32-bit
    change the mov to movl

Reported by chappedm on 2011-10-11 22:09:47

[alk]

OK, thanks.  I changed all the 'mov's like you suggested.

The only freebsd expert I know here is currently on vacation, so I won't be able to
get this in for a few weeks, until he gets back and can review.  But I'll try testing
it out in the meantime.

Reported by csilvers on 2011-10-11 22:18:18

[alk]

So curiosity got the better of me and I setup a vmware player with a Freebsd 8.1 amd-64
image. There is an issue here with PIC that I am fighting with :) Seems that amd-64
doesn't like the use of .curbrk because of some PIC issues. I am looking back over
the sbrk.S freebsd source to try and figure out how to deal with this. I am close to
a solution but my assembly skills are weak sauce. I'll keep you posted. 

Reported by chappedm on 2011-10-13 17:12:42

[alk]

Sounds great!

Reported by csilvers on 2011-10-18 04:55:58

[alk]

I expect to have a patch ready by the end of next week. May need to lean on you a bit
early next week to help me through some background and tricky bits :) 

Reported by chappedm on 2011-10-28 20:20:08

[alk]

Sounds good.  Will do the best I can!

Reported by csilvers on 2011-10-28 22:00:45

[alk]

Any more news on this?  I'm planning on making a new release in the next few days, and
it would be great to have this patch in if possible.

Reported by csilvers on 2012-01-25 23:24:20

[alk]

This went in with all of the other freebsd porting work.

Reported by chappedm on 2012-01-26 01:36:52

[alk]

Closing this off as all of the core patches have been tested and applied. First supported
release for the FreeBSD porting work above is google-perftools-1.9.1 and gperftools-2.0.

Reported by chappedm on 2012-02-07 02:56:39

[alk]

Reported by chappedm on 2012-02-07 02:59:38

• Status changed: Fixed