[PATCH] D16045: [Coverage]: Fixing bug: -fcoverage-mapping does not work with gc-sections

[Justin Bogner via llvm-commits]

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.

> 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?

> 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.