From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <SRS0=QJr3=6Z=emagii.com=binutils@sourceware.org>
Received: from emagii.se (www.emagii.com [185.133.207.17])
	by sourceware.org (Postfix) with ESMTPS id 72DF0384D191
	for <binutils@sourceware.org>; Wed,  1 Mar 2023 14:54:02 +0000 (GMT)
DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 72DF0384D191
Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=emagii.com
Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=emagii.com
Received: from [10.175.196.145] (84-55-68-216.customers.ownit.se [84.55.68.216])
	by emagii.se (Postfix) with ESMTPSA id C58C3120194;
	Wed,  1 Mar 2023 15:53:56 +0100 (CET)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=emagii.com;
	s=default; t=1677682441;
	bh=AVHHsgB68XhrOXdVp792v51GergoeYFHIiIUcz4uJEI=; h=Subject:To:From;
	b=u1aIsb3jE/0c2Jl3uTfbV6cPmCrsj2IKtEjgbdsuCanufR5pU0kqOfPRn+P4Lu0jA
	 7IAqbKcyAPg4xAKba6qyP2GzmwkVScsjQOzSLIfHXAfRkHLjscP7Thm4rfTy3Co23g
	 Dz5J03AkbAsCTchc+fevTm5kBhhwvnStsQWL9weQ=
Authentication-Results: emagii.beebytevps.io;
        spf=pass (sender IP is 84.55.68.216) smtp.mailfrom=binutils@emagii.com smtp.helo=[10.175.196.145]
Received-SPF: pass (emagii.beebytevps.io: connection is authenticated)
Message-ID: <02d7317f-b508-bb35-a387-d40b9f37692e@emagii.com>
Date: Wed, 1 Mar 2023 15:53:56 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.7.1
Subject: Re: [PATCH v6 0/11 Add support for CRC64 generation in linker
Content-Language: en-US
To: binutils@sourceware.org
Cc: nickc@redhat.com
References: <20230301144756.1847278-1-binutils@emagii.com>
From: Ulf Samuelsson <binutils@emagii.com>
In-Reply-To: <20230301144756.1847278-1-binutils@emagii.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
X-PPP-Message-ID: 
 <167768243981.67666.4850171065851337783@localhost.localdomain>
X-PPP-Vhost: emagii.com
X-Spam-Status: No, score=-6.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,SPF_HELO_FAIL,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org
List-Id: <binutils.sourceware.org>

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
>