[llvm] r337596 - [Demangler] Correctly factor in assignment when allocating.

Reid Kleckner via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 23 11:23:09 PDT 2018


Should be fixed in r337720. It wasn't this alignment change that caused the
leaks, it was more likely the change that originally added the MS
demangler, r337584.

On Mon, Jul 23, 2018 at 11:12 AM Zachary Turner <zturner at google.com> wrote:

> I’m OOO until Thursday.  +rnk in case he can help
>
> On Mon, Jul 23, 2018 at 9:37 AM Matt Morehouse <mascasa at google.com> wrote:
>
>> This is causing memory leaks:
>> http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/21207.
>> Please take a look.
>>
>> ==38753==ERROR: LeakSanitizer: detected memory leaks
>>
>> Direct leak of 1776 byte(s) in 74 object(s) allocated from:
>>     #0 0x50cfa8 in operator new(unsigned long) /b/sanitizer-x86_64-linux-fast/build/llvm/projects/compiler-rt/lib/asan/asan_new_delete.cc:92
>>     #1 0x51452e in ArenaAllocator /b/sanitizer-x86_64-linux-fast/build/llvm/lib/Demangle/MicrosoftDemangle.cpp:39:27
>>     #2 0x51452e in Demangler /b/sanitizer-x86_64-linux-fast/build/llvm/lib/Demangle/MicrosoftDemangle.cpp:594
>>     #3 0x51452e in llvm::microsoftDemangle(char const*, char*, unsigned long*, int*) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/Demangle/MicrosoftDemangle.cpp:1535
>>     #4 0x5128c0 in demangle(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) /b/sanitizer-x86_64-linux-fast/build/llvm/tools/llvm-undname/llvm-undname.cpp:34:21
>>     #5 0x5122b9 in main /b/sanitizer-x86_64-linux-fast/build/llvm/tools/llvm-undname/llvm-undname.cpp:67:7
>>     #6 0x7f9756c832e0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202e0)
>>
>> SUMMARY: AddressSanitizer: 1776 byte(s) leaked in 74 allocation(s).
>>
>>
>>
>>
>> On Fri, Jul 20, 2018 at 11:40 AM Zachary Turner via llvm-commits <
>> llvm-commits at lists.llvm.org> wrote:
>>
>>> Author: zturner
>>> Date: Fri Jul 20 11:35:06 2018
>>> New Revision: 337596
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=337596&view=rev
>>> Log:
>>> [Demangler] Correctly factor in assignment when allocating.
>>>
>>> Incidentally all allocations that we currently perform were
>>> properly aligned, but this was only an accident.
>>>
>>> Thanks to Erik Pilkington for catching this.
>>>
>>> Modified:
>>>     llvm/trunk/lib/Demangle/MicrosoftDemangle.cpp
>>>
>>> Modified: llvm/trunk/lib/Demangle/MicrosoftDemangle.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Demangle/MicrosoftDemangle.cpp?rev=337596&r1=337595&r2=337596&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/lib/Demangle/MicrosoftDemangle.cpp (original)
>>> +++ llvm/trunk/lib/Demangle/MicrosoftDemangle.cpp Fri Jul 20 11:35:06
>>> 2018
>>> @@ -46,21 +46,28 @@ public:
>>>      }
>>>    }
>>>
>>> -  void *alloc(size_t Size) {
>>> +  template <typename T, typename... Args> T *alloc(Args &&...
>>> ConstructorArgs) {
>>> +
>>> +    size_t Size = sizeof(T);
>>>      assert(Size < Unit);
>>>      assert(Head && Head->Buf);
>>>
>>> -    uint8_t *P = Head->Buf + Head->Used;
>>> -    Head->Used += Size;
>>> +    size_t P = (size_t)Head->Buf + Head->Used;
>>> +    uintptr_t AlignedP =
>>> +        (((size_t)P + alignof(T) - 1) & ~(size_t)(alignof(T) - 1));
>>> +    uint8_t *PP = (uint8_t *)AlignedP;
>>> +    size_t Adjustment = AlignedP - P;
>>> +
>>> +    Head->Used += Size + Adjustment;
>>>      if (Head->Used < Unit)
>>> -      return P;
>>> +      return new (PP) T(std::forward<Args>(ConstructorArgs)...);
>>>
>>>      AllocatorNode *NewHead = new AllocatorNode;
>>>      NewHead->Buf = new uint8_t[ArenaAllocator::Unit];
>>>      NewHead->Next = Head;
>>>      Head = NewHead;
>>>      NewHead->Used = Size;
>>> -    return NewHead->Buf;
>>> +    return new (NewHead->Buf) T(std::forward<Args>(ConstructorArgs)...);
>>>    }
>>>
>>>  private:
>>> @@ -84,8 +91,6 @@ static void outputSpaceIfNecessary(Outpu
>>>      OS << " ";
>>>  }
>>>
>>> -void *operator new(size_t Size, ArenaAllocator &A) { return
>>> A.alloc(Size); }
>>> -
>>>  // Storage classes
>>>  enum Qualifiers : uint8_t {
>>>    Q_None = 0,
>>> @@ -374,7 +379,7 @@ static void outputName(OutputStream &OS,
>>>  namespace {
>>>
>>>  Type *Type::clone(ArenaAllocator &Arena) const {
>>> -  return new (Arena) Type(*this);
>>> +  return Arena.alloc<Type>(*this);
>>>  }
>>>
>>>  // Write the "first half" of a given type.
>>> @@ -471,7 +476,7 @@ void Type::outputPre(OutputStream &OS) {
>>>  void Type::outputPost(OutputStream &OS) {}
>>>
>>>  Type *PointerType::clone(ArenaAllocator &Arena) const {
>>> -  return new (Arena) PointerType(*this);
>>> +  return Arena.alloc<PointerType>(*this);
>>>  }
>>>
>>>  void PointerType::outputPre(OutputStream &OS) {
>>> @@ -509,7 +514,7 @@ void PointerType::outputPost(OutputStrea
>>>  }
>>>
>>>  Type *FunctionType::clone(ArenaAllocator &Arena) const {
>>> -  return new (Arena) FunctionType(*this);
>>> +  return Arena.alloc<FunctionType>(*this);
>>>  }
>>>
>>>  void FunctionType::outputPre(OutputStream &OS) {
>>> @@ -536,7 +541,7 @@ void FunctionType::outputPost(OutputStre
>>>  }
>>>
>>>  Type *UdtType::clone(ArenaAllocator &Arena) const {
>>> -  return new (Arena) UdtType(*this);
>>> +  return Arena.alloc<UdtType>(*this);
>>>  }
>>>
>>>  void UdtType::outputPre(OutputStream &OS) {
>>> @@ -561,7 +566,7 @@ void UdtType::outputPre(OutputStream &OS
>>>  }
>>>
>>>  Type *ArrayType::clone(ArenaAllocator &Arena) const {
>>> -  return new (Arena) ArrayType(*this);
>>> +  return Arena.alloc<ArrayType>(*this);
>>>  }
>>>
>>>  void ArrayType::outputPre(OutputStream &OS) {
>>> @@ -659,9 +664,9 @@ private:
>>>  void Demangler::parse() {
>>>    // MSVC-style mangled symbols must start with '?'.
>>>    if (!MangledName.consumeFront("?")) {
>>> -    SymbolName = new (Arena) Name;
>>> +    SymbolName = Arena.alloc<Name>();
>>>      SymbolName->Str = MangledName;
>>> -    SymbolType = new (Arena) Type;
>>> +    SymbolType = Arena.alloc<Type>();
>>>      SymbolType->Prim = PrimTy::Unknown;
>>>    }
>>>
>>> @@ -832,7 +837,7 @@ Name *Demangler::demangleName() {
>>>    Name *Head = nullptr;
>>>
>>>    while (!MangledName.consumeFront("@")) {
>>> -    Name *Elem = new (Arena) Name;
>>> +    Name *Elem = Arena.alloc<Name>();
>>>
>>>      assert(!Error);
>>>      demangleNamePiece(*Elem, Head == nullptr);
>>> @@ -1239,7 +1244,7 @@ ReferenceKind Demangler::demangleReferen
>>>  }
>>>
>>>  Type *Demangler::demangleFunctionEncoding() {
>>> -  FunctionType *FTy = new (Arena) FunctionType;
>>> +  FunctionType *FTy = Arena.alloc<FunctionType>();
>>>
>>>    FTy->Prim = PrimTy::Function;
>>>    FTy->FunctionClass = (FuncClass)demangleFunctionClass();
>>> @@ -1265,7 +1270,7 @@ Type *Demangler::demangleFunctionEncodin
>>>
>>>  // Reads a primitive type.
>>>  Type *Demangler::demangleBasicType() {
>>> -  Type *Ty = new (Arena) Type;
>>> +  Type *Ty = Arena.alloc<Type>();
>>>
>>>    switch (MangledName.popFront()) {
>>>    case 'X':
>>> @@ -1335,7 +1340,7 @@ Type *Demangler::demangleBasicType() {
>>>  }
>>>
>>>  UdtType *Demangler::demangleClassType() {
>>> -  UdtType *UTy = new (Arena) UdtType;
>>> +  UdtType *UTy = Arena.alloc<UdtType>();
>>>
>>>    switch (MangledName.popFront()) {
>>>    case 'T':
>>> @@ -1365,7 +1370,7 @@ UdtType *Demangler::demangleClassType()
>>>  // <pointer-type> ::= E? <pointer-cvr-qualifiers> <ext-qualifiers>
>>> <type>
>>>  //                       # the E is required for 64-bit non-static
>>> pointers
>>>  PointerType *Demangler::demanglePointerType() {
>>> -  PointerType *Pointer = new (Arena) PointerType;
>>> +  PointerType *Pointer = Arena.alloc<PointerType>();
>>>
>>>    Pointer->Quals = Q_None;
>>>    switch (MangledName.popFront()) {
>>> @@ -1392,7 +1397,7 @@ PointerType *Demangler::demanglePointerT
>>>    }
>>>
>>>    if (MangledName.consumeFront("6")) {
>>> -    FunctionType *FTy = new (Arena) FunctionType;
>>> +    FunctionType *FTy = Arena.alloc<FunctionType>();
>>>      FTy->Prim = PrimTy::Function;
>>>      FTy->CallConvention = demangleCallingConvention();
>>>
>>> @@ -1435,12 +1440,12 @@ ArrayType *Demangler::demangleArrayType(
>>>      return nullptr;
>>>    }
>>>
>>> -  ArrayType *ATy = new (Arena) ArrayType;
>>> +  ArrayType *ATy = Arena.alloc<ArrayType>();
>>>    ArrayType *Dim = ATy;
>>>    for (int I = 0; I < Dimension; ++I) {
>>>      Dim->Prim = PrimTy::Array;
>>>      Dim->ArrayDimension = demangleNumber();
>>> -    Dim->NextDimension = new (Arena) ArrayType;
>>> +    Dim->NextDimension = Arena.alloc<ArrayType>();
>>>      Dim = Dim->NextDimension;
>>>    }
>>>
>>> @@ -1476,7 +1481,7 @@ ParamList Demangler::demangleParameterLi
>>>        }
>>>        MangledName = MangledName.dropFront();
>>>
>>> -      *Current = new (Arena) ParamList;
>>> +      *Current = Arena.alloc<ParamList>();
>>>        (*Current)->Current = BackRef[N]->clone(Arena);
>>>        Current = &(*Current)->Next;
>>>        continue;
>>> @@ -1484,7 +1489,7 @@ ParamList Demangler::demangleParameterLi
>>>
>>>      size_t ArrayDimension = MangledName.size();
>>>
>>> -    *Current = new (Arena) ParamList;
>>> +    *Current = Arena.alloc<ParamList>();
>>>      (*Current)->Current = demangleType(QualifierMangleMode::Drop);
>>>
>>>      // Single-letter types are ignored for backreferences because
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180723/a47ef6b6/attachment.html>


More information about the llvm-commits mailing list