Bug 174809

Summary: Fix bugs in probe code to change sp on x86, x86_64 and 32-bit ARM.
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, fpizlo, jfbastien, keith_miller, msaboff, saam, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 174645    
Attachments:
Description Flags
proposed patch. fpizlo: review+

Mark Lam
Reported 2017-07-24 20:29:51 PDT
Patch coming.
Attachments
proposed patch. (43.89 KB, patch)
2017-07-24 20:54 PDT, Mark Lam 		fpizlo: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2017-07-24 20:30:41 PDT
<rdar://problem/33504759>
Mark Lam
Comment 2 2017-07-24 20:54:48 PDT
Created attachment 316347 [details] proposed patch.
Build Bot
Comment 3 2017-07-24 20:56:38 PDT
Attachment 316347 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/assembler/testmasm.cpp:226: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/assembler/testmasm.cpp:271: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/assembler/testmasm.cpp:272: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/assembler/testmasm.cpp:318: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/assembler/testmasm.cpp:423: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/assembler/testmasm.cpp:474: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] Total errors found: 6 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 4 2017-07-25 13:09:11 PDT
Comment on attachment 316347 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=316347&action=review Does testmasm run on any bots? Maybe it should! > Source/JavaScriptCore/assembler/testmasm.cpp:321 > +void testProbeModifiesStackPointer(std::function<void*(ProbeContext*)> computeModifiedStack) Would WTF::Function work here?
Mark Lam
Comment 5 2017-07-25 13:57:37 PDT
Thanks for the review. (In reply to Filip Pizlo from comment #4) > Does testmasm run on any bots? Maybe it should! I'll talk to the relevant folks to get this going. > > Source/JavaScriptCore/assembler/testmasm.cpp:321 > > +void testProbeModifiesStackPointer(std::function<void*(ProbeContext*)> computeModifiedStack) > > Would WTF::Function work here? I've changed all uses of std::function in testmasm.cpp to WTF::Function. Landed in r219885: <http://trac.webkit.org/r219885>.
Note You need to log in before you can comment on or make changes to this bug.