lzma: update to lzma sdk 9.20

PATCH New Simon Glass 7 participants 19 messages Nov 3, 2012 – Dec 7, 2012

Subsystems (2)

Library code Generic string library

lore

Share & tools

lore

Simon Glass

author

2012-11-03 21:45 UTC #0 lore raw

PATCH 1/3 lzma: update to lzma sdk 9.20

From: Stefan Reinauer <[email protected]>

Updated code taken from latest lzma sdk release 9.20 at
http://downloads.sourceforge.net/sevenzip/lzma920.tar.bz2

This generates quite a lot of checkpatch warnings, but I guess we need to keep the code style as is to avoid a massive job each time we update this.

Signed-off-by Stefan Reinauer <[email protected]>

Signed-off-by Simon Glass <[email protected]>

---
 lib/lzma/LzmaDec.c   |   14 +++-----------
 lib/lzma/LzmaDec.h   |    6 +++---
 lib/lzma/Types.h     |   36 +++++++++++++++++++++++++++++++-----
 lib/lzma/history.txt |   35 +++++++++++++++++++++++++++++++++++
 lib/lzma/lzma.txt    |   34 +++++++++++++++++++---------------
 5 files changed, 91 insertions(+), 34 deletions(-)

lib/lzma/LzmaDec.c +3 ~~-11~~

index f941da2..4f45f80 100644
@@ -1,5 +1,5 @@
 /* LzmaDec.c -- LZMA Decoder
-2008-11-06 : Igor Pavlov : Public domain */
+2009-09-20 : Igor Pavlov : Public domain */
 
 #include <config.h>
 #include <common.h>

@@ -116,12 +116,6 @@
 StopCompilingDueBUG
 #endif
 
-static const Byte kLiteralNextStates[kNumStates * 2] =
-{
-  0, 0, 0, 0, 1, 2, 3,  4,  5,  6,  4,  5,
-  7, 7, 7, 7, 7, 7, 7, 10, 10, 10, 10, 10
-};
-
 #define LZMA_DIC_MIN (1 << 12)
 
 /* First LZMA-symbol is always decoded.

@@ -180,6 +174,7 @@ static int MY_FAST_CALL LzmaDec_DecodeReal(CLzmaDec *p, SizeT limit, const Byte
 
       if (state < kNumLitStates)
       {
+        state -= (state < 4) ? state : 3;
         symbol = 1;
 
         WATCHDOG_RESET();

@@ -190,6 +185,7 @@ static int MY_FAST_CALL LzmaDec_DecodeReal(CLzmaDec *p, SizeT limit, const Byte
       {
         unsigned matchByte = p->dic[(dicPos - rep0) + ((dicPos < rep0) ? dicBufSize : 0)];
         unsigned offs = 0x100;
+        state -= (state < 10) ? 3 : 6;
         symbol = 1;
 
         WATCHDOG_RESET();

@@ -207,9 +203,6 @@ static int MY_FAST_CALL LzmaDec_DecodeReal(CLzmaDec *p, SizeT limit, const Byte
       }
       dic[dicPos++] = (Byte)symbol;
       processedPos++;
-
-      state = kLiteralNextStates[state];
-      /* if (state < 4) state = 0; else if (state < 10) state -= 3; else state -= 6; */
       continue;
     }
     else

@@ -395,7 +388,6 @@ static int MY_FAST_CALL LzmaDec_DecodeReal(CLzmaDec *p, SizeT limit, const Byte
         else if (distance >= checkDicSize)
           return SZ_ERROR_DATA;
         state = (state < kNumStates + kNumLitStates) ? kNumLitStates : kNumLitStates + 3;
-        /* state = kLiteralNextStates[state]; */
       }
 
       len += kMatchMinLen;

lib/lzma/LzmaDec.h +3 -3

index 7fba87f..63aa505 100644
@@ -1,8 +1,8 @@
 /* LzmaDec.h -- LZMA Decoder
-2008-10-04 : Igor Pavlov : Public domain */
+2009-02-07 : Igor Pavlov : Public domain */
 
-#ifndef __LZMADEC_H
-#define __LZMADEC_H
+#ifndef __LZMA_DEC_H
+#define __LZMA_DEC_H
 
 #include "Types.h"

lib/lzma/Types.h +31 -5

index 1af5cfc..8afcba5 100644
@@ -1,5 +1,5 @@
 /* Types.h -- Basic types
-2008-11-23 : Igor Pavlov : Public domain */
+2010-10-09 : Igor Pavlov : Public domain */
 
 #ifndef __7Z_TYPES_H
 #define __7Z_TYPES_H

@@ -65,9 +65,11 @@ typedef unsigned long UInt64;
 #if defined(_MSC_VER) || defined(__BORLANDC__)
 typedef __int64 Int64;
 typedef unsigned __int64 UInt64;
+#define UINT64_CONST(n) n
 #else
 typedef long long int Int64;
 typedef unsigned long long int UInt64;
+#define UINT64_CONST(n) n ## ULL
 #endif
 
 #endif

@@ -92,13 +94,11 @@ typedef int Bool;
 #endif
 
 #define MY_CDECL __cdecl
-#define MY_STD_CALL __stdcall
-#define MY_FAST_CALL MY_NO_INLINE __fastcall
+#define MY_FAST_CALL __fastcall
 
 #else
 
 #define MY_CDECL
-#define MY_STD_CALL
 #define MY_FAST_CALL
 
 #endif

@@ -108,6 +108,16 @@ typedef int Bool;
 
 typedef struct
 {
+  Byte (*Read)(void *p); /* reads one byte, returns 0 in case of EOF or error */
+} IByteIn;
+
+typedef struct
+{
+  void (*Write)(void *p, Byte b);
+} IByteOut;
+
+typedef struct
+{
   SRes (*Read)(void *p, void *buf, size_t *size);
     /* if (input(*size) != 0 && output(*size) == 0) means end_of_stream.
        (output(*size) < input(*size)) is allowed */

@@ -140,7 +150,7 @@ typedef struct
 
 typedef struct
 {
-  SRes (*Look)(void *p, void **buf, size_t *size);
+  SRes (*Look)(void *p, const void **buf, size_t *size);
     /* if (input(*size) != 0 && output(*size) == 0) means end_of_stream.
        (output(*size) > input(*size)) is not allowed
        (output(*size) < input(*size)) is allowed */

@@ -205,4 +215,20 @@ typedef struct
 #define IAlloc_Alloc(p, size) (p)->Alloc((p), size)
 #define IAlloc_Free(p, a) (p)->Free((p), a)
 
+#ifdef _WIN32
+
+#define CHAR_PATH_SEPARATOR '\\'
+#define WCHAR_PATH_SEPARATOR L'\\'
+#define STRING_PATH_SEPARATOR "\\"
+#define WSTRING_PATH_SEPARATOR L"\\"
+
+#else
+
+#define CHAR_PATH_SEPARATOR '/'
+#define WCHAR_PATH_SEPARATOR L'/'
+#define STRING_PATH_SEPARATOR "/"
+#define WSTRING_PATH_SEPARATOR L"/"
+
+#endif
+
 #endif

lib/lzma/history.txt +35 -0

index aadf825..443511b 100644
@@ -1,6 +1,41 @@
 HISTORY of the LZMA SDK
 -----------------------
 
+9.18 beta      2010-11-02
+-------------------------
+- New small SFX module for installers (SfxSetup).
+
+
+9.12 beta      2010-03-24
+-------------------------
+- The BUG in LZMA SDK 9.* was fixed: LZMA2 codec didn't work,
+  if more than 10 threads were used (or more than 20 threads in some modes).
+
+
+9.11 beta      2010-03-15
+-------------------------
+- PPMd compression method support
+
+
+9.09           2009-12-12
+-------------------------
+- The bug was fixed:
+   Utf16_To_Utf8 funstions in UTFConvert.cpp and 7zMain.c
+   incorrectly converted surrogate characters (the code >= 0x10000) to UTF-8.
+- Some bugs were fixed
+
+
+9.06           2009-08-17
+-------------------------
+- Some changes in ANSI-C 7z Decoder interfaces.
+
+
+9.04           2009-05-30
+-------------------------
+- LZMA2 compression method support
+- xz format support
+
+
 4.65           2009-02-03
 -------------------------
 - Some minor fixes

lib/lzma/lzma.txt +19 ~~-15~~

index aa20f9d..144cd9a 100644
@@ -1,4 +1,4 @@
-LZMA SDK 4.65
+LZMA SDK 9.20
 -------------
 
 LZMA SDK provides the documentation, samples, header files, libraries,

@@ -20,6 +20,10 @@ LICENSE
 
 LZMA SDK is written and placed in the public domain by Igor Pavlov.
 
+Some code in LZMA SDK is based on public domain code from another developers:
+  1) PPMd var.H (2001): Dmitry Shkarin
+  2) SHA-256: Wei Dai (Crypto++ library)
+
 
 LZMA SDK Contents
 -----------------

@@ -33,7 +37,7 @@ LZMA SDK includes:
 UNIX/Linux version
 ------------------
 To compile C++ version of file->file LZMA encoding, go to directory
-C++/7zip/Compress/LZMA_Alone
+CPP/7zip/Bundles/LzmaCon
 and call make to recompile it:
   make -f makefile.gcc clean all

@@ -49,6 +53,7 @@ lzma.txt     - LZMA SDK description (this file)
 7zC.txt      - 7z ANSI-C Decoder description
 methods.txt  - Compression method IDs for .7z
 lzma.exe     - Compiled file->file LZMA encoder/decoder for Windows
+7zr.exe      - 7-Zip with 7z/lzma/xz support.
 history.txt  - history of the LZMA SDK

@@ -66,7 +71,7 @@ C/  - C files
         LzmaEnc.*  - LZMA encoding
         LzmaLib.*  - LZMA Library for DLL calling
         Types.h    - Basic types for another .c files
-	Threads.*  - The code for multithreading.
+        Threads.*  - The code for multithreading.
 
     LzmaLib  - LZMA Library (.DLL for Windows)

@@ -86,12 +91,6 @@ CPP/ -- CPP files
 
     Compress - files related to compression/decompression
 
-      Copy         - Copy coder
-      RangeCoder   - Range Coder (special code of compression/decompression)
-      LZMA         - LZMA compression/decompression on C++
-      LZMA_Alone   - file->file LZMA compression/decompression
-      Branch       - Filters for x86, IA-64, ARM, ARM-Thumb, PowerPC and SPARC code
-
     Archive - files related to archiving
 
       Common   - common files for archive handling

@@ -100,6 +99,7 @@ CPP/ -- CPP files
     Bundles    - Modules that are bundles of other modules
 
       Alone7z           - 7zr.exe: Standalone version of 7z.exe that supports only 7z/LZMA/BCJ/BCJ2
+      LzmaCon           - lzma.exe: LZMA compression/decompression
       Format7zR         - 7zr.dll: Reduced version of 7za.dll: extracting/compressing to 7z/LZMA/BCJ/BCJ2
       Format7zExtractR  - 7zxr.dll: Reduced version of 7zxa.dll: extracting from 7z/LZMA/BCJ/BCJ2.

@@ -369,8 +369,8 @@ Interface:
     propData - LZMA properties  (5 bytes)
     propSize - size of propData buffer (5 bytes)
     finishMode - It has meaning only if the decoding reaches output limit (*destLen).
-	 LZMA_FINISH_ANY - Decode just destLen bytes.
-	 LZMA_FINISH_END - Stream must be finished after (*destLen).
+         LZMA_FINISH_ANY - Decode just destLen bytes.
+         LZMA_FINISH_END - Stream must be finished after (*destLen).
                            You can use LZMA_FINISH_END, when you know that
                            current output buffer covers last bytes of stream.
     alloc    - Memory allocator.

@@ -431,7 +431,7 @@ Memory Requirements:
   {
     ...
     int res = LzmaDec_DecodeToBuf(CLzmaDec *p, Byte *dest, SizeT *destLen,
-	const Byte *src, SizeT *srcLen, ELzmaFinishMode finishMode);
+        const Byte *src, SizeT *srcLen, ELzmaFinishMode finishMode);
     ...
   }

@@ -527,7 +527,8 @@ static ISzAlloc g_Alloc = { SzAlloc, SzFree };
   LzmaEnc_Destroy(enc, &g_Alloc, &g_Alloc);
 
 
-If callback function return some error code, LzmaEnc_Encode also returns that code.
+If callback function return some error code, LzmaEnc_Encode also returns that code
+or it can return the code like SZ_ERROR_READ, SZ_ERROR_WRITE or SZ_ERROR_PROGRESS.
 
 
 Single-call RAM->RAM Compression

@@ -549,8 +550,8 @@ Return code:
 
 
 
-LZMA Defines
-------------
+Defines
+-------
 
 _LZMA_SIZE_OPT - Enable some optimizations in LZMA Decoder to get smaller executable code.

@@ -562,6 +563,9 @@ _LZMA_UINT32_IS_ULONG  - Define it if int is 16-bit on your compiler and long is
 _LZMA_NO_SYSTEM_SIZE_T  - Define it if you don't want to use size_t type.
 
 
+_7ZIP_PPMD_SUPPPORT - Define it if you don't want to support PPMD method in AMSI-C .7z decoder.
+
+
 C++ LZMA Encoder/Decoder
 ~~~~~~~~~~~~~~~~~~~~~~~~
 C++ LZMA code use COM-like interfaces. So if you want to use it,

Signature · 1 line

1.7.7.3

Simon Glass

author

2012-11-03 21:45 UTC #1 lore raw

PATCH 2/3 md5: Fix gcc-4.7 build problem in md5

From: Han Shen <[email protected]>

Fixed by replacing pointer casting with memcpy.

Signed-off-by Simon Glass <[email protected]>

---
 lib/md5.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

lib/md5.c +1 -2

index 2ae4a06..9791e59 100644
@@ -153,8 +153,7 @@ MD5Final(unsigned char digest[16], struct MD5Context *ctx)
 	byteReverse(ctx->in, 14);
 
 	/* Append length in bits and transform */
-	ctx->in32[14] = ctx->bits[0];
-	ctx->in32[15] = ctx->bits[1];
+	memcpy(ctx->in + 14 * sizeof(__u32), ctx->bits, 2 * sizeof(__u32));
 
 	MD5Transform(ctx->buf, (__u32 *) ctx->in);
 	byteReverse((unsigned char *) ctx->buf, 4);

Signature · 1 line

1.7.7.3

Wolfgang Denk

2012-11-04 00:32 UTC #2 lore raw

PATCH 2/3 md5: Fix gcc-4.7 build problem in md5

Dear Simon Glass,

Quoted 14 lines · In message <[email protected]> you wrote:

From: Han Shen <[email protected]>
Fixed by replacing pointer casting with memcpy.
Signed-off-by Simon Glass <[email protected]>
---
 lib/md5.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)
lib/md5.c +0 -0
index 2ae4a06..9791e59 100644

lib/md5.c MD5Final(unsigned char digest[16], struct MD5Context *ctx) · line 156

Quoted 6 lines

lib/md5.c +1 -2

@@ -153,8 +153,7 @@ MD5Final(unsigned char digest[16], struct MD5Context *ctx)
 	byteReverse(ctx->in, 14);
 
 	/* Append length in bits and transform */
-	ctx->in32[14] = ctx->bits[0];
-	ctx->in32[15] = ctx->bits[1];
+	memcpy(ctx->in + 14 * sizeof(__u32), ctx->bits, 2 * sizeof(__u32));

This makes the code actually unreadable. Please add at least a comment what this is doing.

Best regards,

Wolfgang Denk

Signature · 4 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Of course there's no reason for it, it's just our policy.

Wolfgang Denk

2012-11-04 00:39 UTC #3 lore raw

PATCH 2/3 md5: Fix gcc-4.7 build problem in md5

Dear Simon,

Quoted 8 lines · In message <[email protected]> I wrote:

lib/md5.c MD5Final(unsigned char digest[16], struct MD5Context *ctx) · line 156
Quoted 5 lines
lib/md5.c +1 -2
 	/* Append length in bits and transform */
-	ctx->in32[14] = ctx->bits[0];
-	ctx->in32[15] = ctx->bits[1];
+	memcpy(ctx->in + 14 * sizeof(__u32), ctx->bits, 2 * sizeof(__u32));
This makes the code actually unreadable. Please add at least a comment what this is doing.

Actually I think this shoul dbe split into two memcpy commands, using the addresses of the respective array elements directly, without such manual pointer arithmetics.

Best regards,

Wolfgang Denk

Signature · 5 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"The number  of  Unix  installations  has  grown  to  10,  with  more
expected."    - The Unix Programmer's Manual, 2nd Edition, June, 1972

Pavel Machek

2012-11-05 20:03 UTC #4 lore raw

PATCH 2/3 md5: Fix gcc-4.7 build problem in md5

Hi!

Quoted 10 lines

Quoted 8 lines · In message <[email protected]> I wrote:
lib/md5.c MD5Final(unsigned char digest[16], struct MD5Context *ctx) · line 156
Quoted 5 lines
lib/md5.c +1 -2
 	/* Append length in bits and transform */
-	ctx->in32[14] = ctx->bits[0];
-	ctx->in32[15] = ctx->bits[1];
+	memcpy(ctx->in + 14 * sizeof(__u32), ctx->bits, 2 * sizeof(__u32));
This makes the code actually unreadable. Please add at least a comment what this is doing.

Quoted 3 lines

Actually I think this shoul dbe split into two memcpy commands, using the addresses of the respective array elements directly, without such manual pointer arithmetics.

I guess bigger question is: why does gcc miscompile that, and is it guaranteed that it will not miscompile the memcpy?

Is compiler barrier somewhere better solution?
Pavel

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Wolfgang Denk

2012-11-05 20:50 UTC #5 lore raw

PATCH 2/3 md5: Fix gcc-4.7 build problem in md5

Dear Pavel,

Quoted 9 lines · In message <[email protected]> you wrote:

Quoted 9 lines
Quoted 8 lines
lib/md5.c MD5Final(unsigned char digest[16], struct MD5Context *ctx) · line 156
Quoted 5 lines
lib/md5.c +1 -2
 	/* Append length in bits and transform */
-	ctx->in32[14] = ctx->bits[0];
-	ctx->in32[15] = ctx->bits[1];
+	memcpy(ctx->in + 14 * sizeof(__u32), ctx->bits, 2 * sizeof(__u32));
This makes the code actually unreadable. Please add at least a comment what this is doing.

Quoted 6 lines

Quoted 3 lines
Actually I think this shoul dbe split into two memcpy commands, using the addresses of the respective array elements directly, without such manual pointer arithmetics.
I guess bigger question is: why does gcc miscompile that, and is it guaranteed that it will not miscompile the memcpy?

I did not see Simon mentioning anythin about incorrect compilation. My understanding was that it's just the usual "dereferencing type-punned pointer" warnings issue.

Simon, what was the actual problem this was supposed to fix?

Best regards,

Wolfgang Denk

Signature · 5 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Let's say the docs present a simplified view of reality...    :-)
                      - Larry Wall in  <[email protected]>

Han Shen(沈涵)

2012-11-05 21:15 UTC #6 lore raw

PATCH 2/3 md5: Fix gcc-4.7 build problem in md5

Quoted 17 lines · Wolfgang Denk

Dear Pavel,
Quoted 5 lines · In message <[email protected]> you wrote:
Quoted 5 lines
Quoted 5 lines
lib/md5.c MD5Final(unsigned char digest[16], struct MD5Context *ctx)
Quoted 5 lines
        /* Append length in bits and transform */
lib/md5.c +1 -2
-       ctx->in32[14] = ctx->bits[0];
-       ctx->in32[15] = ctx->bits[1];
+       memcpy(ctx->in + 14 * sizeof(__u32), ctx->bits, 2 *
sizeof(__u32));
Quoted 8 lines
Quoted 7 lines
Quoted 3 lines
This makes the code actually unreadable. Please add at least a comment what this is doing.
Actually I think this shoul dbe split into two memcpy commands, using the addresses of the respective array elements directly, without such manual pointer arithmetics.

Quoted 7 lines

Quoted 2 lines
I guess bigger question is: why does gcc miscompile that, and is it guaranteed that it will not miscompile the memcpy?
I did not see Simon mentioning anythin about incorrect compilation. My understanding was that it's just the usual "dereferencing type-punned pointer" warnings issue.

Yup, it's a compilation warning. issue. (but warnings being treated as errors.)

Quoted 15 lines

Simon, what was the actual problem this was supposed to fix?
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Let's say the docs present a simplified view of reality... :-)
- Larry Wall in <[email protected]>

Signature · 1 line

 Han Shen |  Software Engineer |  shenhan at google.com |  +1-650-440-3330

Pavel Machek

2012-11-05 22:51 UTC #7 lore raw

PATCH 2/3 md5: Fix gcc-4.7 build problem in md5

Hi!

    In message <[email protected]> you wrote:

Quoted 14 lines

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
lib/md5.c MD5Final(unsigned char digest[16], struct MD5Context *ctx)
Quoted 5 lines
        /* Append length in bits and transform */
lib/md5.c +1 -2
-       ctx->in32[14] = ctx->bits[0];
-       ctx->in32[15] = ctx->bits[1];
+       memcpy(ctx->in + 14 * sizeof(__u32), ctx->bits, 2 *
sizeof(__u32));
Quoted 5 lines
Quoted 5 lines
Quoted 3 lines
This makes the code actually unreadable. Please add at least a comment what this is doing.
Actually I think this shoul dbe split into two memcpy commands,
using
Quoted 1 line
Quoted 1 line
the addresses of the respective array elements directly, without
such

Quoted 8 lines

Quoted 4 lines
Quoted 1 line
manual pointer arithmetics.
I guess bigger question is: why does gcc miscompile that, and is it guaranteed that it will not miscompile the memcpy?
I did not see Simon mentioning anythin about incorrect compilation. My understanding was that it's just the usual "dereferencing type-punned pointer" warnings issue.

Is there some alternate solution? The memcpy is really ugly...

Plus... does it solve the issue? The code does not look like being compatible with strict pointer aliasing... and I don't think memcpy() helps.

arch/nds32/config.mk:PLATFORM_RELFLAGS += -fno-strict-aliasing -fno-common -mrelax arch/x86/config.mk:PLATFORM_CPPFLAGS += -fno-strict-aliasing

We should really do that globally.
Pavel

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Marek Vasut

2012-11-06 00:56 UTC #8 lore raw

PATCH 2/3 md5: Fix gcc-4.7 build problem in md5

Dear Pavel Machek,

Quoted 37 lines

Hi!
    In message <[email protected]> you wrote:
Quoted 19 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Review
Quoted 5 lines
        /* Append length in bits and transform */
(quoted diff) +1 -2
-       ctx->in32[14] = ctx->bits[0];
-       ctx->in32[15] = ctx->bits[1];
+       memcpy(ctx->in + 14 * sizeof(__u32), ctx->bits, 2 *
   sizeof(__u32));
Quoted 4 lines
Quoted 4 lines
Quoted 2 lines
This makes the code actually unreadable. Please add at least a comment what this is doing.
Actually I think this shoul dbe split into two memcpy commands,
using
Quoted 1 line
Quoted 1 line
the addresses of the respective array elements directly, without
such
Quoted 8 lines
Quoted 5 lines
Quoted 1 line
manual pointer arithmetics.
I guess bigger question is: why does gcc miscompile that, and is it guaranteed that it will not miscompile the memcpy?
I did not see Simon mentioning anythin about incorrect compilation. My understanding was that it's just the usual "dereferencing type-punned pointer" warnings issue.
Is there some alternate solution? The memcpy is really ugly...
Plus... does it solve the issue? The code does not look like being compatible with strict pointer aliasing... and I don't think memcpy() helps.

Quoted 6 lines

arch/nds32/config.mk:PLATFORM_RELFLAGS += -fno-strict-aliasing -fno-common -mrelax arch/x86/config.mk:PLATFORM_CPPFLAGS += -fno-strict-aliasing
We should really do that globally.
Pavel

Were you even able to replicate this problem in the first place? Isn't this whole "problem" a problem of a broken (ubuntu/linaro) toolchain again?

Best regards,
Marek Vasut

Pavel Machek

2012-11-06 11:07 UTC #9 lore raw

PATCH 2/3 md5: Fix gcc-4.7 build problem in md5

Quoted 18 lines · Marek Vasut

Dear Pavel Machek,
Quoted 16 lines
Hi!
    In message <[email protected]> you wrote:
Quoted 7 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Review
Quoted 5 lines
        /* Append length in bits and transform */
(quoted diff) +1 -2
-       ctx->in32[14] = ctx->bits[0];
-       ctx->in32[15] = ctx->bits[1];
+       memcpy(ctx->in + 14 * sizeof(__u32), ctx->bits, 2 *
   sizeof(__u32));
Is there some alternate solution? The memcpy is really ugly...
Plus... does it solve the issue? The code does not look like being compatible with strict pointer aliasing... and I don't think memcpy() helps.

Quoted 8 lines

Quoted 5 lines
arch/nds32/config.mk:PLATFORM_RELFLAGS += -fno-strict-aliasing -fno-common -mrelax arch/x86/config.mk:PLATFORM_CPPFLAGS += -fno-strict-aliasing
We should really do that globally.
Were you even able to replicate this problem in the first place? Isn't this whole "problem" a problem of a broken (ubuntu/linaro) toolchain again?

This is not something you can replicate. At least md5 code is unsafe with strict aliasing, probably most of u-boot, because low-level people write code like that. Thus we should do -fno-strict-aliasing. Otherwise compiler may decide in future to "miscompile" our code, even if it compiles it correctly now.

Pavel

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Marek Vasut

2012-11-06 22:30 UTC #10 lore raw

PATCH 2/3 md5: Fix gcc-4.7 build problem in md5

Dear Pavel Machek,

Quoted 31 lines · Marek Vasut

Quoted 24 lines
Dear Pavel Machek,
Quoted 22 lines
Hi!
    In message <[email protected]> you wrote:
Quoted 8 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
        /* Append length in bits and transform */
(quoted diff) +1 -2
-       ctx->in32[14] = ctx->bits[0];
-       ctx->in32[15] = ctx->bits[1];
+       memcpy(ctx->in + 14 * sizeof(__u32), ctx->bits, 2
*
   sizeof(__u32));
Is there some alternate solution? The memcpy is really ugly...
Plus... does it solve the issue? The code does not look like being compatible with strict pointer aliasing... and I don't think memcpy() helps.
arch/nds32/config.mk:PLATFORM_RELFLAGS += -fno-strict-aliasing -fno-common -mrelax arch/x86/config.mk:PLATFORM_CPPFLAGS += -fno-strict-aliasing
Quoted 5 lines
Quoted 1 line
We should really do that globally.
Were you even able to replicate this problem in the first place? Isn't this whole "problem" a problem of a broken (ubuntu/linaro) toolchain again?

Quoted 7 lines

This is not something you can replicate. At least md5 code is unsafe with strict aliasing, probably most of u-boot, because low-level people write code like that. Thus we should do -fno-strict-aliasing. Otherwise compiler may decide in future to "miscompile" our code, even if it compiles it correctly now.
Pavel

I dont think -fno-strict-aliasing is the way to go, it'll only hide the problem, no?

Best regards,
Marek Vasut

Simon Glass

author

2012-11-07 22:18 UTC #11 lore raw

PATCH 2/3 md5: Fix gcc-4.7 build problem in md5

Hi,

Quoted 39 lines · Marek Vasut

Dear Pavel Machek,
Quoted 37 lines · Marek Vasut
Quoted 24 lines
Dear Pavel Machek,
Quoted 22 lines
Hi!
    In message <[email protected]> you wrote:
Quoted 8 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
        /* Append length in bits and transform */
(quoted diff) +1 -2
-       ctx->in32[14] = ctx->bits[0];
-       ctx->in32[15] = ctx->bits[1];
+       memcpy(ctx->in + 14 * sizeof(__u32), ctx->bits, 2
*
   sizeof(__u32));
Is there some alternate solution? The memcpy is really ugly...
Plus... does it solve the issue? The code does not look like being compatible with strict pointer aliasing... and I don't think memcpy() helps.
arch/nds32/config.mk:PLATFORM_RELFLAGS += -fno-strict-aliasing -fno-common -mrelax arch/x86/config.mk:PLATFORM_CPPFLAGS += -fno-strict-aliasing
Quoted 5 lines
Quoted 1 line
We should really do that globally.
Were you even able to replicate this problem in the first place? Isn't this whole "problem" a problem of a broken (ubuntu/linaro) toolchain again?
This is not something you can replicate. At least md5 code is unsafe with strict aliasing, probably most of u-boot, because low-level people write code like that. Thus we should do -fno-strict-aliasing. Otherwise compiler may decide in future to "miscompile" our code, even if it compiles it correctly now.

Quoted 4 lines

Quoted 1 line
Pavel
I dont think -fno-strict-aliasing is the way to go, it'll only hide the problem, no?

Yes I agree.

I believe this problem might have been a mistake on my part. I think I incorrectly merged Marek's change 'b68d63c GCC47: Fix warning in md5.c' when I was testing. Or it could me that my compiler was broken, as for a while it was generating these errors when it should not.

So I believe we can drop this patch as I am no longer seeing the warning.

If so, sorry for the noise.

Regards,
Simon

Quoted 7 lines

Best regards,
Marek Vasut
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Simon Glass

author

2012-11-03 21:45 UTC #12 lore raw

PATCH 3/3 Add stricmp() and strnicmp()

In reply to Simon Glass

strnicmp() is present but disabled. Make it available and define stricmp() also. There is a only a small performance penalty to having stricmp() call strnicmp(), so do this instead of a standalone function, to save code space.

BRANCH=none

Signed-off-by Simon Glass <[email protected]>

---
 include/linux/string.h |    4 ++++
 lib/string.c           |   12 ++++++++++--
 2 files changed, 14 insertions(+), 2 deletions(-)

include/linux/string.h +4 -0

index 9a8cbc2..77fd1e9 100644
@@ -20,6 +20,10 @@ extern __kernel_size_t strspn(const char *,const char *);
  */
 #include <asm/string.h>
 
+int strnicmp(const char *s1, const char *s2, size_t len);
+
+int stricmp(const char *s1, const char *s2);
+
 #ifndef __HAVE_ARCH_STRCPY
 extern char * strcpy(char *,const char *);
 #endif

lib/string.c +10 -2

index c3ad055..f73df3f 100644
@@ -21,7 +21,6 @@
 #include <malloc.h>
 
 
-#if 0 /* not used - was: #ifndef __HAVE_ARCH_STRNICMP */
 /**
  * strnicmp - Case insensitive, length-limited string comparison
  * @s1: One string

@@ -52,7 +51,16 @@ int strnicmp(const char *s1, const char *s2, size_t len)
 	}
 	return (int)c1 - (int)c2;
 }
-#endif
+
+/**
+ * stricmp - Case insensitive string comparison
+ * @s1: One string
+ * @s2: The other string
+ */
+int stricmp(const char *s1, const char *s2)
+{
+	return strnicmp(s1, s2, -1U);
+}
 
 char * ___strtok;

Signature · 1 line

1.7.7.3

Wolfgang Denk

2012-11-04 00:31 UTC #13 lore raw

PATCH 3/3 Add stricmp() and strnicmp()

Dear Simon Glass,

Quoted 6 lines · In message <[email protected]> you wrote:

strnicmp() is present but disabled. Make it available and define stricmp() also. There is a only a small performance penalty to having stricmp() call strnicmp(), so do this instead of a standalone function, to save code space.
BRANCH=none

Please get rid of such entries in the commit messages!!!!

Consider all patches that contain such entries as NAKed.

include/linux/string.h extern __kernel_size_t strspn(const char *,const char *); · line 25

Quoted 3 lines

include/linux/string.h +3 -0

+int strnicmp(const char *s1, const char *s2, size_t len);
+
+int stricmp(const char *s1, const char *s2);

Who are the users of this? I object against adding dead code.

If users will be added later, this patch should go into the series adding the users.

Best regards,

Wolfgang Denk

Signature · 7 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Severe culture shock results when experts from another protocol suite
[...] try to read OSI documents. The term "osified" is used to  refer
to  such  documents. [...] Any relationship to the word "ossified" is
purely intentional.                                - Marshall T. Rose

Simon Glass

author

2012-11-04 04:38 UTC #14 lore raw

PATCH 3/3 Add stricmp() and strnicmp()

Hi Wolfgang,

Quoted 8 lines · Wolfgang Denk

Dear Simon Glass,
Quoted 5 lines · In message <[email protected]> you wrote:
strnicmp() is present but disabled. Make it available and define stricmp() also. There is a only a small performance penalty to having stricmp() call strnicmp(), so do this instead of a standalone function, to save code space.

Quoted 5 lines

Quoted 1 line
BRANCH=none
Please get rid of such entries in the commit messages!!!!
Consider all patches that contain such entries as NAKed.

Yes it is annoying - I submitted a patch to patman to remove this and another gerrit one also. In the meantime I have to be careful!

Quoted 9 lines

include/linux/string.h extern __kernel_size_t strspn(const char *,const char *); · line 25
Quoted 4 lines
include/linux/string.h +3 -0
+int strnicmp(const char *s1, const char *s2, size_t len);
+
+int stricmp(const char *s1, const char *s2);
Who are the users of this? I object against adding dead code.
If users will be added later, this patch should go into the series adding the users.

OK, messaged received loud and clear. It does require a change of process at my end - now I have to find relationships between commits in different series going to different maintainers and try to tie them together. Just one more thing to worry about.

But I understand your concern that, in fact, if there is no user immediately forthcoming, then it will just sit there and no one will notice if it is dead code.

BTW, is there any easy way to obtain build-coverage information for U-Boot? In other words, can we easily find code that is not enabled by any existing board? That might be an interesting investigation.

Regards,
Simon

Quoted 13 lines

Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Severe culture shock results when experts from another protocol suite [...] try to read OSI documents. The term "osified" is used to refer to such documents. [...] Any relationship to the word "ossified" is
purely intentional. - Marshall T. Rose

Albert ARIBAUD

2012-11-04 22:47 UTC #15 lore raw

PATCH 3/3 Add stricmp() and strnicmp()

Hi Simon,

Quoted 4 lines

OK, messaged received loud and clear. It does require a change of process at my end - now I have to find relationships between commits in different series going to different maintainers and try to tie them together. Just one more thing to worry about.

Consider a 'paradigm shift' here: instead of building series according to intended custodians, one builds them according to functional relationship. If a series needs one custodian more than the others, he'll take it and ask for the others' ack. If there's a tie, this can be resolved between custodians. If one foresees a tie, then one can proactively suggest a resolution.

Quoted 7 lines

But I understand your concern that, in fact, if there is no user immediately forthcoming, then it will just sit there and no one will notice if it is dead code.
BTW, is there any easy way to obtain build-coverage information for U-Boot? In other words, can we easily find code that is not enabled by any existing board? That might be an interesting investigation.

Seconded -- with the added note that we need coverage across all architectures.

Quoted 2 lines

Regards,
Simon

Amicalement,

Signature · 1 line

Albert.

Simon Glass

author

2012-11-07 22:00 UTC #16 lore raw

PATCH 3/3 Add stricmp() and strnicmp()

Hi,

Quoted 18 lines · Albert ARIBAUD

Hi Simon,
Quoted 4 lines
OK, messaged received loud and clear. It does require a change of process at my end - now I have to find relationships between commits in different series going to different maintainers and try to tie them together. Just one more thing to worry about.
Consider a 'paradigm shift' here: instead of building series according to intended custodians, one builds them according to functional relationship. If a series needs one custodian more than the others, he'll take it and ask for the others' ack. If there's a tie, this can be resolved between custodians. If one foresees a tie, then one can proactively suggest a resolution.
Quoted 4 lines
But I understand your concern that, in fact, if there is no user immediately forthcoming, then it will just sit there and no one will notice if it is dead code.

Quoted 6 lines

Quoted 3 lines
BTW, is there any easy way to obtain build-coverage information for U-Boot? In other words, can we easily find code that is not enabled by any existing board? That might be an interesting investigation.
Seconded -- with the added note that we need coverage across all architectures.

This patch is needed by new exynos memory init code. I am copying Hatim so that he is aware that it will need to be sent as part of his memory init series, which I believe is coming soon.

We can leave this for now.

Regards,
Simon

Quoted 7 lines

Quoted 3 lines
Regards,
Simon
Amicalement,
--
Albert.

Simon Glass

author

2012-11-22 02:51 UTC #17 lore raw

PATCH 3/3 Add stricmp() and strnicmp()

Hi,

Quoted 26 lines · Simon Glass

Hi,
Quoted 22 lines · Albert ARIBAUD
Hi Simon,
Quoted 4 lines
OK, messaged received loud and clear. It does require a change of process at my end - now I have to find relationships between commits in different series going to different maintainers and try to tie them together. Just one more thing to worry about.
Consider a 'paradigm shift' here: instead of building series according to intended custodians, one builds them according to functional relationship. If a series needs one custodian more than the others, he'll take it and ask for the others' ack. If there's a tie, this can be resolved between custodians. If one foresees a tie, then one can proactively suggest a resolution.
Quoted 7 lines
But I understand your concern that, in fact, if there is no user immediately forthcoming, then it will just sit there and no one will notice if it is dead code.
BTW, is there any easy way to obtain build-coverage information for U-Boot? In other words, can we easily find code that is not enabled by any existing board? That might be an interesting investigation.

Quoted 8 lines

Quoted 2 lines
Seconded -- with the added note that we need coverage across all architectures.
This patch is needed by new exynos memory init code. I am copying Hatim so that he is aware that it will need to be sent as part of his memory init series, which I believe is coming soon.
We can leave this for now.

As it happens I think I need this for the hashing code, so I will include this patch when I resend the common/ series.

Regards,
Simon

Quoted 11 lines

Regards,
Simon
Quoted 7 lines
Quoted 3 lines
Regards,
Simon
Amicalement,
--
Albert.

Tom Rini

2012-12-07 15:49 UTC #18 lore raw

PATCH 1/3 lzma: update to lzma sdk 9.20

In reply to Simon Glass

Quoted 5 lines · Simon Glass

From: Stefan Reinauer <[email protected]>
Updated code taken from latest lzma sdk release 9.20 at
http://downloads.sourceforge.net/sevenzip/lzma920.tar.bz2

Quoted 7 lines

This generates quite a lot of checkpatch warnings, but I guess we need to keep the code style as is to avoid a massive job each time we update this.
Signed-off-by Stefan Reinauer <[email protected]>
Signed-off-by Simon Glass <[email protected]>

This (and only this part of the series) are now applied to u-boot/master, thanks!

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20121207/b703d94b/attachment.pgp>