patch llvm win64 dwarf4 relocations #15859

vtjnash · 2016-04-13T08:36:56Z

afaict, this is how other platforms treat this flag. and it makes the dwarf4 info correct without seeming to corrupt anything else.

(the IsPCRel case is never triggered, the useful change is switching from the invalid relocation type IMAGE_REL_AMD64_ADDR32 to IMAGE_REL_AMD64_SECREL for FK_Data_4)

tkelman · 2016-04-13T10:26:54Z

Will upstream take this?

edit: by which I mean, merge this as soon as you post an upstream review link

tkelman · 2016-04-13T17:55:51Z

Oh and let me know if there's going to be a test that will urgently need llvm binaries rebuilt to include this patch on appveyor. I was hoping we would sort out the llvm 3.8 subarray segfault before needing to do that.

JeffBezanson · 2016-04-15T18:01:29Z

💯

vtjnash · 2016-04-19T03:47:29Z

@tkelman i'm uncertain yet if this makes sense to promote upstream. it is simply helping to cover up a bug in the relocations for debug info (as i believe happens in the other platforms code). but, while we will never care about the alternative code path this overrides, and neither will any other users of llvm, i'm assuming that clang devs might.

tkelman · 2016-04-19T15:12:06Z

Can you raise the issue at least, on either llvm-dev or bugzilla? It would be unfortunate to have to carry a patch indefinitely, especially one without a corresponding test.

vtjnash · 2016-04-19T15:22:38Z

related julia bug: #10595 (comment)

related clang bug: https://llvm.org/bugs/show_bug.cgi?id=15393

tkelman · 2016-05-11T17:17:43Z

Bump. I went and rebased this for you, since my PR introduced the conflict a few weeks ago.

Any news from upstream about this?

vtjnash · 2016-06-26T02:29:52Z

correct patch:

diff -rpu llvm-3.8.0/include/llvm/MC/MCStreamer.h llvm-3.8.0-reloc/include/llvm/MC/MCStreamer.h
--- llvm-3.8.0/include/llvm/MC/MCStreamer.h 2016-01-12 08:38:15.000000000 -0500
+++ llvm-3.8.0-reloc/include/llvm/MC/MCStreamer.h   2016-05-23 18:54:01.830143100 -0400
@@ -456,7 +456,7 @@ public:
   /// \brief Emits a COFF section relative relocation.
   ///
   /// \param Symbol - Symbol the section relative relocation should point to.
-  virtual void EmitCOFFSecRel32(MCSymbol const *Symbol);
+  virtual void EmitCOFFSecRel32(MCSymbol const *Symbol, uint64_t Offset);

   /// \brief Emit an ELF .size directive.
   ///
diff -rpu llvm-3.8.0/include/llvm/MC/MCWinCOFFStreamer.h llvm-3.8.0-reloc/include/llvm/MC/MCWinCOFFStreamer.h
--- llvm-3.8.0/include/llvm/MC/MCWinCOFFStreamer.h  2015-11-17 05:00:43.000000000 -0500
+++ llvm-3.8.0-reloc/include/llvm/MC/MCWinCOFFStreamer.h    2016-05-23 18:54:01.845722400 -0400
@@ -52,7 +52,7 @@ public:
   void EndCOFFSymbolDef() override;
   void EmitCOFFSafeSEH(MCSymbol const *Symbol) override;
   void EmitCOFFSectionIndex(MCSymbol const *Symbol) override;
-  void EmitCOFFSecRel32(MCSymbol const *Symbol) override;
+  void EmitCOFFSecRel32(MCSymbol const *Symbol, uint64_t Offset) override;
   void EmitCommonSymbol(MCSymbol *Symbol, uint64_t Size,
                         unsigned ByteAlignment) override;
   void EmitLocalCommonSymbol(MCSymbol *Symbol, uint64_t Size,
diff -rpu llvm-3.8.0/lib/CodeGen/AsmPrinter/AsmPrinter.cpp llvm-3.8.0-reloc/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
--- llvm-3.8.0/lib/CodeGen/AsmPrinter/AsmPrinter.cpp    2016-01-12 20:18:13.000000000 -0500
+++ llvm-3.8.0-reloc/lib/CodeGen/AsmPrinter/AsmPrinter.cpp  2016-05-23 18:54:01.845722400 -0400
@@ -1729,7 +1729,7 @@ void AsmPrinter::EmitLabelPlusOffset(con
                                      unsigned Size,
                                      bool IsSectionRelative) const {
   if (MAI->needsDwarfSectionOffsetDirective() && IsSectionRelative) {
-    OutStreamer->EmitCOFFSecRel32(Label);
+    OutStreamer->EmitCOFFSecRel32(Label, Offset);
     return;
   }

diff -rpu llvm-3.8.0/lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp llvm-3.8.0-reloc/lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp
--- llvm-3.8.0/lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp   2015-11-17 19:34:10.000000000 -0500
+++ llvm-3.8.0-reloc/lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp 2016-05-23 20:17:42.105695800 -0400
@@ -150,7 +150,7 @@ void AsmPrinter::emitDwarfSymbolReferenc
   if (!ForceOffset) {
     // On COFF targets, we have to emit the special .secrel32 directive.
     if (MAI->needsDwarfSectionOffsetDirective()) {
-      OutStreamer->EmitCOFFSecRel32(Label);
+      OutStreamer->EmitCOFFSecRel32(Label, /*offset*/0);
       return;
     }

diff -rpu llvm-3.8.0/lib/CodeGen/AsmPrinter/DIE.cpp llvm-3.8.0-reloc/lib/CodeGen/AsmPrinter/DIE.cpp
--- llvm-3.8.0/lib/CodeGen/AsmPrinter/DIE.cpp   2016-01-07 09:28:20.000000000 -0500
+++ llvm-3.8.0-reloc/lib/CodeGen/AsmPrinter/DIE.cpp 2016-05-23 19:08:35.434799900 -0400
@@ -487,7 +487,7 @@ void DIEEntry::EmitValue(const AsmPrinte
     Addr += CU->getDebugInfoOffset();
     if (AP->MAI->doesDwarfUseRelocationsAcrossSections())
       AP->EmitLabelPlusOffset(CU->getSectionSym(), Addr,
-                              DIEEntry::getRefAddrSize(AP));
+                              DIEEntry::getRefAddrSize(AP), true);
     else
       AP->OutStreamer->EmitIntValue(Addr, DIEEntry::getRefAddrSize(AP));
   } else
diff -rpu llvm-3.8.0/lib/CodeGen/AsmPrinter/WinCodeViewLineTables.cpp llvm-3.8.0-reloc/lib/CodeGen/AsmPrinter/WinCodeViewLineTables.cpp
--- llvm-3.8.0/lib/CodeGen/AsmPrinter/WinCodeViewLineTables.cpp 2016-01-12 20:05:23.000000000 -0500
+++ llvm-3.8.0-reloc/lib/CodeGen/AsmPrinter/WinCodeViewLineTables.cpp   2016-05-23 21:19:31.454460900 -0400
@@ -231,7 +231,7 @@ void WinCodeViewLineTables::emitDebugInf
     // code is located and what's its size:
     EmitLabelDiff(*Asm->OutStreamer, Fn, FI.End);
     Asm->OutStreamer->EmitFill(12, 0);
-    Asm->OutStreamer->EmitCOFFSecRel32(Fn);
+    Asm->OutStreamer->EmitCOFFSecRel32(Fn, /*offset*/0);
     Asm->OutStreamer->EmitCOFFSectionIndex(Fn);
     Asm->EmitInt8(0);
     // Emit the function display name as a null-terminated string.
@@ -272,7 +272,7 @@ void WinCodeViewLineTables::emitDebugInf
   Asm->OutStreamer->EmitLabel(LineTableBegin);

   // Identify the function this subsection is for.
-  Asm->OutStreamer->EmitCOFFSecRel32(Fn);
+  Asm->OutStreamer->EmitCOFFSecRel32(Fn, /*offset*/0);
   Asm->OutStreamer->EmitCOFFSectionIndex(Fn);
   // Insert flags after a 16-bit section index.
   Asm->EmitInt16(COFF::DEBUG_LINE_TABLES_HAVE_COLUMN_RECORDS);
diff -rpu llvm-3.8.0/lib/MC/MCAsmStreamer.cpp llvm-3.8.0-reloc/lib/MC/MCAsmStreamer.cpp
--- llvm-3.8.0/lib/MC/MCAsmStreamer.cpp 2015-11-12 08:33:00.000000000 -0500
+++ llvm-3.8.0-reloc/lib/MC/MCAsmStreamer.cpp   2016-05-23 18:54:01.859727900 -0400
@@ -143,7 +143,7 @@ public:
   void EndCOFFSymbolDef() override;
   void EmitCOFFSafeSEH(MCSymbol const *Symbol) override;
   void EmitCOFFSectionIndex(MCSymbol const *Symbol) override;
-  void EmitCOFFSecRel32(MCSymbol const *Symbol) override;
+  void EmitCOFFSecRel32(MCSymbol const *Symbol, uint64_t Offset) override;
   void emitELFSize(MCSymbolELF *Symbol, const MCExpr *Value) override;
   void EmitCommonSymbol(MCSymbol *Symbol, uint64_t Size,
                         unsigned ByteAlignment) override;
@@ -525,7 +525,7 @@ void MCAsmStreamer::EmitCOFFSectionIndex
   EmitEOL();
 }

-void MCAsmStreamer::EmitCOFFSecRel32(MCSymbol const *Symbol) {
+void MCAsmStreamer::EmitCOFFSecRel32(MCSymbol const *Symbol, uint64_t Offset) {
   OS << "\t.secrel32\t";
   Symbol->print(OS, MAI);
   EmitEOL();
diff -rpu llvm-3.8.0/lib/MC/MCParser/COFFAsmParser.cpp llvm-3.8.0-reloc/lib/MC/MCParser/COFFAsmParser.cpp
--- llvm-3.8.0/lib/MC/MCParser/COFFAsmParser.cpp    2015-11-18 01:02:15.000000000 -0500
+++ llvm-3.8.0-reloc/lib/MC/MCParser/COFFAsmParser.cpp  2016-05-23 18:54:15.937859600 -0400
@@ -450,7 +450,7 @@ bool COFFAsmParser::ParseDirectiveSecRel
   MCSymbol *Symbol = getContext().getOrCreateSymbol(SymbolID);

   Lex();
-  getStreamer().EmitCOFFSecRel32(Symbol);
+  getStreamer().EmitCOFFSecRel32(Symbol, /*Offset=*/0);
   return false;
 }

diff -rpu llvm-3.8.0/lib/MC/MCStreamer.cpp llvm-3.8.0-reloc/lib/MC/MCStreamer.cpp
--- llvm-3.8.0/lib/MC/MCStreamer.cpp    2015-11-04 18:59:18.000000000 -0500
+++ llvm-3.8.0-reloc/lib/MC/MCStreamer.cpp  2016-05-23 18:54:15.953281800 -0400
@@ -119,7 +119,7 @@ void MCStreamer::EmitSymbolValue(const M
   if (!IsSectionRelative)
     EmitValueImpl(MCSymbolRefExpr::create(Sym, getContext()), Size);
   else
-    EmitCOFFSecRel32(Sym);
+    EmitCOFFSecRel32(Sym, /*Offset=*/0);
 }

 void MCStreamer::EmitGPRel64Value(const MCExpr *Value) {
@@ -570,7 +570,7 @@ void MCStreamer::EmitCOFFSafeSEH(MCSymbo
 void MCStreamer::EmitCOFFSectionIndex(MCSymbol const *Symbol) {
 }

-void MCStreamer::EmitCOFFSecRel32(MCSymbol const *Symbol) {
+void MCStreamer::EmitCOFFSecRel32(MCSymbol const *Symbol, uint64_t Offset) {
 }

 /// EmitRawText - If this file is backed by an assembly streamer, this dumps
diff -rpu llvm-3.8.0/lib/MC/WinCOFFStreamer.cpp llvm-3.8.0-reloc/lib/MC/WinCOFFStreamer.cpp
--- llvm-3.8.0/lib/MC/WinCOFFStreamer.cpp   2015-11-17 05:00:43.000000000 -0500
+++ llvm-3.8.0-reloc/lib/MC/WinCOFFStreamer.cpp 2016-06-25 22:00:26.530421900 -0400
@@ -199,14 +199,21 @@ void MCWinCOFFStreamer::EmitCOFFSectionI
   DF->getContents().resize(DF->getContents().size() + 2, 0);
 }

-void MCWinCOFFStreamer::EmitCOFFSecRel32(MCSymbol const *Symbol) {
+void MCWinCOFFStreamer::EmitCOFFSecRel32(MCSymbol const *Symbol, uint64_t Offset) {
   MCDataFragment *DF = getOrCreateDataFragment();
-  const MCSymbolRefExpr *SRE = MCSymbolRefExpr::create(Symbol, getContext());
-  MCFixup Fixup = MCFixup::create(DF->getContents().size(), SRE, FK_SecRel_4);
+  // Create Symbol A for the relocation relative reference.
+  const MCExpr *MCE = MCSymbolRefExpr::create(Symbol, getContext());
+  // Add the constant offset, if given
+  if (Offset)
+      MCE = MCBinaryExpr::createAdd(MCE, MCConstantExpr::create(Offset, getContext()), getContext());
+  // Build the secrel32 relocation.
+  MCFixup Fixup = MCFixup::create(DF->getContents().size(), MCE, FK_SecRel_4);
+  // Record the relocation.
   DF->getFixups().push_back(Fixup);
+  // Emit 4 bytes (zeros) to the object file.
   DF->getContents().resize(DF->getContents().size() + 4, 0);
 }

 void MCWinCOFFStreamer::EmitCommonSymbol(MCSymbol *Symbol, uint64_t Size,
                                          unsigned ByteAlignment) {
   assert((!Symbol->isInSection() ||

afaict, this is how other platforms treat this flag

tkelman · 2016-06-26T12:05:51Z

Doesn't seem to fix the type info in my usual smoke test:

$ julia/usr/bin/julia --precompiled=yes
               _
   _       _ _(_)_     |  A fresh approach to technical computing
  (_)     | (_) (_)    |  Documentation: http://docs.julialang.org
   _ _   _| |_  __ _   |  Type "?help" for help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 0.5.0-dev+4939 (2016-06-26 03:01 UTC)
 _/ |\__'_|_|_|\__'_|  |  jn/win64dwarf/664f8b7* (fork: 1 commits, 0 days)
|__/                   |  x86_64-w64-mingw32

julia> include("nonexistent")
ERROR: could not open file C:\cygwin64\home\Tony\nonexistent
 in include_from_node1 at .\loading.jl:426
 in eval at .\boot.jl:234
 in macro expansion at .\REPL.jl:92 [inlined]
 in (::Base.REPL.##1#2{Base.REPL.REPLBackend})() at .\event.jl:46

julia> exit()

Tony@LAPTOP-O230JCFF ~
$ julia/usr/bin/julia --precompiled=no
               _
   _       _ _(_)_     |  A fresh approach to technical computing
  (_)     | (_) (_)    |  Documentation: http://docs.julialang.org
   _ _   _| |_  __ _   |  Type "?help" for help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 0.5.0-dev+4939 (2016-06-26 03:01 UTC)
 _/ |\__'_|_|_|\__'_|  |  jn/win64dwarf/664f8b7* (fork: 1 commits, 0 days)
|__/                   |  x86_64-w64-mingw32

julia> include("nonexistent")
ERROR: could not open file C:\cygwin64\home\Tony\nonexistent
 in include_from_node1(::String) at .\loading.jl:426
 in eval(::Module, ::Any) at .\boot.jl:234
 in macro expansion at .\REPL.jl:92 [inlined]
 in (::Base.REPL.##1#2{Base.REPL.REPLBackend})() at .\event.jl:46

vtjnash · 2016-06-26T18:50:28Z

That's actually completely unrelated, I didn't know that was broken too, but that probably eventually should be addressed too.

tkelman · 2016-06-26T21:25:52Z

That was what I meant by "missing the type signature" in February - #10595 (comment). How possible is that to fix, because that will be noticeable unlike whatever difference this patch is supposed to make.

edit: rebased branch here for you at tkelman@664f8b7 if you want it

vtjnash · 2016-06-27T04:59:36Z

Checking the debuginfo code again, it seems this fix is perhaps necessary but not sufficient. Although if it is necessary, that's due to another, different bug (could be llvm, but since gcc has the same issue, it's probably actually a linker bug).

yes, it would be great if you could force-push that here so that we can merge that.

vtjnash · 2016-06-29T03:18:10Z

deps/llvm.mk

@@ -430,6 +430,7 @@ $(eval $(call LLVM_PATCH,llvm-3.8.0_bindir))
 $(eval $(call LLVM_PATCH,llvm-D14260))
 $(eval $(call LLVM_PATCH,llvm-nodllalias))
 $(eval $(call LLVM_PATCH,llvm-D21271-instcombine-tbaa-3.7))
+$(eval $(call LLVM_PATCH,llvm-win64-reloc-dwarf))


this backported cleanly to 3.7.1? that's super awesome

except on osx where patch is pickier, PR incoming - let me know if the adjusted version works on 3.8 too

ViralBShah added the system:windows Affects only Windows label Apr 16, 2016

vtjnash mentioned this pull request Apr 20, 2016

Remove precompiled=no on windows. #15956

Closed

tkelman force-pushed the jn/win64dwarf branch from 594025f to a485acf Compare May 11, 2016 17:17

patch llvm win64 dwarf4 relocations

664f8b7

afaict, this is how other platforms treat this flag

tkelman force-pushed the jn/win64dwarf branch from a485acf to 664f8b7 Compare June 27, 2016 12:21

vtjnash merged commit cfc10f0 into master Jun 29, 2016

vtjnash deleted the jn/win64dwarf branch June 29, 2016 01:57

vtjnash reviewed Jun 29, 2016
View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

patch llvm win64 dwarf4 relocations #15859

patch llvm win64 dwarf4 relocations #15859

vtjnash commented Apr 13, 2016

tkelman commented Apr 13, 2016 •

edited

Loading

tkelman commented Apr 13, 2016

JeffBezanson commented Apr 15, 2016

vtjnash commented Apr 19, 2016

tkelman commented Apr 19, 2016

vtjnash commented Apr 19, 2016

tkelman commented May 11, 2016

vtjnash commented Jun 26, 2016

tkelman commented Jun 26, 2016

vtjnash commented Jun 26, 2016

tkelman commented Jun 26, 2016 •

edited

Loading

vtjnash commented Jun 27, 2016

vtjnash Jun 29, 2016

tkelman Jun 29, 2016

patch llvm win64 dwarf4 relocations #15859

patch llvm win64 dwarf4 relocations #15859

Conversation

vtjnash commented Apr 13, 2016

tkelman commented Apr 13, 2016 • edited Loading

tkelman commented Apr 13, 2016

JeffBezanson commented Apr 15, 2016

vtjnash commented Apr 19, 2016

tkelman commented Apr 19, 2016

vtjnash commented Apr 19, 2016

tkelman commented May 11, 2016

vtjnash commented Jun 26, 2016

tkelman commented Jun 26, 2016

vtjnash commented Jun 26, 2016

tkelman commented Jun 26, 2016 • edited Loading

vtjnash commented Jun 27, 2016

vtjnash Jun 29, 2016

Choose a reason for hiding this comment

tkelman Jun 29, 2016

Choose a reason for hiding this comment

tkelman commented Apr 13, 2016 •

edited

Loading

tkelman commented Jun 26, 2016 •

edited

Loading