[PATCH] D38588: Clear LastMappingSymbols and LastEMS(Info) when resetting the ARM(AArch64)ELFStreamer

[Yichao Yu via Phabricator via llvm-commits]

yuyichao created this revision.
Herald added subscribers: kristof.beyls, javed.absar, rengolin, aemerson.

This causes a segfault on ARM when (I think) the pass manager is used multiple times.

Reset set the (last) current section to NULL without saving the corresponding LastEMSInfo back into the map. The next use of the streamer then save the LastEMSInfo for the NULL section leaving the LastEMSInfo mapping for the last current section (the one that was there before the reset) NULL which cause the LastEMSInfo to be set to NULL when the section is being used again.

The reuse of the section (pointer) might mean that the map was holding dangling pointers previously which is why I went for clearing the map and resetting the info, making it as similar to the state right after the constructor run as possible. The AArch64 one doesn't have segfault (since LastEMS isn't a pointer) but it seems to have the same issue.

Not really sure how this can be tested since I can only trigger this when reusing the pass in (julia) JIT.

The segfault is likely caused by https://reviews.llvm.org/D30724 which turns LastEMSInfo into a pointer. As mentioned above, it seems that the actual issue was older though.


https://reviews.llvm.org/D38588

Files:
  lib/Target/AArch64/MCTargetDesc/AArch64ELFStreamer.cpp
  lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp


Index: lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
===================================================================
--- lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
+++ lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
@@ -1169,6 +1169,8 @@
   ATS.reset();
   MappingSymbolCounter = 0;
   MCELFStreamer::reset();
+  LastMappingSymbols.clear();
+  LastEMSInfo.reset();
   // MCELFStreamer clear's the assembler's e_flags. However, for
   // arm we manually set the ABI version on streamer creation, so
   // do the same here
Index: lib/Target/AArch64/MCTargetDesc/AArch64ELFStreamer.cpp
===================================================================
--- lib/Target/AArch64/MCTargetDesc/AArch64ELFStreamer.cpp
+++ lib/Target/AArch64/MCTargetDesc/AArch64ELFStreamer.cpp
@@ -101,6 +101,14 @@
     MCELFStreamer::ChangeSection(Section, Subsection);
   }
 
+  // Reset state between object emissions
+  void reset() override {
+    MappingSymbolCounter = 0;
+    MCELFStreamer::reset();
+    LastMappingSymbols.clear();
+    LastEMS = EMS_None;
+  }
+
   /// This function is the one used to emit instruction data into the ELF
   /// streamer. We override it to add the appropriate mapping symbol if
   /// necessary.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D38588.117826.patch
Type: text/x-patch
Size: 1222 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171005/d5228f35/attachment.bin>