View Issue Details

IDProjectCategoryView StatusLast Update
0037136FPCCompilerpublic2020-05-26 23:05
ReporterOndrej Pokorny Assigned ToFlorian  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Platformwin32 i386OSWindows 
Product Version3.3.1 
Fixed in Version3.3.1 
Summary0037136: SIGFPE in an assignment after a float comparison winth NaN
DescriptionA SIGFPE is raised in an assignment right after a NaN Extended value has been compared to another value.
Steps To Reproduceprogram NaNTest;
{$mode objfpc}
uses Math;
var
  B: Boolean;
  N1, N2: Extended;
begin
  N1 := NaN;
  B := N1<4;
  N2 := 0; // << SIGFPE
end.
TagsNo tags attached.
Fixed in Revision45500
FPCOldBugId
FPCTarget-
Attached Files

Activities

Jonas Maebe

2020-05-24 20:19

manager   ~0123039

By default, the FPU raises exceptions if you try to perform invalid operations. If you don't want them, you can mask them using https://www.freepascal.org/docs-html/rtl/math/setexceptionmask.html

Ondrej Pokorny

2020-05-24 20:22

developer   ~0123040

But "N2 := 0;" is not an invalid operation.

If "B := N1<4;" is an invalid operation, the exception MUST be raised there and not (possibly many lines) after.

Ondrej Pokorny

2020-05-24 20:24

developer   ~0123041

How should a person be able to find the issue in code like this:

program NaNTest;
{$mode objfpc}
{$SAFEFPUEXCEPTIONS on} // does not change anything
uses Math;
var
  B: Boolean;
  N1, N2: Extended;
  S: string;
begin
  N1 := NaN;
  B := N1<4;
  // do something - still no exception
  S := 'string';
  S := S+S;
  // exception on next line
  N2 := 0; // << SIGFPE
end.

when the exception is raised at a completely wrong position possibly many steps after the correct line?

Jonas Maebe

2020-05-24 20:36

manager   ~0123042

You will have to ask the designers of the Intel processor.

Ondrej Pokorny

2020-05-24 20:47

developer   ~0123043

Last edited: 2020-05-24 20:48

View 2 revisions

You don't get the point.

{$SAFEFPUEXCEPTIONS on}

should add a WAIT instruction after an FPE exception can be triggered so that SIGFPE gets raised at the correct line. Yet it does not.

Delphi manages to do it (even by default), so why FPC doesn't? {$SAFEFPUEXCEPTIONS on} is absolutely no help here, yet it should help.

Jonas Maebe

2020-05-24 20:55

manager   ~0123044

Last edited: 2020-05-24 20:56

View 2 revisions

As https://www.freepascal.org/docs-html/prog/progsu69.html says: this adds fwait instructions when FPU values get stored. The comparison does not store an FPU value and hence no fwait gets inserted. Inserting it after every single FPU instruction would result in a significant slowdown. Maybe an option could be added to insert it at the end of all statements that include x87 opcodes that may trigger an exception.

nanobit

2020-05-24 21:36

reporter   ~0123045

Last edited: 2020-05-24 21:50

View 2 revisions

{$SAFEFPUEXCEPTIONS on} is intentional slowdown,
so maybe more fwaits could be inserted here. This would be closer to documentation:
"The generated code is slower, but errors are reported at the location where the instruction was executed "

Also catching fpu-exception is not possible with:
try failingFpuOp; ... except ... end;
At least one fwait (or another fpuOp) before "except" would be needed.

Ondrej Pokorny

2020-05-24 21:45

developer   ~0123046

> Inserting it after every single FPU instruction would result in a significant slowdown.

I seriously doubt this.

I tested a small program on my PC in Delphi and FPC:

program Project1;
{$APPTYPE CONSOLE}
{$R *.res}
uses Windows, Math;
var
  B: Boolean;
  N1, N2: Extended;
  I: Integer;
  Q: Cardinal;
begin
  Q := GetTickCount;
  N1 := 0;
  for I := 0 to High(I) do
    B := N1<4;
  Writeln(GetTickCount-Q);
  Readln;
end.

The results are:

Delphi Debug: 3000 (with FPU exception checking and WAIT instructions)
Delphi Release: 500 (no FPU exception checking and no WAIT instructions)

FPC Debug: 4000 (-Scagi -Cirot -O1 -gw2 -godwarfsets -gl -gh -Xg -gt -l -vewnhibq -gh -gl -Ci -Cr -Co -Ct -Sa)
FPC Release: 2500 (-Scghi -CX -O3 -XX -l -vewnhibq -gw -godwarfsets -gl -Xg)

Delphi with a WAIT instruction has still about the same performance as FPC does. Delphi Release is much better. (OK, maybe the FPC options can be tuned as well.)

> As https://www.freepascal.org/docs-html/prog/progsu69.html says: this adds fwait instructions when FPU values get stored. The comparison does not store an FPU value and hence no fwait gets inserted.

Yes. And it does not make sense. I either want good performance so no WAIT instructions whatsoever (Release) or I want good debugging info and performance is not important (Debug) and so I want to have WAIT everywhere.

Having an option for fine-tuning of SAFEFPUEXCEPTIONS (to decide after what operations I want save FPU exceptions) is not necessary at all. I either want all of them or I don't want any. Everything in between is useless.

You mean that if $SAFEFPUEXCEPTIONS is documented somehow it cannot be changed anymore?

Ondrej Pokorny

2020-05-24 21:53

developer   ~0123047

> As https://www.freepascal.org/docs-html/prog/progsu69.html says: this adds fwait instructions when FPU values get stored. The comparison does not store an FPU value and hence no fwait gets inserted.

You picked up only the part of the documentation that suits you. The second part is "This ensures that any errors are reported immediately. The generated code is slower, but errors are reported at the location where the instruction was executed."

If I take this part of the documentation into account, $SAFEFPUEXCEPTIONS should "ensure that any errors are reported immediately". Yet it does not, as shown with the examples above.

Jonas Maebe

2020-05-24 21:59

manager   ~0123048

Last edited: 2020-05-24 22:11

View 4 revisions

I was just explaining why it didn't catch the error in your program, that this behaviour is documented, and the rationale behind the current behaviour. I did not say it cannot be changed after you clarified that your comment was about the behaviour of {$safefpuexeptions on} (in fact, I said the exact opposite).

Ondrej Pokorny

2020-05-24 22:05

developer   ~0123049

OK, thank you!

Florian

2020-05-25 22:40

administrator   ~0123064

Note that only i386 (i8086?) is affected and only if the selected cpu is older than Pentium II. So I really wonder that somebody hit this.

nanobit

2020-05-26 07:00

reporter   ~0123067

Last edited: 2020-05-26 07:06

View 2 revisions

Good to know, because the default setting is lower than -CpPentium2

Ondrej Pokorny

2020-05-26 15:49

developer   ~0123074

Florian, thanks for the fix. Yes - that helped! Good after Jonas rejected the issue twice ;)

> Note that only i386 (i8086?) is affected and only if the selected cpu is older than Pentium II. So I really wonder that somebody hit this.

I have an Intel Core i7-8700 CPU that is 2 years old.

The "default_settings: TSettings" in globals.pas default to cpu_Pentium for me:
{ Note: GENERIC_CPU is used together with generic subdirectory to
  be able to compile some of the units without any real CPU.
  This is used to generate a CPU independant PPUDUMP utility. PM }
{$else not GENERIC_CPU}
  {$ifdef i386}
// this section is active
        fcputype : cpu_Pentium;
        optimizecputype : cpu_Pentium3;
        asmcputype : cpu_none;
        fputype : fpu_x87;
  {$endif i386}

I didn't fiddle with the default CPU target settings.

Florian

2020-05-26 20:24

administrator   ~0123078

@Ondrej: yes, I know this (this is why I always pass a processor type). But each time I propose to rise the default processor, it is not wanted :)

Ondrej Pokorny

2020-05-26 23:05

developer   ~0123081

I am sure you know that - I just wanted to answer you and explain why I hit this: because I was the one that didn't know about the processor settings and I kept them default. I learned something again :) And I also learned why SIGFPE can raise at a correct line. Thanks.

Issue History

Date Modified Username Field Change
2020-05-24 20:12 Ondrej Pokorny New Issue
2020-05-24 20:19 Jonas Maebe Assigned To => Jonas Maebe
2020-05-24 20:19 Jonas Maebe Status new => resolved
2020-05-24 20:19 Jonas Maebe Resolution open => no change required
2020-05-24 20:19 Jonas Maebe FPCTarget => -
2020-05-24 20:19 Jonas Maebe Note Added: 0123039
2020-05-24 20:22 Ondrej Pokorny Status resolved => new
2020-05-24 20:22 Ondrej Pokorny Resolution no change required => reopened
2020-05-24 20:22 Ondrej Pokorny Note Added: 0123040
2020-05-24 20:24 Ondrej Pokorny Note Added: 0123041
2020-05-24 20:36 Jonas Maebe Status new => resolved
2020-05-24 20:36 Jonas Maebe Resolution reopened => not fixable
2020-05-24 20:36 Jonas Maebe Note Added: 0123042
2020-05-24 20:47 Ondrej Pokorny Status resolved => new
2020-05-24 20:47 Ondrej Pokorny Resolution not fixable => reopened
2020-05-24 20:47 Ondrej Pokorny Note Added: 0123043
2020-05-24 20:48 Ondrej Pokorny Note Edited: 0123043 View Revisions
2020-05-24 20:55 Jonas Maebe Note Added: 0123044
2020-05-24 20:56 Jonas Maebe Assigned To Jonas Maebe =>
2020-05-24 20:56 Jonas Maebe Note Edited: 0123044 View Revisions
2020-05-24 21:36 nanobit Note Added: 0123045
2020-05-24 21:45 Ondrej Pokorny Note Added: 0123046
2020-05-24 21:50 nanobit Note Edited: 0123045 View Revisions
2020-05-24 21:53 Ondrej Pokorny Note Added: 0123047
2020-05-24 21:59 Jonas Maebe Note Added: 0123048
2020-05-24 22:01 Jonas Maebe Note Edited: 0123048 View Revisions
2020-05-24 22:05 Ondrej Pokorny Note Added: 0123049
2020-05-24 22:11 Jonas Maebe Note Edited: 0123048 View Revisions
2020-05-24 22:11 Jonas Maebe Note Edited: 0123048 View Revisions
2020-05-25 22:40 Florian Assigned To => Florian
2020-05-25 22:40 Florian Status new => resolved
2020-05-25 22:40 Florian Resolution reopened => fixed
2020-05-25 22:40 Florian Fixed in Version => 3.3.1
2020-05-25 22:40 Florian Fixed in Revision => 45500
2020-05-25 22:40 Florian Note Added: 0123064
2020-05-26 07:00 nanobit Note Added: 0123067
2020-05-26 07:06 nanobit Note Edited: 0123067 View Revisions
2020-05-26 15:49 Ondrej Pokorny Note Added: 0123074
2020-05-26 20:24 Florian Note Added: 0123078
2020-05-26 23:05 Ondrej Pokorny Status resolved => closed
2020-05-26 23:05 Ondrej Pokorny Note Added: 0123081