Re: [fpc-devel] Data flow analysis (dfa) and "case ... of"

[Tomas Hajny]

On Sat, July 1, 2017 16:45, Martok wrote:


Hi,

> The attitude displayed over on #32079 is, quite frankly, terrifying.
> Apparently a language which from the beginning has intrinsics for reading
> and writing files must never be used for doing so, or wild things may
> happen /and that's okay/.
>
> Implying that input should already be sanitized on a bug about something
> that breaks input sanitation code (but only sometimes) is just... wow.
>
> If anybody wants it, here's the patch I'll be rolling on the windows
> snapshots from now on.

I agree that it might be useful to extend the documentation of case ... of
as suggested in one of the comments.

I also agree to Jonas that it should not be a problem of the compiler to
perform these checks. However, there's one thing that worries me - I
assumed that the respective checks should be performed when reading the
input from a file and an error condition should be raised when
encountering a value not fitting the respective type. Unfortunately, this
is apparently not the case, because the following test program happily
runs even when the test file contains a value outside the TEnum range:

type
 TEnum = (one, two, three);

var
 F: file of TEnum;
 E: TEnum;

begin
{$I+}
{$R+}
 Assign (F, 'testfile.tst');
 Reset (F, 1);
 while not (Eof (F)) do
  begin
   Read (F, E);
   WriteLn (Ord (E));
  end;
 Close (F);
end.

Interestingly, the error is caught if I try to write out the read value
back to console (i.e. add WriteLn (E) after the WriteLn line). I believe
that the same error should already happen while reading...

Tomas


___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Re: [fpc-devel] Data flow analysis (dfa) and "case ... of"

[Martok]

The attitude displayed over on #32079 is, quite frankly, terrifying. Apparently
a language which from the beginning has intrinsics for reading and writing files
must never be used for doing so, or wild things may happen /and that's okay/.

Implying that input should already be sanitized on a bug about something that
breaks input sanitation code (but only sometimes) is just... wow.

If anybody wants it, here's the patch I'll be rolling on the windows snapshots
from now on.


Have a good weekend,
Martok
Index: compiler/ncgset.pas
===
--- compiler/ncgset.pas (revision 36620)
+++ compiler/ncgset.pas (working copy)
@@ -1080,7 +1080,7 @@
labelcnt:=case_count_labels(labels);
{ can we omit the range check of the jump table ? }
getrange(left.resultdef,lv,hv);
-   jumptable_no_range:=(lv=min_label) and (hv=max_label);
+   jumptable_no_range:=(lv=min_label) and (hv=max_label) and 
(cs_opt_level4 in current_settings.optimizerswitches) and not (cs_check_range 
in current_settings.localswitches);
{ hack a little bit, because the range can be greater }
{ than the positive range of a aint}
 
___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Re: [fpc-devel] Data flow analysis (dfa) and "case ... of"

[Marco van de Voort]

In our previous episode, Martin said:
> > Interestingly, I just ran into "bad" code generation with exactly the 
> > properties
> > discussed in this thread.
> > Because every declared element is covered, the generated code for it ends up
> > being a computed goto:
> > and most importantly, not into any else statement.
> 
> No, it is not bad code generation.
> It is exactly the expected result. At least that is what I understood 
> from previous mails in this thread, specifically (in the context of 
> out-of-range input):

Afaik intermediate values in a sparse enum are not rangechecked at all? They
fall within the range (lowestvalue..highestvalue), but are not valid anyway.
___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Re: [fpc-devel] Data flow analysis (dfa) and "case ... of"

[Martin]

On 28/06/2017 20:03, Martok wrote:

Interestingly, I just ran into "bad" code generation with exactly the properties
discussed in this thread.
Because every declared element is covered, the generated code for it ends up
being a computed goto:
and most importantly, not into any else statement.


No, it is not bad code generation.
It is exactly the expected result. At least that is what I understood 
from previous mails in this thread, specifically (in the context of 
out-of-range input):



Jonas Maebe wrote:
That is why I said "If range checking is off there or disabled via an 
explicit type cast, then the result is undefined by definition." 


"undefined result" does include the possibility of a crash.

This is the same as accessing an
  Array[enum] of foo;
if enum is out of range, then it may crash, or behave in any undefined way.

The only thing that could be added here is, that if dfa detects that all 
enums are used in a case statement, and yet there is an "else", then it 
should warn about "unreachable code".


___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Re: [fpc-devel] Data flow analysis (dfa) and "case ... of"

[Martok]

Interestingly, I just ran into "bad" code generation with exactly the properties
discussed in this thread.

Take a function like this:

function SignatureSubpacketTypeToStr(const X: TSignatureSubpacketType): String;
begin
  case X of
sstReserved00 : Result:= 'Reserved00';
sstReserved01 : Result:= 'Reserved01';
sstCreationTime   : Result:= 'CreationTime';


Because every declared element is covered, the generated code for it ends up
being a computed goto:

   0x10047c4c <+28>:mov-0x4(%ebp),%al
   0x10047c4f <+31>:and$0xff,%eax
   0x10047c54 <+36>:jmp*0x10071d08(,%eax,4)

Which is perfectly fine if X is guaranteed to be in range of the elements the
case statement matches to. If it is not, as it may be with invalid input data
(as read from a file), that jump goes somewhere undefined - and most
importantly, not into any else statement.

So, while we have code that looks like Result is always properly initialized,
what we get instead is code that doesn't actually work. And no kind of DFA could
detect that, except also range-checking everything.

Just thought I'd share that, as a less synthetic example than some discussed 
here.


Regards,
Martok

___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Re: [fpc-devel] Data flow analysis (dfa) and "case ... of"

[Juha Manninen]

On Thu, Jun 8, 2017 at 3:22 AM, Denis Kozlov  wrote:
> I can't control what values other users/developers may supply, nor I
> restrict the use of different compiler versions and build flags.

Compiler flags have nothing to do with the fundamental issue here.
If other developers supply buggy code to you, then they have to fix it
obviously. This holds true for any bugs.

> It is because "TConvertType(-1)" and the likes are an acceptable statement for
> compiler, I make it my responsibility to handle all possible input values,
> no matter how illogical and invalid they may be. This is a design pattern
> that I follow.

You have good intentions but somehow your logic slipped to a wrong track.
You must prevent invalid data entering an enum variable in the first place.
So, if you get integer data from eg. stream or user input and convert
to enum, you must ensure the integer is within range _before_
typecasting it.
Like:
  if (i<0) or (i>Integer(High(TConvertType))) then
raise Exception.CreateFmt('Invalid convert value (%d)', [i]);
  ctvar := TConvertType(i);

Typecast is always a "serious" thing. It means the original type is
wrong and must be forced to something else.
Obviously you need a sanity check there!
There is only a limited number of places where you set an enum
variable. It is rather easy to make sure the value is sane. After that
you don't need to worry about it, the value is valid always.

Your extra checks for enum values are like a desperate band-aid for a
bug that happened already elsewhere. No, a bug must be fixed where it
is.

Juha
___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Re: [fpc-devel] Data flow analysis (dfa) and "case ... of"

[Denis Kozlov]

On 07/06/2017 15:18, Juha Manninen wrote:

The compiler trusts that data in an enum variable is legal, within the range.
It should trust the same way when doing DFA. It is logical and consistent.


I understand your reasons now and agree that DFA should cover only 
logical (according to the compiler) scenarios.


Juha Manninen wrote:

What you must do is add sanity checks for code that initializes enum
data based on some other data.
.
The same way if you get integer data from I/O, you must verify the
integer is within range _before_ typecasting it.


Sanity checks is exactly what I do, but in both directions (see example 
below).


I can't control what values other users/developers may supply, nor I 
restrict the use of different compiler versions and build flags. It is 
because "TConvertType(-1)" and the likes are an acceptable statement for 
compiler, I make it my responsibility to handle all possible input 
values, no matter how illogical and invalid they may be. This is a 
design pattern that I follow.


If the 'else' condition is optimized away in certain compiler versions 
and/or by certain complier optimization flags, that is no problem, at 
least not for me. Then, as Martin highlighted, "unreachable code" 
warning may be appropriate.


Currently, TConvertType(X) doesn't produce any errors where X is out of 
declared range of TConvertType. So I shall continue to account for 
potential invalid values, even if my attempts may be optimized away in 
some circumstances. I might be trying to account for an illegal or 
undefined behaviour, but I do nonetheless, if it improves the chances of 
catching errors earlier.


type
  TConvertType = (ctA, ctB);

function StrToConvert(const Value: String): TConvertType;
begin
  case Value of
'A': Result := ctA;
'B': Result := ctB;
else raise Exception.CreateFmt('Invalid convert value string (%s)', 
[Value]);

  end;
end;

function Convert(Value: TConvertType): Integer;
begin
  case Value of
ctA: Result := 1;
ctB: Result := 2;
else raise Exception.CreateFmt('Invalid convert value (%d)', 
[Ord(Value)]);

  end;
end;
___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Re: [fpc-devel] Data flow analysis (dfa) and "case ... of"

[Juha Manninen]

On Wed, Jun 7, 2017 at 2:04 AM, Denis Kozlov  wrote:
> You suggest that only Convert3 function should raise "uninitialized result"
> warning, while Convert1 and Convert2 should not. I find this less useful,
> and, again, you can rightfully disagree, but it won't change the fact that
> it is still less useful for me (and possibly others).

Sorry but this is not a matter of opinion. Your view is just flawed and wrong.
FPC developers tried to explain that your "else" can be optimized away
and the program may crash.
The compiler trusts that data in an enum variable is legal, within the range.
It should trust the same way when doing DFA. It is logical and consistent.

If you let your enum data be out of range then your code is seriously
buggy. Why would you write such buggy code?
What you must do is add sanity checks for code that initializes enum
data based on some other data.
For example you are streaming and get data as string:

type
  TEnumType = (ctA, ctB);

function StrToEnum(s: string): TEnumType;
begin
  case s of
"ctA": Result := ctA;
"ctB": Result := ctB;
else raise Exception.Create('Illegal TEnumType value!');
  end;
end;

The same way if you get integer data from I/O, you must verify the
integer is within range _before_ typecasting it.

Logical and robust.

Juha
___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Re: [fpc-devel] Data flow analysis (dfa) and "case ... of"

[Martin]

On 07/06/2017 00:04, Denis Kozlov wrote:


Consider the code fragment below. Currently, FCP 3.0.2 with -O3 shows 
"uninitialized result" warning for Convert2 and Convert3 functions, 
but not for Convert1. I find this perfect as is, and, of course, you 
can rightfully disagree.


You suggest that only Convert3 function should raise "uninitialized 
result" warning, while Convert1 and Convert2 should not. I find this 
less useful, and, again, you can rightfully disagree, but it won't 
change the fact that it is still less useful for me (and possibly 
others).


You did of course read Jonas' statement, that if you call
   Convertn(TConvertType(-1))

then Convert1 and Convert2 may crash, depending on compiler version and 
optimization level (this may also depend on how many elements 
TConvertType has)


Btw, from Jonas statement follows, that the "else" part in Convert1 may 
be silently removed by the compiler. (maybe now or maybe by future fpc 
versions). This may then give an "unreachable code" warning)


The warning however is only useful for Convert2, if you speculate that 
the "else" in Convert1 is used for TConvertType(-1).
If it isn't then you have a bigger problem, and that is not caught by 
this warning anyway.




type
  TConvertType = (ctA, ctB);

function Convert1(Value: TConvertType): Integer;
begin
  case Value of
ctA: Result := 1;
ctB: Result := 2;
else Result := 0;
  end;
end;

function Convert2(Value: TConvertType): Integer;
begin
  case Value of
ctA: Result := 1;
ctB: Result := 2;
  end;
end;

function Convert3(Value: TConvertType): Integer;
begin
  case Value of
ctA: Result := 1;
  end;
end;


___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Re: [fpc-devel] Data flow analysis (dfa) and "case ... of"

[Denis Kozlov]

Juha,

Just to get this straight, I am talking about DFA and your proposal to 
remove "uninitialized result" warning when all declared values of a set 
have been enumerated in a case statement. I don't have any issues with 
range check errors, they were brought up and became part of 
conversation. Now, put aside range checks and focus on DFA and case 
statements.


Consider the code fragment below. Currently, FCP 3.0.2 with -O3 shows 
"uninitialized result" warning for Convert2 and Convert3 functions, but 
not for Convert1. I find this perfect as is, and, of course, you can 
rightfully disagree.


You suggest that only Convert3 function should raise "uninitialized 
result" warning, while Convert1 and Convert2 should not. I find this 
less useful, and, again, you can rightfully disagree, but it won't 
change the fact that it is still less useful for me (and possibly others).


I won't indulge in any further debate. I voiced my opinion/concern and 
will leave it at that. Feel free to ignore it.



type
  TConvertType = (ctA, ctB);

function Convert1(Value: TConvertType): Integer;
begin
  case Value of
ctA: Result := 1;
ctB: Result := 2;
else Result := 0;
  end;
end;

function Convert2(Value: TConvertType): Integer;
begin
  case Value of
ctA: Result := 1;
ctB: Result := 2;
  end;
end;

function Convert3(Value: TConvertType): Integer;
begin
  case Value of
ctA: Result := 1;
  end;
end;



On 05/06/2017 22:11, Juha Manninen wrote:

On Mon, Jun 5, 2017 at 9:37 PM, Denis Kozlov  wrote:

I just wanted to highlight that these cases as legal and I presume not
uncommon, particularly if values are deserialized and typecasted.

No they are not legal. Why you write this nonsense?
Values outside the range dictated by the type system are always wrong.
No excuses, no exceptions.
For the purposes of DFA the compiler can always assume the values are in range.

- Later: -

Complier currently warns (cares) about such execution flows
which are triggered by invalid data, and I happen to find it useful :)

Again nonsense.
The range check -Cr warns about out-of-range data, yes, but this
thread is about DFA and not about range checks.
I feel you hijacked my DFA thread for your own purpose but I don't
really know what that purpose is.
Are you saying that typecasts should be forbidden to prevent range errors?
No, it would not prevent bugs in code.
Anyway, please start a new thread about range check issues.

Juha


___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Re: [fpc-devel] Data flow analysis (dfa) and "case ... of"

[Florian Klämpfl]

Am 05.06.2017 um 20:49 schrieb Jonas Maebe:
> On 05/06/17 20:37, Denis Kozlov wrote:
>>
>>
>> On 05/06/2017 18:59, Jonas Maebe wrote:
>>> That is why I said "If range checking is off there or disabled via an 
>>> explicit type cast, then
>>> the result is undefined by definition." You use an explicit typecast above.
>>
>> I just wanted to highlight that these cases as legal and I presume not
>> uncommon, particularly if values are deserialized and typecasted.
> 
> Then this de-serialisation code must perform range checking. Again: if you 
> assign an invalid value
> to a variable by typecasting, disabling range checking, inline assembly, 
> passing a pointer to C code
> and overwriting the value there, or in any other way, Pascal code that works 
> with the resulting
> value has undefined behaviour. The program could crash, silently fail, raise 
> a random exception at
> some point, or do anything else.
> 

For the record: this can already happen. If a jump table for a case statement 
is generated, the
compiler does not check the boundaries if the jump table covers the whole 
declared (!) range of the
case variable.

___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Re: [fpc-devel] Data flow analysis (dfa) and "case ... of"

[Juha Manninen]

On Mon, Jun 5, 2017 at 9:37 PM, Denis Kozlov  wrote:
> I just wanted to highlight that these cases as legal and I presume not
> uncommon, particularly if values are deserialized and typecasted.

No they are not legal. Why you write this nonsense?
Values outside the range dictated by the type system are always wrong.
No excuses, no exceptions.
For the purposes of DFA the compiler can always assume the values are in range.

- Later: -
> Complier currently warns (cares) about such execution flows
> which are triggered by invalid data, and I happen to find it useful :)

Again nonsense.
The range check -Cr warns about out-of-range data, yes, but this
thread is about DFA and not about range checks.
I feel you hijacked my DFA thread for your own purpose but I don't
really know what that purpose is.
Are you saying that typecasts should be forbidden to prevent range errors?
No, it would not prevent bugs in code.
Anyway, please start a new thread about range check issues.

Juha
___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Re: [fpc-devel] Data flow analysis (dfa) and "case ... of"

[Jonas Maebe]

On 05/06/17 20:37, Denis Kozlov wrote:



On 05/06/2017 18:59, Jonas Maebe wrote:
That is why I said "If range checking is off there or disabled via an 
explicit type cast, then the result is undefined by definition." You 
use an explicit typecast above.

>
> I just wanted to highlight that these cases as legal and I presume not
> uncommon, particularly if values are deserialized and typecasted.

Then this de-serialisation code must perform range checking. Again: if 
you assign an invalid value to a variable by typecasting, disabling 
range checking, inline assembly, passing a pointer to C code and 
overwriting the value there, or in any other way, Pascal code that works 
with the resulting value has undefined behaviour. The program could 
crash, silently fail, raise a random exception at some point, or do 
anything else.


> It seems that there is no way for compiler to know if the result has
> indeed being initialized by reason of enumeration of valid values,
> because one has also account for invalid values.

No, the compiler does not have to care about the fact that you may have 
put invalid data in that variable by bypassing the type system. If you 
do that, you are on your own. The compiler also uses type information to 
perform optimizations (e.g. "high(TAnkorKind) < Kind" will be replaced 
by "false" at compile time, because there is no valid program flow with 
which this statement can become false).


> I think that compiler should always warn (as it does now) if result is
> not initialized against all possible cases.

The DFA simply does not take into account the result of case-statements 
at all right now. If you have a "case bytevar of" with every possible 
value from 0 to 255, you will still get the warning.



Jonas
___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Re: [fpc-devel] Data flow analysis (dfa) and "case ... of"

[Sven Barth via fpc-devel]

On 05.06.2017 20:37, Denis Kozlov wrote:
> 
> I just wanted to highlight that these cases as legal and I presume not
> uncommon, particularly if values are deserialized and typecasted.

They are legal in the sense that they result in undefined behavior and
in that case the compiler is free to act as it sees fit.

Regards,
Sven
___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Re: [fpc-devel] Data flow analysis (dfa) and "case ... of"

[Denis Kozlov]

I just wanted to highlight that these cases as legal and I presume not 
uncommon, particularly if values are deserialized and typecasted.


It seems that there is no way for compiler to know if the result has 
indeed being initialized by reason of enumeration of valid values, 
because one has also account for invalid values.


I think that compiler should always warn (as it does now) if result is 
not initialized against all possible cases.


Denis


On 05/06/2017 18:59, Jonas Maebe wrote:
That is why I said "If range checking is off there or disabled via an 
explicit type cast, then the result is undefined by definition." You 
use an explicit typecast above.

___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Re: [fpc-devel] Data flow analysis (dfa) and "case ... of"

[Juha Manninen]

On Monday, June 5, 2017, Denis Kozlov  wrote:

> I have just created a new GUI application, enabled range checking "-Cr"
> and executed the following:
> ShowMessage(IntToStr(Form1.BorderSpacing.GetSpace(TAnchorKind(-1;
> The displayed value is a semi-random integer. No compiler errors or
> run-time errors.
>

The typecast suppresses a range error in your code.
However the LCL method GetSpace should give an error if it is compiled with
-Cr.
So, build Lazarus + LCL with profile "Debug IDE" and try again.

Juha
___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Re: [fpc-devel] Data flow analysis (dfa) and "case ... of"

[Jonas Maebe]

On 05/06/17 19:39, Denis Kozlov wrote:
I have just created a new GUI application, enabled range checking "-Cr" 
and executed the following:


ShowMessage(IntToStr(Form1.BorderSpacing.GetSpace(TAnchorKind(-1;

The displayed value is a semi-random integer. No compiler errors or 
run-time errors.


That is why I said "If range checking is off there or disabled via an 
explicit type cast, then the result is undefined by definition." You use 
an explicit typecast above.



Jonas
___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Re: [fpc-devel] Data flow analysis (dfa) and "case ... of"

[Denis Kozlov]

I have just created a new GUI application, enabled range checking "-Cr" 
and executed the following:


ShowMessage(IntToStr(Form1.BorderSpacing.GetSpace(TAnchorKind(-1;

The displayed value is a semi-random integer. No compiler errors or 
run-time errors.


P.S. Same behaviour for Lazarus 1.6.4 + FPC 3.0.2 and Lazarus 1.4.4 + 
FPC 2.6.4.


Denis


On 05/06/2017 17:51, Jonas Maebe wrote:

On 05/06/17 18:46, Denis Kozlov wrote:

Result is undefined if caller passes an out of range value.


That would be an error at the time this out-of-range value would be 
converted to TAnchorKind. If range checking is off there or disabled 
via an explicit type cast, then the result is undefined by definition. 
The compiler can and does assume in various cases that a type only 
holds values valid for that type. 


On 05/06/2017 18:07, Juha Manninen wrote:

On Mon, Jun 5, 2017 at 7:46 PM, Denis Kozlov  wrote:

Result is undefined if caller passes an out of range value.
An example: TControlBorderSpacing.GetSpace(TAnchorKind(-1));
Result is indeed not initialized, so silencing this message may be harmful.

Yes but that is an illegal situation. It is already trapped by range check -Cr.
The compiler can safely assume that such situation does not happen.
Actually the compiler already assumes that in other situations. I
remember a bug years ago in Lazarus, caused by an out of range enum
value. FPC had changed its behavior and the bug got triggered.

___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Re: [fpc-devel] Data flow analysis (dfa) and "case ... of"

[Juha Manninen]

For some reason my reply went only to Denis first. Another try here...

On Mon, Jun 5, 2017 at 7:46 PM, Denis Kozlov  wrote:
> Result is undefined if caller passes an out of range value.
> An example: TControlBorderSpacing.GetSpace(TAnchorKind(-1));
> Result is indeed not initialized, so silencing this message may be harmful.

Yes but that is an illegal situation. It is already trapped by range check -Cr.
The compiler can safely assume that such situation does not happen.
Actually the compiler already assumes that in other situations. I
remember a bug years ago in Lazarus, caused by an out of range enum
value. FPC had changed its behavior and the bug got triggered.

Juha
___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Re: [fpc-devel] Data flow analysis (dfa) and "case ... of"

[Jonas Maebe]

On 05/06/17 18:46, Denis Kozlov wrote:

Result is undefined if caller passes an out of range value.


That would be an error at the time this out-of-range value would be 
converted to TAnchorKind. If range checking is off there or disabled via 
an explicit type cast, then the result is undefined by definition. The 
compiler can and does assume in various cases that a type only holds 
values valid for that type.



Jonas

___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Re: [fpc-devel] Data flow analysis (dfa) and "case ... of"

[Denis Kozlov]

Result is undefined if caller passes an out of range value.

An example: TControlBorderSpacing.GetSpace(TAnchorKind(-1));

Result is indeed not initialized, so silencing this message may be harmful.

Denis


On 05/06/2017 15:22, Juha Manninen wrote:

Wiki tells about problems the dfa encounters:
  http://wiki.freepascal.org/FPC_New_Features_3.0#Node_dfa_for_liveness_analysis

In this code if "b" is true in the first test, it will be true also in
the second test, thus using "i" is safe and valid.
   ...
   if b then
 i:=1;
   writeln;
   if b then
 writeln(i);

I understand that complex code can have many variations of such
constructs and analysing them all is difficult.

However for "case ... of" construct with enum types the dfa could be
improved easily, at least I could imagine so.
There is plenty of code like this:

   function TControlBorderSpacing.GetSpace(Kind: TAnchorKind): Integer;
   begin
 case Kind of
 akLeft: Result:=Left;
 akTop: Result:=Top;
 akRight: Result:=Right;
 akBottom: Result:=Bottom;
 end;
   end;

All the enum values are listed and the Result is set for sure, yet FPC
complains:
  controls.pp(3721,1) Warning: Function result variable does not seem
to be initialized

If the compiler used the information about all enums being present, it
would reduce the number of dfa false positives in eg. Lazarus sources
dramatically.

Juha

___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel