[llvm] 82f8aef - [JITLink][MachO] Handle muliple symbols at same offset when splitting C-strings.

[Lang Hames via llvm-commits]

Author: Lang Hames
Date: 2021-06-09T19:16:49+10:00
New Revision: 82f8aef3deb196ad323a3ef57c03276c6e93a246

URL: https://github.com/llvm/llvm-project/commit/82f8aef3deb196ad323a3ef57c03276c6e93a246
DIFF: https://github.com/llvm/llvm-project/commit/82f8aef3deb196ad323a3ef57c03276c6e93a246.diff

LOG: [JITLink][MachO] Handle muliple symbols at same offset when splitting C-strings.

The C-string section splitting support added in f9649d123db triggered an assert
("Duplicate canonical symbol at address") when multiple symbols were defined at
the the same offset within a C-string block (this triggered on arm64, where we
always add a block start symbol). The bug was caused by a failure to update the
record of the last canonical symbol address. The fix was to maintain this record
correctly, and move the auto-generation of the block-start symbol above the
handling for symbols defined in the object itself so that all symbols
(auto-generated and defined) are processed in address order.

Added: 
    

Modified: 
    llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.cpp b/llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.cpp
index 904f38aac085..9ba523232d50 100644
--- a/llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.cpp
+++ b/llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.cpp
@@ -655,9 +655,20 @@ Error MachOLinkGraphBuilder::graphifyCStringSection(
                << " for \"" << StringRef(B.getContent().data()) << "\"\n";
       });
 
-      // Process any symbols that point into this block.
-      JITTargetAddress LastCanonicalAddr = BlockEnd;
-      Symbol *BlockStartSymbol = nullptr;
+      // If there's no symbol at the start of this block then create one.
+      if (NSyms.empty() || NSyms.back()->Value != B.getAddress()) {
+        auto &S = G->addAnonymousSymbol(B, 0, BlockSize, false, false);
+        setCanonicalSymbol(S);
+        LLVM_DEBUG({
+          dbgs() << "      Adding anonymous symbol for c-string block "
+                 << formatv("{0:x16} -- {1:x16}", S.getAddress(),
+                            S.getAddress() + BlockSize)
+                 << "\n";
+        });
+      }
+
+      // Process any remaining symbols that point into this block.
+      JITTargetAddress LastCanonicalAddr = B.getAddress() + BlockEnd;
       while (!NSyms.empty() &&
              NSyms.back()->Value < (B.getAddress() + BlockSize)) {
         auto &NSym = *NSyms.back();
@@ -665,27 +676,18 @@ Error MachOLinkGraphBuilder::graphifyCStringSection(
         bool SymLive =
             (NSym.Desc & MachO::N_NO_DEAD_STRIP) || SectionIsNoDeadStrip;
 
-        auto &S =
-            createStandardGraphSymbol(NSym, B, SymSize, SectionIsText, SymLive,
-                                      NSym.Value != LastCanonicalAddr);
+        bool IsCanonical = false;
+        if (LastCanonicalAddr != NSym.Value) {
+          IsCanonical = true;
+          LastCanonicalAddr = NSym.Value;
+        }
 
-        if (S.getAddress() == B.getAddress() && !BlockStartSymbol)
-          BlockStartSymbol = &S;
+        createStandardGraphSymbol(NSym, B, SymSize, SectionIsText, SymLive,
+                                  IsCanonical);
 
         NSyms.pop_back();
       }
 
-      if (!BlockStartSymbol) {
-        auto &S = G->addAnonymousSymbol(B, 0, BlockSize, false, false);
-        setCanonicalSymbol(S);
-        LLVM_DEBUG({
-          dbgs() << "      Adding anonymous symbol for "
-                 << formatv("{0:x16} -- {1:x16}", S.getAddress(),
-                            S.getAddress() + BlockSize)
-                 << "\n";
-        });
-      }
-
       BlockStart += BlockSize;
     }