This is an archive of the discontinued LLVM Phabricator instance.

[ESan] [MIPS] Fix workingset-signal-posix.cpp on MIPS
ClosedPublic

Authored by slthakur on Sep 8 2016, 2:10 AM.

Details

Reviewers
zhaoqin
bruening
Summary
  • Used uptr for __sanitizer_kernel_sigset_t.sig to avoid byte order issues on big endian systems
  • Corrected the data type of __sanitizer_kernel_sigaction_t.sa_flags which is unsigned int

Diff Detail

Repository
rL LLVM

Event Timeline

slthakur updated this revision to Diff 70661.Sep 8 2016, 2:10 AM
slthakur retitled this revision from to [ESan] [MIPS] Fix workingset-signal-posix.cpp on MIPS.
slthakur updated this object.
slthakur added a reviewer: bruening.
slthakur set the repository for this revision to rL LLVM.
slthakur added a project: Restricted Project.
slthakur added subscribers: jaydeep, llvm-commits.
bruening added inline comments.Sep 8 2016, 1:21 PM
lib/sanitizer_common/sanitizer_platform_limits_posix.h
610

Aren't there references in source code files that need to be updated for this change? At a glance I see things like "(sizeof(k_set->sig[0]) * 8)".

slthakur added inline comments.Sep 13 2016, 11:34 PM
lib/sanitizer_common/sanitizer_platform_limits_posix.h
610

There the two functions which reference __sanitizer_kernel_sigset_t.sig:

  • internal_sigdelset
  • internal_sigismember

Both of these functions were producing wrong results on big endian machines because __sanitizer_kernel_sigset_t.sig was declared as a char array. Both these functions return correct result with this change.

slthakur removed a subscriber: zhaoqin.
seanbruno added a subscriber: seanbruno.

This review *looks* like it has addressed all comment feedback. Does it need to be recreated against trunk?

bruening accepted this revision.Sep 26 2016, 10:10 AM
bruening edited edge metadata.
bruening added inline comments.
lib/sanitizer_common/sanitizer_platform_limits_posix.h
610

OK, you're saying that the *kernel* defines the signal set array as some multi-byte structure (unsigned long) and thus endian-ness comes into play between the sanitizer code and the kernel.

This revision is now accepted and ready to land.Sep 26 2016, 10:10 AM
slthakur closed this revision.Oct 6 2016, 3:30 AM

Committed revision 283438

lib/sanitizer_common/sanitizer_platform_limits_posix.h
610

Yes.