View Issue Details

IDProjectCategoryView StatusLast Update
0025440FPCCompilerpublic2013-12-21 11:47
ReporterMark Morgan Lloyd Assigned ToSergei Gorelkin  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
PlatformSPARCOSLinux 
Product Version2.7.1 
Fixed in Version3.0.0 
Summary0025440: On SPARC, spurious range error when decrementing a smallint class field.
DescriptionWhen the a smallint field with the value -1 is decremented, a range error is raised. Does not appear to affect ordinary variables.

Affects SPARC Linux, does not affect x86 or PPC Linux, other platforms untested.
Steps To ReproduceRun attached program. Correct output should be numbers 1 0 -1 followed by OK, on SPARC the OK is replaced by an error 201.
TagsNo tags attached.
Fixed in Revision26255,26258
FPCOldBugId
FPCTarget
Attached Files

Activities

Mark Morgan Lloyd

2013-12-18 19:29

reporter  

test_range.pas (596 bytes)   
program TestRange;

{$mode objfpc }

{$Q- }
{$R+ }

type
  TSynPasSynRange = class // (TSynCustomHighlighterRange)
  protected
    FPasFoldFixLevel: Smallint;
  public
    procedure DecLastLinePasFoldFix;
  end;

procedure TSynPasSynRange.DecLastLinePasFoldFix;
begin
  dec(FPasFoldFixLevel);
end;


var	z: TSynPasSynRange;	

begin
  z := TSynPasSynRange.Create;
  z.FPasFoldFixLevel := 1;
  WriteLn(z.FPasFoldFixLevel);
  z.DecLastLinePasFoldFix;
  WriteLn(z.FPasFoldFixLevel);
  z.DecLastLinePasFoldFix;
  WriteLn(z.FPasFoldFixLevel);
  z.DecLastLinePasFoldFix;
  WriteLn('OK');
  z.Free
end.

test_range.pas (596 bytes)   

Sergei Gorelkin

2013-12-20 18:25

developer   ~0071985

Caused by two reasons:
- inc(x)/dec(x) with range checking enabled are "optimized" with taking address of x. They were using untyped address, causing loss of alignment and generating large and slow code with unaligned access. This is fixed in r26256 and improves all alignment-sensitive targets.

- tcg.a_load_ref_reg_unaligned was always zero-extending the result, while it is expected to sign-extend if loading a signed value. This is fixed in r26255.

Jonas Maebe

2013-12-20 18:36

manager   ~0071986

Is this safe if you call inc() on an unaligned field in a packed record? I don't see where the alignment is preserved in the new code. In fact, it doesn't seem like there is a possibility to create an "unaligned" pointer in the compiler currently other than by using e.g. a voidpointer (tcgderefnode.pass_generate_code always forces natural alignment)

Mark Morgan Lloyd

2013-12-20 23:19

reporter   ~0071991

I've tested Sergei's fix on SPARC against the example code that I supplied and it appears OK.

However I lack the expertise to decide that the issue should be closed in light of Jonas's questions.

Sergei Gorelkin

2013-12-21 11:42

developer   ~0071993

Jonas is right, as always :-)
It is impossible to preserve, or even determine the alignment during pass 1, so using a voidpointer is only safe way. However it is possible to increase the node complexity at which the optimization happens, so inc/dec on e.g. record fields do not use the optimization. This is done in r25258.

Jonas Maebe

2013-12-21 11:46

manager   ~0071994

I'm definitely not always right. Case in point, there is a way to make pointer-based accesses unaligned in pass 1: create the equivalent of unaligned(ptr^). Determining the alignment in pass 1 for all cases is however indeed another matter :/ We could add an optimization for certain simple cases (local variables, parameters), but it's not very clean.

Mark Morgan Lloyd

2013-12-21 11:47

reporter   ~0071995

Tested and believed OK. Many thanks for the fix.

Issue History

Date Modified Username Field Change
2013-12-18 19:29 Mark Morgan Lloyd New Issue
2013-12-18 19:29 Mark Morgan Lloyd File Added: test_range.pas
2013-12-20 18:25 Sergei Gorelkin Note Added: 0071985
2013-12-20 18:26 Sergei Gorelkin Fixed in Revision => 26255,26256
2013-12-20 18:26 Sergei Gorelkin Status new => resolved
2013-12-20 18:26 Sergei Gorelkin Fixed in Version => 2.7.1
2013-12-20 18:26 Sergei Gorelkin Resolution open => fixed
2013-12-20 18:26 Sergei Gorelkin Assigned To => Sergei Gorelkin
2013-12-20 18:36 Jonas Maebe Note Added: 0071986
2013-12-20 23:19 Mark Morgan Lloyd Note Added: 0071991
2013-12-20 23:19 Mark Morgan Lloyd Status resolved => feedback
2013-12-20 23:19 Mark Morgan Lloyd Resolution fixed => reopened
2013-12-21 11:42 Sergei Gorelkin Note Added: 0071993
2013-12-21 11:42 Sergei Gorelkin Fixed in Revision 26255,26256 => 26255,26258
2013-12-21 11:42 Sergei Gorelkin Status feedback => resolved
2013-12-21 11:42 Sergei Gorelkin Resolution reopened => fixed
2013-12-21 11:46 Jonas Maebe Note Added: 0071994
2013-12-21 11:47 Mark Morgan Lloyd Note Added: 0071995
2013-12-21 11:47 Mark Morgan Lloyd Status resolved => closed