122610 2013-10-10 11:46:00 -0700 Validate uniform input when setting texture sampler uniforms 2014-01-10 14:40:16 -0800 1 1 1 Unclassified WebKit WebGL 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 122607 126775 1 roger_fong roger_fong commit-queue dino esprehn+autocc gyuyoung.kim kondapallykalyan roger_fong oldest_to_newest 938446 0 roger_fong 2013-10-10 11:46:00 -0700 We have to make sure when we set a texture sampler that the value is less than the max combined texture units, otherwise we throw an INVALID_VALUE error. The conformance test, conformance/uniforms/uniform-samplers-test.html, covers this. 938448 1 roger_fong 2013-10-10 11:46:47 -0700 Fix relies on https://bugs.webkit.org/show_bug.cgi?id=122607 to be committed first. 938518 2 213923 roger_fong 2013-10-10 13:33:38 -0700 Created attachment 213923 Patch 938567 3 213923 dino 2013-10-10 15:10:40 -0700 Comment on attachment 213923 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=213923&action=review > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:4170 > + for (unsigned ii = 0; ii < v->length(); ++ii) { Is there a reason for ii vs i here? > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:4171 > + if (((GC3Dint*)v)[ii] >= (int)m_textureUnits.size()) { Might as well use static_cast > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:4189 > + if (location->type() == GraphicsContext3D::SAMPLER_2D || location->type() == GraphicsContext3D::SAMPLER_CUBE) > + for (unsigned ii = 0; ii < (unsigned)size; ++ii) { > + if (((GC3Dint*)v)[ii] >= (int)m_textureUnits.size()) { Same. 938569 4 roger_fong 2013-10-10 15:19:23 -0700 (In reply to comment #3) > (From update of attachment 213923 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=213923&action=review > > > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:4170 > > + for (unsigned ii = 0; ii < v->length(); ++ii) { > > Is there a reason for ii vs i here? Eh, just saw it in a number of places in the file. There are also plenty of just i's though. I can change. > > > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:4171 > > + if (((GC3Dint*)v)[ii] >= (int)m_textureUnits.size()) { > > Might as well use static_cast > > > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:4189 > > + if (location->type() == GraphicsContext3D::SAMPLER_2D || location->type() == GraphicsContext3D::SAMPLER_CUBE) > > + for (unsigned ii = 0; ii < (unsigned)size; ++ii) { > > + if (((GC3Dint*)v)[ii] >= (int)m_textureUnits.size()) { > > Same. KK 938605 5 roger_fong 2013-10-10 16:33:54 -0700 Committed with fixes: http://trac.webkit.org/changeset/157271 213923 2013-10-10 13:33:38 -0700 2013-10-10 15:10:40 -0700 Patch bug-122610-20131010133338.patch text/plain 2934 roger_fong SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDE1NzI0NykKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE3IEBACisyMDEzLTEwLTEwICBSb2dlciBG b25nICA8cm9nZXJfZm9uZ0BhcHBsZS5jb20+CisKKyAgICAgICAgVmFsaWRhdGUgdW5pZm9ybSBp bnB1dCB3aGVuIHNldHRpbmcgdGV4dHVyZSBzYW1wbGVyIHVuaWZvcm1zLgorICAgICAgICBodHRw czovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTIyNjEwLgorCisgICAgICAgIFJl dmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIENvdmVyZWQgYnkgS2hyb25vcyBD b25mb3JtYW5jZSBTdWl0ZSAxLjAuMi4KKyAgICAgICAgU3BlY2lmaWNhbGx5LCBjb25mb3JtYW5j ZS91bmlmb3Jtcy91bmlmb3JtLXNhbXBsZXJzLXRlc3QuaHRtbC4KKworICAgICAgICAqIGh0bWwv Y2FudmFzL1dlYkdMUmVuZGVyaW5nQ29udGV4dC5jcHA6CisgICAgICAgIChXZWJDb3JlOjpXZWJH TFJlbmRlcmluZ0NvbnRleHQ6OnVuaWZvcm0xaSk6CisgICAgICAgIChXZWJDb3JlOjpXZWJHTFJl bmRlcmluZ0NvbnRleHQ6OnVuaWZvcm0xaXYpOgorCiAyMDEzLTEwLTEwICBSb2dlciBGb25nICA8 cm9nZXJfZm9uZ0BhcHBsZS5jb20+CiAKICAgICAgICAgU3RvcmUgdW5pZm9ybSB0eXBlIGluZm9y bWF0aW9uIGluIFdlYkdMVW5pZm9ybWF0aW9uTG9jYXRpb24uCkluZGV4OiBTb3VyY2UvV2ViQ29y ZS9odG1sL2NhbnZhcy9XZWJHTFJlbmRlcmluZ0NvbnRleHQuY3BwCj09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNv dXJjZS9XZWJDb3JlL2h0bWwvY2FudmFzL1dlYkdMUmVuZGVyaW5nQ29udGV4dC5jcHAJKHJldmlz aW9uIDE1NzI0NykKKysrIFNvdXJjZS9XZWJDb3JlL2h0bWwvY2FudmFzL1dlYkdMUmVuZGVyaW5n Q29udGV4dC5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTQxNTEsNiArNDE1MSwxMSBAQCB2b2lkIFdl YkdMUmVuZGVyaW5nQ29udGV4dDo6dW5pZm9ybTFpKGNvCiAgICAgICAgIHJldHVybjsKICAgICB9 CiAKKyAgICBpZiAoKGxvY2F0aW9uLT50eXBlKCkgPT0gR3JhcGhpY3NDb250ZXh0M0Q6OlNBTVBM RVJfMkQgfHwgbG9jYXRpb24tPnR5cGUoKSA9PSBHcmFwaGljc0NvbnRleHQzRDo6U0FNUExFUl9D VUJFKSAmJiB4ID49IChpbnQpbV90ZXh0dXJlVW5pdHMuc2l6ZSgpKSB7CisgICAgICAgIHN5bnRo ZXNpemVHTEVycm9yKEdyYXBoaWNzQ29udGV4dDNEOjpJTlZBTElEX1ZBTFVFLCAidW5pZm9ybTFp IiwgImludmFsaWQgdGV4dHVyZSB1bml0Iik7CisgICAgICAgIHJldHVybjsKKyAgICB9CisKICAg ICBtX2NvbnRleHQtPnVuaWZvcm0xaShsb2NhdGlvbi0+bG9jYXRpb24oKSwgeCk7CiAgICAgY2xl YW51cEFmdGVyR3JhcGhpY3NDYWxsKGZhbHNlKTsKIH0KQEAgLTQxNjEsNiArNDE2NiwxNCBAQCB2 b2lkIFdlYkdMUmVuZGVyaW5nQ29udGV4dDo6dW5pZm9ybTFpdihjCiAgICAgaWYgKGlzQ29udGV4 dExvc3QoKSB8fCAhdmFsaWRhdGVVbmlmb3JtUGFyYW1ldGVycygidW5pZm9ybTFpdiIsIGxvY2F0 aW9uLCB2LCAxKSkKICAgICAgICAgcmV0dXJuOwogCisgICAgaWYgKGxvY2F0aW9uLT50eXBlKCkg PT0gR3JhcGhpY3NDb250ZXh0M0Q6OlNBTVBMRVJfMkQgfHwgbG9jYXRpb24tPnR5cGUoKSA9PSBH cmFwaGljc0NvbnRleHQzRDo6U0FNUExFUl9DVUJFKQorICAgICAgICBmb3IgKHVuc2lnbmVkIGlp ID0gMDsgaWkgPCB2LT5sZW5ndGgoKTsgKytpaSkgeworICAgICAgICAgICAgaWYgKCgoR0MzRGlu dCopdilbaWldID49IChpbnQpbV90ZXh0dXJlVW5pdHMuc2l6ZSgpKSB7CisgICAgICAgICAgICAg ICAgc3ludGhlc2l6ZUdMRXJyb3IoR3JhcGhpY3NDb250ZXh0M0Q6OklOVkFMSURfVkFMVUUsICJ1 bmlmb3JtMWkiLCAiaW52YWxpZCB0ZXh0dXJlIHVuaXQiKTsKKyAgICAgICAgICAgICAgICByZXR1 cm47CisgICAgICAgICAgICB9CisgICAgICAgIH0KKwogICAgIG1fY29udGV4dC0+dW5pZm9ybTFp dihsb2NhdGlvbi0+bG9jYXRpb24oKSwgdi0+bGVuZ3RoKCksIHYtPmRhdGEoKSk7CiAgICAgY2xl YW51cEFmdGVyR3JhcGhpY3NDYWxsKGZhbHNlKTsKIH0KQEAgLTQxNzEsNiArNDE4NCwxNCBAQCB2 b2lkIFdlYkdMUmVuZGVyaW5nQ29udGV4dDo6dW5pZm9ybTFpdihjCiAgICAgaWYgKGlzQ29udGV4 dExvc3QoKSB8fCAhdmFsaWRhdGVVbmlmb3JtUGFyYW1ldGVycygidW5pZm9ybTFpdiIsIGxvY2F0 aW9uLCB2LCBzaXplLCAxKSkKICAgICAgICAgcmV0dXJuOwogCisgICAgaWYgKGxvY2F0aW9uLT50 eXBlKCkgPT0gR3JhcGhpY3NDb250ZXh0M0Q6OlNBTVBMRVJfMkQgfHwgbG9jYXRpb24tPnR5cGUo KSA9PSBHcmFwaGljc0NvbnRleHQzRDo6U0FNUExFUl9DVUJFKQorICAgICAgICBmb3IgKHVuc2ln bmVkIGlpID0gMDsgaWkgPCAodW5zaWduZWQpc2l6ZTsgKytpaSkgeworICAgICAgICAgICAgaWYg KCgoR0MzRGludCopdilbaWldID49IChpbnQpbV90ZXh0dXJlVW5pdHMuc2l6ZSgpKSB7CisgICAg ICAgICAgICAgICAgc3ludGhlc2l6ZUdMRXJyb3IoR3JhcGhpY3NDb250ZXh0M0Q6OklOVkFMSURf VkFMVUUsICJ1bmlmb3JtMWkiLCAiaW52YWxpZCB0ZXh0dXJlIHVuaXQiKTsKKyAgICAgICAgICAg ICAgICByZXR1cm47CisgICAgICAgICAgICB9CisgICAgICAgIH0KKwogICAgIG1fY29udGV4dC0+ dW5pZm9ybTFpdihsb2NhdGlvbi0+bG9jYXRpb24oKSwgc2l6ZSwgdik7CiAgICAgY2xlYW51cEFm dGVyR3JhcGhpY3NDYWxsKGZhbHNlKTsKIH0K