This is an archive of the discontinued LLVM Phabricator instance.

[sanitizer_common] Use O_TRUNC for WrOnly access mode.
ClosedPublic

Authored by MaskRay on Jun 15 2018, 4:24 PM.

Details

Summary

Otherwise if the file existed and was larger than the write size before the OpenFile call, the file will not be truncated and contain garbage in trailing bytes.

Event Timeline

MaskRay created this revision.Jun 15 2018, 4:24 PM
timurrrr edited reviewers, added: kcc; removed: timurrrr.Jun 15 2018, 4:43 PM

Could you please add corresponding check into "TEST(SanitizerCommon, FileOps)" and make sure that all platforms have the same behavior.

MaskRay added a comment.EditedJun 15 2018, 5:50 PM

Added test. BTW, is my testing correct?

% ninja -C ~/Dev/llvm/release TSanitizer-i386-Test TSanitizer-x86_64-Test TSanitizer-x86_64-Test-Nolibc && ~/Dev/llvm/release/projects/
compiler-rt/lib/sanitizer_common/tests/Sanitizer-i386-Test && ~/Dev/llvm/release/projects/compiler-rt/lib/sanitizer_common/tests/Sanitiz
er-x86_64-Test
# passed

Added test. BTW, is my testing correct?

% ninja -C ~/Dev/llvm/release TSanitizer-i386-Test TSanitizer-x86_64-Test TSanitizer-x86_64-Test-Nolibc && ~/Dev/llvm/release/projects/
compiler-rt/lib/sanitizer_common/tests/Sanitizer-i386-Test && ~/Dev/llvm/release/projects/compiler-rt/lib/sanitizer_common/tests/Sanitiz
er-x86_64-Test
# passed

following should run the test

ninja -C ~/Dev/llvm/release check-sanitizer
vitalybuka added inline comments.Jun 15 2018, 6:32 PM
lib/sanitizer_common/sanitizer_posix.cc
163

please also fix
compiler-rt/lib/sanitizer_common/sanitizer_rtems.cc

windows looks good
compiler-rt/lib/sanitizer_common/sanitizer_win.cc

MaskRay updated this revision to Diff 151600.Jun 15 2018, 7:49 PM

Add other platforms as vitalybuka suggested

Thanks for informing me of the check-sanitizer target. 64 SanitizerCommon-lsan-i386-Linux tests already fail before this revision. All other tests pass.

vitalybuka added inline comments.
lib/sanitizer_common/sanitizer_solaris.cc
81 ↗(On Diff #151600)

This function has different signature.

@krytarowski I don't see any calls to this function. Is this here accidentally?

MaskRay updated this revision to Diff 151601.Jun 15 2018, 8:12 PM

Remove change to solaris which seems already broken.

MaskRay marked an inline comment as done.Jun 15 2018, 8:14 PM
MaskRay added inline comments.
lib/sanitizer_common/sanitizer_solaris.cc
81 ↗(On Diff #151600)

I don't know much about the file history. But I guess the signature of OpenFile has changed from two parameters to three parameters at some point and the solaris version does not catch up.

// Returns kInvalidFd on error.
fd_t OpenFile(const char *filename, FileAccessMode mode,
              error_t *errno_p = nullptr);

Just reverted the change to the solaris file.

vitalybuka accepted this revision.Jun 15 2018, 8:34 PM
This revision is now accepted and ready to land.Jun 15 2018, 8:34 PM
This revision was automatically updated to reflect the committed changes.
MaskRay marked an inline comment as done.
krytarowski added a subscriber: ro.Jun 16 2018, 2:44 AM
krytarowski added inline comments.
lib/sanitizer_common/sanitizer_solaris.cc
81 ↗(On Diff #151600)

Hmm, CC @ro. I'm not sure about SunOS.