[llvm] r210298 - Correctly set the comdat symbol on COFF.

Timur Iskhodzhanov timurrrr at google.com
Fri Jun 6 01:14:54 PDT 2014


Also,
http://build.chromium.org/p/chromium.fyi/builders/Chromium%20Win%20Clang%20%28asan%29/builds/907/steps/compile/logs/stdio
Assertion failed: !COMDATSymbol->Section, file
C:\b\build\slave\Chromium_Win_Clang__asan_\build\src\third_party\llvm\lib\MC\WinCOFFObjectWriter.cpp,
line 352

I'm going to revert your change.


2014-06-06 10:20 GMT+04:00 Timur Iskhodzhanov <timurrrr at google.com>:

> Hi Rafael,
>
> Looks like this broke "check-asan check-sanitizer".
>
>
> 2014-06-06 3:09 GMT+04:00 Rafael Espindola <rafael.espindola at gmail.com>:
>
> Author: rafael
>> Date: Thu Jun  5 18:09:25 2014
>> New Revision: 210298
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=210298&view=rev
>> Log:
>> Correctly set the comdat symbol on COFF.
>>
>> We extended the .section syntax to allow multiple sections with the
>> same name but different comdats, but currently we don't make sure that
>> the output section has that comdat symbol.
>>
>> That happens to work with the code llc produces currently because it
>> looks like
>>
>> .section secName, "dr", one_only, "COMDATSym"
>> .globl COMDATSym
>> COMDATSym:
>> ....
>>
>> but that is not very friendly to anyone coding in assembly or even to
>> llc once we get comdat support in the IR.
>>
>> This patch changes the coff object writer to make sure the comdat symbol
>> is
>> output just after the section symbol, as required by the coff spec.
>>
>> Added:
>>     llvm/trunk/test/MC/COFF/section-comdat-conflict.s
>> Modified:
>>     llvm/trunk/include/llvm/MC/MCSectionCOFF.h
>>     llvm/trunk/lib/MC/WinCOFFObjectWriter.cpp
>>     llvm/trunk/test/MC/COFF/section-comdat.s
>>
>> Modified: llvm/trunk/include/llvm/MC/MCSectionCOFF.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCSectionCOFF.h?rev=210298&r1=210297&r2=210298&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/include/llvm/MC/MCSectionCOFF.h (original)
>> +++ llvm/trunk/include/llvm/MC/MCSectionCOFF.h Thu Jun  5 18:09:25 2014
>> @@ -76,6 +76,7 @@ class MCSymbol;
>>        return SectionName.str() + "_end";
>>      }
>>      unsigned getCharacteristics() const { return Characteristics; }
>> +    const MCSymbol *getCOMDATSymbol() const { return COMDATSymbol; }
>>      int getSelection() const { return Selection; }
>>      const MCSectionCOFF *getAssocSection() const { return Assoc; }
>>
>>
>> Modified: llvm/trunk/lib/MC/WinCOFFObjectWriter.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/WinCOFFObjectWriter.cpp?rev=210298&r1=210297&r2=210298&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/MC/WinCOFFObjectWriter.cpp (original)
>> +++ llvm/trunk/lib/MC/WinCOFFObjectWriter.cpp Thu Jun  5 18:09:25 2014
>> @@ -347,6 +347,11 @@ void WinCOFFObjectWriter::DefineSection(
>>
>>    COFFSection *coff_section = createSection(Sec.getSectionName());
>>    COFFSymbol  *coff_symbol = createSymbol(Sec.getSectionName());
>> +  if (const MCSymbol *S = Sec.getCOMDATSymbol()) {
>> +    COFFSymbol *COMDATSymbol = GetOrCreateCOFFSymbol(S);
>> +    assert(!COMDATSymbol->Section);
>> +    COMDATSymbol->Section = coff_section;
>> +  }
>>
>>    coff_section->Symbol = coff_symbol;
>>    coff_symbol->Section = coff_section;
>> @@ -458,9 +463,15 @@ void WinCOFFObjectWriter::DefineSymbol(M
>>        coff_symbol->Data.SectionNumber = COFF::IMAGE_SYM_ABSOLUTE;
>>      } else {
>>        const MCSymbolData &BaseData = Assembler.getSymbolData(*Base);
>> -      if (BaseData.Fragment)
>> -        coff_symbol->Section =
>> +      if (BaseData.Fragment) {
>> +        COFFSection *Sec =
>>              SectionMap[&BaseData.Fragment->getParent()->getSection()];
>> +
>> +        if (coff_symbol->Section && coff_symbol->Section != Sec)
>> +          report_fatal_error("conflicting sections for symbol");
>> +
>> +        coff_symbol->Section = Sec;
>> +      }
>>      }
>>
>>      coff_symbol->MCData = &ResSymData;
>>
>> Added: llvm/trunk/test/MC/COFF/section-comdat-conflict.s
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/COFF/section-comdat-conflict.s?rev=210298&view=auto
>>
>> ==============================================================================
>> --- llvm/trunk/test/MC/COFF/section-comdat-conflict.s (added)
>> +++ llvm/trunk/test/MC/COFF/section-comdat-conflict.s Thu Jun  5 18:09:25
>> 2014
>> @@ -0,0 +1,13 @@
>> +// RUN: not llvm-mc -triple i386-pc-win32 -filetype=obj < %s 2>&1 |
>>  FileCheck %s
>> +
>> +// CHECK: conflicting sections for symbol
>> +
>> +        .section .xyz
>> +        .global bar
>> +bar:
>> +        .long 42
>> +
>> +        .section        .abcd,"xr",discard,bar
>> +        .global foo
>> +foo:
>> +        .long 42
>>
>> Modified: llvm/trunk/test/MC/COFF/section-comdat.s
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/COFF/section-comdat.s?rev=210298&r1=210297&r2=210298&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/test/MC/COFF/section-comdat.s (original)
>> +++ llvm/trunk/test/MC/COFF/section-comdat.s Thu Jun  5 18:09:25 2014
>> @@ -40,6 +40,11 @@ Symbol6:
>>  Symbol7:
>>  .long 1
>>
>> +.section SecName, "dr", newest, "Symbol8"
>> +.globl AnotherSymbol
>> +AnotherSymbol:
>> +.long 1
>> +
>>  // CHECK: Sections [
>>  // CHECK:   Section {
>>  // CHECK:     Number: 1
>> @@ -114,6 +119,10 @@ Symbol7:
>>  // CHECK:     }
>>  // CHECK:   }
>>  // CHECK:   Symbol {
>> +// CHECK:     Name: Symbol1
>> +// CHECK:     Section: secName (2)
>> +// CHECK:   }
>> +// CHECK:   Symbol {
>>  // CHECK:     Name: secName
>>  // CHECK:     Section: secName (3)
>>  // CHECK:     AuxSectionDef {
>> @@ -121,6 +130,10 @@ Symbol7:
>>  // CHECK:     }
>>  // CHECK:   }
>>  // CHECK:   Symbol {
>> +// CHECK:     Name: Symbol2
>> +// CHECK:     Section: secName (3)
>> +// CHECK:   }
>> +// CHECK:   Symbol {
>>  // CHECK:     Name: SecName
>>  // CHECK:     Section: SecName (4)
>>  // CHECK:     AuxSectionDef {
>> @@ -128,6 +141,10 @@ Symbol7:
>>  // CHECK:     }
>>  // CHECK:   }
>>  // CHECK:   Symbol {
>> +// CHECK:     Name: Symbol3
>> +// CHECK:     Section: SecName (4)
>> +// CHECK:   }
>> +// CHECK:   Symbol {
>>  // CHECK:     Name: SecName
>>  // CHECK:     Section: SecName (5)
>>  // CHECK:     AuxSymbolCount: 1
>> @@ -136,6 +153,10 @@ Symbol7:
>>  // CHECK:     }
>>  // CHECK:   }
>>  // CHECK:   Symbol {
>> +// CHECK:     Name: Symbol4
>> +// CHECK:     Section: SecName (5)
>> +// CHECK:   }
>> +// CHECK:   Symbol {
>>  // CHECK:     Name: SecName
>>  // CHECK:     Section: SecName (6)
>>  // CHECK:     AuxSectionDef {
>> @@ -144,6 +165,10 @@ Symbol7:
>>  // CHECK:     }
>>  // CHECK:   }
>>  // CHECK:   Symbol {
>> +// CHECK:     Name: Symbol5
>> +// CHECK:     Section: SecName (6)
>> +// CHECK:   }
>> +// CHECK:   Symbol {
>>  // CHECK:     Name: SecName
>>  // CHECK:     Section: SecName (7)
>>  // CHECK:     AuxSectionDef {
>> @@ -151,6 +176,10 @@ Symbol7:
>>  // CHECK:     }
>>  // CHECK:   }
>>  // CHECK:   Symbol {
>> +// CHECK:     Name: Symbol6
>> +// CHECK:     Section: SecName (7)
>> +// CHECK:   }
>> +// CHECK:   Symbol {
>>  // CHECK:     Name: SecName
>>  // CHECK:     Section: SecName (8)
>>  // CHECK:     AuxSectionDef {
>> @@ -158,31 +187,22 @@ Symbol7:
>>  // CHECK:     }
>>  // CHECK:   }
>>  // CHECK:   Symbol {
>> -// CHECK:     Name: Symbol1
>> -// CHECK:     Section: secName (2)
>> -// CHECK:   }
>> -// CHECK:   Symbol {
>> -// CHECK:     Name: Symbol2
>> -// CHECK:     Section: secName (3)
>> -// CHECK:   }
>> -// CHECK:   Symbol {
>> -// CHECK:     Name: Symbol3
>> -// CHECK:     Section: SecName (4)
>> -// CHECK:   }
>> -// CHECK:   Symbol {
>> -// CHECK:     Name: Symbol4
>> -// CHECK:     Section: SecName (5)
>> +// CHECK:     Name: Symbol7
>> +// CHECK:     Section: SecName (8)
>>  // CHECK:   }
>>  // CHECK:   Symbol {
>> -// CHECK:     Name: Symbol5
>> -// CHECK:     Section: SecName (6)
>> +// CHECK:     Name: SecName
>> +// CHECK:     Section: SecName (9)
>> +// CHECK:     AuxSectionDef {
>> +// CHECK:       Selection: Newest (0x7)
>> +// CHECK:     }
>>  // CHECK:   }
>>  // CHECK:   Symbol {
>> -// CHECK:     Name: Symbol6
>> -// CHECK:     Section: SecName (7)
>> +// CHECK:     Name: Symbol8
>> +// CHECK:     Section: SecName (9)
>>  // CHECK:   }
>>  // CHECK:   Symbol {
>> -// CHECK:     Name: Symbol7
>> -// CHECK:     Section: SecName (8)
>> +// CHECK:     Name: AnotherSymbol
>> +// CHECK:     Section: SecName (9)
>>  // CHECK:   }
>>  // CHECK: ]
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140606/192eb560/attachment.html>


More information about the llvm-commits mailing list