[PATCH] D16045: [Coverage]: Fixing bug: -fcoverage-mapping does not work with gc-sections
Xinliang David Li via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 11 13:06:29 PST 2016
On Mon, Jan 11, 2016 at 11:24 AM, Justin Bogner <mail at justinbogner.com> wrote:
> David Li <davidxl at google.com> writes:
>> davidxl created this revision.
>> davidxl added reviewers: bogner, vsk.
>> davidxl added a subscriber: llvm-commits.
>> Herald added a subscriber: emaste.
>>
>> (The patch is intended for discussion purpose only)
>>
>> With linker gc turned on, the linker will happily discard all
>> __llvm_covmap sections from input objects so resulting executable file
>> won't be usable with llvm-cov tool -- missing coverage data.
>>
>> To demonstrate the problem, build a program with
>> -fprofile-instr-generate -fcoverage-mapping -Wl,--gc-sections. The
>> resulting exec won't be usable for coverage data dumping.
>
> This is only relevant for ELF, right? The darwin linker doesn't have a
> --gc-sections flag, and similar flags like -dead_strip don't seem to
> exhibit the problem you're discussing here.
>
Looks like so.
>> This patch shows one way to to fix the problem (demonstrated for
>> Linux/FreeBSD only here). The idea is to force referencing coverage
>> data module headers in profile dumper routines.
>>
>> There are a couple of downsides:
>> 1) it works for linux/freeBSD, and can be extended for
>> Darwin. Supporting on other platforms can be more complicated -- need
>> runtime registration of coverage data start/end
>> 2) increases runtime overhead (physical memory and dumping time)
>>
>> Another much better possible solution (verified manually on Linux) is
>> quite simple -- set __llvm_covmap section with the right attributes
>> such that they are not allocatable. When a section does not have the
>> 'a' bit in attribute, the linker won't garbage collect it (just like
>> debug sections).
>>
>> This method has another added benefit --- process virtual memory size
>> is reduced and the covmap section is now strippable. I think we
>> should go with this approach instead (at least for ELF based
>> systems). Thoughts?
>
> Yes, this sounds better. Is there any downside?
Not I am aware of. I will prepare patch that does that.
thanks,
David
>
>> http://reviews.llvm.org/D16045
>>
>> Files:
>> lib/profile/InstrProfiling.h
>> lib/profile/InstrProfilingFile.c
>> lib/profile/InstrProfilingPlatformLinux.c
>>
>>
>> Index: lib/profile/InstrProfilingPlatformLinux.c
>> ===================================================================
>> --- lib/profile/InstrProfilingPlatformLinux.c
>> +++ lib/profile/InstrProfilingPlatformLinux.c
>> @@ -19,6 +19,9 @@
>> #define PROF_CNTS_START INSTR_PROF_SECT_START(INSTR_PROF_CNTS_SECT_NAME)
>> #define PROF_CNTS_STOP INSTR_PROF_SECT_STOP(INSTR_PROF_CNTS_SECT_NAME)
>>
>> +#define COVMAP_START INSTR_PROF_SECT_START(INSTR_PROF_COVMAP_SECT_NAME)
>> +#define COVMAP_STOP INSTR_PROF_SECT_STOP(INSTR_PROF_COVMAP_SECT_NAME)
>> +
>> /* Declare section start and stop symbols for various sections
>> * generated by compiler instrumentation.
>> */
>> @@ -28,6 +31,8 @@
>> extern uint64_t PROF_CNTS_STOP COMPILER_RT_VISIBILITY;
>> extern char PROF_NAME_START COMPILER_RT_VISIBILITY;
>> extern char PROF_NAME_STOP COMPILER_RT_VISIBILITY;
>> +extern __llvm_covmap_header COVMAP_START COMPILER_RT_VISIBILITY;
>> +extern __llvm_covmap_header COVMAP_STOP COMPILER_RT_VISIBILITY;
>>
>> /* Add dummy data to ensure the section is always created. */
>> __llvm_profile_data
>> @@ -56,4 +61,15 @@
>> COMPILER_RT_VISIBILITY uint64_t *__llvm_profile_end_counters(void) {
>> return &PROF_CNTS_STOP;
>> }
>> +
>> +COMPILER_RT_VISIBILITY const __llvm_covmap_header *
>> +__llvm_profile_begin_covmap(void) {
>> + return &COVMAP_START;
>> +}
>> +
>> +COMPILER_RT_VISIBILITY const __llvm_covmap_header *
>> +__llvm_profile_end_covmap(void) {
>> + return &COVMAP_STOP;
>> +}
>> +
>> #endif
>> Index: lib/profile/InstrProfilingFile.c
>> ===================================================================
>> --- lib/profile/InstrProfilingFile.c
>> +++ lib/profile/InstrProfilingFile.c
>> @@ -205,6 +205,7 @@
>>
>> COMPILER_RT_VISIBILITY
>> int __llvm_profile_write_file(void) {
>> + __llvm_covmap_header *CovHeader;
>> int rc;
>>
>> GetEnvHook = &getenv;
>> @@ -227,7 +228,19 @@
>> rc = writeFileWithName(__llvm_profile_CurrentFilename);
>> if (rc)
>> PROF_ERR("LLVM Profile: Failed to write file \"%s\": %s\n",
>> - __llvm_profile_CurrentFilename, strerror(errno));
>> + __llvm_profile_CurrentFilename, strerror(errno));
>> +
>> + /* Force reference to coverage data. */
>> + for (CovHeader = __llvm_profile_begin_covmap();
>> + CovHeader < __llvm_profile_end_covmap();) {
>> + uint32_t CoverageSize = CovHeader->CoverageSize;
>> + uint32_t NR = CovHeader->NRecords;
>> + uint32_t FilenamesSize = CovHeader->FilenamesSize;
>> + CovHeader =
>> + (__llvm_covmap_header *)((char *)CovHeader +
>> + NR * sizeof(__llvm_covmap_func_record) +
>> + CoverageSize + FilenamesSize);
>> + }
>> return rc;
>> }
>>
>> Index: lib/profile/InstrProfiling.h
>> ===================================================================
>> --- lib/profile/InstrProfiling.h
>> +++ lib/profile/InstrProfiling.h
>> @@ -30,6 +30,16 @@
>> #include "InstrProfData.inc"
>> } __llvm_profile_header;
>>
>> +typedef struct __llvm_covmap_header {
>> +#define COVMAP_HEADER(Type, LLVMType, Name, Initializer) Type Name;
>> +#include "InstrProfData.inc"
>> +} __llvm_covmap_header;
>> +
>> +typedef struct __llvm_covmap_func_record {
>> +#define COVMAP_FUNC_RECORD(Type, LLVMType, Name, Initializer) Type Name;
>> +#include "InstrProfData.inc"
>> +} __llvm_covmap_func_record;
>> +
>> /*!
>> * \brief Get number of bytes necessary to pad the argument to eight
>> * byte boundary.
>> @@ -55,6 +65,8 @@
>> const char *__llvm_profile_end_names(void);
>> uint64_t *__llvm_profile_begin_counters(void);
>> uint64_t *__llvm_profile_end_counters(void);
>> +const __llvm_covmap_header *__llvm_profile_begin_covmap(void);
>> +const __llvm_covmap_header *__llvm_profile_end_covmap(void);
>>
>> /*!
>> * \brief Clear profile counters to zero.
More information about the llvm-commits
mailing list