SIGSEV in custom QuickSort implementation

Question

I slept over the answer to question Quicksort drama and wanted to recode it from scratch, implementing your tip with the call-by-reference var. And again: I cannot find any failure I made again. I compare the code to your program one by one and I cannot find the problem. The following code produces an Exception (External:SIGSEV at address 11602) during compilation/run

program quicksort;

var
  iArray : array[0..8] of integer;

procedure fillArray(var iArray : array of integer);
begin;
  iArray[0] := 3;
  iArray[1] := 1;
  iArray[2] := 8;
  iArray[3] := 4;
  iArray[4] := 9;
  iArray[5] := 0;
  iArray[6] := 8;
  iArray[7] := 2;
  iArray[8] := 5;
end;

procedure writeArray(iArray : array of integer);
var i:integer;
begin
  for i:=low(iArray) to high(iArray) do begin
    write(iArray[i]);
  end;
  writeln('');
end;

procedure quickSort(var iArray : array of integer; links : integer; rechts:integer);
var
  l,r,pivot, temp: integer;
begin
  if (rechts > links) then begin
    l := links;
    r := rechts;
    pivot := iArray[(rechts+links) div 2];

    while (l<r) do begin
      while (iArray[l] < pivot) do l:=l+1;
      while (iArray[r] > pivot) do r:=r-1;
      if (l<=r) then begin
       temp := iArray[l];
       iArray[l] := iArray[r];
       iArray[r] := temp;
      end;
    end;
    if (links < r) then quickSort(iArray, links, r);
    if (l < rechts) then quickSort(iArray, l, rechts);
  end;
end;

begin
  fillArray(iArray);
  quickSort(iArray,low(iArray),high(iArray));
  writeArray(iArray);
end.

Answer 1

The block of code that swaps, also needs to increment l and decrement r once the swap is complete:

if (l <= r) then
begin
  temp := iArray[l];
  iArray[l] := iArray[r];
  iArray[r] := temp;
  inc(l); // <-- this was missing
  dec(r); // <-- as was this
end;

The complete program, with some other minor tidy ups:

program quicksort24340509;

var
  iArray: array [0 .. 8] of integer;

Procedure fillArray(var iArray: array of integer);
begin;

  iArray[0] := 3;
  iArray[1] := 1;
  iArray[2] := 8;
  iArray[3] := 4;
  iArray[4] := 9;
  iArray[5] := 0;
  iArray[6] := 8;
  iArray[7] := 2;
  iArray[8] := 5;
end;

Procedure writeArray(const iArray: array of integer);
var
  i: integer;
begin
  for i := low(iArray) to high(iArray) do
  begin
    write(iArray[i], ' ');
  end;
  writeln;
end;

Procedure quickSort(var iArray: array of integer; links, rechts: integer);
var
  l, r, pivot, temp: integer;
begin
  if (rechts > links) then
  begin
    l := links;
    r := rechts;
    pivot := iArray[(rechts + links) div 2];

    while l < r do
    begin
      while iArray[l] < pivot do inc(l);
      while iArray[r] > pivot do dec(r);
      if l <= r then
      begin
        temp := iArray[l];
        iArray[l] := iArray[r];
        iArray[r] := temp;
        inc(l);
        dec(r);
      end;
    end;
    if links < r then
      quickSort(iArray, links, r);
    if l < rechts then
      quickSort(iArray, l, rechts);
  end;
end;

begin
  fillArray(iArray);
  quickSort(iArray, low(iArray), high(iArray));
  writeArray(iArray);
  readln;
end.

Output

0 1 2 3 4 5 8 8 9

---

The reason that your version fails, without the missing lines, is that the recursive calls to quickSort operate on the wrong ranges.

For example, Given your input of

3 1 8 4 9 0 8 2 5

the partitioning step pivots on 9 and results in

3 1 8 4 5 0 8 2 9

Now, the recursive step should be to sort all the values to the left of the pivot, and all the values to the right. And we leave the pivot alone because partitioning ensured that it is in its final position.

There are no values to the right of the pivot so we should be making a recursive call for the range 0 to 7. But if you inspect what happens with your code you will find that it does not. Instead it makes a recursive call for the range 0 to 8. That in itself is a little benign, but once the ranges become small, at the stopping condition, it's different. Try asking your program to sort these values:

1 2

The code pivots on 1. At the end of partitioning we have:

links = 0
rechts = 1
l = 0
r = 0

So we recursively call quickSort passing l and rechts as the ranges. But that's exactly the same call as we initially made. And that therefore leads to a stack overflow.

So the point is that we must make sure that when we partition on a pivot, we exclude that pivot from all future recursive calls to quickSort. If we don't do that we don't sub-divide the problem, and the recursion does not terminate.