[PATCH] D46672: COFF: ICF a section and its associated pdata section as a unit.

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added a comment.

In https://reviews.llvm.org/D46672#1094887, @rnk wrote:

> Looks good to me, but let @ruiu review for more style feedback.
>
> It might be worth pointing out somewhere in comments that it's the **xdata** section that might differ, and the pdata section will refer to the xdata, which makes it appear different during ICF.
>
> We're convinced .pdata is special, right? What about other applications of comdat associative metadata, like ASan global metadata?


I'm not sure that I'm convinced. A simpler rule that I can think of based on the observed behaviour is that

1. an associative reference is like a relocation from the parent to the child
2. pdata is special in the same way as xdata and vtables are: it is a magic section name that allows ICF.

We can see this by taking my example from crbug.com/838449#c8 and renaming the pdata section:

  $ sed -e 's/pdata/qdata/g' 1.obj > 1q.obj
  
  C:\src\tmp\pdata-test>link /noentry /opt:icf 1q.obj  2.obj /dll /map:1q.map /include:?f1@@YAXXZ /include:?f2@@YAXXZ
  Microsoft (R) Incremental Linker Version 14.12.25835.0
  Copyright (C) Microsoft Corporation.  All rights reserved.
  
  
  C:\src\tmp\pdata-test>wsl grep f[12] 1q.map
   0001:00000000       ?f1@@YAXXZ                 0000000180001000 f   1q.obj
   0001:00000010       ?f2@@YAXXZ                 0000000180001010 f   1q.obj
   0002:0000007c       $unwind$?f2@@YAXXZ         000000018000207c     1q.obj
   0002:0000007c       $unwind$?f1@@YAXXZ         000000018000207c     1q.obj
   0003:00000000       $qdata$?f1@@YAXXZ          0000000180003000     1q.obj
   0003:0000000c       $qdata$?f2@@YAXXZ          000000018000300c     1q.obj



> ASan typically doesn't register metadata for readonly globals, but if it did register different metadata for two readonly globals with the same contents that got folded by ICF, would that be correct?

I think that if asan named its associative globals like vtables to get around the ICF rodata rule, or if we started embedding address-significant information in the symbol table to allow for safe ICF, we would probably would want the "relocation" behaviour.

> The other main application of associative comdats is debug info. If we ICF two functions together, we probably want to toss the debug info for the function we choose to discard, otherwise the debugger might get confused. This needs testing and experimentation, but it something to think about, since this is another instance of metadata using labels to refer to offsets in ICF'ed code.

Yes. In fact, debug info may be the special case here: a mismatching debug info shouldn't inhibit ICF.

I'll see how easy it is to implement the "simple" rule. If it doesn't work out, let's go with this patch.


https://reviews.llvm.org/D46672