[libcxxabi] r278579 - Fix ASAN failures in the demangler

Kostya Serebryany via cfe-commits cfe-commits at lists.llvm.org
Fri Aug 12 17:29:38 PDT 2016


On Fri, Aug 12, 2016 at 5:26 PM, Mehdi Amini <mehdi.amini at apple.com> wrote:

> This fixes all the crashers on Darwin (clang+libc++), that I could
> reproduce with ASAN+libFuzzer.
> It does not mean that there is no leaks, or that you won’t find more
> crashes with libstdc++ for instance.
>

Yea... On linux I hit a leak right away:
Direct leak of 12 byte(s) in 1 object(s) allocated from:
    #0 0x4c1afe in realloc
    #1 0x4f1663 in __cxa_demangle cxa_demangle.cpp:5012:47
    #2 0x5215f8 in LLVMFuzzerTestOneInput cxa_demangle_fuzzer.cpp:11:3

With lsan disabled the fuzzer is running for 3 minutes / 20 cores w/o new
crashes.
Great improvement!!!

--kcc


>
>> Mehdi
>
> On Aug 12, 2016, at 5:21 PM, Kostya Serebryany <kcc at google.com> wrote:
>
> Sweet!
> Did you fix all of the known crashers?
>
>
>
> On Fri, Aug 12, 2016 at 5:02 PM, Mehdi Amini via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
>> Author: mehdi_amini
>> Date: Fri Aug 12 19:02:33 2016
>> New Revision: 278579
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=278579&view=rev
>> Log:
>> Fix ASAN failures in the demangler
>>
>> These were found fuzzing with ASAN.
>>
>> Modified:
>>     libcxxabi/trunk/src/cxa_demangle.cpp
>>     libcxxabi/trunk/test/test_demangle.pass.cpp
>>
>> Modified: libcxxabi/trunk/src/cxa_demangle.cpp
>> URL: http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/src/cxa_
>> demangle.cpp?rev=278579&r1=278578&r2=278579&view=diff
>> ============================================================
>> ==================
>> --- libcxxabi/trunk/src/cxa_demangle.cpp (original)
>> +++ libcxxabi/trunk/src/cxa_demangle.cpp Fri Aug 12 19:02:33 2016
>> @@ -624,6 +624,8 @@ parse_const_cast_expr(const char* first,
>>                      return first;
>>                  auto expr = db.names.back().move_full();
>>                  db.names.pop_back();
>> +                if (db.names.empty())
>> +                    return first;
>>                  db.names.back() = "const_cast<" +
>> db.names.back().move_full() + ">(" + expr + ")";
>>                  first = t1;
>>              }
>> @@ -650,6 +652,8 @@ parse_dynamic_cast_expr(const char* firs
>>                      return first;
>>                  auto expr = db.names.back().move_full();
>>                  db.names.pop_back();
>> +                if (db.names.empty())
>> +                    return first;
>>                  db.names.back() = "dynamic_cast<" +
>> db.names.back().move_full() + ">(" + expr + ")";
>>                  first = t1;
>>              }
>> @@ -676,6 +680,8 @@ parse_reinterpret_cast_expr(const char*
>>                      return first;
>>                  auto expr = db.names.back().move_full();
>>                  db.names.pop_back();
>> +                if (db.names.empty())
>> +                    return first;
>>                  db.names.back() = "reinterpret_cast<" +
>> db.names.back().move_full() + ">(" + expr + ")";
>>                  first = t1;
>>              }
>> @@ -1294,6 +1300,8 @@ parse_dot_expr(const char* first, const
>>                      return first;
>>                  auto name = db.names.back().move_full();
>>                  db.names.pop_back();
>> +                if (db.names.empty())
>> +                    return first;
>>                  db.names.back().first += "." + name;
>>                  first = t1;
>>              }
>> @@ -2896,6 +2904,8 @@ base_name(String& s)
>>                  ++c;
>>          }
>>      }
>> +    if (pe - pf <= 1)
>> +      return String();
>>      const char* p0 = pe - 1;
>>      for (; p0 != pf; --p0)
>>      {
>> @@ -3016,7 +3026,8 @@ parse_unnamed_type_name(const char* firs
>>                  const char* t1 = parse_type(t0, last, db);
>>                  if (t1 == t0)
>>                  {
>> -                    db.names.pop_back();
>> +                    if(!db.names.empty())
>> +                        db.names.pop_back();
>>                      return first;
>>                  }
>>                  if (db.names.size() < 2)
>> @@ -3041,17 +3052,21 @@ parse_unnamed_type_name(const char* firs
>>                      }
>>                      t0 = t1;
>>                  }
>> +                if(db.names.empty())
>> +                  return first;
>>                  db.names.back().first.append(")");
>>              }
>>              if (t0 == last || *t0 != 'E')
>>              {
>> +              if(!db.names.empty())
>>                  db.names.pop_back();
>>                  return first;
>>              }
>>              ++t0;
>>              if (t0 == last)
>>              {
>> -                db.names.pop_back();
>> +                if(!db.names.empty())
>> +                  db.names.pop_back();
>>                  return first;
>>              }
>>              if (std::isdigit(*t0))
>> @@ -3064,7 +3079,8 @@ parse_unnamed_type_name(const char* firs
>>              }
>>              if (t0 == last || *t0 != '_')
>>              {
>> -                db.names.pop_back();
>> +                if(!db.names.empty())
>> +                  db.names.pop_back();
>>                  return first;
>>              }
>>              first = t0 + 1;
>> @@ -3251,7 +3267,7 @@ parse_binary_expression(const char* firs
>>                  nm += ')';
>>              first = t2;
>>          }
>> -        else
>> +        else if(!db.names.empty())
>>              db.names.pop_back();
>>      }
>>      return first;
>> @@ -3490,7 +3506,7 @@ parse_expression(const char* first, cons
>>                          db.names.back() = "(" + op1 + ")[" + op2 + "]";
>>                          first = t2;
>>                      }
>> -                    else
>> +                    else if (!db.names.empty())
>>                          db.names.pop_back();
>>                  }
>>              }
>> @@ -3686,11 +3702,13 @@ parse_expression(const char* first, cons
>>                          }
>>                          else
>>                          {
>> +                            if (db.names.size() < 2)
>> +                              return first;
>>                              db.names.pop_back();
>>                              db.names.pop_back();
>>                          }
>>                      }
>> -                    else
>> +                    else if (!db.names.empty())
>>                          db.names.pop_back();
>>                  }
>>              }
>> @@ -3879,8 +3897,9 @@ parse_template_args(const char* first, c
>>                      args += ", ";
>>                  args += db.names[k].move_full();
>>              }
>> -            for (; k1 != k0; --k1)
>> -                db.names.pop_back();
>> +            for (; k1 > k0; --k1)
>> +                if (!db.names.empty())
>> +                    db.names.pop_back();
>>              t = t1;
>>          }
>>          first = t + 1;
>> @@ -3959,6 +3978,8 @@ parse_nested_name(const char* first, con
>>                  {
>>                      auto name = db.names.back().move_full();
>>                      db.names.pop_back();
>> +                    if (db.names.empty())
>> +                        return first;
>>                      if (!db.names.back().first.empty())
>>                      {
>>                          db.names.back().first += "::" + name;
>> @@ -3978,6 +3999,8 @@ parse_nested_name(const char* first, con
>>                  {
>>                      auto name = db.names.back().move_full();
>>                      db.names.pop_back();
>> +                    if (db.names.empty())
>> +                        return first;
>>                      if (!db.names.back().first.empty())
>>                          db.names.back().first += "::" + name;
>>                      else
>> @@ -3997,6 +4020,8 @@ parse_nested_name(const char* first, con
>>                  {
>>                      auto name = db.names.back().move_full();
>>                      db.names.pop_back();
>> +                    if (db.names.empty())
>> +                        return first;
>>                      if (!db.names.back().first.empty())
>>                          db.names.back().first += "::" + name;
>>                      else
>> @@ -4014,6 +4039,8 @@ parse_nested_name(const char* first, con
>>                  {
>>                      auto name = db.names.back().move_full();
>>                      db.names.pop_back();
>> +                    if (db.names.empty())
>> +                        return first;
>>                      db.names.back().first += name;
>>                      db.subs.push_back(typename C::sub_type(1,
>> db.names.back(), db.names.get_allocator()));
>>                      t0 = t1;
>> @@ -4033,6 +4060,8 @@ parse_nested_name(const char* first, con
>>                  {
>>                      auto name = db.names.back().move_full();
>>                      db.names.pop_back();
>> +                    if (db.names.empty())
>> +                        return first;
>>                      if (!db.names.back().first.empty())
>>                          db.names.back().first += "::" + name;
>>                      else
>> @@ -4130,11 +4159,13 @@ parse_local_name(const char* first, cons
>>                                  return first;
>>                              auto name = db.names.back().move_full();
>>                              db.names.pop_back();
>> +                            if (db.names.empty())
>> +                                return first;
>>                              db.names.back().first.append("::");
>>                              db.names.back().first.append(name);
>>                              first = t1;
>>                          }
>> -                        else
>> +                        else if (!db.names.empty())
>>                              db.names.pop_back();
>>                      }
>>                  }
>> @@ -4151,10 +4182,12 @@ parse_local_name(const char* first, cons
>>                              return first;
>>                          auto name = db.names.back().move_full();
>>                          db.names.pop_back();
>> +                        if (db.names.empty())
>> +                            return first;
>>                          db.names.back().first.append("::");
>>                          db.names.back().first.append(name);
>>                      }
>> -                    else
>> +                    else if (!db.names.empty())
>>                          db.names.pop_back();
>>                  }
>>                  break;
>> @@ -4219,6 +4252,8 @@ parse_name(const char* first, const char
>>                              return first;
>>                          auto tmp = db.names.back().move_full();
>>                          db.names.pop_back();
>> +                        if (db.names.empty())
>> +                            return first;
>>                          db.names.back().first += tmp;
>>                          first = t1;
>>                          if (ends_with_template_args)
>> @@ -4241,6 +4276,8 @@ parse_name(const char* first, const char
>>                              return first;
>>                          auto tmp = db.names.back().move_full();
>>                          db.names.pop_back();
>> +                        if (db.names.empty())
>> +                            return first;
>>                          db.names.back().first += tmp;
>>                          first = t1;
>>                          if (ends_with_template_args)
>> @@ -4399,6 +4436,8 @@ parse_special_name(const char* first, co
>>                                  return first;
>>                              auto left = db.names.back().move_full();
>>                              db.names.pop_back();
>> +                            if (db.names.empty())
>> +                                return first;
>>                              db.names.back().first = "construction vtable
>> for " +
>>                                                      std::move(left) +
>> "-in-" +
>>
>>  db.names.back().move_full();
>> @@ -4538,6 +4577,9 @@ parse_encoding(const char* first, const
>>                          if (ret2.empty())
>>                              ret1 += ' ';
>>                          db.names.pop_back();
>> +                        if (db.names.empty())
>> +                            return first;
>> +
>>                          db.names.back().first.insert(0, ret1);
>>                          t = t2;
>>                      }
>> @@ -4565,8 +4607,11 @@ parse_encoding(const char* first, const
>>                                          tmp += ", ";
>>                                      tmp += db.names[k].move_full();
>>                                  }
>> -                                for (size_t k = k0; k < k1; ++k)
>> +                                for (size_t k = k0; k < k1; ++k) {
>> +                                    if (db.names.empty())
>> +                                        return first;
>>                                      db.names.pop_back();
>> +                                }
>>                                  if (!tmp.empty())
>>                                  {
>>                                      if (db.names.empty())
>>
>> Modified: libcxxabi/trunk/test/test_demangle.pass.cpp
>> URL: http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/test/tes
>> t_demangle.pass.cpp?rev=278579&r1=278578&r2=278579&view=diff
>> ============================================================
>> ==================
>> --- libcxxabi/trunk/test/test_demangle.pass.cpp (original)
>> +++ libcxxabi/trunk/test/test_demangle.pass.cpp Fri Aug 12 19:02:33 2016
>> @@ -29591,6 +29591,7 @@ const char* cases[][2] =
>>      {"_ZZ4testvEN1g3fooE5Point", "test()::g::foo(Point)"},
>>      {"_ZThn12_NSt9strstreamD0Ev",   "non-virtual thunk to
>> std::strstream::~strstream()"},
>>      {"_ZTv0_n12_NSt9strstreamD0Ev",     "virtual thunk to
>> std::strstream::~strstream()"},
>> +    {"\x6D", "unsigned long"},         // This use to crash with ASAN
>>  };
>>
>>  const unsigned N = sizeof(cases) / sizeof(cases[0]);
>> @@ -29636,6 +29637,21 @@ const char* invalid_cases[] =
>>  #if !LDBL_FP80
>>      "_ZN5test01hIfEEvRAcvjplstT_Le4001a000000000000000E_c",
>>  #endif
>> +       // The following test cases were found by libFuzzer+ASAN
>> +    "\x44\x74\x70\x74\x71\x75\x34\x43\x41\x72\x4D\x6E\x65\x34\x9
>> F\xC1\x43\x41\x72\x4D\x6E\x77\x38\x9A\x8E\x44\x6F\x64\x6C\
>> x53\xF9\x5F\x70\x74\x70\x69\x45\x34\xD3\x73\x9E\x2A\x37",
>> +    "\x4D\x41\x72\x63\x4E\x39\x44\x76\x72\x4D\x34\x44\x53\x4B\x6
>> F\x44\x54\x6E\x61\x37\x47\x77\x78\x38\x43\x27\x41\x5F\x73\x70\x69\x45*",
>> +    "\x41\x64\x6E\x32*",
>> +    "\x43\x46\x41\x67\x73*",
>> +    "\x72\x3A\x4E\x53\x64\x45\x39\x4F\x52\x4E\x1F\x43\x34\x64\x5
>> 4\x5F\x49\x31\x41\x63\x6C\x37\x2A\x4D\x41\x67\x73\x76\x43\
>> x54\x35\x5F\x49\x4B\x4C\x55\x6C\x73\x4C\x38\x64\x43\x41\
>> x47\x4C\x5A\x28\x4F\x41\x6E\x77\x5F\x53\x6F\x70\x69\x45\
>> x5F\x63\x47\x61\x4C\x31\x4F\x4C\x33\x3E\x41\x4C\x4B\x4C\
>> x55\x6C\x73\x4C\x38\x64\x43\x66\x41\x47\x4C\x5A\x28\x4F\
>> x41\x6E\x77\x5F\x53\x6F\x70\x69\x45\x5F\x37\x41*",
>> +    "\x2D\x5F\x63\x47\x4F\x63\xD3",
>> +    "\x44\x74\x70\x74\x71\x75\x32\x43\x41\x38\x65\x6E\x9B\x72\x4
>> D\xC1\x43\x41\x72\x4D\x6E\x77\x38\x9A\x8E\x44\x6F\x64\xC3\
>> x53\xF9\x5F\x70\x74\x70\x69\x45\x38\xD3\x73\x9E\x2A\x37",
>> +    "\x4C\x5A\x4C\x55\x6C\x4D\x41\x5F\x41\x67\x74\x71\x75\x34\x4
>> D\x41\x64\x73\x4C\x44\x76\x72\x4D\x34\x44\x4B\x44\x54\x6E\
>> x61\x37\x47\x77\x78\x38\x43\x27\x41\x5F\x73\x70\x69\x45\
>> x6D\x73\x72\x53\x41\x6F\x41\x7B",
>> +    "\x44\x74\x70\x74\x71\x75\x32\x43\x41\x38\x65\x6E\x9B\x72\x4
>> D\xC1\x43\x41\x72\x4D\x6E\x77\x38\x9A\x8E\x44\x6F\x64\x2C\
>> x53\xF9\x5F\x70\x74\x70\x69\x45\xB4\xD3\x73\x9F\x2A\x37",
>> +    "\x4C\x5A\x4C\x55\x6C\x69\x4D\x73\x72\x53\x6F\x7A\x41\x5F\x4
>> 1\x67\x74\x71\x75\x32\x4D\x41\x64\x73\x39\x28\x76\x72\x4D\
>> x34\x44\x4B\x45\x54\x6E\x61\x37\x47\x77\x78\x38\x43\x27\
>> x41\x5F\x73\x70\x69\x45\x6F\x45\x49\x6D\x1A\x4C\x53\x38\x6A\x7A\x5A",
>> +    "\x44\x74\x63*",
>> +    "\x44\x74\x71\x75\x35\x2A\xDF\x74\x44\x61\x73\x63\x35\x2A\x3
>> B\x41\x72\x4D\x6E\x65\x34\x9F\xC1\x63\x41\x72\x4D\x6E\x77\
>> x38\x9A\x8E\x44\x6F\x64\x6C\x53\xF9\x5F\x70\x74\x70\x69\
>> x45\x33\x44\x76\x35",
>> +    "\x44\x74\x70\x74\x71\x75\x32\x43\x41\x38\x65\x6E\x9B\x72\x4
>> D\xC1\x43\x41\x72\x4D\x6E\x77\x38\x9A\x8E\x44\x6F\x64\x6C\
>> x53\xF9\x5F\x70\x74\x70\x69\x45\x38\xD3\x73\x9E\x2A\x37",
>> +    "\x46\x44\x74\x70\x74\x71\x75\x32\x43\x41\x72\x4D\x6E\x65\x3
>> 4\x9F\xC1\x43\x41\x72\x4D\x6E\x77\x38\x9A\x8E\x44\x6F\x64\
>> x6C\x53\xF9\x5F\x70\x74\x70\x69\x45\x34\xD3\x73\x9E\x2A\
>> x37\x72\x33\x8E\x3A\x29\x8E\x44\x35",
>>  };
>>
>>  const unsigned NI = sizeof(invalid_cases) / sizeof(invalid_cases[0]);
>>
>>
>> _______________________________________________
>> 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/20160812/28864dad/attachment-0001.html>


More information about the cfe-commits mailing list