Summary: | ANGLE: Stop using unsafe strcpy method | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | David Kilzer (:ddkilzer) <ddkilzer> | ||||||||||
Component: | ANGLE | Assignee: | David Kilzer (:ddkilzer) <ddkilzer> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | commit-queue, dino, kbr, kondapallykalyan, roger_fong | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
URL: | https://code.google.com/p/angleproject/issues/detail?id=307 | ||||||||||||
Attachments: |
|
Description
David Kilzer (:ddkilzer)
2014-02-23 15:55:41 PST
Created attachment 225016 [details]
Patch v1
Note that there are important steps to take when updating ANGLE. See http://trac.webkit.org/wiki/UpdatingANGLE 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.
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(). Created attachment 225017 [details]
Patch v2
Try to fix builds by adding #include <string.h> to ShaderLang.cpp.
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.
(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. Created attachment 225018 [details]
Patch v3
Switch to strncpy() instead of strlcpy().
Created attachment 225020 [details] Patch v4 Update patch after Dean landed updated ANGLE in r164565, r164567. 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.
Comment on attachment 225020 [details]
Patch v4
We should file a bug on ANGLE to pick this up. I'll do so.
(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 Committed r164580: <http://trac.webkit.org/changeset/164580> |