From patchwork Sat Oct 25 10:20:17 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Anthony Brandon X-Patchwork-Id: 403027 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 457EA140077 for ; Sat, 25 Oct 2014 21:20:30 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; q=dns; s=default; b=YmV3Xt50qet5u0zKB/ LgvdDxYS2tn6MGyis2T5T6N7ccFp7UtVSZH1613XU/wbloSPhWzWH5oRKi+wabAh UW9U/5aznSVNK+WCEE0AL3bVAn/u12vAn5IpKI0hO3297TF3ny9Z1tf9D/AjGNOv /lG2GqkfsHv80SR/vdeHsH6c8= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; s=default; bh=37tbReVRmM38Sco0GS2s4LoO f/U=; b=hl2KeHLDRq+lwBGoKw7vs3NFL0isjHWgPqhesBjYW2iXQ+fKmwy6DbtT QZ4otVcr1MyX51R9F2knRU9BHoHFR4vFwq9m8fzI40yDi2hqL+r7r8Guzb/smxhG tgnmksdxsg2MxlkSeP42em/ApYJYTjBGofX3C54VoLJRg8l4dwc= Received: (qmail 28845 invoked by alias); 25 Oct 2014 10:20:23 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 28830 invoked by uid 89); 25 Oct 2014 10:20:22 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-Spam-User: qpsmtpd, 2 recipients X-HELO: mail-qc0-f170.google.com Received: from mail-qc0-f170.google.com (HELO mail-qc0-f170.google.com) (209.85.216.170) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Sat, 25 Oct 2014 10:20:20 +0000 Received: by mail-qc0-f170.google.com with SMTP id l6so1872599qcy.15 for ; Sat, 25 Oct 2014 03:20:17 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.140.27.164 with SMTP id 33mr13103714qgx.57.1414232417771; Sat, 25 Oct 2014 03:20:17 -0700 (PDT) Received: by 10.140.95.193 with HTTP; Sat, 25 Oct 2014 03:20:17 -0700 (PDT) In-Reply-To: References: Date: Sat, 25 Oct 2014 12:20:17 +0200 Message-ID: Subject: Re: [PATCH] PR36312 From: Anthony Brandon To: =?UTF-8?B?TWFudWVsIEzDs3Blei1JYsOhw7Fleg==?= Cc: "gcc-patches@gcc.gnu.org" , "manu@gcc.gnu.org" , DJ Delorie , janisjo@codesourcery.com, mikestump@codesourcery.com, ro@cebitec.uni-bielefeld.de, Ian Lance Taylor X-IsSubscribed: yes Hi, Sorry for the delay. Here are the updated diff and changelog. gcc/testsuite/ChangeLog: 2014-10-25 Anthony Brandon PR driver/36312 * gcc.misc-tests/output.exp: New test case for identical input and output files. include/ChangeLog: 2014-10-25 Anthony Brandon PR driver/36312 * filenames.h: Add prototype for canonical_filename_eq. gcc/ChangeLog: 2014-10-25 Anthony Brandon PR driver/36312 * diagnostic-core.h: Add prototype for fatal_error. * diagnostic.c (fatal_error): New function fatal_error. * gcc.c (store_arg): Remove have_o_argbuf_index. (process_command): Check if input and output files are the same. * toplev.c (init_asm_output): Check if input and output files are the same. libiberty/ChangeLog: 2014-10-25 Anthony Brandon PR driver/36312 * filename_cmp.c (canonical_filename_eq): New function to check if file names are the same. * functions.texi: Updated with documentation for new function. On Sun, Oct 19, 2014 at 11:17 AM, Manuel López-Ibáñez wrote: > On 18 October 2014 14:43, Anthony Brandon wrote: >> Never mind about functions.texi. I figured out how to do it. >> Here is the new diff and changelog. >> >> libiberty/ChangeLog: >> >> 2014-10-18 Anthony Brandon >> >> * filename_cmp.c (filename_eq): No change. > > Unfortunately mklog is not 100% perfect (actually, it is 'diff -p' > which is far from perfect). You need to revise that the ChageLog makes > sense (or make mklog smarter). Thus, drop (filename_eq)... > > Also, the changelogs should say PR driver/36312 > (https://gcc.gnu.org/codingconventions.html#ChangeLogs). > > I think you need to explain the difference between using fatal_error() > and fatal_error(UNKNOWN_LOCATION). Yes, I should know because I wrote > this part of the patch. But to be honest, I don't remember why I did > this change. > > +This function first normalizes the file names so that different file names > +pointing to the same underlying file are treated as being identical. > > I would suggest: "This function compares the canonical versions of the > filenames as returned by @code{lrealpath()}, so that ..." > > I cannot approve the patch, but it looks fine to me. If you don't get > a reply in a few days, you should ping the relevant maintainers: > https://gcc.gnu.org/wiki/Community#ping > > Great first contribution! What are your plans next? > > Cheers, > > Manuel. diff --git a/gcc/diagnostic-core.h b/gcc/diagnostic-core.h index a8245de..2fba279 100644 --- a/gcc/diagnostic-core.h +++ b/gcc/diagnostic-core.h @@ -68,6 +68,8 @@ extern void error_n (location_t, int, const char *, const char *, ...) extern void error_at (location_t, const char *, ...) ATTRIBUTE_GCC_DIAG(2,3); extern void fatal_error (const char *, ...) ATTRIBUTE_GCC_DIAG(1,2) ATTRIBUTE_NORETURN; +extern void fatal_error (location_t, const char *, ...) ATTRIBUTE_GCC_DIAG(2,3) + ATTRIBUTE_NORETURN; /* Pass one of the OPT_W* from options.h as the second parameter. */ extern bool pedwarn (location_t, int, const char *, ...) ATTRIBUTE_GCC_DIAG(3,4); diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c index 642cbe3..f7f8aaa 100644 --- a/gcc/diagnostic.c +++ b/gcc/diagnostic.c @@ -1163,6 +1163,23 @@ fatal_error (const char *gmsgid, ...) gcc_unreachable (); } +/* An error which is severe enough that we make no attempt to + continue. Do not use this for internal consistency checks; that's + internal_error. Use of this function should be rare. */ +void +fatal_error (location_t loc, const char *gmsgid, ...) +{ + diagnostic_info diagnostic; + va_list ap; + + va_start (ap, gmsgid); + diagnostic_set_info (&diagnostic, gmsgid, &ap, loc, DK_FATAL); + report_diagnostic (&diagnostic); + va_end (ap); + + gcc_unreachable (); +} + /* An internal consistency check has failed. We make no attempt to continue. Note that unless there is debugging value to be had from a more specific message, or some other good reason, you should use diff --git a/gcc/gcc.c b/gcc/gcc.c index e013d52..6f144de 100644 --- a/gcc/gcc.c +++ b/gcc/gcc.c @@ -1702,17 +1702,15 @@ typedef const char *const_char_p; /* For DEF_VEC_P. */ static vec argbuf; -/* Position in the argbuf vector containing the name of the output file - (the value associated with the "-o" flag). */ - -static int have_o_argbuf_index = 0; - /* Were the options -c, -S or -E passed. */ static int have_c = 0; /* Was the option -o passed. */ static int have_o = 0; +/* Pointer to output file name passed in with -o. */ +static const char *output_file = 0; + /* This is the list of suffixes and codes (%g/%u/%U/%j) and the associated temp file. If the HOST_BIT_BUCKET is used for %j, no entry is made for it here. */ @@ -1762,8 +1760,6 @@ store_arg (const char *arg, int delete_always, int delete_failure) { argbuf.safe_push (arg); - if (strcmp (arg, "-o") == 0) - have_o_argbuf_index = argbuf.length (); if (delete_always || delete_failure) { const char *p; @@ -3713,6 +3709,7 @@ driver_handle_option (struct gcc_options *opts, #if defined(HAVE_TARGET_EXECUTABLE_SUFFIX) || defined(HAVE_TARGET_OBJECT_SUFFIX) arg = convert_filename (arg, ! have_c, 0); #endif + output_file = arg; /* Save the output name in case -save-temps=obj was used. */ save_temps_prefix = xstrdup (arg); /* On some systems, ld cannot handle "-o" without a space. So @@ -4052,6 +4049,14 @@ process_command (unsigned int decoded_options_count, CL_DRIVER, &handlers, global_dc); } + if (output_file && strcmp (output_file, "-")) + { + int i; + for (i = 0; i < n_infiles; i++) + if (canonical_filename_eq (infiles[i].name, output_file)) + fatal_error ("output file %s is the same as input file", output_file); + } + /* If -save-temps=obj and -o name, create the prefix to use for %b. Otherwise just make -save-temps=obj the same as -save-temps=cwd. */ if (save_temps_flag == SAVE_TEMPS_OBJ && save_temps_prefix != NULL) diff --git a/gcc/testsuite/gcc.misc-tests/output.exp b/gcc/testsuite/gcc.misc-tests/output.exp new file mode 100644 index 0000000..aac7607 --- /dev/null +++ b/gcc/testsuite/gcc.misc-tests/output.exp @@ -0,0 +1,66 @@ +# Copyright (C) 2005-2014 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with GCC; see the file COPYING3. If not see +# . + +# Run GCC with the input file also specified as output file. Check that the +# compiler prints an error message and does not overwrite the input file. + +load_lib gcc-defs.exp +load_lib target-supports.exp + +# These tests don't run runtest_file_p consistently if it +# doesn't return the same values, so disable parallelization +# of this *.exp file. The first parallel runtest to reach +# this will run all the tests serially. +if ![gcc_parallel_test_run_p output] { + return +} + +# I'm not sure if this is needed here. It was in options.exp. +gcc_parallel_test_enable 0 + +proc check_gcc_overwrite_input {} { + set filename test-[pid] + set fd [open $filename.c w] + puts $fd "int main (void) \{ return 0; \}" + close $fd + remote_download host $filename.c + set test "input overwrite test" + set compiler cc1 + set gcc_output [gcc_target_compile $filename.c $filename.c executable ""] + + # Is this right, or do I need to use something like remote_upload? + set fd [open $filename.c r] + set file_data [read $fd] + close $fd + remote_file build delete $filename.c + + # check if the contents of the input file has changed + if {!($file_data eq "int main (void) \{ return 0; \}\n")} { + fail "$test (input overwritten)" + return + } + + # check if the error message was printed + if {![regexp -- "same as input" $gcc_output]} { + fail "$test (no error printed)" + return + } + pass $test +} + +check_gcc_overwrite_input + +gcc_parallel_test_enable 1 diff --git a/gcc/toplev.c b/gcc/toplev.c index adfae0b..5ad7a56 100644 --- a/gcc/toplev.c +++ b/gcc/toplev.c @@ -930,10 +930,17 @@ init_asm_output (const char *name) } if (!strcmp (asm_file_name, "-")) asm_out_file = stdout; - else + else if (!canonical_filename_eq (asm_file_name, name)) asm_out_file = fopen (asm_file_name, "w"); + else + /* Use fatal_error (UNKOWN_LOCATION) instead of just fatal_error to + prevent gcc from printing the first line in the current file. */ + fatal_error (UNKNOWN_LOCATION, + "output file %s is the same as input file", + asm_file_name); if (asm_out_file == 0) - fatal_error ("can%'t open %s for writing: %m", asm_file_name); + fatal_error (UNKNOWN_LOCATION, + "can%'t open %s for writing: %m", asm_file_name); } if (!flag_syntax_only) diff --git a/include/filenames.h b/include/filenames.h index e799a51..470c5e0 100644 --- a/include/filenames.h +++ b/include/filenames.h @@ -90,6 +90,8 @@ extern hashval_t filename_hash (const void *s); extern int filename_eq (const void *s1, const void *s2); +extern int canonical_filename_eq (const char *a, const char *b); + #ifdef __cplusplus } #endif diff --git a/libiberty/filename_cmp.c b/libiberty/filename_cmp.c index 9e16d24..9775cd2 100644 --- a/libiberty/filename_cmp.c +++ b/libiberty/filename_cmp.c @@ -24,8 +24,13 @@ #include #endif +#ifdef HAVE_STDLIB_H +#include +#endif + #include "filenames.h" #include "safe-ctype.h" +#include "libiberty.h" /* @@ -190,3 +195,27 @@ filename_eq (const void *s1, const void *s2) /* The casts are for -Wc++-compat. */ return filename_cmp ((const char *) s1, (const char *) s2) == 0; } + +/* + +@deftypefn Extension int canonical_filename_eq (const char *@var{a}, const char *@var{b}) + +Return non-zero if file names @var{a} and @var{b} are equivalent. +This function compares the canonical versions of the filenames as returned by +@code{lrealpath()}, so that so that different file names pointing to the same +underlying file are treated as being identical. + +@end deftypefn + +*/ + +int +canonical_filename_eq (const char * a, const char * b) +{ + char * ca = lrealpath(a); + char * cb = lrealpath(b); + int res = filename_eq (ca, cb); + free (ca); + free (cb); + return res; +} diff --git a/libiberty/functions.texi b/libiberty/functions.texi index 387aee0..2a759c4 100644 --- a/libiberty/functions.texi +++ b/libiberty/functions.texi @@ -125,6 +125,16 @@ Uses @code{malloc} to allocate storage for @var{nelem} objects of @end deftypefn +@c filename_cmp.c:201 +@deftypefn Extension int canonical_filename_eq (const char *@var{a}, const char *@var{b}) + +Return non-zero if file names @var{a} and @var{b} are equivalent. +This function compares the canonical versions of the filenames as returned by +@code{lrealpath()}, so that so that different file names pointing to the same +underlying file are treated as being identical. + +@end deftypefn + @c choose-temp.c:45 @deftypefn Extension char* choose_temp_base (void) @@ -286,7 +296,7 @@ value 1). If @var{valu} is zero, zero is returned. @end deftypefn -@c filename_cmp.c:32 +@c filename_cmp.c:37 @deftypefn Extension int filename_cmp (const char *@var{s1}, const char *@var{s2}) Return zero if the two file names @var{s1} and @var{s2} are equivalent. @@ -303,7 +313,7 @@ and backward slashes are equal. @end deftypefn -@c filename_cmp.c:178 +@c filename_cmp.c:183 @deftypefn Extension int filename_eq (const void *@var{s1}, const void *@var{s2}) Return non-zero if file names @var{s1} and @var{s2} are equivalent. @@ -311,7 +321,7 @@ This function is for use with hashtab.c hash tables. @end deftypefn -@c filename_cmp.c:147 +@c filename_cmp.c:152 @deftypefn Extension hashval_t filename_hash (const void *@var{s}) Return the hash value for file name @var{s} that will be compared @@ -320,7 +330,7 @@ This function is for use with hashtab.c hash tables. @end deftypefn -@c filename_cmp.c:89 +@c filename_cmp.c:94 @deftypefn Extension int filename_ncmp (const char *@var{s1}, const char *@var{s2}, size_t @var{n}) Return zero if the two file names @var{s1} and @var{s2} are equivalent