[PATCH] D49700: [LLD] [COFF] Treat .xdata/.pdata$<sym> as implicitly associative to <sym> for MinGW

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added a comment.

In https://reviews.llvm.org/D49700#1177295, @rnk wrote:

> In https://reviews.llvm.org/D49700#1177256, @rnk wrote:
>
> > In https://reviews.llvm.org/D49700#1174174, @mstorsjo wrote:
> >
> > > - clang + ld.bfd (@rnk fixed an issue for this case in llvm, which triggered all of this)
> >
> >
> > As long as this works, https://reviews.llvm.org/D49757 is fine with me. I'm going to check.
>
>
> Unfortunately, it doesn't work. ld.bfd will not discard the duplicate associated .pdata and .xdata sections. Here was my program:
>
>   $ cat t.h
>   void bar(int*);
>   inline int foo() {
>     volatile int x = 42;
>     ++x;
>     try {
>     bar((int*)&x);
>     } catch(...) {}
>     return x;
>   }
>  
>   $ cat a.cpp
>   #include "t.h"
>   void baz();
>   void bar(int *) {}
>   int main() {
>     foo();
>     baz();
>   }
>  
>   $ cat b.cpp
>   #include "t.h"
>   void baz() {
>     foo();
>   }
>  
>   $ clang -c a.cpp b.cpp --target=x86_64-windows-gnu && g++ a.o b.o -o t.exe
>  
>   $ llvm-objdump -u -d t.exe | less
>
>
> There's only one _Z3foov copy in .text, so that's good:
>
>   ...
>   _Z3foov:
>     402c60:       48 83 ec 48     subq    $72, %rsp
>     402c64:       c7 44 24 44 2a 00 00 00         movl    $42, 68(%rsp)
>     402c6c:       8b 44 24 44     movl    68(%rsp), %eax
>     402c70:       83 c0 01        addl    $1, %eax
>     402c73:       89 44 24 44     movl    %eax, 68(%rsp)
>     402c77:       48 8d 4c 24 44  leaq    68(%rsp), %rcx
>   ...
>
>
> But there are two pdata / xdata sets for it in the unwind info:


Oh, indeed - sorry for not testing it thoroughly enough myself to realize this, and kudos to you for being thorough.

I just went with your initial testcase from https://github.com/Alexpux/MINGW-packages/issues/1677 and tested that it didn't break...

It's surprising that this didn't work, but I'm not interested enough in the specifics to dive in the binutils source to figure out why it doesn't.

I guess that some form of this patch is what we need to go with then?


https://reviews.llvm.org/D49700