r213120 - Improve error recovery around colon.

Evgeniy Stepanov eugeni.stepanov at gmail.com
Thu Jul 17 01:34:15 PDT 2014


Thanks for the fix.
The problem was in operator=() allocating a buffer of size 0. Note
that the destructor of NestedNameSpecifierLocBuilder only deallocates
the buffer if BufferCapacity != 0. This leaves you with malloc(0)
without a matching free().

There is an issue in ASan here: we reported leak of size 1 instead of size 0.
https://code.google.com/p/address-sanitizer/issues/detail?id=325


On Thu, Jul 17, 2014 at 8:32 AM, Serge Pavlov <sepavloff at gmail.com> wrote:
> Thank you for help.
>
> Now I see that sanitizer-x86_64-linux-bootstrap is green. If it indeed was
> repaired by r213178, it could mean that sanitizer issues false warnings for
> 1-byte blocks, as the only effect of that change is prevention of allocating
> tiny blocks.
>
> Thanks,
> --Serge
>
>
> 2014-07-17 3:15 GMT+07:00 Evgeniy Stepanov <eugeni.stepanov at gmail.com>:
>
>> About right, but for MemorySanitizer you need to link target binaries
>> with instrumented libstdc++/libc++. And it does not detect leaks
>> anyway, ASan does.
>>
>> So, -DLLVM_USE_SANITIZER=Address should do it.
>>
>> On Wed, Jul 16, 2014 at 10:49 PM, Serge Pavlov <sepavloff at gmail.com>
>> wrote:
>> > Hi Evgeniy,
>> >
>> > There is nothing in r213120, that could cause memory leaks in
>> > NestedNameSpecifierLocBuilder, maybe this change triggered existing
>> > problem.
>> > Sanitizer reports leaks of tiny blocks (1 byte), in r213178 such blocks
>> > should not be created. However I am not sure if this can help.
>> >
>> > By the way, how I can build compiler with sanitizer enabled? When I
>> > create
>> > build using command:
>> >
>> > cmake \
>> >     -DCMAKE_BUILD_TYPE=Release \
>> >     -DLLVM_ENABLE_ASSERTIONS=ON \
>> >     -DCMAKE_C_COMPILER=$CLANGPATH/clang \
>> >     -DCMAKE_CXX_COMPILER=$CLANGPATH/clang++ \
>> >     -DLLVM_USE_SANITIZER=Memory \
>> >     -DLLVM_TARGETS_TO_BUILD=X86 \
>> >     ../llvm
>> >
>> > make fails:
>> >
>> > [  6%] Building Intrinsics.gen...
>> > ==25305== WARNING: MemorySanitizer: use-of-uninitialized-value
>> >     #0 0x7fb534d9dedd in _M_dispose
>> >
>> > /usr/lib/gcc/x86_64-redhat-linux/4.8.2/../../../../include/c++/4.8.2/bits/basic_string.h:240:34
>> > ...
>> >
>> >
>> > Thanks,
>> > --Serge
>> >
>> >
>> > 2014-07-16 19:50 GMT+07:00 Evgeniy Stepanov <eugeni.stepanov at gmail.com>:
>> >
>> >> Hi,
>> >>
>> >> is looks like there are new memory leaks from this change:
>> >>
>> >>
>> >> http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap/builds/3963/steps/check-clang%20asan/logs/stdio
>> >>
>> >> On Wed, Jul 16, 2014 at 9:16 AM, Serge Pavlov <sepavloff at gmail.com>
>> >> wrote:
>> >> > Author: sepavloff
>> >> > Date: Wed Jul 16 00:16:52 2014
>> >> > New Revision: 213120
>> >> >
>> >> > URL: http://llvm.org/viewvc/llvm-project?rev=213120&view=rev
>> >> > Log:
>> >> > Improve error recovery around colon.
>> >> >
>> >> > Recognize additional cases, when '::' is mistyped as ':'.
>> >> > This is a fix to RP18587 - colons have too much protection in
>> >> > member-declarations
>> >> > Review is tracked by http://reviews.llvm.org/D3653.
>> >> >
>> >> > This is an attempt to recommit the fix, initially committed as
>> >> > r212957
>> >> > but then
>> >> > reverted in r212965 as it broke self-build. In the updated patch
>> >> > ParseDirectDeclarator
>> >> > turns on colon protection in for context as well.
>> >> >
>> >> > Modified:
>> >> >     cfe/trunk/lib/Parse/ParseDecl.cpp
>> >> >     cfe/trunk/lib/Parse/ParseDeclCXX.cpp
>> >> >     cfe/trunk/test/SemaCXX/enum-bitfield.cpp
>> >> >     cfe/trunk/test/SemaCXX/for-range-examples.cpp
>> >> >     cfe/trunk/test/SemaCXX/nested-name-spec.cpp
>> >> >
>> >> > Modified: cfe/trunk/lib/Parse/ParseDecl.cpp
>> >> > URL:
>> >> >
>> >> > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=213120&r1=213119&r2=213120&view=diff
>> >> >
>> >> >
>> >> > ==============================================================================
>> >> > --- cfe/trunk/lib/Parse/ParseDecl.cpp (original)
>> >> > +++ cfe/trunk/lib/Parse/ParseDecl.cpp Wed Jul 16 00:16:52 2014
>> >> > @@ -2715,24 +2715,23 @@ void Parser::ParseDeclarationSpecifiers(
>> >> >        // typedef-name
>> >> >      case tok::kw_decltype:
>> >> >      case tok::identifier: {
>> >> > +      // This identifier can only be a typedef name if we haven't
>> >> > already seen
>> >> > +      // a type-specifier.  Without this check we misparse:
>> >> > +      //  typedef int X; struct Y { short X; };  as 'short int'.
>> >> > +      if (DS.hasTypeSpecifier())
>> >> > +        goto DoneWithDeclSpec;
>> >> > +
>> >> >        // In C++, check to see if this is a scope specifier like
>> >> > foo::bar::, if
>> >> >        // so handle it as such.  This is important for ctor parsing.
>> >> >        if (getLangOpts().CPlusPlus) {
>> >> >          if (TryAnnotateCXXScopeToken(EnteringContext)) {
>> >> > -          if (!DS.hasTypeSpecifier())
>> >> > -            DS.SetTypeSpecError();
>> >> > +          DS.SetTypeSpecError();
>> >> >            goto DoneWithDeclSpec;
>> >> >          }
>> >> >          if (!Tok.is(tok::identifier))
>> >> >            continue;
>> >> >        }
>> >> >
>> >> > -      // This identifier can only be a typedef name if we haven't
>> >> > already seen
>> >> > -      // a type-specifier.  Without this check we misparse:
>> >> > -      //  typedef int X; struct Y { short X; };  as 'short int'.
>> >> > -      if (DS.hasTypeSpecifier())
>> >> > -        goto DoneWithDeclSpec;
>> >> > -
>> >> >        // Check for need to substitute AltiVec keyword tokens.
>> >> >        if (TryAltiVecToken(DS, Loc, PrevSpec, DiagID, isInvalid))
>> >> >          break;
>> >> > @@ -4529,7 +4528,9 @@ void Parser::ParseDeclaratorInternal(Dec
>> >> >    // Member pointers get special handling, since there's no place
>> >> > for
>> >> > the
>> >> >    // scope spec in the generic path below.
>> >> >    if (getLangOpts().CPlusPlus &&
>> >> > -      (Tok.is(tok::coloncolon) || Tok.is(tok::identifier) ||
>> >> > +      (Tok.is(tok::coloncolon) ||
>> >> > +       (Tok.is(tok::identifier) &&
>> >> > +        (NextToken().is(tok::coloncolon) ||
>> >> > NextToken().is(tok::less)))
>> >> > ||
>> >> >         Tok.is(tok::annot_cxxscope))) {
>> >> >      bool EnteringContext = D.getContext() == Declarator::FileContext
>> >> > ||
>> >> >                             D.getContext() ==
>> >> > Declarator::MemberContext;
>> >> > @@ -4722,6 +4723,14 @@ void Parser::ParseDirectDeclarator(Decla
>> >> >    DeclaratorScopeObj DeclScopeObj(*this, D.getCXXScopeSpec());
>> >> >
>> >> >    if (getLangOpts().CPlusPlus && D.mayHaveIdentifier()) {
>> >> > +    // Don't parse FOO:BAR as if it were a typo for FOO::BAR inside
>> >> > a
>> >> > class, in
>> >> > +    // this context it is a bitfield. Also in range-based for
>> >> > statement
>> >> > colon
>> >> > +    // may delimit for-range-declaration.
>> >> > +    ColonProtectionRAIIObject X(*this,
>> >> > +                                D.getContext() ==
>> >> > Declarator::MemberContext ||
>> >> > +                                    (D.getContext() ==
>> >> > Declarator::ForContext &&
>> >> > +                                     getLangOpts().CPlusPlus11));
>> >> > +
>> >> >      // ParseDeclaratorInternal might already have parsed the scope.
>> >> >      if (D.getCXXScopeSpec().isEmpty()) {
>> >> >        bool EnteringContext = D.getContext() ==
>> >> > Declarator::FileContext
>> >> > ||
>> >> >
>> >> > Modified: cfe/trunk/lib/Parse/ParseDeclCXX.cpp
>> >> > URL:
>> >> >
>> >> > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDeclCXX.cpp?rev=213120&r1=213119&r2=213120&view=diff
>> >> >
>> >> >
>> >> > ==============================================================================
>> >> > --- cfe/trunk/lib/Parse/ParseDeclCXX.cpp (original)
>> >> > +++ cfe/trunk/lib/Parse/ParseDeclCXX.cpp Wed Jul 16 00:16:52 2014
>> >> > @@ -1239,7 +1239,8 @@ void Parser::ParseClassSpecifier(tok::To
>> >> >    // Parse the (optional) nested-name-specifier.
>> >> >    CXXScopeSpec &SS = DS.getTypeSpecScope();
>> >> >    if (getLangOpts().CPlusPlus) {
>> >> > -    // "FOO : BAR" is not a potential typo for "FOO::BAR".
>> >> > +    // "FOO : BAR" is not a potential typo for "FOO::BAR".  In this
>> >> > context it
>> >> > +    // is a base-specifier-list.
>> >> >      ColonProtectionRAIIObject X(*this);
>> >> >
>> >> >      if (ParseOptionalCXXScopeSpecifier(SS, ParsedType(),
>> >> > EnteringContext))
>> >> > @@ -1926,14 +1927,8 @@ void Parser::ParseCXXMemberDeclaratorBef
>> >> >    //   declarator pure-specifier[opt]
>> >> >    //   declarator brace-or-equal-initializer[opt]
>> >> >    //   identifier[opt] ':' constant-expression
>> >> > -  if (Tok.isNot(tok::colon)) {
>> >> > -    // Don't parse FOO:BAR as if it were a typo for FOO::BAR, in
>> >> > this
>> >> > context it
>> >> > -    // is a bitfield.
>> >> > -    // FIXME: This should only apply when parsing the id-expression
>> >> > (see
>> >> > -    // PR18587).
>> >> > -    ColonProtectionRAIIObject X(*this);
>> >> > +  if (Tok.isNot(tok::colon))
>> >> >      ParseDeclarator(DeclaratorInfo);
>> >> > -  }
>> >> >
>> >> >    if (!DeclaratorInfo.isFunctionDeclarator() &&
>> >> > TryConsumeToken(tok::colon)) {
>> >> >      BitfieldSize = ParseConstantExpression();
>> >> > @@ -2015,6 +2010,14 @@ void Parser::ParseCXXClassMemberDeclarat
>> >> >      return;
>> >> >    }
>> >> >
>> >> > +  // Turn on colon protection early, while parsing declspec,
>> >> > although
>> >> > there is
>> >> > +  // nothing to protect there. It prevents from false errors if
>> >> > error
>> >> > recovery
>> >> > +  // incorrectly determines where the declspec ends, as in the
>> >> > example:
>> >> > +  //   struct A { enum class B { C }; };
>> >> > +  //   const int C = 4;
>> >> > +  //   struct D { A::B : C; };
>> >> > +  ColonProtectionRAIIObject X(*this);
>> >> > +
>> >> >    // Access declarations.
>> >> >    bool MalformedTypeSpec = false;
>> >> >    if (!TemplateInfo.Kind &&
>> >> > @@ -2128,13 +2131,11 @@ void Parser::ParseCXXClassMemberDeclarat
>> >> >    if (MalformedTypeSpec)
>> >> >      DS.SetTypeSpecError();
>> >> >
>> >> > -  {
>> >> > -    // Don't parse FOO:BAR as if it were a typo for FOO::BAR, in
>> >> > this
>> >> > context it
>> >> > -    // is a bitfield.
>> >> > -    ColonProtectionRAIIObject X(*this);
>> >> > -    ParseDeclarationSpecifiers(DS, TemplateInfo, AS, DSC_class,
>> >> > -                               &CommonLateParsedAttrs);
>> >> > -  }
>> >> > +  ParseDeclarationSpecifiers(DS, TemplateInfo, AS, DSC_class,
>> >> > +                             &CommonLateParsedAttrs);
>> >> > +
>> >> > +  // Turn off colon protection that was set for declspec.
>> >> > +  X.restore();
>> >> >
>> >> >    // If we had a free-standing type definition with a missing
>> >> > semicolon, we
>> >> >    // may get this far before the problem becomes obvious.
>> >> >
>> >> > Modified: cfe/trunk/test/SemaCXX/enum-bitfield.cpp
>> >> > URL:
>> >> >
>> >> > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/enum-bitfield.cpp?rev=213120&r1=213119&r2=213120&view=diff
>> >> >
>> >> >
>> >> > ==============================================================================
>> >> > --- cfe/trunk/test/SemaCXX/enum-bitfield.cpp (original)
>> >> > +++ cfe/trunk/test/SemaCXX/enum-bitfield.cpp Wed Jul 16 00:16:52 2014
>> >> > @@ -16,3 +16,15 @@ struct Y {
>> >> >    enum E : int(2);
>> >> >    enum E : Z(); // expected-error{{integral constant expression must
>> >> > have integral or unscoped enumeration type, not 'Z'}}
>> >> >  };
>> >> > +
>> >> > +namespace pr18587 {
>> >> > +struct A {
>> >> > +  enum class B {
>> >> > +    C
>> >> > +  };
>> >> > +};
>> >> > +const int C = 4;
>> >> > +struct D {
>> >> > +  A::B : C;
>> >> > +};
>> >> > +}
>> >> >
>> >> > Modified: cfe/trunk/test/SemaCXX/for-range-examples.cpp
>> >> > URL:
>> >> >
>> >> > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/for-range-examples.cpp?rev=213120&r1=213119&r2=213120&view=diff
>> >> >
>> >> >
>> >> > ==============================================================================
>> >> > --- cfe/trunk/test/SemaCXX/for-range-examples.cpp (original)
>> >> > +++ cfe/trunk/test/SemaCXX/for-range-examples.cpp Wed Jul 16 00:16:52
>> >> > 2014
>> >> > @@ -227,3 +227,15 @@ namespace test7 {
>> >> >      for (e [[deprecated]] : arr) { e = 0; } // expected-warning
>> >> > {{deprecated}} expected-note {{here}} expected-warning {{extension}}
>> >> >    }
>> >> >  }
>> >> > +
>> >> > +namespace pr18587 {
>> >> > +  class Arg {};
>> >> > +  struct Cont {
>> >> > +    int *begin();
>> >> > +    int *end();
>> >> > +  };
>> >> > +  void AddAllArgs(Cont &x) {
>> >> > +    for (auto Arg: x) {
>> >> > +    }
>> >> > +  }
>> >> > +}
>> >> >
>> >> > Modified: cfe/trunk/test/SemaCXX/nested-name-spec.cpp
>> >> > URL:
>> >> >
>> >> > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/nested-name-spec.cpp?rev=213120&r1=213119&r2=213120&view=diff
>> >> >
>> >> >
>> >> > ==============================================================================
>> >> > --- cfe/trunk/test/SemaCXX/nested-name-spec.cpp (original)
>> >> > +++ cfe/trunk/test/SemaCXX/nested-name-spec.cpp Wed Jul 16 00:16:52
>> >> > 2014
>> >> > @@ -311,3 +311,102 @@ namespace N {
>> >> >
>> >> >  namespace TypedefNamespace { typedef int F; };
>> >> >  TypedefNamespace::F::NonexistentName BadNNSWithCXXScopeSpec; //
>> >> > expected-error {{'F' (aka 'int') is not a class, namespace, or scoped
>> >> > enumeration}}
>> >> > +
>> >> > +namespace PR18587 {
>> >> > +
>> >> > +struct C1 {
>> >> > +  int a, b, c;
>> >> > +  typedef int C2;
>> >> > +  struct B1 {
>> >> > +    struct B2 {
>> >> > +      int a, b, c;
>> >> > +    };
>> >> > +  };
>> >> > +};
>> >> > +struct C2 { static const unsigned N1 = 1; };
>> >> > +struct B1 {
>> >> > +  enum E1 { B2 = 2 };
>> >> > +  static const int B3 = 3;
>> >> > +};
>> >> > +const int N1 = 2;
>> >> > +
>> >> > +// Function declarators
>> >> > +struct S1a { int f(C1::C2); };
>> >> > +struct S1b { int f(C1:C2); };  // expected-error{{unexpected ':' in
>> >> > nested name specifier; did you mean '::'?}}
>> >> > +
>> >> > +struct S2a {
>> >> > +  C1::C2 f(C1::C2);
>> >> > +};
>> >> > +struct S2c {
>> >> > +  C1::C2 f(C1:C2);  // expected-error{{unexpected ':' in nested name
>> >> > specifier; did you mean '::'?}}
>> >> > +};
>> >> > +
>> >> > +struct S3a {
>> >> > +  int f(C1::C2), C2 : N1;
>> >> > +  int g : B1::B2;
>> >> > +};
>> >> > +struct S3b {
>> >> > +  int g : B1:B2;  // expected-error{{unexpected ':' in nested name
>> >> > specifier; did you mean '::'?}}
>> >> > +};
>> >> > +
>> >> > +// Inside square brackets
>> >> > +struct S4a {
>> >> > +  int f[C2::N1];
>> >> > +};
>> >> > +struct S4b {
>> >> > +  int f[C2:N1];  // expected-error{{unexpected ':' in nested name
>> >> > specifier; did you mean '::'?}}
>> >> > +};
>> >> > +
>> >> > +struct S5a {
>> >> > +  int f(int xx[B1::B3 ? C2::N1 : B1::B2]);
>> >> > +};
>> >> > +struct S5b {
>> >> > +  int f(int xx[B1::B3 ? C2::N1 : B1:B2]);  //
>> >> > expected-error{{unexpected ':' in nested name specifier; did you mean
>> >> > '::'?}}
>> >> > +};
>> >> > +struct S5c {
>> >> > +  int f(int xx[B1:B3 ? C2::N1 : B1::B2]);  //
>> >> > expected-error{{unexpected ':' in nested name specifier; did you mean
>> >> > '::'?}}
>> >> > +};
>> >> > +
>> >> > +// Bit fields
>> >> > +struct S6a {
>> >> > +  C1::C2 m1 : B1::B2;
>> >> > +};
>> >> > +struct S6c {
>> >> > +  C1::C2 m1 : B1:B2;  // expected-error{{unexpected ':' in nested
>> >> > name
>> >> > specifier; did you mean '::'?}}
>> >> > +};
>> >> > +struct S6d {
>> >> > +  int C2:N1;
>> >> > +};
>> >> > +struct S6e {
>> >> > +  static const int N = 3;
>> >> > +  B1::E1 : N;
>> >> > +};
>> >> > +struct S6g {
>> >> > +  C1::C2 : B1:B2;  // expected-error{{unexpected ':' in nested name
>> >> > specifier; did you mean '::'?}}
>> >> > +  B1::E1 : B1:B2;  // expected-error{{unexpected ':' in nested name
>> >> > specifier; did you mean '::'?}}
>> >> > +};
>> >> > +
>> >> > +// Template parameters
>> >> > +template <int N> struct T1 {
>> >> > +  int a,b,c;
>> >> > +  static const unsigned N1 = N;
>> >> > +  typedef unsigned C1;
>> >> > +};
>> >> > +T1<C2::N1> var_1a;
>> >> > +T1<C2:N1> var_1b;  // expected-error{{unexpected ':' in nested name
>> >> > specifier; did you mean '::'?}}
>> >> > +template<int N> int F() {}
>> >> > +int (*X1)() = (B1::B2 ? F<1> : F<2>);
>> >> > +int (*X2)() = (B1:B2 ? F<1> : F<2>);  // expected-error{{unexpected
>> >> > ':'
>> >> > in nested name specifier; did you mean '::'?}}
>> >> > +
>> >> > +// Bit fields + templates
>> >> > +struct S7a {
>> >> > +  T1<B1::B2>::C1 m1 : T1<B1::B2>::N1;
>> >> > +};
>> >> > +struct S7b {
>> >> > +  T1<B1:B2>::C1 m1 : T1<B1::B2>::N1;  // expected-error{{unexpected
>> >> > ':'
>> >> > in nested name specifier; did you mean '::'?}}
>> >> > +};
>> >> > +struct S7c {
>> >> > +  T1<B1::B2>::C1 m1 : T1<B1:B2>::N1;  // expected-error{{unexpected
>> >> > ':'
>> >> > in nested name specifier; did you mean '::'?}}
>> >> > +};
>> >> > +
>> >> > +}
>> >> >
>> >> >
>> >> > _______________________________________________
>> >> > cfe-commits mailing list
>> >> > cfe-commits at cs.uiuc.edu
>> >> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>> >
>> >
>
>



More information about the cfe-commits mailing list