Re: [RFC PATCH 04/10] objtool: add undwarf debuginfo generation

[Jiri Slaby <jslaby@suse.cz>]

On 06/01/2017, 07:44 AM, Josh Poimboeuf wrote:
...
> index 3fb0747..1ca5d9a 100644
> --- a/tools/objtool/elf.c
> +++ b/tools/objtool/elf.c
...
> +int elf_write_to_file(struct elf *elf, char *outfile)
> +{
> +	int fd;
> +	struct section *sec;
> +	Elf *elfout;
> +	GElf_Ehdr eh, ehout;
> +	Elf_Scn *scn;
> +	Elf_Data *data;
> +	GElf_Shdr sh;
> +
> +	fd = creat(outfile, 0777);

0755 even though it is umasked?

> +	if (fd == -1) {
> +		perror("creat");
> +		return -1;
> +	}
> +
> +	elfout = elf_begin(fd, ELF_C_WRITE, NULL);
> +	if (!elfout) {
> +		perror("elf_begin");
> +		return -1;
> +	}
> +
> +	if (!gelf_newehdr(elfout, gelf_getclass(elf->elf))) {
> +		perror("gelf_newehdr");
> +		return -1;
> +	}
> +
> +	if (!gelf_getehdr(elfout, &ehout)) {

This does not make much sense to do. You memset(0) it below.

> +		perror("gelf_getehdr");
> +		return -1;
> +	}
> +
> +	if (!gelf_getehdr(elf->elf, &eh)) {
> +		perror("gelf_getehdr");
> +		return -1;
> +	}
> +
> +	memset(&ehout, 0, sizeof(ehout));
> +	ehout.e_ident[EI_DATA] = eh.e_ident[EI_DATA];
> +	ehout.e_machine = eh.e_machine;
> +	ehout.e_type = eh.e_type;
> +	ehout.e_version = EV_CURRENT;
> +	ehout.e_shstrndx = find_section_by_name(elf, ".shstrtab")->idx;
> +
> +	list_for_each_entry(sec, &elf->sections, list) {
> +		if (sec->idx == 0)
> +			continue;
> +
> +		scn = elf_newscn(elfout);
> +		if (!scn) {
> +			perror("elf_newscn");
> +			return -1;
> +		}
> +
> +		data = elf_newdata(scn);
> +		if (!data) {
> +			perror("elf_newdata");
> +			return -1;
> +		}
> +
> +		if (!elf_flagdata(data, ELF_C_SET, ELF_F_DIRTY)) {
> +			perror("elf_flagdata");
> +			return -1;
> +		}

There is not much point setting DIRTY flag here. elf_newdata does so.

> +		data->d_type = sec->data->d_type;
> +		data->d_buf = sec->data->d_buf;
> +		data->d_size = sec->data->d_size;
> +
> +		if(!gelf_getshdr(scn, &sh)) {
> +			perror("gelf_getshdr");
> +			return -1;
> +		}

This does not make much sense to do again. You overwrite the content
right away:

> +		sh = sec->sh;
> +
> +		if (!gelf_update_shdr(scn, &sh)) {
> +			perror("gelf_update_shdr");
> +			return -1;
> +		}
> +	}
> +
> +	if (!gelf_update_ehdr(elfout, &ehout)) {
> +		perror("gelf_update_ehdr");
> +		return -1;
> +	}
> +
> +	if (elf_update(elfout, ELF_C_WRITE) < 0) {
> +		perror("elf_update");
> +		return -1;
> +	}

elf_end() + close() ?

> +
> +	return 0;
> +}
> +
>  void elf_close(struct elf *elf)
>  {
>  	struct section *sec, *tmpsec;

...

> --- /dev/null
> +++ b/tools/objtool/undwarf.c
> @@ -0,0 +1,308 @@
...
> +int undwarf_dump(const char *_objname)
> +{
> +	struct elf *elf;
> +	struct section *sec;
> +	struct rela *rela;
> +	struct undwarf *undwarf;
> +	int nr, i;
> +
> +	objname = _objname;
> +
> +	elf = elf_open(objname);
> +	if (!elf) {
> +		WARN("error reading elf file %s\n", objname);
> +		return 1;
> +	}
> +
> +	sec = find_section_by_name(elf, ".undwarf");
> +	if (!sec || !sec->rela)
> +		return 0;
> +
> +	nr = sec->len / sizeof(*undwarf);
> +	for (i = 0; i < nr; i++) {
...
> +	}

elf_close() ?

> +
> +	return 0;
> +}

thanks,
-- 
js
suse labs