7-zip/p7zip - new fixes - questions
A free file archiver for extremely high compression
Brought to you by:
ipavlov
Menu
▾
▴
In order to compile p7zip with the sun compiler for Solaris 8,
I have to make some fixes.
7zip/Archive/GZip/GZipHandler.cpp, line 61 :
enum // PropID
{
// kpidExtraIsPresent = kpidUserDefined,
// kpidExtraFlags,
// kpidIsText
};
This useless code do not compile.
Fix : delete these lines ...
in ./7zip/UI/Common/ArchiveCommandLine.h :
void Parse1(const UStringVector commandStrings, CArchiveCommandLineOptions &options);
but in ./7zip/UI/Common/ArchiveCommandLine.cpp :
void CArchiveCommandLineParser::Parse1( UStringVector commandStrings, CArchiveCommandLineOptions &options)
the sun compiler cannot link 7za.exe because there are two different prototypes
for the same function ...
Fix: use the same prototype
example : void Parse1(const UStringVector &commandStrings, CArchiveCommandLineOptions &options);
(keep the "const" and add the "&").
Igor, you have said :
"About 4.27 beta. Estimated release date: August 26. I need to fix some thing there. For example, PPMd doesn't work in 64-bit sytems."
What about the fix for PPMd ?
What about the Estimated release date ?
Thanks for fixes.
> What about the fix for PPMd ?
It's pretty complex to fix it now. PPMd uses pointers and hardcoded that sizeof(void *)=4.
Moreover it can place structures in any place in byte array, so changing pointers to array + indices is not so simple also.
I'll think about fixes in next 3 days. If I will not find simple solution, maybe I'll remove PPMD from 64-bit version. And I'll return it when it will be fixed.
> What about the Estimated release date ?
August 31.
> It's pretty complex to fix it now. PPMd uses pointers and hardcoded that sizeof(void *)=4.
> Moreover it can place structures in any place in byte array, so changing pointers to array + indices is not so simple also.
I was thinking that the fix "array + indices" would be easy for you.
> maybe I'll remove PPMD from 64-bit version. And I'll return it when it will be fixed.
What a pity !
NEW NEEDED FIX :
to fix the bug https://sourceforge.net/tracker/?func=detail&atid=660493&aid=1273057&group_id=111810
file 7zip/Archive/7z/7zOut.cpp, line 144,
HRESULT COutArchive::SkeepPrefixArchiveHeader()
{
#ifdef _7Z_VOL
if (_endMarker)
return S_OK;
#endif
if (!Stream) // FIX for #1273057 <========
return E_FAIL; <========
return Stream->Seek(24, STREAM_SEEK_CUR, NULL);
}
With the command "7z -so -t7z a dummy.7z file >output.7z"
Stream is NULL !
In 7zUpdate.cpp
COutArchive archive;
archive.Create(seqOutStream, false); <==== here, already an error that is not tested (Stream = NULL)
RINOK(archive.SkeepPrefixArchiveHeader());
Please check fixed version:
http://www.7-zip.org/alpha/ppmd427a1.zip
I've checked it only in win32.
> Please check fixed version:
> http://www.7-zip.org/alpha/ppmd427a1.zip
"OK" : means that my (little) PPMD test passes
Your new PPMD code :
Linux (Fedora 4), x86 , gcc 4.0.0 : OK
Linux (Fedora 4), x86_64, gcc 4.0.1 : OK
Solaris 9, sparc, gcc 3.3.2, 32bits build : OK
Solaris 9, sparc, Sun WorkShop 6 update 2, 64bits build : core dump
Your new PPMD code without the "pack" directives :
Linux (Fedora 4), x86 , gcc 4.0.0 : core dump
Your code needs a compiler/architecture that can "pack" !
I was thinking that the attribute "pack" was only for space optimization,
but the attribute "pack" is needed for the correctness of the PPMD algo ...
For portability, can you make a code that does not rely on the attribute "pack" ?
(But you can use the attribute "pack" to optimize needed memory").
New 64 bits fix :
Common/MyGuidDef.h, line 7 :
unsigned long Data1
becomes
unsigned int Data1
because aligneof(long) for 64 bits Solaris programs does not support
item.ClassID = *(const GUID *)prop.bstrVal (ArchiverInfo.cpp, line 303).
You can also fix "long" for "LONG" in your code,
example :
PropVariant.h : CPropVariant(long value, VARTYPE vtSrc = VT_I4)
(VT_I4 but sizeof(long)=8 on Solaris 64bits !)
becomes
CPropVariant(LONG value, VARTYPE vtSrc = VT_I4) ?
Remark : I have Visual Studio 2005 beta2 but I am disapointed because
there are no Win64 projects provided by the "new project" wizard ;)
I hope that the final version of VS2005 will be more friendly
for 64bits development !
Do you know how to add (find on the Net) 64bits projects for the wizard
like Win64 Console, Win64 GUI, Win64 MFC, ... ?
Theses projects should know "32 Debug", "32 Release", "64 Debug", "64 Release".
> For portability, can you make a code that does not rely on the attribute "pack" ?
Now I can't do it. Probably it's pretty complex task.
PPMD uses internal memory manager. And if sizeof(of some structures) != 12, it will not work.
> Do you know how to add (find on the Net) 64bits projects for the wizard
like Win64 Console, Win64 GUI, Win64 MFC, ... ?
Configuration manager / active solution platform / New ... .
But I compile from command line with latest Sdk:
sdk64a.BAT:
set CPU=AMD64
set SdkDir=E:\SDK
set SdkBinDir=%SdkDir%\Bin
set SdkBin64Dir=%SdkBinDir%\Win64\x86\AMD64
set PATH=%SdkBin64Dir%;%SdkBinDir%;%PATH%
set INCLUDE=%SdkDir%\INCLUDE;%SdkDir%\INCLUDE\crt;%INCLUDE%
set LIB=%SdkDir%\LIB\AMD64;%SdkDir%\LIB;%LIB%
%*
compileAMD64.bat:
set CFLAGS=-DUNICODE -D_UNICODE
sdk64a.BAT call compile.bat %*
compile.bat:
pushd ..\..\..\C\7zip\ nmake TARGETS="%*"
popd
What about re-arrange your data ?
For example :
#pragma pack(1)
struct {
BYTE d1;
BYTE d2;
UINT32 d3;
} data[MAX];
date[index].d1 = 0;
becomes
// no pack attribute
struct
BYTE d1[MAX];
BYTE d2[MAX];
UINT32 d3[MAX];
} data;
date.d1[index] = 0;
I do not know if this code is slower than the first code
because of the processor cache ?
Probably it's impossible for PPMD. PPMD allocates 12-byte structures from internal memory pool. And it can not be changed.
please see the tracker "patch" https://sourceforge.net/tracker/index.php?func=detail&aid=1275135&group_id=14481&atid=314481
I upload the source for a new code for PPMD that does not need "pack" attributes.
Check this line:
Successor = (PPM_CONTEXT*) SubAllocator.pText;
So does this line work?
Note that pText is pointer to Byte.
Actually now I don't understand these lines in PPMD, so I can't change them.
> Check this line:
> Successor = (PPM_CONTEXT*) SubAllocator.pText;
SubAllocator.pText is "Byte *"
Successor is "PPM_CONTEXT*"
So "Successor = (PPM_CONTEXT*) SubAllocator.pText" always works because they are pointers.
Successor can have an unaligned address for "PPM_CONTEXT".
The problem only arises when you try something like "*Successor".
In CInfo::UpdateModel, Successor is always used like an address (number)
and not like "*pointer".
> Actually now I don't understand these lines in PPMD, so I can't change them.
Actually, I don't understand how PPMD works !
> The problem only arises when you try something like "*Successor"
And how "*Successor" can work if Successor is unaligned on sparc?
>>The problem only arises when you try something like "*Successor"
> And how "*Successor" can work if Successor is unaligned on sparc?
If you compile with "CC -fast", "*Successor" raises a SIGBUS.
If you compile with "CC -fast -misalign", *Successor" works
but all the code is slower ...
gcc for sparc seems to always use "-misalign" when it compiles.
I have worked on a HP-UX with a PA-RISC processor,
there is no "-misalign" flag, so PPMd should not work for this processor.
But who still buys this kind of machine ?
Remark : this processor has the stricter align needs that I know,
for example alignof(double)=sizeof(double)=8 !
The PPMD code is a nightmare !
I have not found with "google" the PPMd code in Java.
Do you plan to port PPMd code to C# in order to
have a full C# reader for 7-zip archives ?
> I have worked on a HP-UX with a PA-RISC processor,
there is no "-misalign" flag, so PPMd should not work for this processor.
Even with your patch?
> Do you plan to port PPMd code to C# in order to
have a full C# reader for 7-zip archives ?
Now I'm not ready. Now I understand only about 30% of PPMD code.
BTW, RAR also uses PPMD. So now it can not be compiled for 64-bit too. I'll try to add same fixes to rar's code.
> there is no "-misalign" flag, so PPMd should not work for this processor.
> Even with your patch?
Yes, even with my patch ...
> BTW, RAR also uses PPMD. So now it can not be compiled for 64-bit too.
>I'll try to add same fixes to rar's code.
I have read the code from unrarsrc-3.5.3.tar.bz2.
I think that this code support sizeof(PPM_CONTEXT)!=12.
In model.hpp :
const uint UNIT_SIZE=sizeof(PPM_CONTEXT);
const uint FIXED_UNIT_SIZE=12;
suballoc.cpp uses these 2 constants.
Perharps, PPMD from 7-zip should use the same trick than PPMD from unrar ?
PPMD runs fine on x64. You only have to adjust the unit size in PPMDSubAlloc.h:
#ifdef WIN64
const UINT UNIT_SIZE=20, N_INDEXES=N1+N2+N3+N4;
#else
const UINT UNIT_SIZE=12, N_INDEXES=N1+N2+N3+N4;
#endif
and replace U2B:
UINT U2B(int NU) { return UNIT_SIZE * NU; }
But maybe it will be incompatible with 32-bit version. Try to compress 1-4 GB with 32-bit version. And then "Test" archive with such 64-bit version.
> But maybe it will be incompatible with 32-bit version.
Why ?
If 7-zip limits the dictionnary size (your GUI limits the size to 1536 MB),
The 64bits version will always create 7-zip archive that can be
use in a 32bits OS.
> Try to compress 1-4 GB with 32-bit version.
> And then "Test" archive with such 64-bit version.
My machine only have 1GB !
I don't understand the purpose of this test.
PPMd have a limited dictionary size.
If the distionary size is 192MB (given by the archive to extract
or by an option to create an archive),
then 7-zip will use (192*sizeof(PPM_CONTEXT)/FIXED_SIZE) MB.
FIXED_SIZE=12.
Remark : A 64bits program always uses more memory than a 32bits program
when this program need to store pointer !
7-zip PPMd can make a mix :
- keep offset in PPM_CONTEXT to always have 32bits "pointer"
on x86 and x86_64, sizeof(PPM_CONTEXT)=12.
- use the trick of unrar to support sizeof(PPM_CONTECT)!=12
on sparc, sizeof(PPM_CONTEXT)>12.
- drop my "Unaligned32" fix.
I have done a little benchmark : 7za t archive_ppmd.7z on a x86 machine.
PPMD 4.26 : 0m58
PPMD 64 (pavlov) : 0m59
PPMD 64 (myspace) : 1m01
> - use the trick of unrar to support sizeof(PPM_CONTECT)!=12
struct PPM_CONTEXT
{
UInt16 NumStats,SummFreq;
UInt32 Stats;
UInt32 Suffix;
}
Why sizeof(PPM_CONTECT) can be !=12 ?
Please check new version:
http://www.7-zip.org/alpha/ppmd427a2.zip
Do I need "#pragma pack(1)" for that code?
What do you think?
>Please check new version:
> http://www.7-zip.org/alpha/ppmd427a2.zip
> Do I need "#pragma pack(1)" for that code?
> What do you think?
I have downloaded ppmd427a2.zip and delete "#pragma pack"
and _PACK_ATTR.
I have tested your new code on:
linux , x86 , Fedora 4
linux , x86_64 , Fedora 4
Solaris, sparc_64 (with "-fast" flag, but without the "-misalign" flag)
All my PPMD TEST are OK for these 3 platforms :)
So you can delete all "#pragma pack" and _PACK_ATTR.
Remark : the PPMD code for unrar is clearer about what
the 12 Bytes chunks can contain.
I take the worse case : alignof(type)=sizeof(type)
The 12 Bytes chunk can be :
struct PPM_CONTEXT
{
UInt16 NumStats;
UInt16 SummFreq;
UInt32 Stats;
UInt32 Suffix;
}
=> no align problem.
The 12 Bytes chunk can be :
struct MEM_BLK
{
UInt16 Stamp, NU;
UInt32 Next, Prev;
};
=> no problem.
The 12 Bytes chunk can be :
struct MEM_BLK
{
UInt32 Next; // was NODE *Next;
UInt32 garbage1;
UInt32 garbage2;
};
=> no align problem.
Remark : With ppmd427a2.zip, "NODE *Next" cannot be unaligned.
Before ppmd427a2.zip, if sizeof(NODE *)==8 and
MEM_BLK[0].Next is aligned
then MEM_BLK[1].Next is unaligned (and crashed on Solaris
without "-misalign")
Something the program use "SummFreq" and "Stats" from PPM_CONTEXT
like a STATE type.
UInt16 SummFreq;
UInt32 Stats;
becomes
Byte Symbol, Freq;
UInt16 SuccessorLow;
UInt16 SuccessorHigh;
=> not align problem
Very Good work :)
Remark : in PPMDSubAlloc.h, StartSubAllocator does :
Byte *ptr = malloc(...);
Byte *base = ptr - 1;
According to C FAQ (http://www.eskimo.com/~scs/C-faq/q6.17.html),
this code has an undefined behavior.
But, I think that common architectures support this
because
- they use a linear address instead of a segment-offset address
- the address "0" is invalid so "address - 1" >= 0.
Conclusion : keep ppmd427a2.zip and delete all "pack" stuff.
> Remark : the PPMD code for unrar is clearer about what the 12 Bytes chunks can contain.
Probably I'll implement it later.
> According to C FAQ (http://www.eskimo.com/~scs/C-faq/q6.17.html),
this code has an undefined behavior.
I've fixed it:
http://www.7-zip.org/alpha/ppmd427a3.zip
>> Remark : the PPMD code for unrar is clearer about what the 12 Bytes chunks can contain.
>Probably I'll implement it later.
nothing urgent ;)
>> According to C FAQ, this code has an undefined behavior.
> I've fixed it:
> http://www.7-zip.org/alpha/ppmd427a3.zip
I have tested this version on :
- linux ,x86 , Fedora 4 : OK (1)
- linux ,x86_64 , Fedora 4 : OK (1)
- Solaris 9 ,sparc_64 : OK
- MacOSX 10.1 , powerpc : OK (2)
(1) valgrind detects no errors
(2) sizeof(void *)=4, MacOSX 10 is only a 32bits system ?
New fix needed for MacOSX :
---------------------------
Rar20/Rar20Decoder.h, line 13, add "#undef Free" :
#undef Free // to avoid error on MacOSX : ../../Common/InBuffer.h:36: macro `Free' used without args
#include "../../Common/InBuffer.h"
because, /usr/include/net/radix.h for MacOSX defines :
#define Free(p) free((char *)p);
> New fix needed for MacOSX :
But I have many places where I use Free()
> But I have many places where I use Free()
Yes, I know.
But only Rar20/Rar20Decoder.h needs this fix...
surely Rar20Decoder.h includes headers that include headers that include headers ... that define "Free()" ...
I don't know how to find where and when "Free()" is defined :(