Re: [PATCH 1/9] doc: Add elf32_checksum.3 and elf64_checksum.3

2024-08-27 Thread Mark Wielaard
Hi Aaron,

On Wed, 2024-08-14 at 17:33 -0400, Aaron Merey wrote:
> Signed-off-by: Aaron Merey 
> ---
>  doc/elf32_checksum.3 | 47 
>  doc/elf64_checksum.3 | 47 
>  2 files changed, 94 insertions(+)
>  create mode 100644 doc/elf32_checksum.3
>  create mode 100644 doc/elf64_checksum.3
> 
> diff --git a/doc/elf32_checksum.3 b/doc/elf32_checksum.3
> new file mode 100644
> index ..617b955c
> --- /dev/null
> +++ b/doc/elf32_checksum.3
> @@ -0,0 +1,47 @@
> +.TH ELF32_CHECKSUM 3 2024-08-14 "Libelf" "Libelf Programmer's Manual"
> +
> +.SH NAME
> +elf32_checksum \- compute the checksum for an ELF32 object file
> +
> +.SH SYNOPSIS
> +.B #include 
> +
> +.BI "long int elf32_checksum(Elf *" elf ");"
> +
> +.SH DESCRIPTION
> +Compute a checksum for the ELF32 object file referred to by
> +.I elf.
> +The checksum is computed from permanent parts of the ELF file and
> +the result is repeatable.

So this one (and the elf64 variant) are a little funny in that I don't
believe anybody really uses it. Because it is defined so fuzzy.

But I think you should explain what "permanent parts" are. The intent
is that you get a checksum that stays the same whether or not the file
is stripped. So this implementation only computes a checksum over
sections that aren't SECTION_STRIP_P (see libelf/elf-knowledge.h) and
it (obviously) ignores SHT_NOBITS sections.

Another issue is that the result is defined as long int, so depending
on platform/arch long int might be 4 to 8 bytes... But we use the crc32
from lib/crc32.c so the result is always capped to an uint32_t. I am
not sure whether that is sign-extended or not. Might want to double
check.

Finally the checksum would be set as DT_CHECKSUM in the dynamic section
(presumable as a kind of pre-build-id like value). But only prelink
ever did that (to preserve/check the original content before
prelinking). I don't believe any current utility does use DT_CHECKSUM
these days (adding it the the dynamic section would of course break the
checksum, and I don't know how one would handle that...)

> +.SH PARAMETERS
> +.TP
> +.I elf
> +The ELF32 object file for which the checksum is to be computed.
> +
> +.SH RETURN VALUE
> +On success, return the computed checksum. If an error occurs, return -1 and 
> set a libelf error code.
> +
> +.SH SEE ALSO
> +.BR elf_errno (3),
> +.BR elf64_checksum (3),
> +.BR libelf (3),
> +.BR elf (5)
> +
> +.SH ATTRIBUTES
> +For an explanation of the terms used in this section, see
> +.BR attributes (7).
> +.TS
> +allbox;
> +lbx lb lb
> +l l l.
> +InterfaceAttribute   Value
> +T{
> +.na
> +.nh
> +.BR elf32_checksum ()
> +T}   Thread safety   MT-Safe
> +.TE
> +
> +.SH REPORTING BUGS
> +Report bugs to  or 
> https://sourceware.org/bugzilla/.

Cheers,

Mark


Re: [PATCH 2/9] doc: Add elf32_fsize.3 and elf64_fsize.3

2024-08-27 Thread Mark Wielaard
Hi Aaron,

On Wed, 2024-08-14 at 17:33 -0400, Aaron Merey wrote:
> Signed-off-by: Aaron Merey 
> ---
>  doc/elf32_fsize.3 | 59 +++
>  doc/elf64_fsize.3 | 59 +++
>  2 files changed, 118 insertions(+)
>  create mode 100644 doc/elf32_fsize.3
>  create mode 100644 doc/elf64_fsize.3
> 
> diff --git a/doc/elf32_fsize.3 b/doc/elf32_fsize.3
> new file mode 100644
> index ..ea86157f
> --- /dev/null
> +++ b/doc/elf32_fsize.3
> @@ -0,0 +1,59 @@
> +.TH ELF32_FSIZE 3 2024-08-14 "Libelf" "Libelf Programmer's Manual"
> +
> +.SH NAME
> +elf32_fsize \- calculate the file size of various ELF32 data structures
> +
> +.SH SYNOPSIS
> +.B #include 
> +
> +.BI "size_t elf32_fsize(Elf_Type " type ", size_t " count ", unsigned int " 
> version ");"
> +
> +.SH DESCRIPTION
> +Calculate the file size in bytes of various ELF32 data structures,
> +given their type and count.

Maybe try rewording not using the exact names in the function
declaration?

"Given an Elf_Type representation of core Elf structures and the number
of items (and the ELF version) returns the number of bytes needed for
the on-disk representation in a 32-bit ELF file."?

Also you should probably note that this libelf implementation relies on
the in-memory representation being the same size as the on-disk
representation.

Maybe give at least some examples of Elf_Type and which data structure
they represent? ELF_T_ADDR (32 bit address), ELF_T_EHDR (Elf32_Ehdr),
etc.

And mention that elf_getdata will set the Elf_Data d_type to the
Elf_Type of the section?

> +.SH PARAMETERS
> +.TP
> +.I type
> +The ELF32 data structure type for which the file size is to be calculated. 
> See
> +.BR libelf.h
> +for a list of valid
> +.BR Elf_Type

Should we have a libelf man page instead that describes these data
structures?

> +values.
> +
> +.TP
> +.I count
> +The number of elements of the specified type.
> +
> +.TP
> +.I version
> +The ELF version. This should be set to
> +.B EV_CURRENT.

Yeah, maybe explicitly say that this is the only valid value.

> +.SH RETURN VALUE
> +The size in bytes of the specified count and type of data structure.  If an 
> error occurs,
> +return 0 and set a libelf error code.

Maybe explicitly say what an error is?
version != EV_CURRENT or type isn't a valid Elf_Type.

I also note that the result will overflow if
count > SIZE_MAX / elf32_fsize(type, 1, 1)
But doesn't produce an error (should it?)

> +.SH SEE ALSO
> +.BR elf_errno (3),
> +.BR elf64_fsize (3),
> +.BR libelf (3),

Aha, you are already referencing it. But it doesn't exist yet?

> +.BR elf (5)
> +
> +.SH ATTRIBUTES
> +For an explanation of the terms used in this section, see
> +.BR attributes (7).
> +.TS
> +allbox;
> +lbx lb lb
> +l l l.
> +InterfaceAttribute   Value
> +T{
> +.na
> +.nh
> +.BR elf32_fsize ()
> +T}   Thread safety   MT-Safe
> +.TE
> +
> +.SH REPORTING BUGS
> +Report bugs to  or 
> https://sourceware.org/bugzilla/.

Cheers,

Mark


Re: [PATCH 3/9] doc: Add elf32_getchdr.3 and elf64_getchdr.3

2024-08-27 Thread Mark Wielaard
Hi Aaron,

On Wed, 2024-08-14 at 17:33 -0400, Aaron Merey wrote:
> diff --git a/doc/elf32_getchdr.3 b/doc/elf32_getchdr.3
> new file mode 100644
> index ..902ae09c
> --- /dev/null
> +++ b/doc/elf32_getchdr.3
> @@ -0,0 +1,48 @@
> +.TH ELF32_GETCHDR 3 2024-08-14 "Libelf" "Libelf Programmer's Manual"
> +
> +.SH NAME
> +elf32_getchdr \- retrieve the compression header for a section from an ELF32 
> object file.
> +
> +.SH SYNOPSIS
> +.B #include 
> +
> +.BI "Elf32_Chdr *elf32_getchdr(Elf_Scn *" scn ");"
> +
> +.SH DESCRIPTION
> +Retrieve the compression header for a section with compressed data.

This should mention that this works only for sections that are
compressed and have the Shdr SHF_COMPRESSED flag set. Otherwise it will
return NULL. It should also mention what the Elf32_Chdr structure looks
like and what the meaning the fields have. What the legal values of
ch_type are, that ch_size is the uncompressed section data size, and
that ch_addralign is the alignment of the uncompressed data.

> +.SH PARAMETERS
> +.TP
> +.I scn
> +Section whose compression header will be retrieved.
> +
> +.SH RETURN VALUE
> +On success, return a pointer to the compression header. On failure, return 
> NULL and set a libelf error code.

Probably should explicitly mention that one failure type is the section
not being compressed (the Shdr not having the SHF_COMPRESSED flag set).

> +.SH SEE ALSO
> +.BR elf_errno (3),
> +.BR elf64_getchdr (3),
> +.BR libelf (3),
> +.BR elf (5)

elf_compress (3) ?

> +.SH ATTRIBUTES
> +For an explanation of the terms used in this section, see
> +.BR attributes (7).
> +.TS
> +allbox;
> +lbx lb lb
> +l l l.
> +InterfaceAttribute   Value
> +T{
> +.na
> +.nh
> +.BR elf32_getchdr ()
> +T}   Thread safety   MT-Safe
> +.TE
> +
> +.SH REPORTING BUGS
> +Report bugs to  or 
> https://sourceware.org/bugzilla/.
> +
> +.SH HISTORY
> +.B elf32_getchdr
> +first appeared in elfutils 0.165. This elfutils libelf function may not be 
> found in other libelf implementations.

Yes, but it has been standardized:
http://www.linker-aliens.org/blogs/ali/entry/elf_section_compression/

Cheers,

Mark


Re: [PATCH 4/9] doc: Add elf32_getphdr.3 and elf64_getphdr.3

2024-08-27 Thread Mark Wielaard
Hi Aaron,

On Wed, Aug 14, 2024 at 05:33:16PM -0400, Aaron Merey wrote:
> diff --git a/doc/elf32_getphdr.3 b/doc/elf32_getphdr.3
> new file mode 100644
> index ..7245f81b
> --- /dev/null
> +++ b/doc/elf32_getphdr.3
> @@ -0,0 +1,45 @@
> +.TH ELF32_GETPHDR 3 2024-08-14 "Libelf" "Libelf Programmer's Manual"
> +
> +.SH NAME
> +elf32_getphdr \- retrieve the program header table for an ELF32 object file
> +
> +.SH SYNOPSIS
> +.B #include 
> +
> +.BI "Elf32_Phdr *elf32_getphdr(Elf *" elf ");"
> +
> +.SH DESCRIPTION
> +Retrieve the program header table for the given ELF descriptor
> +.I elf.

This should expain what the elements of the table are and how to know
how many elements it has. Calling elf_getphdrnum will give you the
number of elements. When changing an element you have to call
elf_flagphdr with ELF_C_SET ELF_F_DIRTY to write the (new) data out to
disk. To change the size of the table (or to delete it) one has to
call elf32_newphdr.

> +.SH PARAMETERS
> +.TP
> +.I elf
> +ELF descriptor from which to retrieve the program header table.
> +
> +.SH RETURN VALUE
> +On success, return a pointer to the program header table. On failure, return 
> NULL and set a libelf error code.

Note that NULL is also returned if there is no program header (or
would be zero size).

> +
> +.SH SEE ALSO
> +.BR elf_errno (3),
> +.BR elf64_getphdr (3),
> +.BR libelf (3),
> +.BR elf (5)

Add elf_getphdrnum and elf32_newphdr.

> +.SH ATTRIBUTES
> +For an explanation of the terms used in this section, see
> +.BR attributes (7).
> +.TS
> +allbox;
> +lbx lb lb
> +l l l.
> +InterfaceAttribute   Value
> +T{
> +.na
> +.nh
> +.BR elf32_getphdr ()
> +T}   Thread safety   MT-Safe
> +.TE
> +
> +.SH REPORTING BUGS
> +Report bugs to  or 
> https://sourceware.org/bugzilla/.

Cheers,

Mark


Re: [PATCH 5/9] doc: Add elf32_getshdr.3 and elf64_getshdr.3

2024-08-27 Thread Mark Wielaard
Hi Aaron,

Meta. In this series I am only commenting on the elf32 variants,
because the elf64 variants are really just identical. Would it make
sense to combine these into one man page?

e.g readv and writev are really the same:
https://man7.org/linux/man-pages/man2/readv.2.html
https://man7.org/linux/man-pages/man2/writev.2.html

On Wed, Aug 14, 2024 at 05:33:17PM -0400, Aaron Merey wrote:
> diff --git a/doc/elf32_getshdr.3 b/doc/elf32_getshdr.3
> new file mode 100644
> index ..5e01fe1c
> --- /dev/null
> +++ b/doc/elf32_getshdr.3
> @@ -0,0 +1,46 @@
> +.TH ELF32_GETSHDR 3 2024-08-14 "Libelf" "Libelf Programmer's Manual"
> +
> +.SH NAME
> +elf32_getshdr \- retrieve the section header for a section in an ELF32 
> object file
> +
> +.SH SYNOPSIS
> +.B #include 
> +
> +.BI "Elf32_Shdr *elf32_getshdr(Elf_Scn *" scn ");"
> +
> +.SH DESCRIPTION
> +Retrieve the section header for the section referred to by
> +.I scn.

This should imho describe the Shdr members. It also should mention
that if you change any member field you should call elf_flagshdr
ELF_C_SET ELF_F_DIRTY to make sure the (changed) Shdr is written out
to the file again.

> +
> +.SH PARAMETERS
> +.TP
> +.I scn
> +The section descriptor whose section header is to be retrieved.
> +
> +.SH RETURN VALUE
> +On success, return a pointer to the section header. If an error occurs, 
> returns
> +NULL and set a libelf error code.

Should probably mention that if scn is NULL the return value will also
be NULL and the libelf error code will not be changed (to help
propagate errors).

> +.SH SEE ALSO
> +.BR elf_errno (3),
> +.BR elf64_getshdr (3),
> +.BR libelf (3),
> +.BR elf (5)

Add elf_flagshdr.

> +.SH ATTRIBUTES
> +For an explanation of the terms used in this section, see
> +.BR attributes (7).
> +.TS
> +allbox;
> +lbx lb lb
> +l l l.
> +InterfaceAttribute   Value
> +T{
> +.na
> +.nh
> +.BR elf32_getshdr ()
> +T}   Thread safety   MT-Safe
> +.TE
> +
> +.SH REPORTING BUGS
> +Report bugs to  or 
> https://sourceware.org/bugzilla/.

Cheers,

Mark