<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Jul 15, 2016, at 12:58 PM, Ben Langmuir <<a href="mailto:blangmuir@apple.com" class="">blangmuir@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br class="Apple-interchange-newline">On Jul 15, 2016, at 12:22 PM, Argyrios Kyrtzidis via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" class="">cfe-commits@lists.llvm.org</a>> wrote:<br class=""><br class="">Author: akirtzidis<br class="">Date: Fri Jul 15 14:22:34 2016<br class="">New Revision: 275600<br class=""><br class="">URL: <a href="http://llvm.org/viewvc/llvm-project?rev=275600&view=rev" class="">http://llvm.org/viewvc/llvm-project?rev=275600&view=rev</a><br class="">Log:<br class="">[objcmt] Fix a buffer overflow crash than can occur while modernizing enums.<br class=""><br class="">Note that due to the nature of the crash it requires libgmalloc or asan for it to crash consistently.<br class=""><br class=""><a href="rdar://19932927" class="">rdar://19932927</a><br class=""><br class="">Added:<br class=""> cfe/trunk/test/ARCMT/objcmt-ns-enum-crash.m<br class=""> cfe/trunk/test/ARCMT/objcmt-ns-enum-crash.m.result<br class="">Modified:<br class=""> cfe/trunk/lib/ARCMigrate/ObjCMT.cpp<br class=""> cfe/trunk/test/ARCMT/objcmt-ns-macros.m.result<br class=""><br class="">Modified: cfe/trunk/lib/ARCMigrate/ObjCMT.cpp<br class="">URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ARCMigrate/ObjCMT.cpp?rev=275600&r1=275599&r2=275600&view=diff<br class="">==============================================================================<br class="">--- cfe/trunk/lib/ARCMigrate/ObjCMT.cpp (original)<br class="">+++ cfe/trunk/lib/ARCMigrate/ObjCMT.cpp Fri Jul 15 14:22:34 2016<br class="">@@ -771,23 +771,11 @@ static void rewriteToNSMacroDecl(ASTCont<br class=""> ClassString += ", ";<br class=""><br class=""> ClassString += TypedefDcl->getIdentifier()->getName();<br class="">- ClassString += ')';<br class="">- SourceLocation EndLoc;<br class="">- if (EnumDcl->getIntegerTypeSourceInfo()) {<br class="">- TypeSourceInfo *TSourceInfo = EnumDcl->getIntegerTypeSourceInfo();<br class="">- TypeLoc TLoc = TSourceInfo->getTypeLoc();<br class="">- EndLoc = TLoc.getLocEnd();<br class="">- const char *lbrace = Ctx.getSourceManager().getCharacterData(EndLoc);<br class="">- unsigned count = 0;<br class="">- if (lbrace)<br class="">- while (lbrace[count] != '{')<br class="">- ++count;<br class="">- if (count > 0)<br class="">- EndLoc = EndLoc.getLocWithOffset(count-1);<br class="">- }<br class="">- else<br class="">- EndLoc = EnumDcl->getLocStart();<br class="">- SourceRange R(EnumDcl->getLocStart(), EndLoc);<br class="">+ ClassString += ") ";<br class="">+ SourceLocation EndLoc = EnumDcl->getBraceRange().getBegin();<br class="">+ if (EndLoc.isInvalid())<br class="">+ return;<br class="">+ CharSourceRange R = CharSourceRange::getCharRange(EnumDcl->getLocStart(), EndLoc);<br class=""> commit.replace(R, ClassString);<br class=""> // This is to remove spaces between '}' and typedef name.<br class=""> SourceLocation StartTypedefLoc = EnumDcl->getLocEnd();<br class="">@@ -1900,18 +1888,20 @@ void ObjCMigrateASTConsumer::HandleTrans<br class=""> if (++N == DEnd)<br class=""> continue;<br class=""> if (const EnumDecl *ED = dyn_cast<EnumDecl>(*N)) {<br class="">- if (++N != DEnd)<br class="">- if (const TypedefDecl *TDF = dyn_cast<TypedefDecl>(*N)) {<br class="">- // prefer typedef-follows-enum to enum-follows-typedef pattern.<br class="">- if (migrateNSEnumDecl(Ctx, ED, TDF)) {<br class="">- ++D; ++D;<br class="">- CacheObjCNSIntegerTypedefed(TD);<br class="">- continue;<br class="">+ if (canModify(ED)) {<br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Is this change tested anywhere? It looks orthogonal to the source range changes.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></blockquote><div><br class=""></div><div>You are right that it is orthogonal. It avoids unnecessary work, it’s not changing behavior.</div><br class=""><blockquote type="cite" class=""><div class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">+ if (++N != DEnd)<br class="">+ if (const TypedefDecl *TDF = dyn_cast<TypedefDecl>(*N)) {<br class="">+ // prefer typedef-follows-enum to enum-follows-typedef pattern.<br class="">+ if (migrateNSEnumDecl(Ctx, ED, TDF)) {<br class="">+ ++D; ++D;<br class="">+ CacheObjCNSIntegerTypedefed(TD);<br class="">+ continue;<br class="">+ }<br class=""> }<br class="">+ if (migrateNSEnumDecl(Ctx, ED, TD)) {<br class="">+ ++D;<br class="">+ continue;<br class=""> }<br class="">- if (migrateNSEnumDecl(Ctx, ED, TD)) {<br class="">- ++D;<br class="">- continue;<br class=""> }<br class=""> }<br class=""> CacheObjCNSIntegerTypedefed(TD);<br class=""><br class="">Added: cfe/trunk/test/ARCMT/objcmt-ns-enum-crash.m<br class="">URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ARCMT/objcmt-ns-enum-crash.m?rev=275600&view=auto" class="">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ARCMT/objcmt-ns-enum-crash.m?rev=275600&view=auto</a><br class="">==============================================================================<br class="">--- cfe/trunk/test/ARCMT/objcmt-ns-enum-crash.m (added)<br class="">+++ cfe/trunk/test/ARCMT/objcmt-ns-enum-crash.m Fri Jul 15 14:22:34 2016<br class="">@@ -0,0 +1,14 @@<br class="">+// RUN: rm -rf %t<br class="">+// RUN: %clang_cc1 -objcmt-migrate-ns-macros -mt-migrate-directory %t %s -x objective-c -fobjc-runtime-has-weak -fobjc-arc -triple x86_64-apple-darwin11<br class="">+// RUN: c-arcmt-test -mt-migrate-directory %t | arcmt-test -verify-transformed-files %s.result<br class="">+<br class="">+#define NS_ENUM(_type, _name) enum _name : _type _name; enum _name : _type<br class="">+#define NS_OPTIONS(_type, _name) enum _name : _type _name; enum _name : _type<br class="">+typedef long NSInteger;<br class="">+<br class="">+typedef enum : NSInteger {five} ApplicableEnum;<br class="">+<br class="">+typedef unsigned long mytd;<br class="">+<br class="">+#define MY_ENUM(name, type, ...) typedef enum : type { __VA_ARGS__ } name##_t<br class="">+MY_ENUM(MyEnum, unsigned int, One);<br class=""><br class="">Added: cfe/trunk/test/ARCMT/objcmt-ns-enum-crash.m.result<br class="">URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ARCMT/objcmt-ns-enum-crash.m.result?rev=275600&view=auto" class="">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ARCMT/objcmt-ns-enum-crash.m.result?rev=275600&view=auto</a><br class="">==============================================================================<br class="">--- cfe/trunk/test/ARCMT/objcmt-ns-enum-crash.m.result (added)<br class="">+++ cfe/trunk/test/ARCMT/objcmt-ns-enum-crash.m.result Fri Jul 15 14:22:34 2016<br class="">@@ -0,0 +1,14 @@<br class="">+// RUN: rm -rf %t<br class="">+// RUN: %clang_cc1 -objcmt-migrate-ns-macros -mt-migrate-directory %t %s -x objective-c -fobjc-runtime-has-weak -fobjc-arc -triple x86_64-apple-darwin11<br class="">+// RUN: c-arcmt-test -mt-migrate-directory %t | arcmt-test -verify-transformed-files %s.result<br class="">+<br class="">+#define NS_ENUM(_type, _name) enum _name : _type _name; enum _name : _type<br class="">+#define NS_OPTIONS(_type, _name) enum _name : _type _name; enum _name : _type<br class="">+typedef long NSInteger;<br class="">+<br class="">+typedef NS_ENUM(NSInteger, ApplicableEnum) {five};<br class="">+<br class="">+typedef unsigned long mytd;<br class="">+<br class="">+#define MY_ENUM(name, type, ...) typedef enum : type { __VA_ARGS__ } name##_t<br class="">+MY_ENUM(MyEnum, unsigned int, One);<br class=""><br class="">Modified: cfe/trunk/test/ARCMT/objcmt-ns-macros.m.result<br class="">URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ARCMT/objcmt-ns-macros.m.result?rev=275600&r1=275599&r2=275600&view=diff" class="">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ARCMT/objcmt-ns-macros.m.result?rev=275600&r1=275599&r2=275600&view=diff</a><br class="">==============================================================================<br class="">--- cfe/trunk/test/ARCMT/objcmt-ns-macros.m.result (original)<br class="">+++ cfe/trunk/test/ARCMT/objcmt-ns-macros.m.result Fri Jul 15 14:22:34 2016<br class="">@@ -77,7 +77,7 @@ typedef NS_ENUM(NSInteger, UIK) {<br class=""> UIKTwo = 2,<br class="">};<br class=""><br class="">-typedef NS_ENUM(unsigned int, NSTickMarkPosition) {<br class="">+typedef NS_ENUM(unsigned int, NSTickMarkPosition) {<br class=""> NSTickMarkBelow = 0,<br class=""> NSTickMarkAbove = 1,<br class=""> NSTickMarkLeft = NSTickMarkAbove,<br class=""><br class=""><br class="">_______________________________________________<br class="">cfe-commits mailing list<br class=""><a href="mailto:cfe-commits@lists.llvm.org" class="">cfe-commits@lists.llvm.org</a><br class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</blockquote></div></blockquote></div><br class=""></body></html>