January 26, 2006
Okay, I've given this some thought and perhaps the best approach would be to reconsider bounds checking under the looser category of "data access checking."  Bounds checking would be a required minimum and anything beyond that would be left as a QOI issue for the compiler developers.  Adding "write to static data" checking should be a trivial modification of the existing bounds checking code.  If you assume the existing bounds checking code is this:

// assume p is a pointer to the write
// location and a is the array object
if( p < &a[0] || p >= &a[$] ) {
    onArrayBoundsError( __FILE__, __LINE__ );
}

The it would simply be a matter of adding two new constant variables to store the top and bottom of the static area (or determining the locations dynamically as in the current DMD GC code) and adding an additional check:

// assume sb is a pointer to the base of the const data area
// and st is a pointer to one past the top of that area
if( p >= sb && p < st ) {
    onInvalidWriteError( __FILE__, __LINE__ );
}

This eliminates the need for per-variable flag maintenance and offers an easy way to turn off the checking if it is not desired.  And since this is conceptually (and functionally) quite similar to bounds checking anyway, it should be a fairly painless extension of established practice.


Sean
January 26, 2006
"Sean Kelly" <sean@f4.ca> wrote in message news:dr9642$rh$1@digitaldaemon.com...
> Sean Kelly wrote:
>>
>> See above.  I think such a flag may not actually be necessary in this case, simply because code generation for const data tends to be somewhat distinct.  Perhaps some late stage analysis could be performed to detect this problem?  I'm kind of guessing here, but in the small amount of compiler work I've done in the past I think this would have been fairly simple to implement.
>
> I take it back :-P.  Passing through an opaque function call as in the original example tosses the possibility of code analysis out the window. But some detection might be better than none in this case.

It is getting some detection - a seg fault. The whole reason for putting const data into a read-only segment is to get hardware detection and enforcement.

>  Also, it would be nice if the system reported a meaningful error message
> if this occurs--perhaps something indicating that the segfault occurred
> from an attempted write to const data?

You should get such an indication if you're running it under a decent debugger.

>  But once you're stuck with runtime detection, I don't really care if the
> problem is first noticed by a software flag or a hardware fault.  In fact,
> loading a core dump makes reproducing the problem fairly simple in most
> cases.

All seg faults are are the hardware doing the checking for you rather than having to do it by adding instructions. Along with a good debugger, it's pretty good, and has the nice characteristic that it doesn't bloat the code or slow the execution.


January 26, 2006
Walter Bright wrote:
> "Sean Kelly" <sean@f4.ca> wrote in message news:dr9642$rh$1@digitaldaemon.com...
>
>> I take it back :-P.  Passing through an opaque function call as in the original example tosses the possibility of code analysis out the window. But some detection might be better than none in this case.
> 
> It is getting some detection - a seg fault. The whole reason for putting const data into a read-only segment is to get hardware detection and enforcement.

Is there any way to trap such a write attempt in Windows?  For example, this code:

import std.c.stdio;

const char[] c = "hello";

void main()
{
    c[1] = 'a';
    printf( "%.*s\n", c );
}

runs to completion in Windows and prints "hello" (ie. the assignment is effectively ignored).  Removing the 'const' prints "hallo" as expected.  But while this is better than having the const data altered by a write, it also doesn't make bugs known.  All in all, I do really prefer to rely on the hardware to signal this, but if that's not possible I still want to have *some* indication that such a write was attempted--this was one reason I suggested extending bounds checking. Is it simply that Windows doesn't have a trap set up for this situation?

> All seg faults are are the hardware doing the checking for you rather than having to do it by adding instructions. Along with a good debugger, it's pretty good, and has the nice characteristic that it doesn't bloat the code or slow the execution. 

Agreed.  And every debugger I've used can halt on such errors to allow the problem to be debugged.  But I'm not sure whether a debugger would catch the above situation in Windows (I'll admit I've never tried it).


Sean
January 26, 2006
"Sean Kelly" <sean@f4.ca> wrote in message news:dr9teh$hmt$1@digitaldaemon.com...
>> All seg faults are are the hardware doing the checking for you rather than having to do it by adding instructions. Along with a good debugger, it's pretty good, and has the nice characteristic that it doesn't bloat the code or slow the execution.
>
> Agreed.  And every debugger I've used can halt on such errors to allow the problem to be debugged.  But I'm not sure whether a debugger would catch the above situation in Windows (I'll admit I've never tried it).

Using Visual Studio 6 (which uses WinDbg), and converting the given gode into a WinMain() function, the debugger does indeed catch the access violation, but most of the time it breaks at.. some dissasembly in the middle of NTDLL.  Which is useless.  And the call stack in that case doesn't help either - WinDbg doesn't really seem to like the D calling convention, so it somehow just hides calls to D functions, making the call stack something like

NTDLL
WinMain
NTKERNEL

When there are supposed to be calls to any number of D functions between NTDLL and WinMain.

In fact, I've tried several different scenarios, and have yet to get VS6 to break to the line of the access violation.  It always breaks to the middle of NTDLL.


January 26, 2006
Sean Kelly wrote:
> Okay, I've given this some thought and perhaps the best approach would be to reconsider bounds checking under the looser category of "data access checking."  Bounds checking would be a required minimum and anything beyond that would be left as a QOI issue for the compiler developers.  Adding "write to static data" checking should be a trivial modification of the existing bounds checking code.  If you assume the existing bounds checking code is this:
> 
> // assume p is a pointer to the write
> // location and a is the array object
> if( p < &a[0] || p >= &a[$] ) {
>     onArrayBoundsError( __FILE__, __LINE__ );
> }
> 
> The it would simply be a matter of adding two new constant variables to store the top and bottom of the static area (or determining the locations dynamically as in the current DMD GC code) and adding an additional check:
> 
> // assume sb is a pointer to the base of the const data area
> // and st is a pointer to one past the top of that area
> if( p >= sb && p < st ) {
>     onInvalidWriteError( __FILE__, __LINE__ );
> }
> 
> This eliminates the need for per-variable flag maintenance and offers an easy way to turn off the checking if it is not desired.  And since this is conceptually (and functionally) quite similar to bounds checking anyway, it should be a fairly painless extension of established practice.

IMHO, this is a very good idea!  Assuming that it is part of bounds checking, and thus it would disappear on release builds, then this would be a very good thing to do on debug builds.
January 26, 2006
"Russ Lewis" <spamhole-2001-07-16@deming-os.org> wrote in message news:drbdai$20ns$1@digitaldaemon.com...
> Sean Kelly wrote:
>> Okay, I've given this some thought and perhaps the best approach would be to reconsider bounds checking under the looser category of "data access checking."  Bounds checking would be a required minimum and anything beyond that would be left as a QOI issue for the compiler developers. Adding "write to static data" checking should be a trivial modification of the existing bounds checking code.  If you assume the existing bounds checking code is this:
>>
>> // assume p is a pointer to the write
>> // location and a is the array object
>> if( p < &a[0] || p >= &a[$] ) {
>>     onArrayBoundsError( __FILE__, __LINE__ );
>> }
>>
>> The it would simply be a matter of adding two new constant variables to store the top and bottom of the static area (or determining the locations dynamically as in the current DMD GC code) and adding an additional check:
>>
>> // assume sb is a pointer to the base of the const data area
>> // and st is a pointer to one past the top of that area
>> if( p >= sb && p < st ) {
>>     onInvalidWriteError( __FILE__, __LINE__ );
>> }
>>
>> This eliminates the need for per-variable flag maintenance and offers an easy way to turn off the checking if it is not desired.  And since this is conceptually (and functionally) quite similar to bounds checking anyway, it should be a fairly painless extension of established practice.
>
> IMHO, this is a very good idea!  Assuming that it is part of bounds checking, and thus it would disappear on release builds, then this would be a very good thing to do on debug builds.
I agree; integrating this with bounds checks would be real nice.


January 28, 2006
The Wed, 25 Jan 2006 11:48:30 -0800, Walter Bright wrote:
> String literals are read-only. [...] The seg fault comes from attempting to write into that read-only data.

Why does D allow assignment of read only data to a read/write variable (without an explicit cast)?  Why is the following code allowed to compile with no warning and crash at runtime?

    int main()
    {
        const char[] test = "this is a test";
        test[2] = 'b';
        return 0;
    }

Why does the following not crash and yields "String is thbs is a"?

import std.stdio;

    int main()
    {
        const char[10] test = "this is a ";
        test[2] = 'b';
        writefln("String is %s", test);
        return 0;
    }


January 29, 2006
On Thu, 26 Jan 2006 12:45:20 +1100, Walter Bright <newshound@digitalmars.com> wrote:

>
> "Sean Kelly" <sean@f4.ca> wrote in message
> news:dr9642$rh$1@digitaldaemon.com...
>> Sean Kelly wrote:
>>>
>>> See above.  I think such a flag may not actually be necessary in this
>>> case, simply because code generation for const data tends to be somewhat
>>> distinct.  Perhaps some late stage analysis could be performed to detect
>>> this problem?  I'm kind of guessing here, but in the small amount of
>>> compiler work I've done in the past I think this would have been fairly
>>> simple to implement.
>>
>> I take it back :-P.  Passing through an opaque function call as in the
>> original example tosses the possibility of code analysis out the window.
>> But some detection might be better than none in this case.
>
> It is getting some detection - a seg fault. The whole reason for putting
> const data into a read-only segment is to get hardware detection and
> enforcement.

Would it be possible to detect this at compile time rather than run time?

-- 
Derek Parnell
Melbourne, Australia
1 2
Next ›   Last »