<div dir="ltr">r210317.</div><div class="gmail_extra"><br><br><div class="gmail_quote">2014-06-06 12:14 GMT+04:00 Timur Iskhodzhanov <span dir="ltr"><<a href="mailto:timurrrr@google.com" target="_blank">timurrrr@google.com</a>></span>:<br>

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Also,<div><a href="http://build.chromium.org/p/chromium.fyi/builders/Chromium%20Win%20Clang%20%28asan%29/builds/907/steps/compile/logs/stdio" target="_blank">http://build.chromium.org/p/chromium.fyi/builders/Chromium%20Win%20Clang%20%28asan%29/builds/907/steps/compile/logs/stdio</a><br>


</div><div>Assertion failed: !COMDATSymbol->Section, file C:\b\build\slave\Chromium_Win_Clang__asan_\build\src\third_party\llvm\lib\MC\WinCOFFObjectWriter.cpp, line 352<br></div><div><br></div><div>I'm going to revert your change.</div>


</div><div class="gmail_extra"><br><br><div class="gmail_quote">2014-06-06 10:20 GMT+04:00 Timur Iskhodzhanov <span dir="ltr"><<a href="mailto:timurrrr@google.com" target="_blank">timurrrr@google.com</a>></span>:<div>

<div class="h5"><br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hi Rafael,<div><br></div><div>Looks like this broke "check-asan check-sanitizer".</div></div>

<div class="gmail_extra">
<br><br><div class="gmail_quote">2014-06-06 3:09 GMT+04:00 Rafael Espindola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span>:<div><div><br>

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: rafael<br>
Date: Thu Jun  5 18:09:25 2014<br>
New Revision: 210298<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=210298&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=210298&view=rev</a><br>
Log:<br>
Correctly set the comdat symbol on COFF.<br>
<br>
We extended the .section syntax to allow multiple sections with the<br>
same name but different comdats, but currently we don't make sure that<br>
the output section has that comdat symbol.<br>
<br>
That happens to work with the code llc produces currently because it looks like<br>
<br>
.section secName, "dr", one_only, "COMDATSym"<br>
.globl COMDATSym<br>
COMDATSym:<br>
....<br>
<br>
but that is not very friendly to anyone coding in assembly or even to<br>
llc once we get comdat support in the IR.<br>
<br>
This patch changes the coff object writer to make sure the comdat symbol is<br>
output just after the section symbol, as required by the coff spec.<br>
<br>
Added:<br>
    llvm/trunk/test/MC/COFF/section-comdat-conflict.s<br>
Modified:<br>
    llvm/trunk/include/llvm/MC/MCSectionCOFF.h<br>
    llvm/trunk/lib/MC/WinCOFFObjectWriter.cpp<br>
    llvm/trunk/test/MC/COFF/section-comdat.s<br>
<br>
Modified: llvm/trunk/include/llvm/MC/MCSectionCOFF.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCSectionCOFF.h?rev=210298&r1=210297&r2=210298&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCSectionCOFF.h?rev=210298&r1=210297&r2=210298&view=diff</a><br>




==============================================================================<br>
--- llvm/trunk/include/llvm/MC/MCSectionCOFF.h (original)<br>
+++ llvm/trunk/include/llvm/MC/MCSectionCOFF.h Thu Jun  5 18:09:25 2014<br>
@@ -76,6 +76,7 @@ class MCSymbol;<br>
       return SectionName.str() + "_end";<br>
     }<br>
     unsigned getCharacteristics() const { return Characteristics; }<br>
+    const MCSymbol *getCOMDATSymbol() const { return COMDATSymbol; }<br>
     int getSelection() const { return Selection; }<br>
     const MCSectionCOFF *getAssocSection() const { return Assoc; }<br>
<br>
<br>
Modified: llvm/trunk/lib/MC/WinCOFFObjectWriter.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/WinCOFFObjectWriter.cpp?rev=210298&r1=210297&r2=210298&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/WinCOFFObjectWriter.cpp?rev=210298&r1=210297&r2=210298&view=diff</a><br>




==============================================================================<br>
--- llvm/trunk/lib/MC/WinCOFFObjectWriter.cpp (original)<br>
+++ llvm/trunk/lib/MC/WinCOFFObjectWriter.cpp Thu Jun  5 18:09:25 2014<br>
@@ -347,6 +347,11 @@ void WinCOFFObjectWriter::DefineSection(<br>
<br>
   COFFSection *coff_section = createSection(Sec.getSectionName());<br>
   COFFSymbol  *coff_symbol = createSymbol(Sec.getSectionName());<br>
+  if (const MCSymbol *S = Sec.getCOMDATSymbol()) {<br>
+    COFFSymbol *COMDATSymbol = GetOrCreateCOFFSymbol(S);<br>
+    assert(!COMDATSymbol->Section);<br>
+    COMDATSymbol->Section = coff_section;<br>
+  }<br>
<br>
   coff_section->Symbol = coff_symbol;<br>
   coff_symbol->Section = coff_section;<br>
@@ -458,9 +463,15 @@ void WinCOFFObjectWriter::DefineSymbol(M<br>
       coff_symbol->Data.SectionNumber = COFF::IMAGE_SYM_ABSOLUTE;<br>
     } else {<br>
       const MCSymbolData &BaseData = Assembler.getSymbolData(*Base);<br>
-      if (BaseData.Fragment)<br>
-        coff_symbol->Section =<br>
+      if (BaseData.Fragment) {<br>
+        COFFSection *Sec =<br>
             SectionMap[&BaseData.Fragment->getParent()->getSection()];<br>
+<br>
+        if (coff_symbol->Section && coff_symbol->Section != Sec)<br>
+          report_fatal_error("conflicting sections for symbol");<br>
+<br>
+        coff_symbol->Section = Sec;<br>
+      }<br>
     }<br>
<br>
     coff_symbol->MCData = &ResSymData;<br>
<br>
Added: llvm/trunk/test/MC/COFF/section-comdat-conflict.s<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/COFF/section-comdat-conflict.s?rev=210298&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/COFF/section-comdat-conflict.s?rev=210298&view=auto</a><br>




==============================================================================<br>
--- llvm/trunk/test/MC/COFF/section-comdat-conflict.s (added)<br>
+++ llvm/trunk/test/MC/COFF/section-comdat-conflict.s Thu Jun  5 18:09:25 2014<br>
@@ -0,0 +1,13 @@<br>
+// RUN: not llvm-mc -triple i386-pc-win32 -filetype=obj < %s 2>&1 |  FileCheck %s<br>
+<br>
+// CHECK: conflicting sections for symbol<br>
+<br>
+        .section .xyz<br>
+        .global bar<br>
+bar:<br>
+        .long 42<br>
+<br>
+        .section        .abcd,"xr",discard,bar<br>
+        .global foo<br>
+foo:<br>
+        .long 42<br>
<br>
Modified: llvm/trunk/test/MC/COFF/section-comdat.s<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/COFF/section-comdat.s?rev=210298&r1=210297&r2=210298&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/COFF/section-comdat.s?rev=210298&r1=210297&r2=210298&view=diff</a><br>




==============================================================================<br>
--- llvm/trunk/test/MC/COFF/section-comdat.s (original)<br>
+++ llvm/trunk/test/MC/COFF/section-comdat.s Thu Jun  5 18:09:25 2014<br>
@@ -40,6 +40,11 @@ Symbol6:<br>
 Symbol7:<br>
 .long 1<br>
<br>
+.section SecName, "dr", newest, "Symbol8"<br>
+.globl AnotherSymbol<br>
+AnotherSymbol:<br>
+.long 1<br>
+<br>
 // CHECK: Sections [<br>
 // CHECK:   Section {<br>
 // CHECK:     Number: 1<br>
@@ -114,6 +119,10 @@ Symbol7:<br>
 // CHECK:     }<br>
 // CHECK:   }<br>
 // CHECK:   Symbol {<br>
+// CHECK:     Name: Symbol1<br>
+// CHECK:     Section: secName (2)<br>
+// CHECK:   }<br>
+// CHECK:   Symbol {<br>
 // CHECK:     Name: secName<br>
 // CHECK:     Section: secName (3)<br>
 // CHECK:     AuxSectionDef {<br>
@@ -121,6 +130,10 @@ Symbol7:<br>
 // CHECK:     }<br>
 // CHECK:   }<br>
 // CHECK:   Symbol {<br>
+// CHECK:     Name: Symbol2<br>
+// CHECK:     Section: secName (3)<br>
+// CHECK:   }<br>
+// CHECK:   Symbol {<br>
 // CHECK:     Name: SecName<br>
 // CHECK:     Section: SecName (4)<br>
 // CHECK:     AuxSectionDef {<br>
@@ -128,6 +141,10 @@ Symbol7:<br>
 // CHECK:     }<br>
 // CHECK:   }<br>
 // CHECK:   Symbol {<br>
+// CHECK:     Name: Symbol3<br>
+// CHECK:     Section: SecName (4)<br>
+// CHECK:   }<br>
+// CHECK:   Symbol {<br>
 // CHECK:     Name: SecName<br>
 // CHECK:     Section: SecName (5)<br>
 // CHECK:     AuxSymbolCount: 1<br>
@@ -136,6 +153,10 @@ Symbol7:<br>
 // CHECK:     }<br>
 // CHECK:   }<br>
 // CHECK:   Symbol {<br>
+// CHECK:     Name: Symbol4<br>
+// CHECK:     Section: SecName (5)<br>
+// CHECK:   }<br>
+// CHECK:   Symbol {<br>
 // CHECK:     Name: SecName<br>
 // CHECK:     Section: SecName (6)<br>
 // CHECK:     AuxSectionDef {<br>
@@ -144,6 +165,10 @@ Symbol7:<br>
 // CHECK:     }<br>
 // CHECK:   }<br>
 // CHECK:   Symbol {<br>
+// CHECK:     Name: Symbol5<br>
+// CHECK:     Section: SecName (6)<br>
+// CHECK:   }<br>
+// CHECK:   Symbol {<br>
 // CHECK:     Name: SecName<br>
 // CHECK:     Section: SecName (7)<br>
 // CHECK:     AuxSectionDef {<br>
@@ -151,6 +176,10 @@ Symbol7:<br>
 // CHECK:     }<br>
 // CHECK:   }<br>
 // CHECK:   Symbol {<br>
+// CHECK:     Name: Symbol6<br>
+// CHECK:     Section: SecName (7)<br>
+// CHECK:   }<br>
+// CHECK:   Symbol {<br>
 // CHECK:     Name: SecName<br>
 // CHECK:     Section: SecName (8)<br>
 // CHECK:     AuxSectionDef {<br>
@@ -158,31 +187,22 @@ Symbol7:<br>
 // CHECK:     }<br>
 // CHECK:   }<br>
 // CHECK:   Symbol {<br>
-// CHECK:     Name: Symbol1<br>
-// CHECK:     Section: secName (2)<br>
-// CHECK:   }<br>
-// CHECK:   Symbol {<br>
-// CHECK:     Name: Symbol2<br>
-// CHECK:     Section: secName (3)<br>
-// CHECK:   }<br>
-// CHECK:   Symbol {<br>
-// CHECK:     Name: Symbol3<br>
-// CHECK:     Section: SecName (4)<br>
-// CHECK:   }<br>
-// CHECK:   Symbol {<br>
-// CHECK:     Name: Symbol4<br>
-// CHECK:     Section: SecName (5)<br>
+// CHECK:     Name: Symbol7<br>
+// CHECK:     Section: SecName (8)<br>
 // CHECK:   }<br>
 // CHECK:   Symbol {<br>
-// CHECK:     Name: Symbol5<br>
-// CHECK:     Section: SecName (6)<br>
+// CHECK:     Name: SecName<br>
+// CHECK:     Section: SecName (9)<br>
+// CHECK:     AuxSectionDef {<br>
+// CHECK:       Selection: Newest (0x7)<br>
+// CHECK:     }<br>
 // CHECK:   }<br>
 // CHECK:   Symbol {<br>
-// CHECK:     Name: Symbol6<br>
-// CHECK:     Section: SecName (7)<br>
+// CHECK:     Name: Symbol8<br>
+// CHECK:     Section: SecName (9)<br>
 // CHECK:   }<br>
 // CHECK:   Symbol {<br>
-// CHECK:     Name: Symbol7<br>
-// CHECK:     Section: SecName (8)<br>
+// CHECK:     Name: AnotherSymbol<br>
+// CHECK:     Section: SecName (9)<br>
 // CHECK:   }<br>
 // CHECK: ]<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div></div></div><br></div>
</blockquote></div></div></div><br></div>
</blockquote></div><br></div>