[patch] Create a MCSymbolELF

[Jim Grosbach]

Agreed. This is cool. Thanks, Rafael.
-jim
> On Jun 1, 2015, at 4:00 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
> 
> 
>> On 2015-Jun-01, at 15:56, Rafael Espíndola <rafael.espindola at gmail.com> wrote:
>> 
>>>> -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
>> <t.patch>
> 
> LGTM.
>