This is an archive of the discontinued LLVM Phabricator instance.

Sanitizers: Implement `GetRSS` on Mac OS X
ClosedPublic

Authored by ismailp on May 9 2015, 8:01 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

ismailp updated this revision to Diff 25407.May 9 2015, 8:01 AM
ismailp retitled this revision from to Sanitizers: Implement `GetRSS` on Mac OS X.
ismailp updated this object.
ismailp edited the test plan for this revision. (Show Details)
ismailp added reviewers: kcc, glider, dvyukov.
ismailp added a subscriber: Unknown Object (MLST).
dvyukov added inline comments.May 10 2015, 1:06 AM
lib/sanitizer_common/sanitizer_mac.cc
341 ↗(On Diff #25407)

We don't use C89 declaration style, so please do:
unsigned count = ...
and the same for the other variables.

345 ↗(On Diff #25407)

Is it really can happen? Or it is more like "this should never happen"?
If it the latter, then make it fatal by calling Die(). Otherwise print only in verbose more (I think we now have VPrintf(1, "..."). Whenever we print something in normal operation mode, some programs fail. Consider a generator program, this output will become part of generated output and will necessary fail the rest of the test.

347 ↗(On Diff #25407)

s/0U/0/ please
0 should convert to uptr just fine. This code is compiled with -Wall by two compilers clean, that should catch any conversion bugs.

ismailp updated this revision to Diff 25502.May 11 2015, 1:30 PM

Addressed comments.

glider accepted this revision.May 12 2015, 2:36 AM
glider edited edge metadata.

LGTM with two nits.

lib/sanitizer_common/sanitizer_mac.cc
47 ↗(On Diff #25502)

While at it, can you please sort this block of includes alphabetically?

343 ↗(On Diff #25502)

NB: currently task_info() can't possibly return anything but KERN_SUCCESS, so it's perfectly fine to die.

345 ↗(On Diff #25502)

Nit: spare newline.

This revision is now accepted and ready to land.May 12 2015, 2:36 AM
dvyukov accepted this revision.May 12 2015, 3:09 AM
dvyukov edited edge metadata.

LGTM

ismailp updated this revision to Diff 25615.May 12 2015, 1:49 PM
ismailp edited edge metadata.
  • Sort includes alphabetically
  • Remove newline in GetRSS
This revision was automatically updated to reflect the committed changes.

Thank you both for your time!