[PATCH] D32688: [Coverage] Comdat section name should be same as the variable name in COFF format

Adam Folwarczny via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 2 03:50:01 PDT 2017


adamf added a comment.

> Could you update some tests in 'test/Instrumentation/InstrProfiling/' to reflect this change? PR23499.ll and linkage.ll are good candidates.

Thanks, I have updated this test. linkage.ll was broken because of bug in my fix (use after free in ternary operator).

> +1 to tests, it would help illustrate what's changing.

I have added some additional checks in PR23499.ll to emphasize my change.

To avoid XY Problem I will start from test case to reproduce my initial problem. Based on this example you can easily reproduce the problem.
In attachment is source code, compilation command and result error (shortly also below). F3275199: LinkingError.zip <https://reviews.llvm.org/F3275199>

Shortly how to reproduce this linker error:
I have 3 files

"Header.h"

  #pragma once
  inline int compute(int i){return i+1;}

"A.cpp"

  #include "Header.h"
  int main(){return compute(1);}

"B.cpp"

  #include "Header.h"
  int common(){return compute(2);}

Then I compiled it using command (on Windows):

> clang++.exe -fprofile-instr-generate -fcoverage-mapping  A.cpp B.cpp

Finally I received linker error message:

> B-5be2e4.o : error LNK2005: ___profd_?compute@@YAHH at Z already defined in A-a141c9.o

Now I can describe details about my solution:

In above example the coverage data for function "compute" has linkage type "linkeonce_odr" for variables with prefix _profd_  and _profc_ ("_profc_?compute@@YAHH at Z" and "_profc_?compute@@YAHH at Z").
It is because these data are computed from header function file and are duplicated in A.obj and B.obj so linker has to merge these symbols.
But I have observed if the COMDAT section name is different to the variable name then the linker still fails to merge these symbols.
In the above example the COMDAT section name is "_profc_?compute@@YAHH at Z" and it is equal to variable with prefix __profc_. So the linker complains only about variable with prefix _profd_ .
So my fix creates COMDAT section name equals to the variable name in case of triple: x86_64-pc-win32-coff

> Also, do you know why the change in r256220 ([PGO] Fix another comdat related issue for COFF) was not enough to fix linkage issues when using COFF?

COMDAT section generated by the function "getOrCreateProfileComdat" is added to 3 variables: CounterPtr, ValuesVar and Data. 
The COMDAT name was always the same (for each variable) based on return value from function: "getInstrProfCountersVarPrefix" (after r260117), after r256220 it was "getInstrProfNameVarPrefix".
Before my patch the linker properly merged symbols for variables with prefix __profc_ but not for __profd_. After patch r256220, but before r260117 I suppose the problem was with both (__profc_ and __profd_).


https://reviews.llvm.org/D32688





More information about the llvm-commits mailing list