[compiler-rt] r184799 - Address a few of the issues in GCDAProfiling I noted when looking

[Bill Wendling]

Caching the 'filename' won't work because it gets 'freed' after its use in llvm_gcda_start_file.

-bw

On Jun 24, 2013, at 5:37 PM, Chandler Carruth <chandlerc at gmail.com> wrote:

> Author: chandlerc
> Date: Mon Jun 24 19:37:32 2013
> New Revision: 184799
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=184799&view=rev
> Log:
> Address a few of the issues in GCDAProfiling I noted when looking
> through Bill's patch:
> 
> 1) Correctly test the file descriptor after the sceond attempt at
>   creating the file.
> 2) Make the filename a global so that we can issue error messages from
>   other routines.
> 3) Check errno in several places and print it out so that errors are
>   easier to track down.
> 
> I don't really expect any of these to fix the current failures I'm
> seeing, but I'm hopeful they'll at least let me debug them.
> 
> Modified:
>    compiler-rt/trunk/lib/profile/GCDAProfiling.c
> 
> Modified: compiler-rt/trunk/lib/profile/GCDAProfiling.c
> URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/profile/GCDAProfiling.c?rev=184799&r1=184798&r2=184799&view=diff
> ==============================================================================
> --- compiler-rt/trunk/lib/profile/GCDAProfiling.c (original)
> +++ compiler-rt/trunk/lib/profile/GCDAProfiling.c Mon Jun 24 19:37:32 2013
> @@ -20,6 +20,7 @@
> |*
> \*===----------------------------------------------------------------------===*/
> 
> +#include <errno.h>
> #include <fcntl.h>
> #include <stdio.h>
> #include <stdlib.h>
> @@ -45,6 +46,11 @@ typedef unsigned int uint64_t;
>  */
> 
> /*
> + * The current file name we're outputting. Used primarily for error logging.
> + */
> +static char *filename = NULL;
> +
> +/*
>  * The current file we're outputting.
>  */ 
> static FILE *output_file = NULL;
> @@ -196,17 +202,32 @@ static void recursive_mkdir(char *filena
>   }
> }
> 
> -static void map_file() {
> +static int map_file() {
>   fseek(output_file, 0L, SEEK_END);
>   file_size = ftell(output_file);
> 
>   write_buffer = mmap(0, file_size, PROT_READ | PROT_WRITE,
>                       MAP_FILE | MAP_SHARED, fd, 0);
> +  if (write_buffer == (void *)-1) {
> +    int errnum = errno;
> +    fprintf(stderr, "profiling: %s: cannot map: %s\n", filename,
> +            strerror(errnum));
> +    return -1;
> +  }
> +  return 0;
> }
> 
> static void unmap_file() {
> -  msync(write_buffer, file_size, MS_SYNC);
> -  munmap(write_buffer, file_size);
> +  if (msync(write_buffer, file_size, MS_SYNC) == -1) {
> +    int errnum = errno;
> +    fprintf(stderr, "profiling: %s: cannot msync: %s\n", filename,
> +            strerror(errnum));
> +  }
> +
> +  /* We explicitly ignore errors from unmapping because at this point the data
> +   * is written and we don't care.
> +   */
> +  (void)munmap(write_buffer, file_size);
>   write_buffer = NULL;
>   file_size = 0;
> }
> @@ -220,8 +241,8 @@ static void unmap_file() {
>  * started at a time.
>  */
> void llvm_gcda_start_file(const char *orig_filename, const char version[4]) {
> -  char *filename = mangle_filename(orig_filename);
>   const char *mode = "r+b";
> +  filename = mangle_filename(orig_filename);
> 
>   /* Try just opening the file. */
>   new_file = 0;
> @@ -236,9 +257,11 @@ void llvm_gcda_start_file(const char *or
>       /* Try creating the directories first then opening the file. */
>       recursive_mkdir(filename);
>       fd = open(filename, O_RDWR | O_CREAT, 0644);
> -      if (!output_file) {
> +      if (fd == -1) {
>         /* Bah! It's hopeless. */
> -        fprintf(stderr, "profiling:%s: cannot open\n", filename);
> +        int errnum = errno;
> +        fprintf(stderr, "profiling: %s: cannot open: %s\n", filename,
> +                strerror(errnum));
>         free(filename);
>         return;
>       }
> @@ -256,7 +279,14 @@ void llvm_gcda_start_file(const char *or
>     resize_write_buffer(WRITE_BUFFER_SIZE);
>     memset(write_buffer, 0, WRITE_BUFFER_SIZE);
>   } else {
> -    map_file();
> +    if (map_file() == -1) {
> +      /* mmap failed, try to recover by clobbering */
> +      new_file = 1;
> +      write_buffer = NULL;
> +      cur_buffer_size = 0;
> +      resize_write_buffer(WRITE_BUFFER_SIZE);
> +      memset(write_buffer, 0, WRITE_BUFFER_SIZE);
> +    }
>   }
> 
>   /* gcda file, version, stamp LLVM. */
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits