This is an archive of the discontinued LLVM Phabricator instance.

Differential D44713

More OpenBSD fixes
ClosedPublic

Authored by devnexen on Mar 20 2018, 3:46 PM.

Details

Reviewers
krytarowski
vitalybuka
Commits
rG022a82cb15ca: More OpenBSD fixes
rCRT328239: More OpenBSD fixes
rL328239: More OpenBSD fixes
Summary
  • Use internal_syscall_ptr in internal_readlink
  • use sigcontext on OpenBSD

Patch by David CARLIER

Diff Detail

Repository
rL LLVM

Event Timeline

devnexen created this revision.Mar 20 2018, 3:46 PM
Herald added subscribers: Restricted Project, llvm-commits, kubamracek. · View Herald TranscriptMar 20 2018, 3:46 PM
devnexen edited the summary of this revision. (Show Details)Mar 20 2018, 3:49 PM
vitalybuka added inline comments.Mar 21 2018, 12:53 AM
lib/sanitizer_common/sanitizer_linux.cc
72 ↗(On Diff #139216)

this should not be needed after r328077

341 ↗(On Diff #139216)

What about this?

1754 ↗(On Diff #139216)

Why zeroes? Can you get real values?

devnexen added inline comments.Mar 21 2018, 1:48 AM
lib/sanitizer_common/sanitizer_linux.cc
72 ↗(On Diff #139216)

I commented this commit here

341 ↗(On Diff #139216)

I thought I did this one ...

1754 ↗(On Diff #139216)

ucontext not supported here.

devnexen added inline comments.Mar 21 2018, 1:52 AM
lib/sanitizer_common/sanitizer_linux.cc
72 ↗(On Diff #139216)

Nice works better under FreeBSD should do the job under OpenBSD.

devnexen updated this revision to Diff 139262.Mar 21 2018, 1:57 AM
vitalybuka added inline comments.Mar 21 2018, 11:02 AM
lib/sanitizer_common/sanitizer_linux.cc
311 ↗(On Diff #139262)

newfstatat should return int, so no _ptr

317 ↗(On Diff #139262)

same for stat

404 ↗(On Diff #139262)

same for unlinkat, it returns int, on both

72 ↗(On Diff #139216)

Sorry, that was a typo.

341 ↗(On Diff #139216)

Sorry, but I only asked, because I am not sure why you are making this change.
According sanitizer_common/sanitizer_syscall_generic.inc

internal_syscall_ptr for pointer and (s)size_t

Looking on documentation all of them return int and internal_syscall supposed to be used. What am I missing?

1754 ↗(On Diff #139216)

https://man.openbsd.org/sigaction.2

On OpenBSD, ucontext_t is an alias for the sigcontext structure defined in <signal.h>.

devnexen added inline comments.Mar 21 2018, 11:39 AM
lib/sanitizer_common/sanitizer_linux.cc
404 ↗(On Diff #139262)

Seems under Linux it s the same in fact https://github.com/llvm-mirror/compiler-rt/blob/b4e600115ad4c50192095c225aaed3ae9ca99ec1/lib/sanitizer_common/sanitizer_syscall_generic.inc#L53 but I personally do not mind reverting them I think it was kamel who asked me to update in the first place.

vitalybuka added inline comments.Mar 21 2018, 12:36 PM
lib/sanitizer_common/sanitizer_linux.cc
404 ↗(On Diff #139262)

it's the same, but if we introduced internal_syscall_ptr and we specified that it's for pointers and size, we should follow that convention.

devnexen added inline comments.Mar 21 2018, 12:42 PM
lib/sanitizer_common/sanitizer_linux.cc
404 ↗(On Diff #139262)

Good enough for me.

devnexen updated this revision to Diff 139354.Mar 21 2018, 12:47 PM
vitalybuka requested changes to this revision.Mar 21 2018, 1:23 PM
vitalybuka added inline comments.
lib/sanitizer_common/sanitizer_linux.cc
325 ↗(On Diff #139354)

I think it should be internal_syscall() as it's int everywhere, why do you need ptr?
And this is not OpenBSD branch at all

395 ↗(On Diff #139354)

On linux readlinkat is int, on OpenBSD it's size. So it should be two different branches

412 ↗(On Diff #139354)

it's int on OpenBSD as well https://github.com/openbsd/src/blob/master/sys/sys/syscall.h

1754 ↗(On Diff #139216)

Could you please comment about your plans here?

This revision now requires changes to proceed.Mar 21 2018, 1:23 PM
devnexen added inline comments.Mar 21 2018, 1:38 PM
lib/sanitizer_common/sanitizer_linux.cc
1754 ↗(On Diff #139216)

All good points ... Did not know about sigcontext

devnexen updated this revision to Diff 139366.Mar 21 2018, 1:54 PM

Adding sigcontext support

vitalybuka added inline comments.Mar 22 2018, 10:47 AM
lib/sanitizer_common/sanitizer_linux.cc
412 ↗(On Diff #139354)

it's int on OpenBSD as well
https://github.com/openbsd/src/blob/master/sys/sys/syscall.h

What do you think about this one?

devnexen added inline comments.Mar 22 2018, 10:53 AM
lib/sanitizer_common/sanitizer_linux.cc
412 ↗(On Diff #139354)

Sorry I missed this one good point.

devnexen updated this revision to Diff 139477.Mar 22 2018, 11:00 AM
vitalybuka accepted this revision.Mar 22 2018, 11:20 AM
This revision is now accepted and ready to land.Mar 22 2018, 11:20 AM
vitalybuka retitled this revision from Unsubmitted part of D44599 to More OpenBSD fixes.Mar 22 2018, 11:21 AM
vitalybuka edited the summary of this revision. (Show Details)
vitalybuka edited the summary of this revision. (Show Details)
Closed by commit rL328239: More OpenBSD fixes (authored by vitalybuka). · Explain WhyMar 22 2018, 1:46 PM
This revision was automatically updated to reflect the committed changes.
Herald added a subscriber: delcypher. · View Herald TranscriptMar 22 2018, 1:46 PM

Revision Contents

PathSize
compiler-rt/
trunk/
lib/
sanitizer_common/
34 lines

Diff 139504

compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cc

Show First 20 Lines • Show All 62 Lines • ▼ Show 20 Lines
#include <sys/stat.h> #include <sys/stat.h>
#include <sys/syscall.h> #include <sys/syscall.h>
#include <sys/time.h> #include <sys/time.h>
#include <sys/types.h> #include <sys/types.h>
#if !SANITIZER_OPENBSD #if !SANITIZER_OPENBSD
#include <ucontext.h> #include <ucontext.h>
#endif #endif
#if SANITIZER_OPENBSD #if SANITIZER_OPENBSD
#include <signal.h>
#include <sys/futex.h> #include <sys/futex.h>
#endif #endif
#include <unistd.h> #include <unistd.h>
#if SANITIZER_LINUX #if SANITIZER_LINUX
#include <sys/utsname.h> #include <sys/utsname.h>
#endif #endif
▲ Show 20 Lines • Show All 307 Lines • ▼ Show 20 Lines
#if SANITIZER_USES_CANONICAL_LINUX_SYSCALLS #if SANITIZER_USES_CANONICAL_LINUX_SYSCALLS
return internal_syscall(SYSCALL(dup3), oldfd, newfd, 0); return internal_syscall(SYSCALL(dup3), oldfd, newfd, 0);
#else #else
return internal_syscall(SYSCALL(dup2), oldfd, newfd); return internal_syscall(SYSCALL(dup2), oldfd, newfd);
#endif #endif
} }
uptr internal_readlink(const char *path, char *buf, uptr bufsize) { uptr internal_readlink(const char *path, char *buf, uptr bufsize) {
#if SANITIZER_USES_CANONICAL_LINUX_SYSCALLS || SANITIZER_OPENBSD #if SANITIZER_USES_CANONICAL_LINUX_SYSCALLS
return internal_syscall(SYSCALL(readlinkat), AT_FDCWD, (uptr)path, (uptr)buf, return internal_syscall(SYSCALL(readlinkat), AT_FDCWD, (uptr)path, (uptr)buf,
bufsize); bufsize);
#elif SANITIZER_OPENBSD
return internal_syscall_ptr(SYSCALL(readlinkat), AT_FDCWD, (uptr)path,
(uptr)buf, bufsize);
#else #else
return internal_syscall_ptr(SYSCALL(readlink), path, buf, bufsize); return internal_syscall_ptr(SYSCALL(readlink), path, buf, bufsize);
#endif #endif
} }
uptr internal_unlink(const char *path) { uptr internal_unlink(const char *path) {
#if SANITIZER_USES_CANONICAL_LINUX_SYSCALLS || SANITIZER_OPENBSD #if SANITIZER_USES_CANONICAL_LINUX_SYSCALLS || SANITIZER_OPENBSD
return internal_syscall(SYSCALL(unlinkat), AT_FDCWD, (uptr)path, 0); return internal_syscall(SYSCALL(unlinkat), AT_FDCWD, (uptr)path, 0);
▲ Show 20 Lines • Show All 1,292 Lines • ▼ Show 20 Lines if (ctx->magic == kEsrMagic) {
return true; return true;
} }
aux += ctx->size; aux += ctx->size;
} }
return false; return false;
} }
#endif #endif
#if SANITIZER_OPENBSD
using Context = sigcontext;
#else
using Context = ucontext;
#endif // !SANITIZER_OPENBSD
SignalContext::WriteFlag SignalContext::GetWriteFlag() const { SignalContext::WriteFlag SignalContext::GetWriteFlag() const {
#if !SANITIZER_OPENBSD
ucontext_t *ucontext = (ucontext_t *)context; ucontext_t *ucontext = (ucontext_t *)context;
#if defined(__x86_64__) || defined(__i386__) #if defined(__x86_64__) || defined(__i386__)
static const uptr PF_WRITE = 1U << 1; static const uptr PF_WRITE = 1U << 1;
#if SANITIZER_FREEBSD #if SANITIZER_FREEBSD
uptr err = ucontext->uc_mcontext.mc_err; uptr err = ucontext->uc_mcontext.mc_err;
#elif SANITIZER_NETBSD #elif SANITIZER_NETBSD
uptr err = ucontext->uc_mcontext.__gregs[_REG_ERR]; uptr err = ucontext->uc_mcontext.__gregs[_REG_ERR];
#elif SANITIZER_OPENBSD
uptr err = ucontext->sc_err;
#elif SANITIZER_SOLARIS && defined(__i386__) #elif SANITIZER_SOLARIS && defined(__i386__)
# define ERR 13 const int Err = 13;
uptr err = ucontext->uc_mcontext.gregs[ERR]; uptr err = ucontext->uc_mcontext.gregs[Err];
#else #else
uptr err = ucontext->uc_mcontext.gregs[REG_ERR]; uptr err = ucontext->uc_mcontext.gregs[REG_ERR];
#endif #endif // SANITIZER_FREEBSD
return err & PF_WRITE ? WRITE : READ; return err & PF_WRITE ? WRITE : READ;
#elif defined(__arm__) #elif defined(__arm__)
static const uptr FSR_WRITE = 1U << 11; static const uptr FSR_WRITE = 1U << 11;
uptr fsr = ucontext->uc_mcontext.error_code; uptr fsr = ucontext->uc_mcontext.error_code;
return fsr & FSR_WRITE ? WRITE : READ; return fsr & FSR_WRITE ? WRITE : READ;
#elif defined(__aarch64__) #elif defined(__aarch64__)
static const u64 ESR_ELx_WNR = 1U << 6; static const u64 ESR_ELx_WNR = 1U << 6;
u64 esr; u64 esr;
if (!Aarch64GetESR(ucontext, &esr)) return UNKNOWN; if (!Aarch64GetESR(ucontext, &esr)) return UNKNOWN;
return esr & ESR_ELx_WNR ? WRITE : READ; return esr & ESR_ELx_WNR ? WRITE : READ;
#elif SANITIZER_SOLARIS && defined(__sparc__) #elif SANITIZER_SOLARIS && defined(__sparc__)
// Decode the instruction to determine the access type. // Decode the instruction to determine the access type.
// From OpenSolaris $SRC/uts/sun4/os/trap.c (get_accesstype). // From OpenSolaris $SRC/uts/sun4/os/trap.c (get_accesstype).
uptr pc = ucontext->uc_mcontext.gregs[REG_PC]; uptr pc = ucontext->uc_mcontext.gregs[REG_PC];
u32 instr = *(u32 *)pc; u32 instr = *(u32 *)pc;
return (instr >> 21) & 1 ? WRITE: READ; return (instr >> 21) & 1 ? WRITE: READ;
#else #else
(void)ucontext; (void)ucontext;
return UNKNOWN; // FIXME: Implement. return UNKNOWN; // FIXME: Implement.
#endif #endif
#else // !SANITIZER_OPENBSD
return UNKNOWN;
#endif // !SANITIZER_OPENBSD
} }
void SignalContext::DumpAllRegisters(void *context) { void SignalContext::DumpAllRegisters(void *context) {
// FIXME: Implement this. // FIXME: Implement this.
} }
static void GetPcSpBp(void *context, uptr *pc, uptr *sp, uptr *bp) { static void GetPcSpBp(void *context, uptr *pc, uptr *sp, uptr *bp) {
#if SANITIZER_NETBSD #if SANITIZER_NETBSD
Show All 19 Lines#elif defined(__hppa__)
*bp = ucontext->uc_mcontext.sc_gr[3]; *bp = ucontext->uc_mcontext.sc_gr[3];
*sp = ucontext->uc_mcontext.sc_gr[30]; *sp = ucontext->uc_mcontext.sc_gr[30];
#elif defined(__x86_64__) #elif defined(__x86_64__)
# if SANITIZER_FREEBSD # if SANITIZER_FREEBSD
ucontext_t *ucontext = (ucontext_t*)context; ucontext_t *ucontext = (ucontext_t*)context;
*pc = ucontext->uc_mcontext.mc_rip; *pc = ucontext->uc_mcontext.mc_rip;
*bp = ucontext->uc_mcontext.mc_rbp; *bp = ucontext->uc_mcontext.mc_rbp;
*sp = ucontext->uc_mcontext.mc_rsp; *sp = ucontext->uc_mcontext.mc_rsp;
#elif SANITIZER_OPENBSD
sigcontext *ucontext = (sigcontext *)context;
*pc = ucontext->sc_rip;
*bp = ucontext->sc_rbp;
*sp = ucontext->sc_rsp;
# else # else
ucontext_t *ucontext = (ucontext_t*)context; ucontext_t *ucontext = (ucontext_t*)context;
*pc = ucontext->uc_mcontext.gregs[REG_RIP]; *pc = ucontext->uc_mcontext.gregs[REG_RIP];
*bp = ucontext->uc_mcontext.gregs[REG_RBP]; *bp = ucontext->uc_mcontext.gregs[REG_RBP];
*sp = ucontext->uc_mcontext.gregs[REG_RSP]; *sp = ucontext->uc_mcontext.gregs[REG_RSP];
# endif # endif
#elif defined(__i386__) #elif defined(__i386__)
# if SANITIZER_FREEBSD # if SANITIZER_FREEBSD
ucontext_t *ucontext = (ucontext_t*)context; ucontext_t *ucontext = (ucontext_t*)context;
*pc = ucontext->uc_mcontext.mc_eip; *pc = ucontext->uc_mcontext.mc_eip;
*bp = ucontext->uc_mcontext.mc_ebp; *bp = ucontext->uc_mcontext.mc_ebp;
*sp = ucontext->uc_mcontext.mc_esp; *sp = ucontext->uc_mcontext.mc_esp;
#elif SANITIZER_OPENBSD
sigcontext *ucontext = (sigcontext *)context;
*pc = ucontext->sc_eip;
*bp = ucontext->sc_ebp;
*sp = ucontext->sc_esp;
# else # else
ucontext_t *ucontext = (ucontext_t*)context; ucontext_t *ucontext = (ucontext_t*)context;
# if SANITIZER_SOLARIS # if SANITIZER_SOLARIS
/* Use the numeric values: the symbolic ones are undefined by llvm /* Use the numeric values: the symbolic ones are undefined by llvm
include/llvm/Support/Solaris.h. */ include/llvm/Support/Solaris.h. */
# ifndef REG_EIP # ifndef REG_EIP
# define REG_EIP 14 // REG_PC # define REG_EIP 14 // REG_PC
# endif # endif
▲ Show 20 Lines • Show All 121 Lines • Show Last 20 Lines