#3 sparc/solaris: Bus Error at chmfile.cpp:723

closed-fixed
nobody
None
5
2004-07-06
2004-06-28
No

On a sun sparc sunos 5.7 built, xchm 0.9.1 crashes with:

Program received signal SIGSEGV, Segmentation fault.
CHMFile::InfoFromSystem (this=0xc0858) at chmfile.cpp:723

(tested with a number of different *.chm-files, for
example with windows-XP C:\windows\license.chm).

The reason for the crash seems to be odd values of the
variable index in
CHMFile::InfoFromSystem(). With the patch attached the
crash can be safely pre-recognized and avoided, however
processing of the /#SYSTEM file is simply aborted that way.

Tools used:

gcc 3.2.3
wxGTK 2.4.2
chmlib-0.33
xchm 0.9.1

(BTW, thanks for this very nice tool!)

Markus Schwarzenberg

Discussion

  • hack to pre-recognize and avoid crash described

     
    Attachments
  • Logged In: YES
    user_id=162312

    That's probably because of this assignment at the end of
    GetInfoFromSystem():

    index += *cursor + 2;

    Earlier on, this is done:

    FIXENDIAN16(*cursor);

    FIXENDIAN16() is defined like this:

    #define FIXENDIAN16(x) (x = wxUINT16_SWAP_ON_BE(x))

    Which probably means that either wxWidgets thinks that sunos
    is not big endian, or wxUINT16_SWAP_ON_BE() is not
    implemented right.
    The solution, IMHO is to verify if the system is sunos, and
    more importantly, if it is sparc, because on i386 the
    conversion is not required, and properly define
    FIXENDIAN16() and FIXENDIAN32().
    E.g.:

    #ifdef __sun
    # ifdef sparc
    # define FIXENDIAN16(x) (x = ((((x)&0xff)<<8) |
    ((x>>8)&0xff)))
    # define FIXENDIAN32(x) (x = ((((x)&0xff)<<24) |
    ((x&0xff00)<<8) | ((x>>8)&0xff00)))
    # else
    # define FIXENDIAN16(x) (x)
    # define FIXENDIAN32(x) (x)
    # endif
    #else
    # define FIXENDIAN32(x) (x = wxUINT32_SWAP_ON_BE(x))
    # define FIXENDIAN32(x) (x = wxUINT32_SWAP_ON_BE(x))
    #endif

    Please try that and let me know if it works. But first,
    figure that 'sparc' macro out, I don't know if #ifdef sparc
    is correct. I don't know what's defined on sparc. So you
    should check that out and replace it in the code.
    So I'm not going to incorporate the patch because it doesn't
    solve the problem, it just basically removes #SYSTEM
    detection of .chm info. I will accept a patch similar to the
    above description, where everything works :).

    Let me know how that works out,
    Razvan

     
  • Logged In: YES
    user_id=81393

    > That's probably because of this assignment at the end of
    GetInfoFromSystem():
    >
    > index += *cursor + 2;

    yes, that's the place where the odd index is calculated.

    The proposed FIXENDIAN16 changes DON'T help. Besides this,
    wxUINT16_SWAP_ON_BE seems to work correctly, I verified this
    using the debugger (0x0A00 is converted to 0x00A0, for example)

    May the chm-Files be corrupted? (However aparrantly I get
    the odd index in all *chm-FIles tested until now) Is there a
    known good reference *.chm to go on debugging with?

    BTW, on sun sparc there is defined (among others ;-) :
    sun, __sun, sparc, __sparc, unix, __unix
    for gcc as well as for sun cc (for gcc the __ prefixed
    versions of the above macros are defined, too).

    Markus

     
  • Logged In: YES
    user_id=81393

    oops, two typos, should read:

    0x0A00 is converted to 0x000A, for example

    and

    for gcc the __ postfixed versions of the above macros are
    defined, too

     
  • Logged In: YES
    user_id=162312

    Well I don't know what to say. Obviously, I'd rather have
    the thing fixed that hacked but a) I can't ask you to take
    it upon yourself to read on how #SYSTEM detection is done
    and check it on sunos, and b) I don't have access to a
    sunos/sparc machine, so I'll think about adding the hacked
    stuff into the CVS source code.

     
  • Logged In: YES
    user_id=81393

    Well, perhaps you could attach a wellknown CHM file to this
    bug report together with a list of all index / *cursor
    values in CHMFile::InfoFromSystem for this CHM file and I
    could tell you what happens in sunos. If you have a good
    link on how to read #SYSTEM perhaps I could help better. I
    don't have much time for debugging this, but I do have a
    little, so don't hesitate to ask...

     
  • Logged In: YES
    user_id=162312

    Ok, here's the chmfile.cpp diff/patch, and the output of
    loading the PHP manual.
    Hope they're useful in tracking down the sunos bug.

    Output:

    Index: 0
    Cursor: 10
    Cursor at end: 4
    Index at end: 8
    Index: 8
    Cursor: 9
    Cursor at end: 22
    Index at end: 34
    Index: 34
    Cursor: 4
    Cursor at end: 36
    Index at end: 74
    Index: 74
    Cursor: 2
    Cursor at end: 17
    Index at end: 95
    Index: 95
    Cursor: 3
    Cursor at end: 11
    Index at end: 110
    Index: 110
    Cursor: 16
    Cursor at end: 11
    Index at end: 125
    Index: 125
    Cursor: 6
    Cursor at end: 14
    Index at end: 143
    Index: 143
    Cursor: 5
    Cursor at end: 7
    Index at end: 154
    Index: 154
    Cursor: 7
    Cursor at end: 4
    Index at end: 162
    Index: 162
    Cursor: 12
    Cursor at end: 4
    Index at end: 170
    Index: 170
    Cursor: 13
    Cursor at end: 4096
    Index at end: 4270

     
  • Patch for chmfile; outputs index and *cursor values.

     
    Attachments
    • status: open --> open-accepted
     
  • Patch for chmfile working correct on sparc/solaris, with debugging output (index, cursVal)

     
    Attachments
  • Logged In: YES
    user_id=81393

    Thanks. I think now I know the true reason for the crash:
    You cannot access words at odd adresses on sparc. Besides
    this, actually nothing was wrong with the code discussed. I
    changed chmfile.cpp in the following ways:
    1. cursor is now unsigned char*
    2. at the places, where the byte-order corrected contens of
    *cursor is needed (only twice: for switch() and for
    incrementing index) it is calculated depending on the endianess.
    With the patch attached I get exactly the same output as
    below, in other words: it now works on sparc (only the
    debugging output code needs to be removed, again).

    BTW: Is it really legal on all intel architectures to access
    words at odd adresses?

     
  • supposedly working chmfile.cpp (tested on slackware/i386)

     
    Attachments
  • help to make chmfile.cpp work (added macro to get an unsigned 16-bit int from the 1st 2 bytes of an unsigned char array)

     
    Attachments
  • Logged In: YES
    user_id=162312

    Unfortunately it stopped matching the output on i386 :).
    I changed the source to incorporate your suggestion about
    accessing through a unsigned char pointer, and it now looks
    fine on i386.
    Please check with the attached diff files and let me know if
    it works on SunOS/sparc like that, so that I can close the bug.

    An yes, looks like it's legal on i386+ to access odd memory
    addresses since nobody complained about this and xCHM's been
    around for an year or so.

     
  • Logged In: YES
    user_id=81393

    Stupid me. Just noticed a thinko:

    The way we calculate int(odd adress) is independend on the
    processor type, it depends only on the data when we use:

    value = x[0] | ((u_int16_t)x[1] << 8)

    Thats just the way how a word is calculated from two bytes
    in little endian order.
    Conclusion: There is no need distinguishing between
    little/bigendian here. Just use the odd-address-save method
    above instead of the previous pointer/cast conversion and it
    should work on all platforms.

     
  • Logged In: YES
    user_id=162312

    Right. My mistake too, should have checked the code better
    than just throw a glimpse at it.
    Well thanks for helping out, I'm going to release 0.9.2
    tomorrow. Couldn't have solved this without your help.

     
    • status: open-accepted --> closed-fixed