Patch out posix_memalign in the out-of-memory killer....

Patch out posix_memalign in the out-of-memory killer.

BUG=http://crbug.com/44189
TEST=included in base_unittests


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=47714

[Mark Mentovai, 2010-05-19 19:33:22]

http://codereview.chromium.org/2110010/diff/1/2
File base/process_util_mac.mm (right):

http://codereview.chromium.org/2110010/diff/1/2#newcode360
base/process_util_mac.mm:360: // === C malloc/calloc/valloc/realloc ===
C malloc/calloc/valloc/realloc/posix_memalign

http://codereview.chromium.org/2110010/diff/1/2#newcode362
base/process_util_mac.mm:362: // The extended version of malloc_zone_t. From
<malloc/malloc.h>, included here
Say 10.6 in this, because 10.5’s <malloc/malloc.h> obviously doesn’t have this.

http://codereview.chromium.org/2110010/diff/1/2#newcode381
base/process_util_mac.mm:381: void* (*memalign)(struct _malloc_zone_t* zone,
size_t alignment, size_t size);
You should have a comment here saying that memalign field is only present when
version >= 5. Apple should have put this comment in their header, but they
didn’t.

http://codereview.chromium.org/2110010/diff/1/2#newcode382
base/process_util_mac.mm:382: void (*free_definite_size)(struct _malloc_zone_t*
zone, void* ptr,
version >= 6.

http://codereview.chromium.org/2110010/diff/1/2#newcode444
base/process_util_mac.mm:444: if (!result && size)
g_old_memalign can return NULL for reasons other than out-of-memory. You
shouldn’t break in those cases. Copy the logic from Libc-594.1.4/gen/malloc.c’s
posix_memalign to only catch cases where posix_memalign would return ENOMEM, and
include a comment saying what you’re doing and where you (I) copied the logic
from:

  if (!result &&
      alignment >= sizeof(void*) && (alignment & (alignment - 1)) == 0 &&
      size) {
    DebugUtil::BreakDebugger();
  }

http://codereview.chromium.org/2110010/diff/1/2#newcode586
base/process_util_mac.mm:586: if (default_zone->version >= 5) {
Comment here (or alongside the structure definition, maybe by its version field)
that the default zone’s version is 3 on 10.5.8 and 6 on 10.6.3.

http://codereview.chromium.org/2110010/diff/1/2#newcode596
base/process_util_mac.mm:596: // === C batch_malloc ===
batch_malloc isn’t an accessible C function, there’s only
malloc_zone_batch_malloc.

http://codereview.chromium.org/2110010/diff/1/3
File base/process_util_unittest.cc (right):

http://codereview.chromium.org/2110010/diff/1/3#newcode644
base/process_util_unittest.cc:644: LOG(WARNING) <<
I don’t want to warn every time we run this test on 10.5. Leave this out.

I recommend merging this test with the one used for Linux, like so:

(inside #if defiend(OS_POSIX)):

TEST_F(OutOfMemoryTest, Posix_memalign) {
  typedef int (*memalign_t)(void**, size_t, size_t);
#if defined(OS_MACOSX)
  // posix_memalign only exists on >= 10.6. Use dlsym to grab it at runtime
  // because it may not be present in the SDK used for compilation.
  memalign_t memalign =
      reinterpret_cast<memalign_t>(dlsym(RTLD_DEFAULT, "posix_memalign"));
#else
  memalign_t memalign = posix_memalign;
#endif
  if (memalign) {
    // Grab the return value of posix_memalign to silence a compiler warning
    // about unused return values. We don't actually care about the return
    // value, since we're asserting death.
    ASSERT_DEATH(EXPECT_EQ(ENOMEM, memalign(&value_, 8, test_size_)), "");
  }
}

[Avi (use Gerrit), 2010-05-19 20:04:38]

All addressed.

[Mark Mentovai, 2010-05-19 20:11:17]

http://codereview.chromium.org/2110010/diff/6001/7001
File base/process_util_mac.mm (right):

http://codereview.chromium.org/2110010/diff/6001/7001#newcode450
base/process_util_mac.mm:450: && 0 == (alignment & (alignment - 1)))
Use {}. That rule should be applied if the conditional statement is multi-line
too. Especially with the continuation line wrapping to a different indent, the
{} makes the distinction between the condition and the controlled statement
really clear.

http://codereview.chromium.org/2110010/diff/6001/7001#newcode450
base/process_util_mac.mm:450: && 0 == (alignment & (alignment - 1)))
Googlers generally write |expression == 0| and not that nutty |0 == expression|
thing. Googlers are apparently not afraid of accidentally writing expression =
0.

[Mark Mentovai, 2010-05-19 20:11:22]

lg