From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21917 invoked by alias); 28 Feb 2013 20:12:50 -0000 Received: (qmail 21904 invoked by uid 22791); 28 Feb 2013 20:12:48 -0000 X-SWARE-Spam-Status: No, hits=-6.6 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_SPAMHAUS_DROP,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,RP_MATCHES_RCVD,SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 28 Feb 2013 20:12:39 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r1SKCdnb008933 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 28 Feb 2013 15:12:39 -0500 Received: from zalov.cz (vpn1-4-4.ams2.redhat.com [10.36.4.4]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r1SKCb9U001770 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 28 Feb 2013 15:12:38 -0500 Received: from zalov.cz (localhost [127.0.0.1]) by zalov.cz (8.14.5/8.14.5) with ESMTP id r1SKCaZs025014; Thu, 28 Feb 2013 21:12:36 +0100 Received: (from jakub@localhost) by zalov.cz (8.14.5/8.14.5/Submit) id r1SKCaEU025013; Thu, 28 Feb 2013 21:12:36 +0100 Date: Thu, 28 Feb 2013 20:12:00 -0000 From: Jakub Jelinek To: Tom Tromey Cc: gcc-patches@gcc.gnu.org Subject: [PATCH] Fix libcpp memory leak (PR middle-end/56461) Message-ID: <20130228201235.GY12913@tucnak.redhat.com> Reply-To: Jakub Jelinek MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2013-02/txt/msg01341.txt.bz2 Hi! On #include on Linux we leak memory in the preprocessor. The problem is that signal.h doesn't have standard multiple inclusion guards, the multiple inclusion guard is defined only conditionally and the whole header isn't protected by that guard, just big part of it. Furthermore, signal.h includes sys/context.h, which later includes signal.h again. That means we call read_file on the signal.h header while it is stacked (so file->buffer != NULL && !file->buffer_valid) and just overwrite file->buffer with a newly allocated memory. Later on when doing _cpp_pop_file_buffer on the inner signal.h we free the second buffer and clear file->buffer{,_start,_valid} fields, then later on when the outer signal.h (same _cpp_file structure) is being unstacked, those fields are already NULL and we don't free anything, thus we leak the memory. Fixed by making struct cpp_buffer be the owner of the buffer, responsible for freeing it, rather than struct _cpp_file. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2013-02-28 Jakub Jelinek PR middle-end/56461 * internal.h (struct cpp_buffer): Add to_free field. (_cpp_pop_file_buffer): Add third argument. * files.c (_cpp_stack_file): Set buffer->to_free. (_cpp_pop_file_buffer): Add to_free argument. Free to_free if non-NULL, and if equal to file->buffer_start, also clear file->buffer{,_start,_valid}. * directives.c (_cpp_pop_buffer): Pass buffer->to_free to _cpp_pop_file_buffer. --- libcpp/internal.h.jj 2013-02-27 14:05:39.000000000 +0100 +++ libcpp/internal.h 2013-02-28 17:09:41.263168009 +0100 @@ -301,6 +301,8 @@ struct cpp_buffer const unsigned char *buf; /* Entire character buffer. */ const unsigned char *rlimit; /* Writable byte at end of file. */ + const unsigned char *to_free; /* Pointer that should be freed when + popping the buffer. */ _cpp_line_note *notes; /* Array of notes. */ unsigned int cur_note; /* Next note to process. */ @@ -635,7 +637,8 @@ extern int _cpp_compare_file_date (cpp_r extern void _cpp_report_missing_guards (cpp_reader *); extern void _cpp_init_files (cpp_reader *); extern void _cpp_cleanup_files (cpp_reader *); -extern void _cpp_pop_file_buffer (cpp_reader *, struct _cpp_file *); +extern void _cpp_pop_file_buffer (cpp_reader *, struct _cpp_file *, + const unsigned char *); extern bool _cpp_save_file_entries (cpp_reader *pfile, FILE *f); extern bool _cpp_read_file_entries (cpp_reader *, FILE *); extern const char *_cpp_get_file_name (_cpp_file *); --- libcpp/files.c.jj 2013-02-28 10:57:50.000000000 +0100 +++ libcpp/files.c 2013-02-28 17:10:06.023024353 +0100 @@ -877,6 +877,7 @@ _cpp_stack_file (cpp_reader *pfile, _cpp && !CPP_OPTION (pfile, directives_only)); buffer->file = file; buffer->sysp = sysp; + buffer->to_free = file->buffer_start; /* Initialize controlling macro state. */ pfile->mi_valid = true; @@ -1418,7 +1419,8 @@ cpp_push_default_include (cpp_reader *pf /* Do appropriate cleanup when a file INC's buffer is popped off the input stack. */ void -_cpp_pop_file_buffer (cpp_reader *pfile, _cpp_file *file) +_cpp_pop_file_buffer (cpp_reader *pfile, _cpp_file *file, + const unsigned char *to_free) { /* Record the inclusion-preventing macro, which could be NULL meaning no controlling macro. */ @@ -1428,12 +1430,15 @@ _cpp_pop_file_buffer (cpp_reader *pfile, /* Invalidate control macros in the #including file. */ pfile->mi_valid = false; - if (file->buffer_start) + if (to_free) { - free ((void *) file->buffer_start); - file->buffer_start = NULL; - file->buffer = NULL; - file->buffer_valid = false; + if (to_free == file->buffer_start) + { + file->buffer_start = NULL; + file->buffer = NULL; + file->buffer_valid = false; + } + free ((void *) to_free); } } --- libcpp/directives.c.jj 2013-01-15 09:04:55.000000000 +0100 +++ libcpp/directives.c 2013-02-28 17:09:53.239097488 +0100 @@ -2558,6 +2558,7 @@ _cpp_pop_buffer (cpp_reader *pfile) cpp_buffer *buffer = pfile->buffer; struct _cpp_file *inc = buffer->file; struct if_stack *ifs; + const unsigned char *to_free; /* Walk back up the conditional stack till we reach its level at entry to this file, issuing error messages. */ @@ -2571,6 +2572,7 @@ _cpp_pop_buffer (cpp_reader *pfile) /* _cpp_do_file_change expects pfile->buffer to be the new one. */ pfile->buffer = buffer->prev; + to_free = buffer->to_free; free (buffer->notes); /* Free the buffer object now; we may want to push a new buffer @@ -2579,7 +2581,7 @@ _cpp_pop_buffer (cpp_reader *pfile) if (inc) { - _cpp_pop_file_buffer (pfile, inc); + _cpp_pop_file_buffer (pfile, inc, to_free); _cpp_do_file_change (pfile, LC_LEAVE, 0, 0, 0); } Jakub