WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
129169
gatherFromOtherThread() needs to align the sp before gathering roots
https://bugs.webkit.org/show_bug.cgi?id=129169
Summary
gatherFromOtherThread() needs to align the sp before gathering roots
Mark Lam
Reported
2014-02-21 13:58:03 PST
The GC scans the stacks of other threads using MachineThreads::gatherFromOtherThread(). gatherFromOtherThread() defines the range of the other thread's stack as being bounded by the other thread's stack pointer and stack base. While the stack base will always be aligned to sizeof(void*), the stack pointer may not be. This is because the other thread may have just pushed a 32-bit value on its stack before we suspended it for scanning. The fix is to round the stack pointer up to the next aligned address of sizeof(void*) and start scanning from there. We ignore the 32-bit word at the bottom of the stack (top of the stack for stacks growing up) because it cannot be a 64-bit pointer anyway since pointers should be stored on 64-bit aligned boundaries (our conservative scan algorithm already depends on this assumption). ref: <
rdar://problem/15447622
>
Attachments
the patch.
(3.29 KB, patch)
2014-02-21 14:09 PST
,
Mark Lam
ggaren
: review+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Geoffrey Garen
Comment 1
2014-02-21 14:05:42 PST
> The fix is to round the stack pointer up to the next aligned address of sizeof(void*) and start scanning from there. We ignore the 32-bit word at the bottom of the stack (top of the stack for stacks growing up) because it cannot be a 64-bit pointer anyway since pointers should be stored on 64-bit aligned boundaries (our conservative scan algorithm already depends on this assumption).
I think your described fix will work -- but this explanation is wrong because it doesn't consider 32bit.
Mark Lam
Comment 2
2014-02-21 14:09:02 PST
Created
attachment 224905
[details]
the patch.
Mark Lam
Comment 3
2014-02-21 14:09:56 PST
Comment on
attachment 224905
[details]
the patch. View in context:
https://bugs.webkit.org/attachment.cgi?id=224905&action=review
> Source/JavaScriptCore/ChangeLog:19 > + Pointers should always be stored on 64-bit aligned boundaries (our
I'll change "Pointers" to "64-bit pointers" to be even more clearer.
Geoffrey Garen
Comment 4
2014-02-21 14:14:21 PST
Comment on
attachment 224905
[details]
the patch. View in context:
https://bugs.webkit.org/attachment.cgi?id=224905&action=review
r=me
> Source/JavaScriptCore/heap/MachineStackMarker.cpp:448 > + stackPointer = reinterpret_cast<void*>(WTF::roundUpToMultipleOf<sizeof(void*)>(reinterpret_cast<size_t>(stackPointer)));
Let's use uintptr_t instead of size_t.
Mark Lam
Comment 5
2014-02-21 14:16:04 PST
(In reply to
comment #4
)
> > Source/JavaScriptCore/heap/MachineStackMarker.cpp:448 > > + stackPointer = reinterpret_cast<void*>(WTF::roundUpToMultipleOf<sizeof(void*)>(reinterpret_cast<size_t>(stackPointer))); > > Let's use uintptr_t instead of size_t.
Sure, but StdLibExtras.h defined WTF::roundUpToMultipleOf() as taking a size_t.
Mark Lam
Comment 6
2014-02-21 14:22:47 PST
Thanks for the review. Landed in
r164500
: <
http:/trac.webkit.org/r164500
>.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug