From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from emagii.se (www.emagii.com [185.133.207.17]) by sourceware.org (Postfix) with ESMTPS id 2072B38702D4 for ; Tue, 21 Feb 2023 13:11:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2072B38702D4 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 4350A120280; Tue, 21 Feb 2023 14:10:52 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=emagii.com; s=default; t=1676985058; bh=hd2wSEufpf165+Q+nUZaW4rDUe7atVyfeiEe3BIntqE=; h=Subject:To:From; b=Hmk6aGcLnAygPGpEDa3gwd7fmcgHS27AO/iV+UuoOSeM3Vr+FHmTAhvv2vJnlK6JI W9n85G9qn9EWvX5He9QbK/McDfNYgOkgnSNdURpGXOOaVSZK15dpb4DMu/SVbR2Az/ WJXw3Ee8BIZGNlbbwvYyVzKGvEsrqRW84enlqVZw= 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: <318da134-56d4-ac29-cffe-e8282e714b6f@emagii.com> Date: Tue, 21 Feb 2023 14:10:52 +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 v4 0/5] Add support for CRC64 generation in linker Content-Language: sv-FI To: Nick Clifton , binutils@sourceware.org References: <20230219194549.22554-1-binutils@emagii.com> <20230219194549.22554-2-binutils@emagii.com> From: Ulf Samuelsson In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-PPP-Message-ID: <167698505646.1661424.9629926507072171957@localhost.localdomain> X-PPP-Vhost: emagii.com X-Spam-Status: No, score=-4.9 required=5.0 tests=BAYES_00,BODY_8BITS,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: Den 2023-02-21 kl. 13:02, skrev Nick Clifton: > Hi Ulf, > >> Fourth patchset. >> Get rid of binaries in testsuite. > > Thanks. > > I do wonder if it would be possible to simplify some of the extensions > to the linker script syntax, in order to make it easier to use and to > allow for future extensions.  For example: > >   DIGEST "algorithm" ( start_symbol,  end_symbol ) Yes, I am open to suggestions. > > So "algorithm" could be "CRC" or "CRC32" or "CRC64" or "CRC64-WE" or > "CRC TABLE" or any of the other algorithms/features that you want to > support[1].  And more could be added in the future without having to > update the linker script syntax. > CRC TABLE is different from the other, since it generates a CRC table and the others generate the checksum. > If the algorithm needs extra data apart from the start and end symbols > then this could be included in the name, eg: > >   DIGEST "CRC64-POLY-0xDEADBEEFDEADBEEF" (ecc_start , ecc_end) I think it make the code a little more complex and thus higher cost of maintenance. Then you have to parse the name in the code, and it seems easier to let 'bison' do the job. Then you would get. DIGEST ""        (ecc_start , ecc_end)               if (get_crc_algo($2, &size, &polynom)) {                     lang_add_crc_syndrome (false, size, polynome);               } else if (get_sha_algo(&size)) {                     lang_add_sha(size);               } Supported algo = { CRC64-ECMA, CRC64-ISO, CRC64-WE, CRC32 } right now. DIGEST  POLY (0xDEADBEEFDEADBEEF, )   (ecc_start , ecc_end)                     lang_add_crc64_syndrome (false, $6, $3); DIGEST  POLYI (0xDEADBEEFDEADBEEF, )   (ecc_start , ecc_end)                     lang_add_crc64_syndrome (true, $6, $3); DIGEST TABLE lang_add_assignment (exp_assign (CRC64_TABLE, exp_nameop (NAME,"."), false)); lang_add_table (); /* A generic version requires that the table is after the DIGEST */ > I would also suggest that functionality should not be restricted to > just the .text section, but that it should work for any section. (It > is reasonable to restrict it to only working inside a single section > however). I am not as well educated on different sections in an image except as you guys, so I thought restricting things to the ".text" section should work. The question is how to know which section to parse. Is there a way to tell which section a symbol is located in? Then no directive is needed. You just get the section of CRC_ADDRESS. Otherwise, there needs to be a way of specifying the section. > >> An algorithm typically needs >> *init__tab( poly ) >> calc__inv( const unsigned char *input_str, size_t >> num_bytes ); >> calc_    ( const unsigned char *input_str, size_t >> num_bytes ); >> void lang_add__syndrome(bool invert, bfd_vma poly) >> void lang_add__table(void) > > Since there are going to be several different versions of these > functions, > and possibly more to come in the future, I think that it would be best to > put them into a new file. eg lddigest.c  This might also make it > easier for > other linkers to make use of the code, should they decide to implement a > similar feature. > Yes, that could be done. > > I am going to post some individual reviews of the patches as well, but I > wanted to give my overall impressions here. > The patches need additional testing, so this is for review of the concept only. I checked that the tables generated by CRC64 TABLE and CRC32 TABLE are the same. One bug I found is the crc32_invert variable which should be crc_invert, the same as used for crc64. > Cheers >   Nick > > [1] Well except for TIMESTAMP and DEBUG ON/OFF which are valid new > features in their own rights. > I find them quite useful. I have one more addition which I would like to add, and that is knowledge about flash sectors. I would like to tell the linker ALIGN to the end of the current flash sector. Those are chip dependent, so the linker needs to know the flash organisation of the specific chip for this to work. ============================= BTW, I did some calculation of code size. The initial patch had 350 lines of code, where about 100 are the core CRC64 routines. The CRC calculation is low complexity code which fetches the text section and calculates the CRC over an array of characters. The ielftool application which can insert a CRC into an ELF file is 11-12,000 lines of C code that needs to understand the ELF format and all sectors generated. If someone "invents" a new sector, or otherwise changes the ELF file, then the tool needs to be updated. That is difficult to support. This is the case for ALL file formats and all variants. If there is just one such tool per file format, then with a dozen file formats, there is a need to maintain 130,000-150,000 lines of code. Now, since this tool is not standardized, multiple companies need to maintain their own version, so these 350 lines (or whatever it will end up in) saves millions of lines of code worldwide. Best Regards Ulf Samuelsson