This is the mail archive of the binutils@sourceware.org mailing list for the binutils 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]

[Ian Lance Taylor] PATCH RFA: Permit objcopy -add-section to work with /dev/null


Ping.  I guess I'm still a binutils global maintainer but these days I
would prefer that somebody else take a look before I make this sort of
change.  Thanks.

Ian

--- Begin Message ---
Sometimes it's handy to build an object using objcopy -I -O.  Those
objects naturally don't have a .note.GNU-stack section.  Therefore,
when they are linked into an executable, they cause to be executable
to be marked as requiring an executable stack.  One obvious way to
avoid that is to run
    objcopy --add-section .note.GNU-stack=/dev/null
Unfortunately, that fails in two ways.

First, it fails because objcopy silently fails when --add-section is
used with a zero length file:

	    size = get_file_size (s + 1);
	    if (size < 1)
	      {
		status = 1;
		break;
	      }

That just seems like an obvious bug.

Secondly, it fails because objcopy uses get_file_size to get the size
of the file being added into the section.  get_file_size fails on
/dev/null because /dev/null is not an ordinary file.

This patch fixes both problems, and adds a couple of test cases.  To
make the test cases easier to write, I introduce a simple variable
substitution in the options in run_dump_test tests: they can use
$srcdir, and it will be replaced with the source directory in which
they are being run.  I can do that in a different way if people don't
like it.

OK for mainline?

Ian


binutils/ChangeLog:

2010-01-15  Ian Lance Taylor  <iant@google.com>

	* objcopy.c (copy_main): Rewrite OPTION_ADD_SECTION code to work
	with non-ordinary files like /dev/null.

binutils/testsuite/ChangeLog:

2010-01-15  Ian Lance Taylor  <iant@google.com>

	* lib/utils-lib.exp (run_dump_test): Permit option values to use
	$srcdir to refer to the source directory.
	* binutils-all/add-section.d: New test.
	* binutils-all/add-empty-section.d: New test.
	* binutils-all/empty-file: New test input file.
	* binutils-all/objcopy.exp: Run new tests.


Index: objcopy.c
===================================================================
RCS file: /cvs/src/src/binutils/objcopy.c,v
retrieving revision 1.140
diff -u -p -r1.140 objcopy.c
--- objcopy.c	5 Jan 2010 00:41:54 -0000	1.140
+++ objcopy.c	15 Jan 2010 21:55:58 -0000
@@ -3306,10 +3306,8 @@ copy_main (int argc, char *argv[])
 	case OPTION_ADD_SECTION:
 	  {
 	    const char *s;
-	    off_t size;
+	    size_t off, alloc;
 	    struct section_add *pa;
-	    int len;
-	    char *name;
 	    FILE *f;
 
 	    s = strchr (optarg, '=');
@@ -3317,34 +3315,40 @@ copy_main (int argc, char *argv[])
 	    if (s == NULL)
 	      fatal (_("bad format for %s"), "--add-section");
 
-	    size = get_file_size (s + 1);
-	    if (size < 1)
-	      {
-		status = 1;
-		break;
-	      }
-
 	    pa = (struct section_add *) xmalloc (sizeof (struct section_add));
-
-	    len = s - optarg;
-	    name = (char *) xmalloc (len + 1);
-	    strncpy (name, optarg, len);
-	    name[len] = '\0';
-	    pa->name = name;
-
+	    pa->name = xstrndup (optarg, s - optarg);
 	    pa->filename = s + 1;
-	    pa->size = size;
-	    pa->contents = (bfd_byte *) xmalloc (size);
 
-	    f = fopen (pa->filename, FOPEN_RB);
+	    /* We don't use get_file_size so that we can do
+	         --add-section .note.GNU_stack=/dev/null
+	       get_file_size doesn't work on /dev/null.  */
 
+	    f = fopen (pa->filename, FOPEN_RB);
 	    if (f == NULL)
 	      fatal (_("cannot open: %s: %s"),
 		     pa->filename, strerror (errno));
 
-	    if (fread (pa->contents, 1, pa->size, f) == 0
-		|| ferror (f))
-	      fatal (_("%s: fread failed"), pa->filename);
+	    off = 0;
+	    alloc = 4096;
+	    pa->contents = (bfd_byte *) xmalloc (alloc);
+	    while (!feof (f))
+	      {
+		off_t got;
+
+		if (off == alloc)
+		  {
+		    alloc <<= 1;
+		    pa->contents = (bfd_byte *) xrealloc (pa->contents, alloc);
+		  }
+
+		got = fread (pa->contents + off, 1, alloc - off, f);
+		if (ferror (f))
+		  fatal (_("%s: fread failed"), pa->filename);
+
+		off += got;
+	      }
+
+	    pa->size = off;
 
 	    fclose (f);
 
Index: testsuite/binutils-all/add-empty-section.d
===================================================================
RCS file: testsuite/binutils-all/add-empty-section.d
diff -N testsuite/binutils-all/add-empty-section.d
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/binutils-all/add-empty-section.d	15 Jan 2010 21:55:58 -0000
@@ -0,0 +1,9 @@
+#PROG: objcopy
+#name: objcopy add-empty-section
+#source: empty.s
+#objcopy: --add-section NEW=$srcdir/empty-file
+#readelf: -S --wide
+
+#...
+  \[[ 0-9]+\] NEW[ \t]+PROGBITS[ \t]+[0-9a-f]+[ \t]+[0-9a-f]+[ \t]+0+[ \t]+[ \t0-9a-f]+
+#...
Index: testsuite/binutils-all/add-section.d
===================================================================
RCS file: testsuite/binutils-all/add-section.d
diff -N testsuite/binutils-all/add-section.d
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/binutils-all/add-section.d	15 Jan 2010 21:55:58 -0000
@@ -0,0 +1,11 @@
+#PROG: objcopy
+#name: objcopy add-section
+#source: empty.s
+#objcopy: --add-section NEW=$srcdir/empty.s
+#objdump: -s -j NEW
+
+.*: +file format .*
+
+Contents of section NEW:
+ 0000 2320416e 20656d70 74792066 696c652e  # An empty file.
+ 0010 0a                                   .               
Index: testsuite/binutils-all/empty-file
===================================================================
RCS file: testsuite/binutils-all/empty-file
diff -N testsuite/binutils-all/empty-file
Index: testsuite/binutils-all/objcopy.exp
===================================================================
RCS file: /cvs/src/src/binutils/testsuite/binutils-all/objcopy.exp,v
retrieving revision 1.62
diff -u -p -r1.62 objcopy.exp
--- testsuite/binutils-all/objcopy.exp	2 Sep 2009 07:22:32 -0000	1.62
+++ testsuite/binutils-all/objcopy.exp	15 Jan 2010 21:55:58 -0000
@@ -875,5 +875,8 @@ if [is_elf_format] {
     run_dump_test "localize-hidden-1"
     run_dump_test "testranges"
     run_dump_test "testranges-ia64"
+
+    run_dump_test "add-section"
+    run_dump_test "add-empty-section"
 }
 run_dump_test "localize-hidden-2"
Index: testsuite/lib/utils-lib.exp
===================================================================
RCS file: /cvs/src/src/binutils/testsuite/lib/utils-lib.exp,v
retrieving revision 1.17
diff -u -p -r1.17 utils-lib.exp
--- testsuite/lib/utils-lib.exp	6 Jan 2010 16:52:15 -0000	1.17
+++ testsuite/lib/utils-lib.exp	15 Jan 2010 21:55:58 -0000
@@ -327,6 +327,11 @@ proc run_dump_test { name {extra_options
 	    unresolved $subdir/$name
 	    return
 	}
+
+	# Permit the option to use $srcdir to refer to the source
+	# directory.
+	regsub -all "\\\$srcdir" "$opt_val" "$srcdir/$subdir" opt_val
+
 	if [string length $opts($opt_name)] {
 	    perror "option $opt_name multiply set in $file.d"
 	    unresolved $subdir/$name
@@ -343,6 +348,11 @@ proc run_dump_test { name {extra_options
 	    unresolved $subdir/$name
 	    return
 	}
+
+	# Permit the option to use $srcdir to refer to the source
+	# directory.
+	regsub -all "\\\$srcdir" "$opt_val" "$srcdir/$subdir" opt_val
+
 	# add extra option to end of existing option, adding space
 	# if necessary.
 	if [string length $opts($opt_name)] {

--- End Message ---

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