diff mbox series

[RFA] Incremental LTO linking part 1: simple-object bits

Message ID 20180508150531.GA16916@kam.mff.cuni.cz
State New
Headers show
Series [RFA] Incremental LTO linking part 1: simple-object bits | expand

Commit Message

Jan Hubicka May 8, 2018, 3:05 p.m. UTC
Hi,
for incremental linking of LTO objects we need to copy debug sections from
source object files into destination without renaming them from .gnu.debuglto
into the standard debug section (because they will again be LTO debug section
in the resulting object file).

I have discussed this with Richard on IRC and I hope it is fine to change the
API here becuase lto-wrapper is the only user of this function.  I will send
lto-wrapper support in separate patch.

I have lto-bootstrapped/regtested the whole incremental linking patchet on
x86-64-linux with libbackend being incrementaly linked and also experimented
with extra testcases and tested that debugging works on resulting cc1 binary.
OK?

Honza

	* simple-object.h (simple_object_copy_lto_debug_sections): Add rename
	parameter.
	* simple-object.c (handle_lto_debug_sections): Add rename parameter.
	(handle_lto_debug_sections_rename): New function.
	(handle_lto_debug_sections_norename): New function.
	(simple_object_copy_lto_debug_sections): Add rename parameter.

Comments

Richard Biener May 9, 2018, 10 a.m. UTC | #1
On Tue, 8 May 2018, Jan Hubicka wrote:

> Hi,
> for incremental linking of LTO objects we need to copy debug sections from
> source object files into destination without renaming them from .gnu.debuglto
> into the standard debug section (because they will again be LTO debug section
> in the resulting object file).
> 
> I have discussed this with Richard on IRC and I hope it is fine to change the
> API here becuase lto-wrapper is the only user of this function.  I will send
> lto-wrapper support in separate patch.
> 
> I have lto-bootstrapped/regtested the whole incremental linking patchet on
> x86-64-linux with libbackend being incrementaly linked and also experimented
> with extra testcases and tested that debugging works on resulting cc1 binary.
> OK?

Works for me.

Richard.

> Honza
> 
> 	* simple-object.h (simple_object_copy_lto_debug_sections): Add rename
> 	parameter.
> 	* simple-object.c (handle_lto_debug_sections): Add rename parameter.
> 	(handle_lto_debug_sections_rename): New function.
> 	(handle_lto_debug_sections_norename): New function.
> 	(simple_object_copy_lto_debug_sections): Add rename parameter.
> Index: include/simple-object.h
> ===================================================================
> --- include/simple-object.h	(revision 260042)
> +++ include/simple-object.h	(working copy)
> @@ -198,12 +198,15 @@
>  simple_object_release_write (simple_object_write *);
>  
>  /* Copy LTO debug sections from SRC_OBJECT to DEST.
> +   If RENAME is true, rename LTO debug section into debug section (i.e.
> +   when producing final binary) and if it is false, keep the sections with
> +   original names (when incrementally linking).
>     If an error occurs, return the errno value in ERR and an error string.  */
>  
>  extern const char *
>  simple_object_copy_lto_debug_sections (simple_object_read *src_object,
>  				       const char *dest,
> -				       int *err);
> +				       int *err, int rename);
>  
>  #ifdef __cplusplus
>  }
> Index: libiberty/simple-object.c
> ===================================================================
> --- libiberty/simple-object.c	(revision 260042)
> +++ libiberty/simple-object.c	(working copy)
> @@ -251,12 +251,15 @@
>  }
>  
>  /* Callback to identify and rename LTO debug sections by name.
> -   Returns 1 if NAME is a LTO debug section, 0 if not.  */
> +   Returns non-NULL if NAME is a LTO debug section, NULL if not.
> +   If RENAME is true it will rename LTO debug sections to non-LTO
> +   ones.  */
>  
>  static char *
> -handle_lto_debug_sections (const char *name)
> +handle_lto_debug_sections (const char *name, int rename)
>  {
> -  char *newname = XCNEWVEC (char, strlen (name) + 1);
> +  char *newname = rename ? XCNEWVEC (char, strlen (name) + 1)
> +	  	         : xstrdup (name);
>  
>    /* ???  So we can't use .gnu.lto_ prefixed sections as the assembler
>       complains about bogus section flags.  Which means we need to arrange
> @@ -265,12 +268,14 @@
>    /* Also include corresponding reloc sections.  */
>    if (strncmp (name, ".rela", sizeof (".rela") - 1) == 0)
>      {
> -      strncpy (newname, name, sizeof (".rela") - 1);
> +      if (rename)
> +        strncpy (newname, name, sizeof (".rela") - 1);
>        name += sizeof (".rela") - 1;
>      }
>    else if (strncmp (name, ".rel", sizeof (".rel") - 1) == 0)
>      {
> -      strncpy (newname, name, sizeof (".rel") - 1);
> +      if (rename)
> +        strncpy (newname, name, sizeof (".rel") - 1);
>        name += sizeof (".rel") - 1;
>      }
>    /* ???  For now this handles both .gnu.lto_ and .gnu.debuglto_ prefixed
> @@ -277,10 +282,10 @@
>       sections.  */
>    /* Copy LTO debug sections and rename them to their non-LTO name.  */
>    if (strncmp (name, ".gnu.debuglto_", sizeof (".gnu.debuglto_") - 1) == 0)
> -    return strcat (newname, name + sizeof (".gnu.debuglto_") - 1);
> +    return rename ? strcat (newname, name + sizeof (".gnu.debuglto_") - 1) : newname;
>    else if (strncmp (name, ".gnu.lto_.debug_",
>  		    sizeof (".gnu.lto_.debug_") -1) == 0)
> -    return strcat (newname, name + sizeof (".gnu.lto_") - 1);
> +    return rename ? strcat (newname, name + sizeof (".gnu.lto_") - 1) : newname;
>    /* Copy over .note.GNU-stack section under the same name if present.  */
>    else if (strcmp (name, ".note.GNU-stack") == 0)
>      return strcpy (newname, name);
> @@ -289,14 +294,31 @@
>       COMDAT sections in objects produced by GCC.  */
>    else if (strcmp (name, ".comment") == 0)
>      return strcpy (newname, name);
> +  free (newname);
>    return NULL;
>  }
>  
> +/* Wrapper for handle_lto_debug_sections.  */
> +
> +static char *
> +handle_lto_debug_sections_rename (const char *name)
> +{
> +  return handle_lto_debug_sections (name, 1);
> +}
> +
> +/* Wrapper for handle_lto_debug_sections.  */
> +
> +static char *
> +handle_lto_debug_sections_norename (const char *name)
> +{
> +  return handle_lto_debug_sections (name, 0);
> +}
> +
>  /* Copy LTO debug sections.  */
>  
>  const char *
>  simple_object_copy_lto_debug_sections (simple_object_read *sobj,
> -				       const char *dest, int *err)
> +				       const char *dest, int *err, int rename)
>  {
>    const char *errmsg;
>    simple_object_write *dest_sobj;
> @@ -317,9 +339,10 @@
>    if (! dest_sobj)
>      return errmsg;
>  
> -  errmsg = sobj->functions->copy_lto_debug_sections (sobj, dest_sobj,
> -						     handle_lto_debug_sections,
> -						     err);
> +  errmsg = sobj->functions->copy_lto_debug_sections
> +	 	 (sobj, dest_sobj,
> +		  rename ? handle_lto_debug_sections_rename
> +			 : handle_lto_debug_sections_norename,  err);
>    if (errmsg)
>      {
>        simple_object_release_write (dest_sobj);
> 
>
H.J. Lu May 30, 2018, 5:02 p.m. UTC | #2
On Tue, May 8, 2018 at 8:05 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
> Hi,
> for incremental linking of LTO objects we need to copy debug sections from
> source object files into destination without renaming them from .gnu.debuglto
> into the standard debug section (because they will again be LTO debug section
> in the resulting object file).
>
> I have discussed this with Richard on IRC and I hope it is fine to change the
> API here becuase lto-wrapper is the only user of this function.  I will send
> lto-wrapper support in separate patch.
>
> I have lto-bootstrapped/regtested the whole incremental linking patchet on
> x86-64-linux with libbackend being incrementaly linked and also experimented
> with extra testcases and tested that debugging works on resulting cc1 binary.
> OK?
>
> Honza
>
>         * simple-object.h (simple_object_copy_lto_debug_sections): Add rename
>         parameter.
>         * simple-object.c (handle_lto_debug_sections): Add rename parameter.
>         (handle_lto_debug_sections_rename): New function.
>         (handle_lto_debug_sections_norename): New function.
>         (simple_object_copy_lto_debug_sections): Add rename parameter.
> Index: include/simple-object.h
> ===================================================================
> --- include/simple-object.h     (revision 260042)
> +++ include/simple-object.h     (working copy)
> @@ -198,12 +198,15 @@
>  simple_object_release_write (simple_object_write *);
>
>  /* Copy LTO debug sections from SRC_OBJECT to DEST.
> +   If RENAME is true, rename LTO debug section into debug section (i.e.
> +   when producing final binary) and if it is false, keep the sections with
> +   original names (when incrementally linking).
>     If an error occurs, return the errno value in ERR and an error string.  */
>
>  extern const char *
>  simple_object_copy_lto_debug_sections (simple_object_read *src_object,
>                                        const char *dest,
> -                                      int *err);
> +                                      int *err, int rename);
>
>  #ifdef __cplusplus
>  }

This part is missing from r260956:

https://gcc.gnu.org/ml/gcc-cvs/2018-05/msg01177.html
Jan Hubicka May 30, 2018, 5:11 p.m. UTC | #3
> On Tue, May 8, 2018 at 8:05 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
> > Hi,
> > for incremental linking of LTO objects we need to copy debug sections from
> > source object files into destination without renaming them from .gnu.debuglto
> > into the standard debug section (because they will again be LTO debug section
> > in the resulting object file).
> >
> > I have discussed this with Richard on IRC and I hope it is fine to change the
> > API here becuase lto-wrapper is the only user of this function.  I will send
> > lto-wrapper support in separate patch.
> >
> > I have lto-bootstrapped/regtested the whole incremental linking patchet on
> > x86-64-linux with libbackend being incrementaly linked and also experimented
> > with extra testcases and tested that debugging works on resulting cc1 binary.
> > OK?
> >
> > Honza
> >
> >         * simple-object.h (simple_object_copy_lto_debug_sections): Add rename
> >         parameter.
> >         * simple-object.c (handle_lto_debug_sections): Add rename parameter.
> >         (handle_lto_debug_sections_rename): New function.
> >         (handle_lto_debug_sections_norename): New function.
> >         (simple_object_copy_lto_debug_sections): Add rename parameter.
> > Index: include/simple-object.h
> > ===================================================================
> > --- include/simple-object.h     (revision 260042)
> > +++ include/simple-object.h     (working copy)
> > @@ -198,12 +198,15 @@
> >  simple_object_release_write (simple_object_write *);
> >
> >  /* Copy LTO debug sections from SRC_OBJECT to DEST.
> > +   If RENAME is true, rename LTO debug section into debug section (i.e.
> > +   when producing final binary) and if it is false, keep the sections with
> > +   original names (when incrementally linking).
> >     If an error occurs, return the errno value in ERR and an error string.  */
> >
> >  extern const char *
> >  simple_object_copy_lto_debug_sections (simple_object_read *src_object,
> >                                        const char *dest,
> > -                                      int *err);
> > +                                      int *err, int rename);
> >
> >  #ifdef __cplusplus
> >  }
> 
> This part is missing from r260956:
> 
> https://gcc.gnu.org/ml/gcc-cvs/2018-05/msg01177.html

I have managed to commit partially, it should be fixed now.  My apologizes for this!

Honza
> 
> -- 
> H.J.
Rainer Orth May 30, 2018, 5:33 p.m. UTC | #4
Hi Jan,

>> > Index: include/simple-object.h
>> > ===================================================================
>> > --- include/simple-object.h     (revision 260042)
>> > +++ include/simple-object.h     (working copy)
>> > @@ -198,12 +198,15 @@
>> >  simple_object_release_write (simple_object_write *);
>> >
>> >  /* Copy LTO debug sections from SRC_OBJECT to DEST.
>> > +   If RENAME is true, rename LTO debug section into debug section (i.e.
>> > +   when producing final binary) and if it is false, keep the sections with
>> > +   original names (when incrementally linking).
>> >     If an error occurs, return the errno value in ERR and an error string.  */
>> >
>> >  extern const char *
>> >  simple_object_copy_lto_debug_sections (simple_object_read *src_object,
>> >                                        const char *dest,
>> > -                                      int *err);
>> > +                                      int *err, int rename);
>> >
>> >  #ifdef __cplusplus
>> >  }
>> 
>> This part is missing from r260956:
>> 
>> https://gcc.gnu.org/ml/gcc-cvs/2018-05/msg01177.html
>
> I have managed to commit partially, it should be fixed now.  My apologizes
> for this!

unfortunately, it isn't: the include/simple-object.h part is still
missing.

	Rainer
diff mbox series

Patch

Index: include/simple-object.h
===================================================================
--- include/simple-object.h	(revision 260042)
+++ include/simple-object.h	(working copy)
@@ -198,12 +198,15 @@ 
 simple_object_release_write (simple_object_write *);
 
 /* Copy LTO debug sections from SRC_OBJECT to DEST.
+   If RENAME is true, rename LTO debug section into debug section (i.e.
+   when producing final binary) and if it is false, keep the sections with
+   original names (when incrementally linking).
    If an error occurs, return the errno value in ERR and an error string.  */
 
 extern const char *
 simple_object_copy_lto_debug_sections (simple_object_read *src_object,
 				       const char *dest,
-				       int *err);
+				       int *err, int rename);
 
 #ifdef __cplusplus
 }
Index: libiberty/simple-object.c
===================================================================
--- libiberty/simple-object.c	(revision 260042)
+++ libiberty/simple-object.c	(working copy)
@@ -251,12 +251,15 @@ 
 }
 
 /* Callback to identify and rename LTO debug sections by name.
-   Returns 1 if NAME is a LTO debug section, 0 if not.  */
+   Returns non-NULL if NAME is a LTO debug section, NULL if not.
+   If RENAME is true it will rename LTO debug sections to non-LTO
+   ones.  */
 
 static char *
-handle_lto_debug_sections (const char *name)
+handle_lto_debug_sections (const char *name, int rename)
 {
-  char *newname = XCNEWVEC (char, strlen (name) + 1);
+  char *newname = rename ? XCNEWVEC (char, strlen (name) + 1)
+	  	         : xstrdup (name);
 
   /* ???  So we can't use .gnu.lto_ prefixed sections as the assembler
      complains about bogus section flags.  Which means we need to arrange
@@ -265,12 +268,14 @@ 
   /* Also include corresponding reloc sections.  */
   if (strncmp (name, ".rela", sizeof (".rela") - 1) == 0)
     {
-      strncpy (newname, name, sizeof (".rela") - 1);
+      if (rename)
+        strncpy (newname, name, sizeof (".rela") - 1);
       name += sizeof (".rela") - 1;
     }
   else if (strncmp (name, ".rel", sizeof (".rel") - 1) == 0)
     {
-      strncpy (newname, name, sizeof (".rel") - 1);
+      if (rename)
+        strncpy (newname, name, sizeof (".rel") - 1);
       name += sizeof (".rel") - 1;
     }
   /* ???  For now this handles both .gnu.lto_ and .gnu.debuglto_ prefixed
@@ -277,10 +282,10 @@ 
      sections.  */
   /* Copy LTO debug sections and rename them to their non-LTO name.  */
   if (strncmp (name, ".gnu.debuglto_", sizeof (".gnu.debuglto_") - 1) == 0)
-    return strcat (newname, name + sizeof (".gnu.debuglto_") - 1);
+    return rename ? strcat (newname, name + sizeof (".gnu.debuglto_") - 1) : newname;
   else if (strncmp (name, ".gnu.lto_.debug_",
 		    sizeof (".gnu.lto_.debug_") -1) == 0)
-    return strcat (newname, name + sizeof (".gnu.lto_") - 1);
+    return rename ? strcat (newname, name + sizeof (".gnu.lto_") - 1) : newname;
   /* Copy over .note.GNU-stack section under the same name if present.  */
   else if (strcmp (name, ".note.GNU-stack") == 0)
     return strcpy (newname, name);
@@ -289,14 +294,31 @@ 
      COMDAT sections in objects produced by GCC.  */
   else if (strcmp (name, ".comment") == 0)
     return strcpy (newname, name);
+  free (newname);
   return NULL;
 }
 
+/* Wrapper for handle_lto_debug_sections.  */
+
+static char *
+handle_lto_debug_sections_rename (const char *name)
+{
+  return handle_lto_debug_sections (name, 1);
+}
+
+/* Wrapper for handle_lto_debug_sections.  */
+
+static char *
+handle_lto_debug_sections_norename (const char *name)
+{
+  return handle_lto_debug_sections (name, 0);
+}
+
 /* Copy LTO debug sections.  */
 
 const char *
 simple_object_copy_lto_debug_sections (simple_object_read *sobj,
-				       const char *dest, int *err)
+				       const char *dest, int *err, int rename)
 {
   const char *errmsg;
   simple_object_write *dest_sobj;
@@ -317,9 +339,10 @@ 
   if (! dest_sobj)
     return errmsg;
 
-  errmsg = sobj->functions->copy_lto_debug_sections (sobj, dest_sobj,
-						     handle_lto_debug_sections,
-						     err);
+  errmsg = sobj->functions->copy_lto_debug_sections
+	 	 (sobj, dest_sobj,
+		  rename ? handle_lto_debug_sections_rename
+			 : handle_lto_debug_sections_norename,  err);
   if (errmsg)
     {
       simple_object_release_write (dest_sobj);