> On Jul 15, 2016, at 1:03 PM, Argyrios Kyrtzidis <akyr...@gmail.com> wrote:
>
>>
>> On Jul 15, 2016, at 12:58 PM, Ben Langmuir <blangm...@apple.com
>> <mailto:blangm...@apple.com>> wrote:
>>
>>>
>>> On Jul 15, 2016, at 12:22 PM, Argyrios Kyrtzidis via cfe-commits
>>> <cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>> wrote:
>>>
>>> Author: akirtzidis
>>> Date: Fri Jul 15 14:22:34 2016
>>> New Revision: 275600
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=275600&view=rev
>>> <http://llvm.org/viewvc/llvm-project?rev=275600&view=rev>
>>> Log:
>>> [objcmt] Fix a buffer overflow crash than can occur while modernizing enums.
>>>
>>> Note that due to the nature of the crash it requires libgmalloc or asan for
>>> it to crash consistently.
>>>
>>> rdar://19932927 <rdar://19932927>
>>>
>>> Added:
>>> cfe/trunk/test/ARCMT/objcmt-ns-enum-crash.m
>>> cfe/trunk/test/ARCMT/objcmt-ns-enum-crash.m.result
>>> Modified:
>>> cfe/trunk/lib/ARCMigrate/ObjCMT.cpp
>>> cfe/trunk/test/ARCMT/objcmt-ns-macros.m.result
>>>
>>> Modified: cfe/trunk/lib/ARCMigrate/ObjCMT.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ARCMigrate/ObjCMT.cpp?rev=275600&r1=275599&r2=275600&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/lib/ARCMigrate/ObjCMT.cpp (original)
>>> +++ cfe/trunk/lib/ARCMigrate/ObjCMT.cpp Fri Jul 15 14:22:34 2016
>>> @@ -771,23 +771,11 @@ static void rewriteToNSMacroDecl(ASTCont
>>> ClassString += ", ";
>>>
>>> ClassString += TypedefDcl->getIdentifier()->getName();
>>> - ClassString += ')';
>>> - SourceLocation EndLoc;
>>> - if (EnumDcl->getIntegerTypeSourceInfo()) {
>>> - TypeSourceInfo *TSourceInfo = EnumDcl->getIntegerTypeSourceInfo();
>>> - TypeLoc TLoc = TSourceInfo->getTypeLoc();
>>> - EndLoc = TLoc.getLocEnd();
>>> - const char *lbrace = Ctx.getSourceManager().getCharacterData(EndLoc);
>>> - unsigned count = 0;
>>> - if (lbrace)
>>> - while (lbrace[count] != '{')
>>> - ++count;
>>> - if (count > 0)
>>> - EndLoc = EndLoc.getLocWithOffset(count-1);
>>> - }
>>> - else
>>> - EndLoc = EnumDcl->getLocStart();
>>> - SourceRange R(EnumDcl->getLocStart(), EndLoc);
>>> + ClassString += ") ";
>>> + SourceLocation EndLoc = EnumDcl->getBraceRange().getBegin();
>>> + if (EndLoc.isInvalid())
>>> + return;
>>> + CharSourceRange R =
>>> CharSourceRange::getCharRange(EnumDcl->getLocStart(), EndLoc);
>>> commit.replace(R, ClassString);
>>> // This is to remove spaces between '}' and typedef name.
>>> SourceLocation StartTypedefLoc = EnumDcl->getLocEnd();
>>> @@ -1900,18 +1888,20 @@ void ObjCMigrateASTConsumer::HandleTrans
>>> if (++N == DEnd)
>>> continue;
>>> if (const EnumDecl *ED = dyn_cast<EnumDecl>(*N)) {
>>> - if (++N != DEnd)
>>> - if (const TypedefDecl *TDF = dyn_cast<TypedefDecl>(*N)) {
>>> - // prefer typedef-follows-enum to enum-follows-typedef
>>> pattern.
>>> - if (migrateNSEnumDecl(Ctx, ED, TDF)) {
>>> - ++D; ++D;
>>> - CacheObjCNSIntegerTypedefed(TD);
>>> - continue;
>>> + if (canModify(ED)) {
>>
>> Is this change tested anywhere? It looks orthogonal to the source range
>> changes.
>
> You are right that it is orthogonal. It avoids unnecessary work, it’s not
> changing behavior.
On second look, test case in r275609 shows behavior improvement.
Without this change, 'ARCMT/whitelisted/objcmt-with-whitelist.m’ will fail.
>
>>
>>> + if (++N != DEnd)
>>> + if (const TypedefDecl *TDF = dyn_cast<TypedefDecl>(*N)) {
>>> + // prefer typedef-follows-enum to enum-follows-typedef
>>> pattern.
>>> + if (migrateNSEnumDecl(Ctx, ED, TDF)) {
>>> + ++D; ++D;
>>> + CacheObjCNSIntegerTypedefed(TD);
>>> + continue;
>>> + }
>>> }
>>> + if (migrateNSEnumDecl(Ctx, ED, TD)) {
>>> + ++D;
>>> + continue;
>>> }
>>> - if (migrateNSEnumDecl(Ctx, ED, TD)) {
>>> - ++D;
>>> - continue;
>>> }
>>> }
>>> CacheObjCNSIntegerTypedefed(TD);
>>>
>>> Added: cfe/trunk/test/ARCMT/objcmt-ns-enum-crash.m
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ARCMT/objcmt-ns-enum-crash.m?rev=275600&view=auto
>>>
>>> <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ARCMT/objcmt-ns-enum-crash.m?rev=275600&view=auto>
>>> ==============================================================================
>>> --- cfe/trunk/test/ARCMT/objcmt-ns-enum-crash.m (added)
>>> +++ cfe/trunk/test/ARCMT/objcmt-ns-enum-crash.m Fri Jul 15 14:22:34 2016
>>> @@ -0,0 +1,14 @@
>>> +// RUN: rm -rf %t
>>> +// 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
>>> +// RUN: c-arcmt-test -mt-migrate-directory %t | arcmt-test
>>> -verify-transformed-files %s.result
>>> +
>>> +#define NS_ENUM(_type, _name) enum _name : _type _name; enum _name : _type
>>> +#define NS_OPTIONS(_type, _name) enum _name : _type _name; enum _name :
>>> _type
>>> +typedef long NSInteger;
>>> +
>>> +typedef enum : NSInteger {five} ApplicableEnum;
>>> +
>>> +typedef unsigned long mytd;
>>> +
>>> +#define MY_ENUM(name, type, ...) typedef enum : type { __VA_ARGS__ }
>>> name##_t
>>> +MY_ENUM(MyEnum, unsigned int, One);
>>>
>>> Added: cfe/trunk/test/ARCMT/objcmt-ns-enum-crash.m.result
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ARCMT/objcmt-ns-enum-crash.m.result?rev=275600&view=auto
>>>
>>> <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ARCMT/objcmt-ns-enum-crash.m.result?rev=275600&view=auto>
>>> ==============================================================================
>>> --- cfe/trunk/test/ARCMT/objcmt-ns-enum-crash.m.result (added)
>>> +++ cfe/trunk/test/ARCMT/objcmt-ns-enum-crash.m.result Fri Jul 15 14:22:34
>>> 2016
>>> @@ -0,0 +1,14 @@
>>> +// RUN: rm -rf %t
>>> +// 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
>>> +// RUN: c-arcmt-test -mt-migrate-directory %t | arcmt-test
>>> -verify-transformed-files %s.result
>>> +
>>> +#define NS_ENUM(_type, _name) enum _name : _type _name; enum _name : _type
>>> +#define NS_OPTIONS(_type, _name) enum _name : _type _name; enum _name :
>>> _type
>>> +typedef long NSInteger;
>>> +
>>> +typedef NS_ENUM(NSInteger, ApplicableEnum) {five};
>>> +
>>> +typedef unsigned long mytd;
>>> +
>>> +#define MY_ENUM(name, type, ...) typedef enum : type { __VA_ARGS__ }
>>> name##_t
>>> +MY_ENUM(MyEnum, unsigned int, One);
>>>
>>> Modified: cfe/trunk/test/ARCMT/objcmt-ns-macros.m.result
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ARCMT/objcmt-ns-macros.m.result?rev=275600&r1=275599&r2=275600&view=diff
>>>
>>> <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ARCMT/objcmt-ns-macros.m.result?rev=275600&r1=275599&r2=275600&view=diff>
>>> ==============================================================================
>>> --- cfe/trunk/test/ARCMT/objcmt-ns-macros.m.result (original)
>>> +++ cfe/trunk/test/ARCMT/objcmt-ns-macros.m.result Fri Jul 15 14:22:34 2016
>>> @@ -77,7 +77,7 @@ typedef NS_ENUM(NSInteger, UIK) {
>>> UIKTwo = 2,
>>> };
>>>
>>> -typedef NS_ENUM(unsigned int, NSTickMarkPosition) {
>>> +typedef NS_ENUM(unsigned int, NSTickMarkPosition) {
>>> NSTickMarkBelow = 0,
>>> NSTickMarkAbove = 1,
>>> NSTickMarkLeft = NSTickMarkAbove,
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
