r275600 - [objcmt] Fix a buffer overflow crash than can occur while modernizing enums.

[Ben Langmuir via cfe-commits]

> 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.

> +            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