This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Make global file function names link unique



On 18/12/2006, at 1:26 PM, John David Anglin wrote:


If you can't guarantee that any name is unique to this file, you need
to fall down to the last case in this function, where it already does
all kinds of crc32 operations.  If you're going to do that, or do
something like what you have in the patch below, please only do it on
targets that actually need it.

The enclosed version only adds the crc32 for 'I' and 'D' when the
target doesn't handle constructors/destructors (i.e., it uses collect2).
I believe that adding the crc32 is necessary in all cases when 'F'
and 'N' are used.


Adding randomness based on flag_random_seed causes the assembly
output to change from one compilation to another.  This creates
several new testsuite regressions that I would prefer to avoid.

I agree that you should avoid testsuite regressions, but not at the cost of introducing actual bugs! It would be better to just fix the testsuite, for example by having it pass -frandom-seed.


Also note that the last case uses different names for the crc32
string.

Aside from changes to comments and the 'if', the patch is identical
to the one approved here:
http://gcc.gnu.org/ml/gcc-patches/2006-01/msg00804.html
and subsequently removed here:
http://gcc.gnu.org/ml/gcc-patches/2006-11/msg00003.html

Tested on hppa2.0-hp-hpux11.11 and hppa-unknown-linux-gnu.

OK?

The property of first_global_object_name is that first_global_object_name is set to this value only for this compilation in the whole link. If this is not happening on HP-UX, it would be better to fix that rather than work around it. For example, in the first patch you quoted, this happens "because ... overrides new", so perhaps an override of new should not be placed in first_global_object_name (although I do not see why this causes a problem; there can be only one override of new in a whole program). This is the only purpose of first_global_object_name, so if this can not be made to work then first_global_object_name is useless and should be removed.


I think you need to add a comment explaining what's really going on in detail. Even after your reading the URLs you quoted, reading the original PR, and reading your mail about this patch, I do not understand why there is a problem and am very suspicious that you are merely working around a problem that is really somewhere else.

Certainly if you do not add a comment you can expect that at some point in the future this patch will be removed just like last time, because no-one will understand why it is necessary.

Dave
--
J. David Anglin dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada (613) 990-0752 (FAX: 952-6602)


2006-12-18 John David Anglin <dave.anglin@nrc-cnrc.gc.ca>

	PR middle-end/30151
	PR middle-end/30174
	* tree.c (get_file_function_name): Make global names link unique.

Index: tree.c
===================================================================
--- tree.c	(revision 119963)
+++ tree.c	(working copy)
@@ -6283,7 +6283,7 @@
       *p = '_';
 }

-/* Generate a name for a special-purpose function function.
+/* Generate a name for a special-purpose function.
The generated name may need to be unique across the whole link.
TYPE is some string to identify the purpose of this function to the
linker or collect2; it must start with an uppercase letter,
@@ -6300,9 +6300,29 @@
const char *p;
char *q;


- /* If we already have a name we know to be unique, just use that. */
+ /* If we already have a name we know to be unique to this translation
+ unit, just use it. */
if (first_global_object_name)
- p = first_global_object_name;
+ {
+ p = first_global_object_name;
+
+ /* The generated name needs to be unique across the whole link for
+ types 'I' and 'D' when constructors/destructors are not handled
+ by the target, and for types 'F' and 'N'. */
+ if ((type[0] != 'I' && type[0] != 'D') || ! targetm.have_ctors_dtors)
+ {
+ const char *file = main_input_filename;
+
+ if (!file)
+ file = input_filename;
+
+ /* Use the CRC32 string for file, because the full pathname
+ might be quite long. */
+ q = alloca (strlen (p) + 10);
+ sprintf (q, "%s_%08X", p, crc32_string (0, file));
+ p = q;
+ }
+ }
/* If the target is handling the constructors/destructors, they
will be local to this file and the name is only necessary for
debugging purposes. */

Attachment: smime.p7s
Description: S/MIME cryptographic signature


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]