Discussion:
Test for nil causes AV in D4
(too old to reply)
P E Schoen
2010-12-25 20:03:03 UTC
Permalink
I was getting access violation errors with the following D4 code:

var tblOld,tblNew: TTable;

if tblNew = nil then //This causes an AV
tblNew := TTable.Create(fmReclData);
if tblNew <> nil then begin
tblNew.Active := False;
....

In the finally section I changed this to

finally
if assigned(tblNew) then begin
tblNew.Free;
tblNew := nil; end;

There is a compiler warning for the finally section that tblNew might not
have been initialized. In the first instance I simply removed the check for
nil. But what I found in the following link indicates that the two are
equivalent.

I suppose it is OK to omit a check for assignment in both cases of creating
the table and freeing it. However I suppose it might cause an AV for
tblNew.Free. Would it be better to assign a nil value to tblNew at the
beginning of the code? Is that any different than not assigning anything to
the variable at all?

Thanks, and Merry Christmas!

Paul
P E Schoen
2010-12-25 20:04:55 UTC
Permalink
"P E Schoen" wrote in message news:b6sRo.38481$***@newsfe04.iad...

I was getting access violation errors with the following D4 code:

var tblOld,tblNew: TTable;

if tblNew = nil then //This causes an AV
tblNew := TTable.Create(fmReclData);
if tblNew <> nil then begin
tblNew.Active := False;
....

In the finally section I changed this to

finally
if assigned(tblNew) then begin
tblNew.Free;
tblNew := nil; end;

There is a compiler warning for the finally section that tblNew might not
have been initialized. In the first instance I simply removed the check for
nil. But what I found in the following link indicates that the two are
equivalent.

http://delphi.about.com/od/delphitips2007/qt/nil_assigned.htm

I suppose it is OK to omit a check for assignment in both cases of creating
the table and freeing it. However I suppose it might cause an AV for
tblNew.Free. Would it be better to assign a nil value to tblNew at the
beginning of the code? Is that any different than not assigning anything to
the variable at all?

Thanks, and Merry Christmas!

Paul
Jamie
2010-12-25 21:24:45 UTC
Permalink
Post by P E Schoen
var tblOld,tblNew: TTable;
if tblNew = nil then //This causes an AV
tblNew := TTable.Create(fmReclData);
if tblNew <> nil then begin
tblNew.Active := False;
....
In the finally section I changed this to
finally
if assigned(tblNew) then begin
tblNew.Free;
tblNew := nil; end;
There is a compiler warning for the finally section that tblNew might not
have been initialized. In the first instance I simply removed the check for
nil. But what I found in the following link indicates that the two are
equivalent.
http://delphi.about.com/od/delphitips2007/qt/nil_assigned.htm
I suppose it is OK to omit a check for assignment in both cases of creating
the table and freeing it. However I suppose it might cause an AV for
tblNew.Free. Would it be better to assign a nil value to tblNew at the
beginning of the code? Is that any different than not assigning anything to
the variable at all?
Thanks, and Merry Christmas!
Paul
That is a bad idea to test a pointer for NIL if no initializing code
exist before.

Doing a NIL check is fine as long as the pointer is properly managed
before had. You can use that check to test if you have it assigned or
not how ever, at the start of a procedure or function, these pointers
are just space on the stack and what ever was sitting there before hand.

By testing for a NIL pointer that just may not be NIL, due to some
other crap that was being used before in that same stack location, is
going to mislead your code.. On top of that, if further steps are taken
on this pointer, assuming it may point to something valid, the RTL
background test for classes etc, may just trip off a fault, because it
used a pointer out side it's limits or it points to something with in
the limits and assumes to be valid and your code then uses it instead of
creating a new one..

If you have complex code that is deleting and re-assigning the
pointer, you should be forcing this stack pointer to NIL with out even
assuming it to be valid at the start of the code block.

Checking for NIL pointers is good, only when you know that somewhere
at the start of your code you forced it to NIL...
P E Schoen
2010-12-26 19:57:40 UTC
Permalink
Post by Jamie
That is a bad idea to test a pointer for NIL if no initializing
code exist before.
Doing a NIL check is fine as long as the pointer is properly
managed before had. You can use that check to test if you have
it assigned or not how ever, at the start of a procedure or function,
these pointers are just space on the stack and what ever was
sitting there before hand.
By testing for a NIL pointer that just may not be NIL, due
to some other crap that was being used before in that same
stack location, is going to mislead your code.. On top of that,
if further steps are taken on this pointer, assuming it may point
to something valid, the RTL background test for classes etc, may
just trip off a fault, because it used a pointer out side it's limits
or it points to something with in the limits and assumes to be
valid and your code then uses it instead of creating a new one..
If you have complex code that is deleting and re-assigning
the pointer, you should be forcing this stack pointer to NIL with
out even assuming it to be valid at the start of the code block.
Checking for NIL pointers is good, only when you know that
somewhere at the start of your code you forced it to NIL...
I can see your point and it certainly makes sense to set the pointer to nil
at the time of assignment. I realize that the pointer resides in stack space
of the function, which probably contains the leftovers of other functions
that may have used the stack. So it will be probably a 32 bit pointer which
may point to any address in RAM.

However, I can't see why simply checking the pointer for a particular value
such as NIL should cause an AV. The 32 bits should contain a numeric value
which should be able to be compared to any other value, just as it could be
assigned a value of NIL or zero or any other value. In this case it was
useful to have the AV because it prevented me from making a wrong assumption
about the value of a non-assigned variable. The likelihood of the pointer
containing the special value NIL, is essentially NIL, or 1/2^32.

Thanks,

Paul
Jamie
2010-12-26 20:27:01 UTC
Permalink
Post by P E Schoen
Post by Jamie
That is a bad idea to test a pointer for NIL if no initializing
code exist before.
Doing a NIL check is fine as long as the pointer is properly
managed before had. You can use that check to test if you have
it assigned or not how ever, at the start of a procedure or function,
these pointers are just space on the stack and what ever was
sitting there before hand.
By testing for a NIL pointer that just may not be NIL, due
to some other crap that was being used before in that same
stack location, is going to mislead your code.. On top of that,
if further steps are taken on this pointer, assuming it may point
to something valid, the RTL background test for classes etc, may
just trip off a fault, because it used a pointer out side it's limits
or it points to something with in the limits and assumes to be
valid and your code then uses it instead of creating a new one..
If you have complex code that is deleting and re-assigning
the pointer, you should be forcing this stack pointer to NIL with
out even assuming it to be valid at the start of the code block.
Checking for NIL pointers is good, only when you know that
somewhere at the start of your code you forced it to NIL...
I can see your point and it certainly makes sense to set the pointer to
nil at the time of assignment. I realize that the pointer resides in
stack space of the function, which probably contains the leftovers of
other functions that may have used the stack. So it will be probably a
32 bit pointer which may point to any address in RAM.
However, I can't see why simply checking the pointer for a particular
value such as NIL should cause an AV. The 32 bits should contain a
numeric value which should be able to be compared to any other value,
just as it could be assigned a value of NIL or zero or any other value.
In this case it was useful to have the AV because it prevented me from
making a wrong assumption about the value of a non-assigned variable.
The likelihood of the pointer containing the special value NIL, is
essentially NIL, or 1/2^32.
Thanks,
Paul
It's possible that extra code is being generated if it thinks that
value may sit in an area where the dynamic heap is and use the pointer
to examine in more details if the pointer is looking at a 0 use of
memory or not.
To confirm that extra code is being generated and doing such test on
a pointer (wild one), set the Assembler View on that line and study the
assembler code.

For a sure way, type case it to a DWORD..

If DWORD(MyPointer) Then it's not 0.

But that really still does not tell you much.. Because going back to
the RTL way of doing it, the pointer maybe looking at code space, which
is protected.

I would go with a assembly view of it.
P E Schoen
2010-12-27 06:39:44 UTC
Permalink
It's possible that extra code is being generated if it thinks that value
may sit in an area where the dynamic heap is and use the
pointer to examine in more details if the pointer is looking at a 0
use of memory or not.
To confirm that extra code is being generated and doing such
test on a pointer (wild one), set the Assembler View on that line
and study the assembler code.
For a sure way, type case it to a DWORD..
If DWORD(MyPointer) Then it's not 0.
But that really still does not tell you much.. Because going back
to the RTL way of doing it, the pointer maybe looking at code
space, which is protected.
I would go with a assembly view of it.
I just found another place in my code where a *global* variable pointer
apparently is set to nil when declared:

var ReclCurveData: TStringList;

if ReclCurveData = nil then
ReclCurveData := TStringList.Create;

I also make sure it is freed when the form is destroyed:

ReclCurveData.Free;
ReclCurveData := nil;

So, it seems that stack space memory is treated differently. Maybe that is
related to the fact that you can't initialize variables when they are
declared within a function or procedure. It is good that the compiler flags
a warning for a pointer in stack space possibly not having been initialized.

Paul
Maarten Wiltink
2010-12-27 09:15:08 UTC
Permalink
Post by P E Schoen
Post by Jamie
It's possible that extra code is being generated if it thinks that
value may sit in an area where the dynamic heap is and use the
pointer to examine in more details if the pointer is looking at a 0
use of memory or not.
Jamie, you're teaching voodoo again. No extra code is being generated.
The compiler isn't that smart.


[...]
Post by P E Schoen
I just found another place in my code where a *global* variable pointer
var ReclCurveData: TStringList;
if ReclCurveData = nil then
ReclCurveData := TStringList.Create;
Because it's a global variable, it lives in memory that is loaded from
the executable image. Therefore, it can be initialised for free and
will be zero if you don't specify anything else.
Post by P E Schoen
ReclCurveData.Free;
ReclCurveData := nil;
So, it seems that stack space memory is treated differently. Maybe that
is related to the fact that you can't initialize variables when they are
declared within a function or procedure. It is good that the compiler
flags a warning for a pointer in stack space possibly not having been
initialized.
Stack space can't be initialised for free and therefore under the
language specification it can't be initialised at all. Yes, the compiler
is that cheap. There's no other reason for it.

The consequence is that you need to treat local variables different
from global ones. Local variables start out uninitialised, so you MUST
NOT check their value before assigning a value. They're uninitialised -
just assign the value. By the same token, at the end of the variable's
lifetime, don't worry about its value, it will go out of scope anyway.
Free the object, but don't bother with clearing the reference if you
don't need to. The pattern for a local object is

ObjRef:=ClassRef.Create;
try
ObjRef.Work;
finally
ObjRef.Free;
end;

Note the absence of the nil check at the start, and of the reset at the
end. The first is just wrong, and the second superfluous.

Groetjes,
Maarten Wiltink
P E Schoen
2010-12-27 17:51:22 UTC
Permalink
Post by Maarten Wiltink
Stack space can't be initialised for free and therefore under
the language specification it can't be initialised at all. Yes,
the compiler is that cheap. There's no other reason for it.
The consequence is that you need to treat local variables
different from global ones. Local variables start out uninitialised,
so you MUST NOT check their value before assigning a value.
It might be interesting to check the code to see why it should raise an AV
to compare a pointer's value to some other value, while setting it to some
value does not. In both cases as I understand it the pointer is just an
address (or just a 32 bit or 64 bit value) in memory that is of the right
size to hold an address. However, in global space it is referenced by an
absolute address, while an address in stack space would be referenced as an
offset from the stack pointer.

In either case it should resolve to a memory address which may or may not
point to valid memory. I don't see why this value can't be examined as a
simple number. Maybe the typecast to DWORD would prevent the compiler from
treating it as a pointer, where it might determine that the address is
invalid. I think the real problem is that any operation on a pointer, other
than assignment, assumes the pointer refers to a memory address containing a
structure (or object), and raises the AV. And I wonder if the pointer
actually was 0, or nil, or a valid address, then maybe a read of its value
might not raise the AV.

But your input was very helpful, and now I understand the problem AND the
proper way to use pointers within a procedure as well as a global.

Thanks!

Paul
Jamie
2010-12-27 21:34:10 UTC
Permalink
Post by P E Schoen
Post by Maarten Wiltink
Stack space can't be initialised for free and therefore under
the language specification it can't be initialised at all. Yes,
the compiler is that cheap. There's no other reason for it.
The consequence is that you need to treat local variables
different from global ones. Local variables start out uninitialised,
so you MUST NOT check their value before assigning a value.
It might be interesting to check the code to see why it should raise an
AV to compare a pointer's value to some other value, while setting it to
some value does not. In both cases as I understand it the pointer is
just an address (or just a 32 bit or 64 bit value) in memory that is of
the right size to hold an address. However, in global space it is
referenced by an absolute address, while an address in stack space would
be referenced as an offset from the stack pointer.
In either case it should resolve to a memory address which may or may
not point to valid memory. I don't see why this value can't be examined
as a simple number. Maybe the typecast to DWORD would prevent the
compiler from treating it as a pointer, where it might determine that
the address is invalid. I think the real problem is that any operation
on a pointer, other than assignment, assumes the pointer refers to a
memory address containing a structure (or object), and raises the AV.
And I wonder if the pointer actually was 0, or nil, or a valid address,
then maybe a read of its value might not raise the AV.
But your input was very helpful, and now I understand the problem AND
the proper way to use pointers within a procedure as well as a global.
Thanks!
Paul
I just checked my old D3, I didn't do the code you had how ever, I did a
simple If LocalPointer is NIl then....
just to see what it generated. I must say the D3 compiler is very
efficient when it comes to optimization. It just did a simple XOr
EAX,EAX when EAX was loaded from the object. Now in your case, it would
be a good idea to ASM dump it and see if the compiler is tripping over
itself some how. I know when D4 came out, there were some issues that
were compiler specific and not user problems. But those were fixed with
a SP from borland..

Jamie
Maarten Wiltink
2010-12-28 08:28:46 UTC
Permalink
[...]
Post by P E Schoen
Post by Maarten Wiltink
The consequence is that you need to treat local variables
different from global ones. Local variables start out uninitialised,
so you MUST NOT check their value before assigning a value.
It might be interesting to check the code to see why it should raise
an AV to compare a pointer's value to some other value, while setting
it to some value does not.
Turn off optimisation. You _are_ going to find that comparing a pointer
does not cause an access violation.

Reading the value can't fail, because it lives either in your data
segment or in your stack frame. Testing the value does just that, and
only that. There is no magic. Casting to a dword adds nothing.

When I say that you mustn't use the value of an uninitialised variable,
I don't mean that you can get an access violation for looking at the
value itself[0]. It's just that the value is undefined, unpredictable,
and you will gain nothing from looking at it.

Groetjes,
Maarten Wiltink

[0] If it's of a subrange or enumerated type, you could fail the bounds
check. But that's not an access violation.
Jamie
2010-12-27 21:21:26 UTC
Permalink
Post by Maarten Wiltink
Post by P E Schoen
Post by Jamie
It's possible that extra code is being generated if it thinks that
value may sit in an area where the dynamic heap is and use the
pointer to examine in more details if the pointer is looking at a 0
use of memory or not.
Jamie, you're teaching voodoo again. No extra code is being generated.
The compiler isn't that smart.
[...]
Post by P E Schoen
I just found another place in my code where a *global* variable pointer
var ReclCurveData: TStringList;
if ReclCurveData = nil then
ReclCurveData := TStringList.Create;
Because it's a global variable, it lives in memory that is loaded from
the executable image. Therefore, it can be initialised for free and
will be zero if you don't specify anything else.
Post by P E Schoen
ReclCurveData.Free;
ReclCurveData := nil;
So, it seems that stack space memory is treated differently. Maybe that
is related to the fact that you can't initialize variables when they are
declared within a function or procedure. It is good that the compiler
flags a warning for a pointer in stack space possibly not having been
initialized.
Stack space can't be initialised for free and therefore under the
language specification it can't be initialised at all. Yes, the compiler
is that cheap. There's no other reason for it.
The consequence is that you need to treat local variables different
from global ones. Local variables start out uninitialised, so you MUST
NOT check their value before assigning a value. They're uninitialised -
just assign the value. By the same token, at the end of the variable's
lifetime, don't worry about its value, it will go out of scope anyway.
Free the object, but don't bother with clearing the reference if you
don't need to. The pattern for a local object is
ObjRef:=ClassRef.Create;
try
ObjRef.Work;
finally
ObjRef.Free;
end;
Note the absence of the nil check at the start, and of the reset at the
end. The first is just wrong, and the second superfluous.
Groetjes,
Maarten Wiltink
If you only knew how much I learned how the compiler does things in the
background that explains much, just using the ASM dump, You would then
understand the reason why I suggest such things.. I've found many quirks
in the compiler over the years generating problems that just could not
be explained with out the ASM dump. Of course, you need to understand
ASM code for that to work. Many of these issues are due to optimizing
errors and D4 had a few new nifty ones introduced when it came out to
be corrected later. These were mostly in memory handling of pointers and
clean ups. D2 had it's problems handling COMs/activeX, D3 had it's
issues with generating type info for COMs/ACtiveX, D4 had memory
handling problems with strings and other dynamic goodies that borland
release a fixed patch for. D5 and D6 was kind of ok. D7 is when things
started to get a little bloated and if you had lots of nice components
on your palette, you may run into periodic problems of the system losing
resources to the IDE. Never was sure about what caused that. I
still use D7 but found that the added bloat to the RTL has cause a
couple of my older apps written in D3 that needs all the CPU time it
can muster, and ends up falling behind..


My reasoning for him to do a code dump on that was to see if the
compiler was actually doing something different.. Something that could
some how generated code out of place. I've seen where code got bombarded
from other bad code and effected code elsewhere However, I take the
reason of why I suggested he do this due to how the memory handler
takes care of pointers using the New, Delete etc... the system will
process a pointer or at least look at it if, it isn't NIL.

Those that think using an Exception Block on an unknown state of a
pointer should be shot!


Jamie
Maarten Wiltink
2010-12-28 08:42:30 UTC
Permalink
Post by Jamie
Post by Maarten Wiltink
Jamie, you're teaching voodoo again.
If you only knew how much I learned how the compiler does things in the
background that explains much, just using the ASM dump, You would then
understand the reason why I suggest such things..
I'll accept that you know what you're talking about. But all the same I
do feel that when presented with incorrect code, you shouldn't start with
seeing how a specific compiler reacts to it. Fix the code first.
Post by Jamie
[...] Many of these issues are due to optimizing errors ...
The manual says Delphi only uses safe optimisations, ever. It does!

But they mess with debugging, so during development, I leave it off.

Then, for release, I still leave it off. You've obviously seen it cause
problems - have you ever seen it result in a measurable improvement?
Post by Jamie
Those that think using an Exception Block on an unknown state of a
pointer should be shot!
...from a circus cannon. But yes, that's a mortal sin for a programmer.

Groetjes,
Maarten Wiltink
Hans-Peter Diettrich
2010-12-27 11:54:54 UTC
Permalink
Post by P E Schoen
var tblOld,tblNew: TTable;
if tblNew = nil then //This causes an AV
There is a compiler warning for the finally section that tblNew might
not have been initialized.
This warning suggests that tblNew is a local variable. Local variables
are not initialized, so that a check for Nil produces random results.
Post by P E Schoen
Would it be better to assign a nil value to tblNew
at the beginning of the code?
Yes, that's the best choice for local objects. Then you can safely use
Free later in the code, regardless of wheter an object has been created
or not.
Post by P E Schoen
Is that any different than not assigning
anything to the variable at all?
Yes.

DoDi

Loading...