[webkit-reviews] review granted: [Bug 198414] [WHLSL] Implement arrays and MakeArrayReference : [Attachment 372859] patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jun 26 17:21:15 PDT 2019


Myles C. Maxfield <mmaxfield at apple.com> has granted Saam Barati
<sbarati at apple.com>'s request for review:
Bug 198414: [WHLSL] Implement arrays and MakeArrayReference
https://bugs.webkit.org/show_bug.cgi?id=198414

Attachment 372859: patch

https://bugs.webkit.org/attachment.cgi?id=372859&action=review




--- Comment #4 from Myles C. Maxfield <mmaxfield at apple.com> ---
Comment on attachment 372859
  --> https://bugs.webkit.org/attachment.cgi?id=372859
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=372859&action=review

> Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLFunctionWriter.cpp:635
> +	   m_stringBuilder.append(makeString(mangledTypeName, ' ',
variableName, " = { ", takeLastValue(), ", 1 };\n"));

I know you didn’t change this, but shouldn’t this be 0 if the original pointer
is null?

> Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLNativeFunctionWriter.cpp:-178
> -	       stringBuilder.append(makeString("    return ",
arrayParameterType.numElements(), "u;\n"));

The “u” suffix was to make it an unsigned, in case it’s > 2B

> Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLTypeNamer.cpp:343
> +    return &*iterator;

Isn’t returning a pointer pointing into the array a use after free if something
else gets added to the array before you dereference the pointer? I think that’s
why I made it an index instead of a pointer.

> LayoutTests/webgpu/whlsl-huge-array.html:97
> +    shouldBe("resultsInt32Array[0]", "1");

Can we update these tests to use the pattern that caides tests to pass if the
machine doesn’t support Meta? I’ve migrated all the existing tests.


More information about the webkit-reviews mailing list