Skip to content

Commit

Permalink
Revert "Make TType store a const char * for mangled name."
Browse files Browse the repository at this point in the history
This reverts commit dc7bffd.

Reason for revert: Causes a memory leak, detected by ASAN bot:

https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_asan_rel_ng/builds/494713

Direct leak of 24 byte(s) in 1 object(s) allocated from:
    #0 0x847aa2 in operator new(unsigned long) /b/build/slave/linux_upload_clang/build/src/third_party/llvm/compiler-rt/lib/asan/asan_new_delete.cc:92:3
    #1 0x193a833 in sh::TType::buildMangledName() const third_party/angle/src/compiler/translator/Types.cpp:545:21
    #2 0x193d2e8 in getMangledName third_party/angle/src/compiler/translator/Types.cpp:751:24
    #3 0x193d2e8 in sh::TType::realize() third_party/angle/src/compiler/translator/Types.cpp:759
    #4 0x1834474 in sh::TCache::getType(sh::TBasicType, sh::TPrecision, sh::TQualifier, unsigned char, unsigned char) third_party/angle/src/compiler/translator/Cache.cpp:89:11
    #5 0x1859ac7 in getType third_party/angle/src/compiler/translator/Cache.h:36:16
    #6 0x1859ac7 in sh::InsertBuiltInFunctions(unsigned int, ShShaderSpec, ShBuiltInResources const&, sh::TSymbolTable&) third_party/angle/src/compiler/translator/Initialize.cpp:28

Bug: angleproject:1432

Original change's description:
> Make TType store a const char * for mangled name.
> 
> We would only ever use the c_str value from the mangled name. This
> makes it easier to make constexpr TTypes.
> 
> Bug: angleproject:1432
> Change-Id: I147b3a85f9b8b2453e2d7f4a713d767b22036cc9
> Reviewed-on: https://chromium-review.googlesource.com/776277
> Commit-Queue: Jamie Madill <jmadill@chromium.org>
> Reviewed-by: Kai Ninomiya <kainino@chromium.org>

TBR=jmadill@chromium.org,kainino@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: angleproject:1432
Change-Id: Ib112a2ce9871a4f4afc53101ac1a3ddd166008cf
Reviewed-on: https://chromium-review.googlesource.com/780420
Reviewed-by: Jamie Madill <jmadill@chromium.org>
  • Loading branch information
null77 committed Nov 21, 2017
1 parent 85c93c4 commit c796500
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 27 deletions.
4 changes: 2 additions & 2 deletions src/compiler/translator/SymbolTable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ const TString *TFunction::buildMangledName() const

for (const auto &p : parameters)
{
newName += p.type->getMangledName();
newName += p.type->getMangledName().c_str();
}
return NewPoolTString(newName.c_str());
}
Expand All @@ -79,7 +79,7 @@ const TString &TFunction::GetMangledNameFromCall(const TString &functionName,

for (TIntermNode *argument : arguments)
{
newName += argument->getAsTyped()->getType().getMangledName();
newName += argument->getAsTyped()->getType().getMangledName().c_str();
}
return *NewPoolTString(newName.c_str());
}
Expand Down
34 changes: 12 additions & 22 deletions src/compiler/translator/Types.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,7 @@ TType::TType()
secondarySize(0),
mInterfaceBlock(nullptr),
mStructure(nullptr),
mIsStructSpecifier(false),
mMangledName(nullptr)
mIsStructSpecifier(false)
{
}

Expand All @@ -141,8 +140,7 @@ TType::TType(TBasicType t, unsigned char ps, unsigned char ss)
secondarySize(ss),
mInterfaceBlock(0),
mStructure(0),
mIsStructSpecifier(false),
mMangledName(nullptr)
mIsStructSpecifier(false)
{
}

Expand All @@ -157,8 +155,7 @@ TType::TType(TBasicType t, TPrecision p, TQualifier q, unsigned char ps, unsigne
secondarySize(ss),
mInterfaceBlock(0),
mStructure(0),
mIsStructSpecifier(false),
mMangledName(nullptr)
mIsStructSpecifier(false)
{
}

Expand All @@ -173,8 +170,7 @@ TType::TType(const TPublicType &p)
secondarySize(p.getSecondarySize()),
mInterfaceBlock(nullptr),
mStructure(nullptr),
mIsStructSpecifier(false),
mMangledName(nullptr)
mIsStructSpecifier(false)
{
ASSERT(primarySize <= 4);
ASSERT(secondarySize <= 4);
Expand All @@ -200,8 +196,7 @@ TType::TType(TStructure *userDef)
secondarySize(1),
mInterfaceBlock(nullptr),
mStructure(userDef),
mIsStructSpecifier(false),
mMangledName(nullptr)
mIsStructSpecifier(false)
{
}

Expand All @@ -218,8 +213,7 @@ TType::TType(TInterfaceBlock *interfaceBlockIn,
secondarySize(1),
mInterfaceBlock(interfaceBlockIn),
mStructure(0),
mIsStructSpecifier(false),
mMangledName(nullptr)
mIsStructSpecifier(false)
{
}

Expand Down Expand Up @@ -381,7 +375,7 @@ TString TType::getCompleteString() const
//
// Recursively generate mangled names.
//
const char *TType::buildMangledName() const
TString TType::buildMangledName() const
{
TString mangledName;
if (isMatrix())
Expand Down Expand Up @@ -538,12 +532,7 @@ const char *TType::buildMangledName() const
mangledName += buf;
mangledName += ']';
}

mangledName += ';';

// We allocate with the pool allocator, so it's fine that the TString goes out of scope.
TString *temp = new TString(mangledName);
return temp->c_str();
return mangledName;
}

size_t TType::getObjectSize() const
Expand Down Expand Up @@ -744,11 +733,12 @@ void TType::setStruct(TStructure *s)
}
}

const char *TType::getMangledName() const
const TString &TType::getMangledName() const
{
if (mMangledName == nullptr)
if (mMangledName.empty())
{
mMangledName = buildMangledName();
mMangledName += ';';
}

return mMangledName;
Expand All @@ -761,7 +751,7 @@ void TType::realize()

void TType::invalidateMangledName()
{
mMangledName = nullptr;
mMangledName = "";
}

// TStructure implementation.
Expand Down
6 changes: 3 additions & 3 deletions src/compiler/translator/Types.h
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ class TType
const TStructure *getStruct() const { return mStructure; }
void setStruct(TStructure *s);

const char *getMangledName() const;
const TString &getMangledName() const;

bool sameNonArrayType(const TType &right) const;

Expand Down Expand Up @@ -368,7 +368,7 @@ class TType

private:
void invalidateMangledName();
const char *buildMangledName() const;
TString buildMangledName() const;

TBasicType type;
TPrecision precision;
Expand All @@ -393,7 +393,7 @@ class TType
TStructure *mStructure;
bool mIsStructSpecifier;

mutable const char *mMangledName;
mutable TString mMangledName;
};

// TTypeSpecifierNonArray stores all of the necessary fields for type_specifier_nonarray from the
Expand Down

0 comments on commit c796500

Please sign in to comment.