[llvm] r210298 - Correctly set the comdat symbol on COFF.
Timur Iskhodzhanov
timurrrr at google.com
Fri Jun 6 01:26:11 PDT 2014
r210317.
2014-06-06 12:14 GMT+04:00 Timur Iskhodzhanov <timurrrr at google.com>:
> 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/1d0bf69b/attachment.html>
More information about the llvm-commits
mailing list