Fwd: TkHTML 64-bit Windows issue due to Microsoft 'C' Compiler (2005 DevStudio) bug

54 views

Skip to first unread message

Dan Kennedy

unread,

Mar 19, 2010, 12:19:57 AM3/19/10

to tkh...@googlegroups.com, Anil E. Sahukar

Dan,

I have found areas in your code where initializing a pointer-based parameter with a literal integer ‘0’ leads to the Windows x64 crash reported to you by my colleague, Ira some time back.

Analysis:

The compiler improperly pushes a DWORD PTR onto the stack at the call site of the fifth or higher parameter, not the QWORD PTR that is appropriate (params 1-4 are managed in registers and the compiler uses an XOR optimization to avoid pushing bits when the value is 0…we get lucky there).

Line 3932, css.c (call site – with my fix)

        /* If the selector is a match for our node, apply the rule properties */
        nSelectorMatch +=
                applyRule(pTree, pNode, pRule, aPropDone, (char**)0, &sCreator);

As long as the upper portion of the QWORD is 0, all is well. After a few iterations the upper portion gets set to a non-zero value and pzIfMatch is now considered non-zero despite the fact the caller passed 0 for that param.

Line 3752, css.c (error site)
        if (pzIfMatch) {
            HtmlComputedValuesInit(pTree, pNode, pNode, pCreator);
            pCreator->pzContent = pzIfMatch;
        }

When this case arises, the HtmlComputedValuesCreator is not fully initialized and fFont is NULL. Memcpy crashes.

Line 1752, htmlprop.c (crash site – pValues->fFont is NULL)
    if (pParent) {
        int nBytes = sizeof(HtmlComputedValues) - iCopyBytes;
        char *pvalues = (char *)((HtmlElementNode *)pParent)->pPropertyValues;
        HtmlComputedValues *pParentValues = (HtmlComputedValues *)pValues;
        memcpy(&values[iCopyBytes], &pvalues[iCopyBytes], nBytes);
        memcpy(&p->fontKey, pValues->fFont->pKey, sizeof(HtmlFontKey));
        pValues->mask =
            (pValues->mask & iCopyMask) | (pParentValues->mask & !iCopyMask);
    }

The general fix we’ve chosen is to explicitly cast all pointers initialized with zero to their appropriate type.   This seems to be done throughout the Tcl/Tk code base (usually).

You have at least 20 instances of this particular issue in your code base. I found at risk calls by scanning the code for ‘0,’ and checking the function prototypes.

If you are interested, the compiler bug can be easily demonstrated with a simple ‘C’ program that has a function with at least five parameters, the fifth or higher of which is a pointer. If the call site passes all zeros, you will see the register optimization in the generated assembly code.

If you have further questions, please feel free to contact me.

Best Regards,

AJ Sahukar

From: Anil E. Sahukar
Sent: Friday, March 05, 2010 1:34 PM
To: 'Dan Kennedy'
Subject: RE: TkHTML widget usage question

Dan,

I appreciate your help (and your keen eyes).

Fixing that gets me closer to the desired outcome, but not all the way there.

My sample HTML input (attached) gives the following rendering:

I see that this is not the preferred method of posting and I have joined the tkhtml3 Google group.

Would you like me to repost on that group?

Thanks in advance for the help,

AJ

-----Original Message-----
From: Dan Kennedy [mailto:danie...@gmail.com]
Sent: Friday, March 05, 2010 12:11 PM
To: Anil E. Sahukar
Cc: Ira Mahajan
Subject: Re: TkHTML widget usage question

On Mar 6, 2010, at 12:06 AM, Anil E. Sahukar wrote:

> Dan,
>
> My name is AJ Sahukar and I work with Ira Mahajan using your TkHTML
> widget.
>
> After studying your documentation, I understood that the recommended
> way to apply default formatting is via a Style Sheet specification.
>
> I have a style sheet I render as a string as follows:
>
>     ostr << "<HEAD>" << std::endl
>          << "<TITLE>TKHTML STYLESHEET</TITLE>" << std::endl
>          << "<STYLE TYPE=\"text/css\">" << std::endl
>          << " BODY { ";
>
>     ostr << "background_color:" << bg_color.GetRGBHexString() << ";"

Supposed to be "background-color" (with a '-' not '_') I think.

Dan.

>          << "color:" << fg_color.GetRGBHexString() << ";"
>          << "font-family:" << text_font.GetName() << ";"
>          << "size:" << text_font.GetSize() << "pt;"
>          << "font-weight:" << weight_spec << ";"
>          << "font-style:" << style_spec << " }" << std::endl;
>
>     ostr << "</STYLE>" << std::endl
>          << "</HEAD>" << std::endl;
>
>     return ostr.str();
>
> I render this on the Tk side as shown below (the $format_spec param
> is the output of the above ostr, the $texteditor::eval_text_string
> is the HTML text to be rendered.
>
>     $control_path reset;
>     $control_path style $format_spec;
>     $control_path parse -final $texteditor::eval_text_string;
>
> Everything but the background color renders as I expect it to. Am I
> missing something in my implementation?
>
> Thanks in advance for the help,
>
> AJ
>
>

Reply all

Reply to author

Forward

0 new messages