public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Ulf Samuelsson <binutils@emagii.com>
To: binutils@sourceware.org
Cc: nickc@redhat.com
Subject: Re: [PATCH v6 0/11 Add support for CRC64 generation in linker
Date: Wed, 1 Mar 2023 15:53:56 +0100	[thread overview]
Message-ID: <02d7317f-b508-bb35-a387-d40b9f37692e@emagii.com> (raw)
In-Reply-To: <20230301144756.1847278-1-binutils@emagii.com>

Forgot to run "indent" on the files after some modifications.
Have already run "indent" once, so most should be OK.

The new "lddigest_tab.c" is manually indented for clarity.
"indent" wreaks havoc on the readability.

Best Regards

Ulf Samuelsson

On 2023-03-01 15:47, Ulf Samuelsson via Binutils wrote:
> Patchset six
> Commands changes so you always get little endian
> but can override this in the linker command file.
>
>    DIGEST "<label>[.BE]" "CRC32"              (start, end)
>    DIGEST "<label>[.BE]" "CRC64-ECMA"         (start, end)
>    DIGEST "<label>[.BE]" "CRC64-GO-ISO"       (start, end)
>    DIGEST "<label>[.BE]" "CRC64-GO-ISO-R"     (start, end)
>    DIGEST "<label>[.BE]" "CRC64-WE"           (start, end)
>    DIGEST "<label>[.BE]" "CRC64-XZ"           (start, end)
>    DIGEST "<label>[.BE]"  POLY (<params>)     (start, end)
>    DIGEST TABLE "<label>[.BE]"
>    DIGEST SECTION "<section>"
>
> Each command has a label, which gets defined (not include ".BE")
>
> It has been built natively as well as with the following setup
> to configure.
>
>    --target=x86_64-pc-linux-gnu
>    --target=x86_64-w64-mingw32
>    --target=aarch64-linux-gnu
>    --target=powerpc64-linux-gnu
>    --target=mips-elf
>    --enable-targets=all --enable-64-bit-bfd
>
> A testsuite for all predefined algorithms are added.
> The testsuite calculates the CRC for the "123456789"
> string which is common.
> The expected checksum value is provided.
> The check ensures that a table is generated, but does not
> check the binary contents of the table.
> It is indirectly checked by using the same table for the
> algorithm
>
> The "DIGEST POLY" commands are checked by using
> the the "CRC32" and "CRC64-ECMA" polynomes and parameters.
>
> There is an online calculator at
> http://www.sunshine2k.de/coding/javascript/crc/crc_js.html
>
> There are no testsuites for the DEBUG ON|OFF, TIMESTAMP and
> "DIGEST SECTION" commands yet.
>
> The testsuite will fail if "DEBUG ON" is executed due to
> verbose output.
>
> The TIMESTAMP output varies with time and is difficult to test.
>
> The DIGEST SECTION command is not tested, because this
> need a little extra discussion.
> I have not seen any other suitable sections being generated.
>
> Since the information at http://www.sunshine2k.de/ has
> been very useful, their license (MIT) is added.
>
> Code examples for the CRC check routines that needs to be in the
> user source is added to the documentation with copyright statements.
> Should they be there, or is it enough to have the license
> in the source code?
>
> The Makefile.in is manually generated since I had a HDD crash
> and upgraded to a new disk and installed Ubuntu 22.04.
> The autotools are upgraded to 2.71. The build system
> did not like this, so I installed 2.69.
> This changes a lot more than the autotools-2.69 in Ubuntu 18.04
> so I resorted to manual edit of Makefile.in.
>
> Maybe someone should run autotools and generate this automatically
> instead of applying the the manual edit.
>
> Added instructions to build ldint.pdf which is not buildable today.
>
> The system has only been tested on a PC == little endian.
> I do not have a big endian host, but maybe someone could
> test it on such a host.
>
> The way endianess is implemented, it is easy for the user
> to supply a workaround by specifying another endianess.
>
>
>
> Fifth patchset.
>
> +* The linker script has a new command to insert a timestamp
> +  TIMESTAMP
> +  inserts the current time (seconds since Epoch) as a 64-bit value
> +
> +* The linker script syntax has new commands for debugging a linker script
> +  DEBUG ON  turns on debugging
> +  DEBUG OFF turns off debugging
> +
> +* The linker script syntax has new commands for handling CRC-32/64 calculations
> +  on the '.text' section
> +  It uses code from https://www.libcrc.org/
> +
> +  DIGEST "CRC32"              (start, end)
> +  DIGEST "CRC64-ECMA"         (start, end)
> +  DIGEST "CRC64-ISO"          (start, end)
> +  DIGEST "CRC64-WE"           (start, end)
> +  DIGEST POLY (size, polynome)(start, end)
> +  DIGEST POLYI(size, polynome)(start, end) (inversion during CRC calculation)
> +  DIGEST TABLE
> +  DIGEST SECTION "<section>
>
> ran indent on the new files.
>
> The *.d files in the testsuite was edited to
> reduce size and to remove all '(' characters.
> The new linker can link using the testsuite/ld-script/*.t
> linker files, but they do not pass the 'make check-ld'
> test.
>
> Fourth patchset.
> Get rid of binaries in testsuite.
>
> Here is the third patchset for introducing CRC64 generation.
> This has focused on supporting maintainability of the feature.
> In order to do so, CRC32 generation has been introduced to
> detect issues when extending the linker with another algoritm
> for checking integrity of the image.
> This will simplify adding other algorithms like SHA algoritms
> in the future.
>
> In addition, the TIMESTAMP command is used to store the current time
> in seconds since Epoch is stored in the image.
>
> The main routine for calculating CRCs has been broken down
> in subroutines which allow other algorithms.
>
> An algorithm typically needs
> <type> *init_<algo>_tab( <type> poly )
> <type> calc_<algo>_inv( const unsigned char *input_str, size_t num_bytes );
> <type> calc_<algo>    ( const unsigned char *input_str, size_t num_bytes );
> void lang_add_<algo>_syndrome(bool invert, bfd_vma poly)
> void lang_add_<algo>_table(void)
>
> The common functions are:
> * bool get_text_section_contents(void)
>    This reads the '.text' section and its contents
>    Pointers are stored in the global variables
>    asection *text_section
>    char     *text_contents
> * bool set_crc_checksum(bfd_vma crc, bfd_vma addr, bfd_vma size)
>    Stores the CRC at the index addr. The size of the CRC in bytes is specified.
>    Storing something larger that 64-bit is not supported here.
> * bool symbol_lookup(char *name, bfd_vma *val)
>    Gets the value of a symbol into 'val'. Returns false if not found.
>
> To add CRC32 support means adding the following commands
> * CRC32 CRC32                    '(' crc_start ',' crc_end ')'
> * CRC32 POLY  '[' mustbe_exp ']' '(' crc_start ',' crc_end ')'
> * CRC32 POLYI '[' mustbe_exp ']' '(' crc_start ',' crc_end ')'
>
> ================
> Here is the second patchset for introducing CRC64 generation in the linker
> This is mainly looking at indentation and whitespace errors.
> The patches applies cleanly without whitespace error
> except for the last patch which contains the testsuite.
> When the contents of .text is printed out, sometimes
> the last byte of a line in the CRC table is a space.
> This is reported as a whitespace error.
>
> Supports the following new linker commands:
> * CRC64 ECMA                     '(' crc_start ',' crc_end ')'
> * CRC64 ISO                      '(' crc_start ',' crc_end ')'
> * CRC64 POLY  '[' mustbe_exp ']' '(' crc_start ',' crc_end ')'
> * CRC64 POLYI '[' mustbe_exp ']' '(' crc_start ',' crc_end ')'
> * CRC64 TABLE
>
> ECMA  == 0x42F0E1EBA9EA3693
> ISO   == 0xD800000000000000
> POLY  == Allows your own polynome
> POLYI == Allows your own polynome, with inversion during calc
>
> The CRC is calculated from crc_start to crc_end (not included)
>
> The "CRC64 <polynome> command
> * Allows specifying the polynom (ECMA(default), ISO or your own)
> * Allows for inversion in the CRC calculation (CRC64-WE)
> * Allows specifying the area that should be checked.
> * reserves room for the CRC (8 bytes)
> * Declares a symbol ___CRC64___ for the address of the CRC.
> * Declares a symbol ___CRC64_START___ for the first address of the checked area
> * Declares a symbol ___CRC64_END___ for the first address after the checked area
>
> The symbols names are declared in "checksum.h".
> If the names are unsuitable, it is easy to change.
>
> The CRC TABLE command
>    This is used to speed up the CRC calculation.
> * Creates a 2kB table which speeds up the CRC calculation
> * Can insert the 2kB table into the .text section at current location.
> * Declares a symbol ___CRC64_TABLE___ if the table is inserted.
>
> Comments on the code:
>
> The process of calculation involves:
> * Check if CRC is requested
> * Validation of CRC prerequisites
> * get .text section
> * get .text section contents
> * calculate CRC over the area
> * insert the CRC in the correct location
> ====================================
>
> This version also supports the
> * DEBUG ON
> * DEBUG OFF
> turning on/off yydebug
> I find it useful for debugging the build, but it can easly be removed.
>
> This patch contains a lot of debug output, that is enabled by
> #define DEBUG_CRC 1
> This should be removed in the final version.
>
> The ld.texi stuff needs some more work. Not very experienced with that.
>
> Added an entry in NEWS
>
> Added 4 test cases for the different CRC64 polynome commands.
> All testcases generate a CRC table.
>
> The code is using the libcrc released under an MIT license found in
> * https://www.libcrc.org/
> * https://github.com/lammertb/libcrc/tree/v2.0
> Author:  Lammert Bies
> A license for libcrc is added to the patch.
>
>
> [PATCH v6 01/11] DIGEST: LICENSING
> [PATCH v6 02/11] DIGEST: NEWS
> [PATCH v6 03/11] DIGEST: Documentation
> [PATCH v6 04/11] DIGEST: testsuite
> [PATCH v6 05/11] DIGEST: ldlex.l
> [PATCH v6 06/11] DIGEST: ldgram.y
> [PATCH v6 07/11] DIGEST: ldmain.c
> [PATCH v6 08/11] DIGEST: ldlang.*: add timestamp
> [PATCH v6 09/11] DIGEST: calculation
> [PATCH v6 10/11] DIGEST: Makefile.*
> [PATCH v6 11/11] Build ldint
>

      parent reply	other threads:[~2023-03-01 14:54 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-01 14:47 binutils
2023-03-01 14:47 ` [PATCH v6 01/11] DIGEST: LICENSING binutils
2023-03-01 15:04   ` Nick Clifton
2023-03-01 15:30     ` Ulf Samuelsson
2023-03-01 17:15       ` Nick Clifton
2023-03-01 17:19         ` Ian Lance Taylor
2023-03-01 14:47 ` [PATCH v6 02/11] DIGEST: NEWS binutils
2023-03-01 14:57   ` Ulf Samuelsson
2023-03-01 14:47 ` [PATCH v6 03/11] DIGEST: Documentation binutils
2023-03-01 14:47 ` [PATCH v6 04/11] DIGEST: testsuite binutils
2023-03-01 16:48   ` Nick Clifton
2023-03-01 17:33     ` Ulf Samuelsson
2023-03-01 14:47 ` [PATCH v6 05/11] DIGEST: ldlex.l binutils
2023-03-01 14:47 ` [PATCH v6 06/11] DIGEST: ldgram.y binutils
2023-03-01 14:47 ` [PATCH v6 07/11] DIGEST: ldmain.c binutils
2023-03-01 14:47 ` [PATCH v6 08/11] DIGEST: ldlang.*: add timestamp binutils
2023-03-01 14:47 ` [PATCH v6 09/11] DIGEST: calculation binutils
2023-03-01 14:47 ` [PATCH v6 10/11] DIGEST: Makefile.* binutils
2023-03-01 14:47 ` [PATCH v6 11/11] Build ldint binutils
2023-03-01 14:53 ` Ulf Samuelsson [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=02d7317f-b508-bb35-a387-d40b9f37692e@emagii.com \
    --to=binutils@emagii.com \
    --cc=binutils@sourceware.org \
    --cc=nickc@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).