Re: Patch regarding addition of .symtab while generating object file from libiberty [WIP]
On Mon, 26 Jun 2023 at 23:24, Jan Hubicka wrote:
> > > > +simple_object_write_add_symbol(simple_object_write *sobj, const char
> > > *name,
> > > > +size_t size, unsigned int align);
> > >
> > > Symbols has much more properties in addition to sizes and alignments.
> > > We will eventually need to get dwarf writting, so we will need to
> > > support them. However right now we only do these fake lto object
> > > symbols, so perhaps for start we could kep things simple and assume
> that
> > > size is always 0 and align always 1 or so.
> > >
> > > Overall this looks like really good start to me (both API and
> > > imllementation looks reasonable to me and it is good that you follow
> the
> > > coding convention). I guess you can create a branch (see git info on
> > > the gcc homepage) and put the patch there?
> > >
> > I can, but I don't have write access to git. Also, for the next part, I
> am
> This is easy to fix. Follow instructions here
> https://gcc.gnu.org/gitwrite.html
> and add me as sponsor. There are also instructions how to produce a
> branch on git correctly.
>
Ok.
> > thinking of properly configuring the driver
> > to directly produce an executable or adding debug info. What do you
> think?
>
> I think we should try to aim to something that can be tested. If we add
> symbol table and get the ELF header right, linker should accept the
> object files and properly recognize them as LTO.
>
I have done some testing, readelf can read the symbol table and display it
correctly ( using your updated patch to produce object files:
https://gcc.gnu.org/pipermail/gcc/2022-May/238670.html).
Apart from this, I also ran GCC LTO's test suite, and all test cases passed
as expected.
I am also able to produce an executable and run it. The only thing I might
be missing is producing the ELF header directly instead of reading it from
crtbegin.o.
I am thinking about it but can't think where to find architecture and
header's field details without crtbegin.o.
What exactly you mean by configuring the driver?
>
I was talking about right now in the patch, to produce an executable, we
have to stop before the assembly step(-S flag) and then invoke the linker.
I am thinking of fixing it.
--
Regards
Rishi
>
> Honza
> > --
> > Regards
> > Rishi
> >
> > >
> > > I am also adding Ian to CC as he is maintainer of the simple-object and
> > > he may have some ideas.
> > >
> > > Honza
> > > >
> > > > /* Release all resources associated with SIMPLE_OBJECT, including
> any
> > > > simple_object_write_section's that may have been created. */
> > > > diff --git a/libiberty/simple-object-common.h
> > > > b/libiberty/simple-object-common.h
> > > > index b9d10550d88..df99c9d85ac 100644
> > > > --- a/libiberty/simple-object-common.h
> > > > +++ b/libiberty/simple-object-common.h
> > > > @@ -58,6 +58,24 @@ struct simple_object_write_struct
> > > >simple_object_write_section *last_section;
> > > >/* Private data for the object file format. */
> > > >void *data;
> > > > + /*The start of the list of symbols.*/
> > > > + simple_object_symbol *symbols;
> > > > + /*The last entry in the list of symbols*/
> > > > + simple_object_symbol *last_symbol;
> > > > +};
> > > > +
> > > > +/*A symbol in object file being created*/
> > > > +struct simple_object_symbol_struct
> > > > +{
> > > > + /*Next in the list of symbols attached to an
> > > > + simple_object_write*/
> > > > + simple_object_symbol *next;
> > > > + /*The name of this symbol. */
> > > > + char *name;
> > > > + /* Symbol value */
> > > > + unsigned int align;
> > > > + /* Symbol size */
> > > > + size_t size;
> > > > };
> > > >
> > > > /* A section in an object file being created. */
> > > > diff --git a/libiberty/simple-object-elf.c
> > > b/libiberty/simple-object-elf.c
> > > > index eee07039984..cbba88186bd 100644
> > > > --- a/libiberty/simple-object-elf.c
> > > > +++ b/libiberty/simple-object-elf.c
> > > > @@ -787,9 +787,9 @@ simple_object_elf_write_ehdr (simple_object_write
> > > > *sobj, int descriptor,
> > > > ++shnum;
> > > >if (shnum > 0)
> > > > {
> > > > - /* Add a section header for the dummy section and one for
> > > > - .shstrtab. */
> > > > - shnum += 2;
> > > > + /* Add a section header for the dummy section,
> > > > + .shstrtab, .symtab and .strtab. */
> > > > + shnum += 4;
> > > > }
> > > >
> > > >ehdr_size = (cl == ELFCLASS32
> > > > @@ -882,6 +882,51 @@ simple_object_elf_write_shdr
> (simple_object_write
> > > > *sobj, int descriptor,
> > > > errmsg, err);
> > > > }
> > > >
> > > > +/* Write out an ELF Symbol*/
> > > > +
> > > > +static int
> > > > +simple_object_elf_write_symbol(simple_object_write *sobj, int
> > > descriptor,
> > > > +off_t offset, unsigned int st_name, unsigned int
> st_value,
> > > > size_t st_size,
> > > > +unsigned char st_info, unsigned char st_other, unsigned
> int
> > > > st_shndx,
> > > > +
Re: Patch regarding addition of .symtab while generating object file from libiberty [WIP]
> > > +simple_object_write_add_symbol(simple_object_write *sobj, const char
> > *name,
> > > +size_t size, unsigned int align);
> >
> > Symbols has much more properties in addition to sizes and alignments.
> > We will eventually need to get dwarf writting, so we will need to
> > support them. However right now we only do these fake lto object
> > symbols, so perhaps for start we could kep things simple and assume that
> > size is always 0 and align always 1 or so.
> >
> > Overall this looks like really good start to me (both API and
> > imllementation looks reasonable to me and it is good that you follow the
> > coding convention). I guess you can create a branch (see git info on
> > the gcc homepage) and put the patch there?
> >
> I can, but I don't have write access to git. Also, for the next part, I am
This is easy to fix. Follow instructions here
https://gcc.gnu.org/gitwrite.html
and add me as sponsor. There are also instructions how to produce a
branch on git correctly.
> thinking of properly configuring the driver
> to directly produce an executable or adding debug info. What do you think?
I think we should try to aim to something that can be tested. If we add
symbol table and get the ELF header right, linker should accept the
object files and properly recognize them as LTO.
What exactly you mean by configuring the driver?
Honza
> --
> Regards
> Rishi
>
> >
> > I am also adding Ian to CC as he is maintainer of the simple-object and
> > he may have some ideas.
> >
> > Honza
> > >
> > > /* Release all resources associated with SIMPLE_OBJECT, including any
> > > simple_object_write_section's that may have been created. */
> > > diff --git a/libiberty/simple-object-common.h
> > > b/libiberty/simple-object-common.h
> > > index b9d10550d88..df99c9d85ac 100644
> > > --- a/libiberty/simple-object-common.h
> > > +++ b/libiberty/simple-object-common.h
> > > @@ -58,6 +58,24 @@ struct simple_object_write_struct
> > >simple_object_write_section *last_section;
> > >/* Private data for the object file format. */
> > >void *data;
> > > + /*The start of the list of symbols.*/
> > > + simple_object_symbol *symbols;
> > > + /*The last entry in the list of symbols*/
> > > + simple_object_symbol *last_symbol;
> > > +};
> > > +
> > > +/*A symbol in object file being created*/
> > > +struct simple_object_symbol_struct
> > > +{
> > > + /*Next in the list of symbols attached to an
> > > + simple_object_write*/
> > > + simple_object_symbol *next;
> > > + /*The name of this symbol. */
> > > + char *name;
> > > + /* Symbol value */
> > > + unsigned int align;
> > > + /* Symbol size */
> > > + size_t size;
> > > };
> > >
> > > /* A section in an object file being created. */
> > > diff --git a/libiberty/simple-object-elf.c
> > b/libiberty/simple-object-elf.c
> > > index eee07039984..cbba88186bd 100644
> > > --- a/libiberty/simple-object-elf.c
> > > +++ b/libiberty/simple-object-elf.c
> > > @@ -787,9 +787,9 @@ simple_object_elf_write_ehdr (simple_object_write
> > > *sobj, int descriptor,
> > > ++shnum;
> > >if (shnum > 0)
> > > {
> > > - /* Add a section header for the dummy section and one for
> > > - .shstrtab. */
> > > - shnum += 2;
> > > + /* Add a section header for the dummy section,
> > > + .shstrtab, .symtab and .strtab. */
> > > + shnum += 4;
> > > }
> > >
> > >ehdr_size = (cl == ELFCLASS32
> > > @@ -882,6 +882,51 @@ simple_object_elf_write_shdr (simple_object_write
> > > *sobj, int descriptor,
> > > errmsg, err);
> > > }
> > >
> > > +/* Write out an ELF Symbol*/
> > > +
> > > +static int
> > > +simple_object_elf_write_symbol(simple_object_write *sobj, int
> > descriptor,
> > > +off_t offset, unsigned int st_name, unsigned int st_value,
> > > size_t st_size,
> > > +unsigned char st_info, unsigned char st_other, unsigned int
> > > st_shndx,
> > > +const char **errmsg, int *err)
> > > +{
> > > + struct simple_object_elf_attributes *attrs =
> > > +(struct simple_object_elf_attributes *) sobj->data;
> > > + const struct elf_type_functions* fns;
> > > + unsigned char cl;
> > > + size_t sym_size;
> > > + unsigned char buf[sizeof (Elf64_External_Shdr)];
> > > +
> > > + fns = attrs->type_functions;
> > > + cl = attrs->ei_class;
> > > +
> > > + sym_size = (cl == ELFCLASS32
> > > + ? sizeof (Elf32_External_Shdr)
> > > + : sizeof (Elf64_External_Shdr));
> > > + memset (buf, 0, sizeof (Elf64_External_Shdr));
> > > +
> > > + if(cl==ELFCLASS32)
> > > + {
> > > +ELF_SET_FIELD(fns, cl, Sym, buf, st_name, Elf_Word, st_name);
> > > +ELF_SET_FIELD(fns, cl, Sym, buf, st_value, Elf_Addr, st_value);
> > > +ELF_SET_FIELD(fns, cl, Sym, buf, st_size, Elf_Addr, st_size);
> > > +buf[4]=st_info;
> > > +buf[5]=st_other;
> > > +ELF_SET_FIELD(fns, cl, Sym, buf, st_shndx, Elf_Half, st_shndx);
> > > + }
> > > + else
> > > + {
> > > +
Re: Patch regarding addition of .symtab while generating object file from libiberty [WIP]
On Sun, 25 Jun 2023 at 04:18, Jan Hubicka wrote:
> > Hi,
> Hi,
> I am sorry for late reaction.
>
No problem!
> > I am working on the GSOC project "Bypass Assembler when generating LTO
> > object files." So as a first step, I am adding .symtab along with
> > __gnu_lto_slim symbol into it so that at a later stage, it can be
> > recognized that this object file has been produced using -flto enabled.
> > This patch is regarding the same. Although I am still testing this
> patch, I
> > want general feedback on my code and design choice.
> > I have extended simple_object_wrtie_struct to hold a list of symbols (
> > similar to sections ). A function in simple-object.c to add symbols. I am
> > calling this function in lto-object.cc to add __gnu_lto_v1.
> > Right now, as we are only working on ELF support first, I am adding
> .symtab
> > in elf object files only.
> >
> > ---
> > gcc/lto-object.cc| 4 +-
> > include/simple-object.h | 10 +++
> > libiberty/simple-object-common.h | 18 +
> > libiberty/simple-object-elf.c| 130 +--
> > libiberty/simple-object.c| 32
> > 5 files changed, 187 insertions(+), 7 deletions(-)
> >
> > diff --git a/gcc/lto-object.cc b/gcc/lto-object.cc
> > index cb1c3a6cfb3..680977cb327 100644
> > --- a/gcc/lto-object.cc
> > +++ b/gcc/lto-object.cc
> > @@ -187,7 +187,9 @@ lto_obj_file_close (lto_file *file)
> >int err;
> >
> >gcc_assert (lo->base.offset == 0);
> > -
> > + /*Add __gnu_lto_slim symbol*/
> > + if(flag_bypass_asm)
> > +simple_object_write_add_symbol (lo->sobj_w,
> "__gnu_lto_slim",1,1);
>
> You can probably do this unconditionally. The ltrans files we produce
> are kind of wrong by missing the symbol table currently.
>
I see.
> > +simple_object_write_add_symbol(simple_object_write *sobj, const char
> *name,
> > +size_t size, unsigned int align);
>
> Symbols has much more properties in addition to sizes and alignments.
> We will eventually need to get dwarf writting, so we will need to
> support them. However right now we only do these fake lto object
> symbols, so perhaps for start we could kep things simple and assume that
> size is always 0 and align always 1 or so.
>
> Overall this looks like really good start to me (both API and
> imllementation looks reasonable to me and it is good that you follow the
> coding convention). I guess you can create a branch (see git info on
> the gcc homepage) and put the patch there?
>
I can, but I don't have write access to git. Also, for the next part, I am
thinking of properly configuring the driver
to directly produce an executable or adding debug info. What do you think?
--
Regards
Rishi
>
> I am also adding Ian to CC as he is maintainer of the simple-object and
> he may have some ideas.
>
> Honza
> >
> > /* Release all resources associated with SIMPLE_OBJECT, including any
> > simple_object_write_section's that may have been created. */
> > diff --git a/libiberty/simple-object-common.h
> > b/libiberty/simple-object-common.h
> > index b9d10550d88..df99c9d85ac 100644
> > --- a/libiberty/simple-object-common.h
> > +++ b/libiberty/simple-object-common.h
> > @@ -58,6 +58,24 @@ struct simple_object_write_struct
> >simple_object_write_section *last_section;
> >/* Private data for the object file format. */
> >void *data;
> > + /*The start of the list of symbols.*/
> > + simple_object_symbol *symbols;
> > + /*The last entry in the list of symbols*/
> > + simple_object_symbol *last_symbol;
> > +};
> > +
> > +/*A symbol in object file being created*/
> > +struct simple_object_symbol_struct
> > +{
> > + /*Next in the list of symbols attached to an
> > + simple_object_write*/
> > + simple_object_symbol *next;
> > + /*The name of this symbol. */
> > + char *name;
> > + /* Symbol value */
> > + unsigned int align;
> > + /* Symbol size */
> > + size_t size;
> > };
> >
> > /* A section in an object file being created. */
> > diff --git a/libiberty/simple-object-elf.c
> b/libiberty/simple-object-elf.c
> > index eee07039984..cbba88186bd 100644
> > --- a/libiberty/simple-object-elf.c
> > +++ b/libiberty/simple-object-elf.c
> > @@ -787,9 +787,9 @@ simple_object_elf_write_ehdr (simple_object_write
> > *sobj, int descriptor,
> > ++shnum;
> >if (shnum > 0)
> > {
> > - /* Add a section header for the dummy section and one for
> > - .shstrtab. */
> > - shnum += 2;
> > + /* Add a section header for the dummy section,
> > + .shstrtab, .symtab and .strtab. */
> > + shnum += 4;
> > }
> >
> >ehdr_size = (cl == ELFCLASS32
> > @@ -882,6 +882,51 @@ simple_object_elf_write_shdr (simple_object_write
> > *sobj, int descriptor,
> > errmsg, err);
> > }
> >
> > +/* Write out an ELF Symbol*/
> > +
> > +static int
> > +simple_object_elf_write_symbol(simple_object_write *sobj, int
> descriptor,
> > +off_t offset, unsigned int
Re: Patch regarding addition of .symtab while generating object file from libiberty [WIP]
> Hi,
Hi,
I am sorry for late reaction.
> I am working on the GSOC project "Bypass Assembler when generating LTO
> object files." So as a first step, I am adding .symtab along with
> __gnu_lto_slim symbol into it so that at a later stage, it can be
> recognized that this object file has been produced using -flto enabled.
> This patch is regarding the same. Although I am still testing this patch, I
> want general feedback on my code and design choice.
> I have extended simple_object_wrtie_struct to hold a list of symbols (
> similar to sections ). A function in simple-object.c to add symbols. I am
> calling this function in lto-object.cc to add __gnu_lto_v1.
> Right now, as we are only working on ELF support first, I am adding .symtab
> in elf object files only.
>
> ---
> gcc/lto-object.cc| 4 +-
> include/simple-object.h | 10 +++
> libiberty/simple-object-common.h | 18 +
> libiberty/simple-object-elf.c| 130 +--
> libiberty/simple-object.c| 32
> 5 files changed, 187 insertions(+), 7 deletions(-)
>
> diff --git a/gcc/lto-object.cc b/gcc/lto-object.cc
> index cb1c3a6cfb3..680977cb327 100644
> --- a/gcc/lto-object.cc
> +++ b/gcc/lto-object.cc
> @@ -187,7 +187,9 @@ lto_obj_file_close (lto_file *file)
>int err;
>
>gcc_assert (lo->base.offset == 0);
> -
> + /*Add __gnu_lto_slim symbol*/
> + if(flag_bypass_asm)
> +simple_object_write_add_symbol (lo->sobj_w, "__gnu_lto_slim",1,1);
You can probably do this unconditionally. The ltrans files we produce
are kind of wrong by missing the symbol table currently.
> +simple_object_write_add_symbol(simple_object_write *sobj, const char *name,
> +size_t size, unsigned int align);
Symbols has much more properties in addition to sizes and alignments.
We will eventually need to get dwarf writting, so we will need to
support them. However right now we only do these fake lto object
symbols, so perhaps for start we could kep things simple and assume that
size is always 0 and align always 1 or so.
Overall this looks like really good start to me (both API and
imllementation looks reasonable to me and it is good that you follow the
coding convention). I guess you can create a branch (see git info on
the gcc homepage) and put the patch there?
I am also adding Ian to CC as he is maintainer of the simple-object and
he may have some ideas.
Honza
>
> /* Release all resources associated with SIMPLE_OBJECT, including any
> simple_object_write_section's that may have been created. */
> diff --git a/libiberty/simple-object-common.h
> b/libiberty/simple-object-common.h
> index b9d10550d88..df99c9d85ac 100644
> --- a/libiberty/simple-object-common.h
> +++ b/libiberty/simple-object-common.h
> @@ -58,6 +58,24 @@ struct simple_object_write_struct
>simple_object_write_section *last_section;
>/* Private data for the object file format. */
>void *data;
> + /*The start of the list of symbols.*/
> + simple_object_symbol *symbols;
> + /*The last entry in the list of symbols*/
> + simple_object_symbol *last_symbol;
> +};
> +
> +/*A symbol in object file being created*/
> +struct simple_object_symbol_struct
> +{
> + /*Next in the list of symbols attached to an
> + simple_object_write*/
> + simple_object_symbol *next;
> + /*The name of this symbol. */
> + char *name;
> + /* Symbol value */
> + unsigned int align;
> + /* Symbol size */
> + size_t size;
> };
>
> /* A section in an object file being created. */
> diff --git a/libiberty/simple-object-elf.c b/libiberty/simple-object-elf.c
> index eee07039984..cbba88186bd 100644
> --- a/libiberty/simple-object-elf.c
> +++ b/libiberty/simple-object-elf.c
> @@ -787,9 +787,9 @@ simple_object_elf_write_ehdr (simple_object_write
> *sobj, int descriptor,
> ++shnum;
>if (shnum > 0)
> {
> - /* Add a section header for the dummy section and one for
> - .shstrtab. */
> - shnum += 2;
> + /* Add a section header for the dummy section,
> + .shstrtab, .symtab and .strtab. */
> + shnum += 4;
> }
>
>ehdr_size = (cl == ELFCLASS32
> @@ -882,6 +882,51 @@ simple_object_elf_write_shdr (simple_object_write
> *sobj, int descriptor,
> errmsg, err);
> }
>
> +/* Write out an ELF Symbol*/
> +
> +static int
> +simple_object_elf_write_symbol(simple_object_write *sobj, int descriptor,
> +off_t offset, unsigned int st_name, unsigned int st_value,
> size_t st_size,
> +unsigned char st_info, unsigned char st_other, unsigned int
> st_shndx,
> +const char **errmsg, int *err)
> +{
> + struct simple_object_elf_attributes *attrs =
> +(struct simple_object_elf_attributes *) sobj->data;
> + const struct elf_type_functions* fns;
> + unsigned char cl;
> + size_t sym_size;
> + unsigned char buf[sizeof (Elf64_External_Shdr)];
> +
> + fns = attrs->type_functions;
> + cl = attrs->ei_class;
> +
> + sym_size = (cl ==
