[PATCH] D82883: [LLD][COFF] Deduplicate .pdata entries

Thu Sep 10 15:42:12 PDT 2020

aganea added subscribers: ruiu, majnemer, rnk.
aganea added a comment.

I've digged a bit more. The issue is that the ICF improperly folds `.text$x` sections (catch funclets) but not always the "associated" `.pdata` and `.xdata` records, in part because those additional sections are attached to `.text$mn` not to `.text$x`. This occurs only with MSVC-cl built OBJs, Clang does not create separate `.text$x` sections for catch functlets (rather, they are merged into the function `.text`). I'm seeing the bug when linking with the FBX SDK 2016.1 VS2015 <https://www.autodesk.com/developer-network/platform-technologies/fbx-sdk-2016-1> (`lib\vs2015\x64\release\libfbxsdk-mt.lib`).

The COFF table for `fbxmesh.obj` in the archive above looks like this:

  11C 00000000 SECT88 notype       Static       | .text$mn
      Section length   52, #relocs    2, #linenums    0, checksum 270664C2, selection    2 (pick any)
  11E 00000000 SECT89 notype       Static       | .text$x            <<<< ----- this is de-duplicated, probably it shouldn't be ----- >>>>
      Section length   20, #relocs    2, #linenums    0, checksum 7C6EA17D, selection    5 (pick associative Section 0x88)
  ...
  817 00000000 SECT160 notype       Static      | .xdata
      Section length   10, #relocs    2, #linenums    0, checksum DFA6EA80, selection    5 (pick associative Section 0x88)
  819 00000000 SECT160 notype       Static      | $unwind$?_Buyheadnode@?$_Tree_comp_alloc at V?$_Tmap_traits at HHU?$less at H@std@@V?$allocator at U?$pair@$$CBHH at std@@@2@$00 at std@@@std@@QEAAPEAU?$_Tree_node at U?$pair@$$CBHH at std@@PEAX at 2@XZ
  81A 00000000 SECT161 notype       Static      | .pdata
      Section length    C, #relocs    3, #linenums    0, checksum 1703F4BB, selection    5 (pick associative Section 0x88)
  81C 00000000 SECT161 notype       Static      | $pdata$?_Buyheadnode@?$_Tree_comp_alloc at V?$_Tmap_traits at HHU?$less at H@std@@V?$allocator at U?$pair@$$CBHH at std@@@2@$00 at std@@@std@@QEAAPEAU?$_Tree_node at U?$pair@$$CBHH at std@@PEAX at 2@XZ
  81D 00000000 SECT162 notype       Static      | .rdata
      Section length   28, #relocs    3, #linenums    0, checksum  849A3B3, selection    5 (pick associative Section 0x88)
  81F 00000000 SECT162 notype       Static      | $cppxdata$?_Buyheadnode@?$_Tree_comp_alloc at V?$_Tmap_traits at HHU?$less at H@std@@V?$allocator at U?$pair@$$CBHH at std@@@2@$00 at std@@@std@@QEAAPEAU?$_Tree_node at U?$pair@$$CBHH at std@@PEAX at 2@XZ
  820 00000000 SECT163 notype       Static      | .xdata
      Section length   10, #relocs    0, #linenums    0, checksum 8999943C, selection    5 (pick associative Section 0x88)
  822 00000000 SECT163 notype       Static      | $stateUnwindMap$?_Buyheadnode@?$_Tree_comp_alloc at V?$_Tmap_traits at HHU?$less at H@std@@V?$allocator at U?$pair@$$CBHH at std@@@2@$00 at std@@@std@@QEAAPEAU?$_Tree_node at U?$pair@$$CBHH at std@@PEAX at 2@XZ
  823 00000000 SECT164 notype       Static      | .xdata
      Section length   14, #relocs    1, #linenums    0, checksum 570F4CF1, selection    5 (pick associative Section 0x88)
  825 00000000 SECT164 notype       Static      | $tryMap$?_Buyheadnode@?$_Tree_comp_alloc at V?$_Tmap_traits at HHU?$less at H@std@@V?$allocator at U?$pair@$$CBHH at std@@@2@$00 at std@@@std@@QEAAPEAU?$_Tree_node at U?$pair@$$CBHH at std@@PEAX at 2@XZ
  826 00000000 SECT165 notype       Static      | .xdata
      Section length   14, #relocs    1, #linenums    0, checksum   677058, selection    5 (pick associative Section 0x88)
  828 00000000 SECT165 notype       Static      | $handlerMap$0$?_Buyheadnode@?$_Tree_comp_alloc at V?$_Tmap_traits at HHU?$less at H@std@@V?$allocator at U?$pair@$$CBHH at std@@@2@$00 at std@@@std@@QEAAPEAU?$_Tree_node at U?$pair@$$CBHH at std@@PEAX at 2@XZ
  829 00000000 SECT166 notype       Static      | .xdata
      Section length   28, #relocs    5, #linenums    0, checksum F9326582, selection    5 (pick associative Section 0x88)
  82B 00000000 SECT166 notype       Static      | $ip2state$?_Buyheadnode@?$_Tree_comp_alloc at V?$_Tmap_traits at HHU?$less at H@std@@V?$allocator at U?$pair@$$CBHH at std@@@2@$00 at std@@@std@@QEAAPEAU?$_Tree_node at U?$pair@$$CBHH at std@@PEAX at 2@XZ
  82C 00000000 SECT167 notype       Static      | .xdata
      Section length   10, #relocs    2, #linenums    0, checksum 5840A276, selection    5 (pick associative Section 0x88)
  82E 00000000 SECT167 notype       Static      | $unwind$?catch$0@?0??_Buyheadnode@?$_Tree_comp_alloc at V?$_Tmap_traits at HHU?$less at H@std@@V?$allocator at U?$pair@$$CBHH at std@@@2@$00 at std@@@std@@QEAAPEAU?$_Tree_node at U?$pair@$$CBHH at std@@PEAX at 2@XZ at 4HA
  82F 00000000 SECT168 notype       Static      | .pdata            <<<< ----- but this is not de-duplicated ----- >>>>
      Section length    C, #relocs    3, #linenums    0, checksum F9766256, selection    5 (pick associative Section 0x88)
  831 00000000 SECT168 notype       Static      | $pdata$?catch$0@?0??_Buyheadnode@?$_Tree_comp_alloc at V?$_Tmap_traits at HHU?$less at H@std@@V?$allocator at U?$pair@$$CBHH at std@@@2@$00 at std@@@std@@QEAAPEAU?$_Tree_node at U?$pair@$$CBHH at std@@PEAX at 2@XZ at 4HA

Note how all associative COMDATs are attached to `.text$mn`, none to `.text$x`, this seems to be by design.

This gives this non-trivial graph: https://miro.com/app/board/o9J_klyfWoY=/
F12911723: image.png <https://reviews.llvm.org/F12911723>

In essence, we end up with several `.pdata` entries in the PE pointing to the same `.text$x` address.

I think there should be a rule in the ICF to prevent `.text$x` (SECT89) from being folded if its pointee .xdata and .pdata cannot be folded as well, like here. Although the ICF algorithm seems to work based only on forward dependencies.

I can't find a repro, something else could be involved (the counter for the equivalence classes?).
I've tried the following:

  // --- foo.h
  void bar(int);
  void bar(unsigned);
  template <typename T> struct A {
    template <typename X> void foo(X a) {
      try {
        bar(a);
      } catch (...) {
        throw;
      }
    }
  };
  // --- a.cpp
  #include "foo.h"
  void test(int a) {
    A<int> val;
    val.foo(a);
  }
  // --- b.cpp
  #include "foo.h"
  template <> struct A<unsigned> {
    void foo(unsigned a) {
      try {
        bar(a);
        bar(a);
      } catch (...) {
        throw;
      }
    }
  };
  void alttest(unsigned a) {
    A<unsigned> val;
    val.foo(a);
  }

But that doesn't trigger the bug. The issue seems to be occuring on the MS-STL `std::_Tree::_Insert_nohint` function, although from MS-STL VS2015.

There are many other libraries linked in, compiled with VS2019 or VS2017, so the issue could be from elsewhere.

I'm a bit stuck at the point. Would you go with the current patch, or something different in `lld/COFF/ICF.cpp`?
Ideas? @majnemer @rnk @ruiu

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82883/new/

https://reviews.llvm.org/D82883