[patch] Create a MCSymbolELF

Rafael EspĂ­ndola rafael.espindola at gmail.com
Mon Jun 1 15:56:02 PDT 2015


>> -MCSymbol *MCContext::getOrCreateSectionSymbol(const MCSectionELF &Section) {
>> -  MCSymbol *&Sym = SectionSymbols[&Section];
>> +MCSymbolELF *MCContext::getOrCreateSectionSymbol(const MCSectionELF &Section) {
>> +  MCSymbolELF *&Sym = SectionSymbols[&Section];
>>    if (Sym)
>>      return Sym;
>>
>> @@ -128,12 +128,12 @@ MCSymbol *MCContext::getOrCreateSectionSymbol(const MCSectionELF &Section) {
>>
>>    MCSymbol *&OldSym = Symbols[Name];
>>    if (OldSym && OldSym->isUndefined()) {
>> -    Sym = OldSym;
>> -    return OldSym;
>> +    Sym = cast<MCSymbolELF>(OldSym);
>> +    return Sym;
>>    }
>>
>>    auto NameIter = UsedNames.insert(std::make_pair(Name, true)).first;
>> -  Sym = new (*this) MCSymbol(&*NameIter, /*isTemporary*/ false);
>> +  Sym = new (*this) MCSymbolELF(&*NameIter, /*isTemporary*/ false);
>
> It'd be nice to `assert(isELF())` at the top of this function, if that's
> not already there.

It is already passed a MCSectionELF, so that is statically known.

>>
>>    if (!OldSym)
>>      OldSym = Sym;
>> @@ -163,8 +163,14 @@ MCSymbol *MCContext::CreateSymbol(StringRef Name, bool AlwaysAddSuffix) {
>>    if (AllowTemporaryLabels)
>>      IsTemporary = Name.startswith(MAI->getPrivateGlobalPrefix());
>>
>> -  if (IsTemporary && AlwaysAddSuffix && !UseNamesOnTempLabels)
>> -    return new (*this) MCSymbol(nullptr, true);
>> +  bool IsELF = MOFI && MOFI->getObjectFileType() == MCObjectFileInfo::IsELF;
>
> Should this be in a function?  When is `MOFI` not available?

MOFI is null in llvm-objdump. Running

 ./bin/llvm-objdump -m -d
~/llvm/llvm/test/tools/llvm-objdump/X86/Inputs/exeThread.macho-x86_64

Fails without this.

>>    SmallString<128> NewName = Name;
>>    bool AddSuffix = AlwaysAddSuffix;
>> @@ -178,7 +184,11 @@ MCSymbol *MCContext::CreateSymbol(StringRef Name, bool AlwaysAddSuffix) {
>>      if (NameEntry.second) {
>>        // Ok, we found a name. Have the MCSymbol object itself refer to the copy
>>        // of the string that is embedded in the UsedNames entry.
>> -      MCSymbol *Result = new (*this) MCSymbol(&*NameEntry.first, IsTemporary);
>> +      MCSymbol *Result;
>> +      if (IsELF)
>> +        Result = new (*this) MCSymbolELF(&*NameEntry.first, IsTemporary);
>> +      else
>> +        Result = new (*this) MCSymbol(false, &*NameEntry.first, IsTemporary);
>
> Actually, this is somewhat awkward code duplication.  It might be better
> to add a private function like this:
>
>     MCSymbol *MCContext::createSymbolImpl(
>         const StringMapEntry<...> *Name, bool IsTemporary) {
>       if (IsELF)
>         return new (*this) MCSymbolELF(Name, IsTemporary);
>       return new (*this) MCSymbol(/* IsELF */ false, Name, IsTemporary);
>     }

Done. An updated patch is attached.

Cheers,
Rafael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: t.patch
Type: text/x-patch
Size: 21732 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150601/78e254f5/attachment.bin>


More information about the llvm-commits mailing list