On Sun, 25 Jun 2023 at 04:18, Jan Hubicka <hubi...@ucw.cz> 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 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
> > + {
> > + ELF_SET_FIELD(fns, cl, Sym, buf, st_name, Elf_Word, st_name);
> > + buf[4]=st_info;
> > + buf[5]=st_other;
> > + ELF_SET_FIELD(fns, cl, Sym, buf, st_shndx, Elf_Half, st_shndx);
> > + 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);
> > + }
> > + return simple_object_internal_write(descriptor, offset,buf,sym_size,
> > + errmsg,err);
> > +}
> > +
> > /* Write out a complete ELF file.
> > Ehdr
> > initial dummy Shdr
> > @@ -932,8 +977,8 @@ simple_object_elf_write_to_file (simple_object_write
> > *sobj, int descriptor,
> > if (shnum == 0)
> > return NULL;
> >
> > - /* Add initial dummy Shdr and .shstrtab. */
> > - shnum += 2;
> > + /* Add initial dummy Shdr, .shstrtab, .symtab and .strtab. */
> > + shnum += 4;
> >
> > shdr_offset = ehdr_size;
> > sh_offset = shdr_offset + shnum * shdr_size;
> > @@ -1036,6 +1081,70 @@ simple_object_elf_write_to_file
> (simple_object_write
> > *sobj, int descriptor,
> > sh_offset += sh_size;
> > }
> >
> > + unsigned int num_sym=1;
> > + simple_object_symbol *symbol;
> > +
> > + for(symbol=sobj->symbols;symbol!=NULL;symbol=symbol->next)
> > + {
> > + ++num_sym;
> > + }
> > +
> > + size_t
> >
> sym_size=cl==ELFCLASS32?sizeof(Elf32_External_Sym):sizeof(Elf64_External_Sym);
> > + size_t sh_addralign=cl==ELFCLASS32?0x04:0x08;
> > + size_t sh_entsize=sym_size;
> > + size_t sh_size=num_sym*sym_size;
> > + unsigned int sh_info=2;
> > + if (!simple_object_elf_write_shdr (sobj, descriptor, shdr_offset,
> > + sh_name, SHT_SYMTAB, 0, 0, sh_offset,
> > + sh_size, shnum-2, sh_info,
> > + sh_addralign,sh_entsize, &errmsg, err))
> > + return errmsg;
> > + shdr_offset += shdr_size;
> > + sh_name+=strlen(".symtab")+1;
> > + /*Writes out the dummy symbol */
> > +
> > + if(!simple_object_elf_write_symbol(sobj, descriptor, sh_offset,
> > + 0,0,0,0,0,SHN_UNDEF,&errmsg,err))
> > + return errmsg;
> > + sh_offset+=sym_size;
> > + unsigned int st_name=1;
> > + for(symbol=sobj->symbols;symbol!=NULL;symbol=symbol->next)
> > + {
> > + unsigned int st_value=1;
> > + unsigned int st_size=1;
> > + unsigned char st_info=17;
> > + if(!simple_object_elf_write_symbol(sobj, descriptor, sh_offset,
> > + st_name,st_value,st_size,st_info,0,SHN_COMMON,&errmsg,err))
> > + return errmsg;
> > + sh_offset+=sym_size;
> > + st_name+=strlen(symbol->name)+1;
> > +
> > + }
> > +
> > + if (!simple_object_elf_write_shdr (sobj, descriptor, shdr_offset,
> > + sh_name, SHT_STRTAB, 0, 0, sh_offset,
> > + st_name, 0, 0,
> > + 1, 0, &errmsg, err))
> > + return errmsg;
> > + shdr_offset += shdr_size;
> > + sh_name+=strlen(".strtab")+1;
> > + /*.strtab has a leading zero byte*/
> > + zero = 0;
> > + if (!simple_object_internal_write (descriptor, sh_offset, &zero, 1,
> > + &errmsg, err))
> > + return errmsg;
> > + ++sh_offset;
> > +
> > + for(symbol=sobj->symbols;symbol!=NULL;symbol=symbol->next)
> > + {
> > + size_t len=strlen(symbol->name)+1;
> > + if (!simple_object_internal_write (descriptor, sh_offset,
> > + (const unsigned char *) symbol->name,
> > + len, &errmsg, err))
> > + return errmsg;
> > + sh_offset += len;
> > +
> > + }
> > if (!simple_object_elf_write_shdr (sobj, descriptor, shdr_offset,
> > sh_name, SHT_STRTAB, 0, 0, sh_offset,
> > sh_name + strlen (".shstrtab") + 1, 0, 0,
> > @@ -1060,7 +1169,16 @@ simple_object_elf_write_to_file
> (simple_object_write
> > *sobj, int descriptor,
> > return errmsg;
> > sh_offset += len;
> > }
> > -
> > + if (!simple_object_internal_write (descriptor, sh_offset,
> > + (const unsigned char *) ".symtab",
> > + strlen (".symtab") + 1, &errmsg, err))
> > + return errmsg;
> > + sh_offset+=strlen(".symtab")+1;
> > + if (!simple_object_internal_write (descriptor, sh_offset,
> > + (const unsigned char *) ".strtab",
> > + strlen (".strtab") + 1, &errmsg, err))
> > + return errmsg;
> > + sh_offset+=strlen(".strtab")+1;
> > if (!simple_object_internal_write (descriptor, sh_offset,
> > (const unsigned char *) ".shstrtab",
> > strlen (".shstrtab") + 1, &errmsg, err))
> > diff --git a/libiberty/simple-object.c b/libiberty/simple-object.c
> > index 163e58a2f3b..73622160248 100644
> > --- a/libiberty/simple-object.c
> > +++ b/libiberty/simple-object.c
> > @@ -455,6 +455,8 @@ simple_object_start_write (simple_object_attributes
> > *attrs,
> > ret->sections = NULL;
> > ret->last_section = NULL;
> > ret->data = data;
> > + ret->symbols=NULL;
> > + ret->last_symbol=NULL;
> > return ret;
> > }
> >
> > @@ -538,6 +540,28 @@ simple_object_write_to_file (simple_object_write
> > *sobj, int descriptor,
> > {
> > return sobj->functions->write_to_file (sobj, descriptor, err);
> > }
> > +/*Adds a symbol in to common*/
> > +void
> > +simple_object_write_add_symbol(simple_object_write *sobj, const char
> *name,
> > +size_t size, unsigned int align)
> > +{
> > + simple_object_symbol *symbol;
> > + symbol=XNEW(simple_object_symbol);
> > + symbol->next=NULL;
> > + symbol->name=xstrdup(name);
> > + symbol->align=align;
> > + symbol->size=size;
> > + if(sobj->last_symbol==NULL)
> > + {
> > + sobj->symbols=symbol;
> > + sobj->last_symbol=symbol;
> > + }
> > + else
> > + {
> > + sobj->last_symbol->next=symbol;
> > + sobj->last_symbol=symbol;
> > + }
> > +}
> >
> > /* Release an simple_object_write. */
> >
> > @@ -571,6 +595,14 @@ simple_object_release_write (simple_object_write
> *sobj)
> > XDELETE (section);
> > section = next_section;
> > }
> > + simple_object_symbol *symbol;
> > + symbol=sobj->symbols;
> > + while(symbol!=NULL)
> > + {
> > + free(symbol->name);
> > + XDELETE(symbol);
> > + symbol=symbol->next;
> > + }
> >
> > sobj->functions->release_write (sobj->data);
> > XDELETE (sobj);
> > --
> > 2.40.1
>
