Re: [gold][patch] Add execute permission to output file

[Cary Coutant <ccoutant at google dot com>]

> Gold normally unlinks an output file before creating it, so that the
> permissions are correctly set. If the output file exists but is empty,
> however, gold assumes that it was pre-created with tight permissions,
> and leaves it alone. Unfortunately, the ffmpeg configure script
> creates an empty but non-executable file, then expects the output file
> to be executable. This patch modifies gold to add execute permission
> to the file wherever read permission was already granted. (It still
> applies the umask to the final mode.)
>
> Since we already do a stat and check for a regular file in order to do
> the chmod, I changed it to call ::unlink directly instead of using
> libiberty's unlink_if_ordinary.
>
> OK?

Ugh. I left a debugging statement in there. Here's the correct patch.

-cary


	* output.cc (Output_file::open): Add execute permission to empty file.
	* testsuite/Makefile.am (permission_test): New test.
	* testsuite/Makefile.in: Regenerate.


Index: output.cc
===================================================================
RCS file: /cvs/src/src/gold/output.cc,v
retrieving revision 1.94
diff -u -p -r1.94 output.cc
--- output.cc	2 Sep 2009 16:39:06 -0000	1.94
+++ output.cc	4 Sep 2009 22:25:08 -0000
@@ -30,7 +30,7 @@
 #include <sys/mman.h>
 #include <sys/stat.h>
 #include <algorithm>
-#include "libiberty.h"   // for unlink_if_ordinary()
+#include "libiberty.h"   // for lbasename()

 #include "parameters.h"
 #include "object.h"
@@ -3461,8 +3461,22 @@ Output_file::open(off_t file_size)
       else
 	{
 	  struct stat s;
-	  if (::stat(this->name_, &s) == 0 && s.st_size != 0)
-	    unlink_if_ordinary(this->name_);
+	  if (::stat(this->name_, &s) == 0
+	      && (S_ISREG (s.st_mode) || S_ISLNK (s.st_mode)))
+	    {
+	      if (s.st_size != 0)
+		::unlink(this->name_);
+	      else if (!parameters->options().relocatable())
+		{
+		  // If we don't unlink the existing file, add execute
+		  // permission where read permissions already exist
+		  // and where the umask permits.
+		  int mask = ::umask(0);
+		  ::umask(mask);
+		  s.st_mode |= (s.st_mode & 0444) >> 2;
+		  ::chmod(this->name_, s.st_mode & ~mask);
+		}
+	    }

 	  int mode = parameters->options().relocatable() ? 0666 : 0777;
 	  int o = open_descriptor(-1, this->name_, O_RDWR | O_CREAT | O_TRUNC,
Index: testsuite/Makefile.am
===================================================================
RCS file: /cvs/src/src/gold/testsuite/Makefile.am,v
retrieving revision 1.100
diff -u -p -r1.100 Makefile.am
--- testsuite/Makefile.am	22 Aug 2009 19:02:57 -0000	1.100
+++ testsuite/Makefile.am	4 Sep 2009 22:25:08 -0000
@@ -1139,5 +1139,15 @@ hidden_test: hidden_test_main.o libhidde
 hidden_test.err: hidden_test
 	@touch hidden_test.err

+# Test that if the output file already exists and is empty,
+# it will get execute permission.
+check_PROGRAMS += permission_test
+permission_test: basic_test.o gcctestdir/ld
+	umask 022; \
+	rm -f $@; \
+	touch $@; \
+	chmod 600 $@; \
+	$(CXXLINK) -Bgcctestdir/ basic_test.o
+
 endif GCC
 endif NATIVE_LINKER