r275600 - [objcmt] Fix a buffer overflow crash than can occur while modernizing enums.
Argyrios Kyrtzidis via cfe-commits
cfe-commits at lists.llvm.org
Fri Jul 15 13:03:16 PDT 2016
> On Jul 15, 2016, at 12:58 PM, Ben Langmuir <blangmuir at apple.com> wrote:
>
>>
>> On Jul 15, 2016, at 12:22 PM, Argyrios Kyrtzidis via cfe-commits <cfe-commits at 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
>> 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
>>
>> 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.
>
>> + 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
>> ==============================================================================
>> --- 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
>> ==============================================================================
>> --- 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
>> ==============================================================================
>> --- 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 at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160715/2ab485f8/attachment-0001.html>
More information about the cfe-commits
mailing list