Bug 129237 – ANGLE: Stop using unsafe strcpy method

• Status: RESOLVED
• Resolution: FIXED
• Component: ANGLE

[David Kilzer (:ddkilzer)]

ANGLE should stop using the unsafe strcpy method.

[David Kilzer (:ddkilzer)]

<rdar://problem/11077580>

[David Kilzer (:ddkilzer)]

Created attachment 225016 [details]
Patch v1

[WebKit Commit Bot]

Note that there are important steps to take when updating ANGLE. See http://trac.webkit.org/wiki/UpdatingANGLE

[WebKit Commit Bot]

Attachment 225016 [details] did not pass style-queue:


ERROR: Source/ThirdParty/ANGLE/src/libGLESv2/Program.cpp:118:  Declaration has space between type name and * in char *newLog  [whitespace/declaration] [3]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[David Kilzer (:ddkilzer)]

Comment on attachment 225016 [details]
Patch v1

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

> Source/ThirdParty/ANGLE/src/common/angleutils.h:47
> +#define strlcpy(dst, src, dstsize) strcpy_s(dst, dstsize, src)

Waiting for the EWS bots to tell me if this works on Windows.

> Source/ThirdParty/ANGLE/src/compiler/ShaderLang.cpp:226
> +    size_t infoLogLength = 0;
> +    ShGetInfo(compiler, SH_INFO_LOG_LENGTH, &infoLogLength);

A better design would be to pass in the buffer length to ShGetInfoLog() as a parameter, but since it's a C API, a new function name would have to be created.

> Source/ThirdParty/ANGLE/src/compiler/ShaderLang.cpp:245
> +    size_t objCodeLength = 0;
> +    ShGetInfo(handle, SH_OBJECT_CODE_LENGTH, &objCodeLength);

Ditto for passing in the length to ShGetObjectCode().

> Source/ThirdParty/ANGLE/src/libGLESv2/Program.cpp:112
> +        const size_t newInfoLogLength = infoLength + 2;
> +        mInfoLog = new char[newInfoLogLength];
> +        strlcpy(mInfoLog, info, newInfoLogLength);
> +        strlcpy(mInfoLog + infoLength, "\n", newInfoLogLength - infoLength);

I think using snprintf() here would be clearer, but probably not as performant.

> Source/ThirdParty/ANGLE/src/libGLESv2/Program.cpp:121
> +        const size_t newInfoLogLength = logLength + infoLength + 2;
> +        char *newLog = new char[newInfoLogLength];
> +        strlcpy(newLog, mInfoLog, newInfoLogLength);
> +        strlcpy(newLog + logLength, info, newInfoLogLength - logLength);
> +        strlcpy(newLog + logLength + infoLength, "\n", newInfoLogLength - logLength - infoLength);

Ditto for using snprintf().

[David Kilzer (:ddkilzer)]

Created attachment 225017 [details]
Patch v2

Try to fix builds by adding #include <string.h> to ShaderLang.cpp.

[WebKit Commit Bot]

Attachment 225017 [details] did not pass style-queue:


ERROR: Source/ThirdParty/ANGLE/src/libGLESv2/Program.cpp:118:  Declaration has space between type name and * in char *newLog  [whitespace/declaration] [3]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[David Kilzer (:ddkilzer)]

(In reply to comment #6)
> Created an attachment (id=225017) [details]
> Patch v2
> 
> Try to fix builds by adding #include <string.h> to ShaderLang.cpp.

Hmm, strlcpy doesn't seem to be used in WebCore, so we should probably use strncpy with an explicit '\0' set at the end.

[David Kilzer (:ddkilzer)]

Created attachment 225018 [details]
Patch v3

Switch to strncpy() instead of strlcpy().

[David Kilzer (:ddkilzer)]

Created attachment 225020 [details]
Patch v4

Update patch after Dean landed updated ANGLE in r164565, r164567.

[WebKit Commit Bot]

Attachment 225020 [details] did not pass style-queue:


ERROR: Source/ThirdParty/ANGLE/src/libGLESv2/Program.cpp:119:  Declaration has space between type name and * in char *newLog  [whitespace/declaration] [3]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Dean Jackson]

Comment on attachment 225020 [details]
Patch v4

We should file a bug on ANGLE to pick this up. I'll do so.

[David Kilzer (:ddkilzer)]

(In reply to comment #12)
> (From update of attachment 225020 [details])
> We should file a bug on ANGLE to pick this up. I'll do so.

https://code.google.com/p/angleproject/issues/detail?id=307

[David Kilzer (:ddkilzer)]

Committed r164580: <http://trac.webkit.org/changeset/164580>