Some of these tests will crash when validated untagging is enabled.
<rdar://problem/61556785>
Created attachment 396032 [details] proposed patch.
Comment on attachment 396032 [details] proposed patch. r=me
Thanks for the review. Landed in r259848: <http://trac.webkit.org/r259848>.
Comment on attachment 396032 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=396032&action=review > Source/JavaScriptCore/assembler/testmasm.cpp:2136 > + CHECK_NOT_EQ(invoke<void*>(cage, taggedNotCagedPtr, 1), ptr); This isn't quite a strong as the old test. It doesn't guarantee that the cage gave you a pointer that will crash when dereferenced. However, I think that can be tested by also asserting, CHECK_NOT_EQ(invoke<void*>(cage, taggedNotCagedPtr, 1), removeArrayPtrTag(invoke<void*>(cage, taggedNotCagedPtr, 1))));
Landed the additional test in r259890: <http://trac.webkit.org/r259890>.
Comment on attachment 396032 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=396032&action=review > Source/JavaScriptCore/ChangeLog:9 > + Some of these tests will crash when validated untagging is enabled. why?
Comment on attachment 396032 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=396032&action=review >> Source/JavaScriptCore/ChangeLog:9 >> + Some of these tests will crash when validated untagging is enabled. > > why? (You should say that here, not just what, but why)
(In reply to Saam Barati from comment #8) > Comment on attachment 396032 [details] > proposed patch. > > View in context: > https://bugs.webkit.org/attachment.cgi?id=396032&action=review > > >> Source/JavaScriptCore/ChangeLog:9 > >> + Some of these tests will crash when validated untagging is enabled. > > > > why? > > (You should say that here, not just what, but why) It's in the title: "when validated untagging is enabled". When we start validating the untagged pointer (as in crash if validation fails), we will crash if we are untagging badly tagged pointers on purpose in the test.
(In reply to Mark Lam from comment #9) > (In reply to Saam Barati from comment #8) > > Comment on attachment 396032 [details] > > proposed patch. > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=396032&action=review > > > > >> Source/JavaScriptCore/ChangeLog:9 > > >> + Some of these tests will crash when validated untagging is enabled. > > > > > > why? > > > > (You should say that here, not just what, but why) > > It's in the title: "when validated untagging is enabled". When we start > validating the untagged pointer (as in crash if validation fails), we will > crash if we are untagging badly tagged pointers on purpose in the test. Sorry, not title. It's in the bug and ChangeLog description.
Comment on attachment 396032 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=396032&action=review >> Source/JavaScriptCore/assembler/testmasm.cpp:2136 >> + CHECK_NOT_EQ(invoke<void*>(cage, taggedNotCagedPtr, 1), ptr); > > This isn't quite a strong as the old test. It doesn't guarantee that the cage gave you a pointer that will crash when dereferenced. However, I think that can be tested by also asserting, > > CHECK_NOT_EQ(invoke<void*>(cage, taggedNotCagedPtr, 1), removeArrayPtrTag(invoke<void*>(cage, taggedNotCagedPtr, 1)))); can't we just assert using some functions that we're looking at unmapped memory?
Comment on attachment 396032 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=396032&action=review >>> Source/JavaScriptCore/assembler/testmasm.cpp:2136 >>> + CHECK_NOT_EQ(invoke<void*>(cage, taggedNotCagedPtr, 1), ptr); >> >> This isn't quite a strong as the old test. It doesn't guarantee that the cage gave you a pointer that will crash when dereferenced. However, I think that can be tested by also asserting, >> >> CHECK_NOT_EQ(invoke<void*>(cage, taggedNotCagedPtr, 1), removeArrayPtrTag(invoke<void*>(cage, taggedNotCagedPtr, 1)))); > > can't we just assert using some functions that we're looking at unmapped memory? The point is to ensure you have PAC bits.