This is an archive of the discontinued LLVM Phabricator instance.

Differential D57863

[safestack] Explain why tinfo at the end of the buffer
ClosedPublic

Authored by vitalybuka on Feb 6 2019, 4:14 PM.

Details

Reviewers
pcc
eugenis
vlad.tsyrklevich
Commits
rG871b2bd98030: [safestack] Explain why tinfo at the end of the buffer
rCRT353482: [safestack] Explain why tinfo at the end of the buffer
rL353482: [safestack] Explain why tinfo at the end of the buffer

Diff Detail

Event Timeline

vitalybuka created this revision.Feb 6 2019, 4:14 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 6 2019, 4:15 PM
Herald added subscribers: Restricted Project, jfb. · View Herald Transcript
Harbormaster completed remote builds in B27851: Diff 185665.Feb 6 2019, 4:15 PM
vitalybuka edited the summary of this revision. (Show Details)Feb 6 2019, 4:18 PM
vitalybuka added reviewers: pcc, eugenis.
vitalybuka added a reviewer: vlad.tsyrklevich.Feb 7 2019, 1:30 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 7 2019, 1:30 PM
Herald added a subscriber: llvm-commits. · View Herald Transcript
vlad.tsyrklevich added inline comments.Feb 7 2019, 1:39 PM
compiler-rt/lib/safestack/safestack.cc
235

Shouldn't this be addr+guard?

vitalybuka marked an inline comment as done.Feb 7 2019, 2:13 PM
vitalybuka added inline comments.
compiler-rt/lib/safestack/safestack.cc
235

unsafe_stack_alloc already does that

vlad.tsyrklevich accepted this revision.Feb 7 2019, 2:15 PM
This revision is now accepted and ready to land.Feb 7 2019, 2:15 PM
vitalybuka updated this revision to Diff 185877.Feb 7 2019, 3:17 PM

do nothing

vitalybuka retitled this revision from [safestack] Simplify tinfo location calculation to [safestack] Explain why tinfo at the end of the buffer.Feb 7 2019, 3:18 PM
vitalybuka edited the summary of this revision. (Show Details)
Harbormaster completed remote builds in B27924: Diff 185877.Feb 7 2019, 3:18 PM
vitalybuka updated this revision to Diff 185878.Feb 7 2019, 3:19 PM

typo

Harbormaster completed remote builds in B27925: Diff 185878.Feb 7 2019, 3:20 PM
Closed by commit rL353482: [safestack] Explain why tinfo at the end of the buffer (authored by vitalybuka). · Explain WhyFeb 7 2019, 3:23 PM
This revision was automatically updated to reflect the committed changes.
Herald added a subscriber: delcypher. · View Herald TranscriptFeb 7 2019, 3:23 PM

Revision Contents

PathSize
compiler-rt/
lib/
safestack/
3 lines

Diff 185665

compiler-rt/lib/safestack/safestack.cc

Show First 20 Lines • Show All 226 Lines • ▼ Show 20 Lines if (attr) {
pthread_attr_destroy(&tmpattr); pthread_attr_destroy(&tmpattr);
} }
SFS_CHECK(size); SFS_CHECK(size);
SFS_CHECK((size & (kStackAlign - 1)) == 0); SFS_CHECK((size & (kStackAlign - 1)) == 0);
SFS_CHECK((guard & (pageSize - 1)) == 0); SFS_CHECK((guard & (pageSize - 1)) == 0);
void *addr = unsafe_stack_alloc(size, guard); void *addr = unsafe_stack_alloc(size, guard);
struct tinfo *tinfo = struct tinfo *tinfo = reinterpret_cast<struct tinfo *>(addr);
vlad.tsyrklevichUnsubmitted
Not Done
ReplyInline Actions

Shouldn't this be addr+guard?

vlad.tsyrklevich: Shouldn't this be addr+guard?
vitalybukaAuthorUnsubmitted
Done
ReplyInline Actions

unsafe_stack_alloc already does that

vitalybuka: unsafe_stack_alloc already does that
(struct tinfo *)(((char *)addr) + size - sizeof(struct tinfo));
tinfo->start_routine = start_routine; tinfo->start_routine = start_routine;
tinfo->start_routine_arg = arg; tinfo->start_routine_arg = arg;
tinfo->unsafe_stack_start = addr; tinfo->unsafe_stack_start = addr;
tinfo->unsafe_stack_size = size; tinfo->unsafe_stack_size = size;
tinfo->unsafe_stack_guard = guard; tinfo->unsafe_stack_guard = guard;
return REAL(pthread_create)(thread, attr, thread_start, tinfo); return REAL(pthread_create)(thread, attr, thread_start, tinfo);
} }
▲ Show 20 Lines • Show All 71 Lines • Show Last 20 Lines