This is an archive of the discontinued LLVM Phabricator instance.

[ASAN] Port asan to FreeBSD/mips64.
Needs RevisionPublic

Authored by bsdjhb on Jul 27 2018, 3:04 PM.

Details

Reviewers
kcc
vitalybuka
morehouse
Group Reviewers
Restricted Project
Summary

FreeBSD/mips64 uses the same shadow offset as MIPS64 on Linux.

Event Timeline

bsdjhb created this revision.Jul 27 2018, 3:04 PM
Herald added subscribers: Restricted Project, atanasyan, delcypher and 4 others. · View Herald TranscriptJul 27 2018, 3:04 PM

The LLVM changes to use the correct shadow offset on FreeBSD/mips64 are in D49939.

lib/sanitizer_common/sanitizer_linux.cc
1249

Rather than adding '&& SANITIZER_LINUX' to the #ifdef for each architecture as they are ported to other systems, we might consider wrapping all of the 'internal_clone' implementations in a single '#if SANITIZER_LINUX'.

lib/sanitizer_common/sanitizer_syscall_generic.inc
46

__syscall is valid for all FreeBSD platforms.

bsdjhb added a reviewer: kcc.Aug 1 2018, 3:59 PM
jrtc27 added a subscriber: jrtc27.Aug 1 2018, 4:17 PM
jrtc27 added inline comments.
lib/asan/asan_mapping.h
220

Is there a reason why you don't just hoist the __mips64 case up to line 215 to come before this, like is done for 32-bit MIPS on line 188?

krytarowski added inline comments.Aug 1 2018, 4:33 PM
lib/sanitizer_common/sanitizer_syscall_generic.inc
46

NetBSD will switch away from the syscall(2)/__syscall(2) approach and use libc natively. These indirect syscall functions are not portable enough and generate problems for various ABIs.

bsdjhb added inline comments.Aug 2 2018, 1:25 PM
lib/asan/asan_mapping.h
220

Actually, I meant to ask that as a comment when I posted this in terms of what do folks think is the better approach. Thanks for the reminder.

do we still need this patch?

vitalybuka requested changes to this revision.Nov 29 2018, 2:02 PM

Just to push from "Ready to Review" list.

This revision now requires changes to proceed.Nov 29 2018, 2:02 PM

Sorry for the delay. This does still need review from folks familiar with sanitizers. I haven't yet rebased it to a more recent tree due to lack of review feedback. If there is interest in reviewing it I can update it to a newer tree and retest (or if there is feedback on the current patches I'm happy to update them).

This code is out of sync with HEAD.

arichardson added a reviewer: Restricted Project.May 8 2019, 10:48 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMay 8 2019, 10:48 AM