Tricky
Tricky

Reputation: 4471

Pointers to Pointers - should old pointer still exist?

In ActiveHDL, I get en error of a NULL pointer reference on the report line. I argue that ptr should still hold the old pointer value, and hence should not be NULL. I've raised this as a ticket and the response was that the behaviour is correct, and in VHDL 2018 it is still correct due to garbage collection.

I would argue that it is wrong in both standards as ptr still holds the pointer reference to the old data, so shouldn't have been made null and/or should not be garbage collected (in VHDL2018).

What can I try to resolve this?

Edit

Here's the code from the defect:

entity int_append_test is
end entity;

architecture test of int_append_test is

  type int_vector_ptr_t is access integer_vector;
  type int_vector_ptr_ptr_t is access int_vector_ptr_t;

  procedure append( variable ptr  : inout int_vector_ptr_t;
                    constant val  : in    integer ) is
    variable p  : int_vector_ptr_t;
  begin
    p := ptr;

    if ptr = null then
      ptr := new integer_vector'( (0 => val) );
    else
      ptr := new integer_vector'(p.all & val);
    end if;

    DEALLOCATE(p);  -- Remove old data, prevent memory leak
  end procedure append;



begin

  process
    variable ptr      : int_vector_ptr_t;
    variable ptr_ptr  : int_vector_ptr_ptr_t;

    function to_string(iv : integer_vector) return string is
      variable l : std.textio.line;
    begin
      for i in iv'range loop
        std.textio.write(l, to_string(iv(i)) & " ");
      end loop;

      return l.all; -- I know this is memory leak, but its easy
    end function;

  begin
    for i in 0 to 10 loop
      append(ptr, i);
    end loop;

    for i in ptr.all'range loop
      report "ptr=" & to_string(ptr.all(i));
    end loop;


    ptr_ptr := new int_vector_ptr_t;

    for i in 0 to 10 loop
      --append(ptr_ptr.all, i);

      ptr := ptr_ptr.all;

      if i = 0 then
        ptr_ptr.all := new integer_vector'( (0 => i) );
      else
        ptr_ptr.all := new integer_vector'(ptr.all & i);
      end if;

      -- If you add these two lines, you get a NULL pointer, but why don't you get a null pointer
      -- reference above?

      report "ptr_data = " & to_string(ptr.all);
      --report "ptr_data = " & to_string(ptr.all(i));

      DEALLOCATE(ptr);  -- Remove old data, prevent memory leak

    end loop;

    for i in ptr_ptr.all.all'range loop
      report "ptr=" & to_string(ptr_ptr.all.all(i));
    end loop;

    wait;

  end process;

end architecture;

Upvotes: 1

Views: 192

Answers (1)

user1155120
user1155120

Reputation:

The process in architecture test of int_append_test can be modified to demonstrate the issue is algorithmic and not language implementation related:

architecture test of int_append_test is

  type int_vector_ptr_t is access integer_vector;
  type int_vector_ptr_ptr_t is access int_vector_ptr_t;

  procedure append (variable ptr: inout int_vector_ptr_t;
                    constant val: in    integer ) is
    variable p: int_vector_ptr_t;
    begin
        p := ptr;

        if ptr = null then
            ptr := new integer_vector'( (0 => val) );
        else
            ptr := new integer_vector'(p.all & val);
        end if;

        DEALLOCATE(p);  -- Remove old data, prevent memory leak
    end procedure append;

begin

    process
        variable ptr:       int_vector_ptr_t;
        variable ptr_ptr:   int_vector_ptr_ptr_t;

    function to_string(iv:  integer_vector) return string is
        variable l:  std.textio.line;
    begin
        for i in iv'range loop
          std.textio.write(l, to_string(iv(i)) & " ");
        end loop;

        return l.all;
    end function;

    begin
        for i in 0 to 10 loop
            append(ptr, i);
        end loop;

        for i in ptr.all'range loop
            report "ptr = " & to_string(ptr.all(i));
        end loop;

        ptr_ptr := new int_vector_ptr_t;

        for i in 0 to 10 loop

            ptr := ptr_ptr.all;

            if i = 0 then
                ptr_ptr.all := new integer_vector'( (0 => i) );
            else
                ptr_ptr.all := new integer_vector'(ptr.all & i);
            end if;

            if ptr = NULL then   -- test for NULL ptr ADDED
                report "ptr = NULL for i = " & integer'image(i); -- ADDED
            else
                report "ptr_data = " & to_string(ptr.all); -- MOVED HERE
            end if;

            DEALLOCATE(ptr);

        end loop;

        for i in ptr_ptr.all.all'range loop
            report "ptr = " & to_string(ptr_ptr.all.all(i));
        end loop;

        wait;

    end process;

end architecture;

The value of NULL is detected in the access value held by ptr and when run this gives:

ghdl -r --std=08 int_append_test
int_append_test.vhdl:46:13:@0ms:(report note): ptr = 0
int_append_test.vhdl:46:13:@0ms:(report note): ptr = 1
int_append_test.vhdl:46:13:@0ms:(report note): ptr = 2
int_append_test.vhdl:46:13:@0ms:(report note): ptr = 3
int_append_test.vhdl:46:13:@0ms:(report note): ptr = 4
int_append_test.vhdl:46:13:@0ms:(report note): ptr = 5
int_append_test.vhdl:46:13:@0ms:(report note): ptr = 6
int_append_test.vhdl:46:13:@0ms:(report note): ptr = 7
int_append_test.vhdl:46:13:@0ms:(report note): ptr = 8
int_append_test.vhdl:46:13:@0ms:(report note): ptr = 9
int_append_test.vhdl:46:13:@0ms:(report note): ptr = 10
int_append_test.vhdl:62:17:@0ms:(report note): ptr = NULL for i = 0
int_append_test.vhdl:64:17:@0ms:(report note): ptr_data = 0
int_append_test.vhdl:64:17:@0ms:(report note): ptr_data = 0 1
int_append_test.vhdl:64:17:@0ms:(report note): ptr_data = 0 1 2
int_append_test.vhdl:64:17:@0ms:(report note): ptr_data = 0 1 2 3
int_append_test.vhdl:64:17:@0ms:(report note): ptr_data = 0 1 2 3 4
int_append_test.vhdl:64:17:@0ms:(report note): ptr_data = 0 1 2 3 4 5
int_append_test.vhdl:64:17:@0ms:(report note): ptr_data = 0 1 2 3 4 5 6
int_append_test.vhdl:64:17:@0ms:(report note): ptr_data = 0 1 2 3 4 5 6 7
int_append_test.vhdl:64:17:@0ms:(report note): ptr_data = 0 1 2 3 4 5 6 7 8
int_append_test.vhdl:64:17:@0ms:(report note): ptr_data = 0 1 2 3 4 5 6 7 8 9
int_append_test.vhdl:72:13:@0ms:(report note): ptr = 0
int_append_test.vhdl:72:13:@0ms:(report note): ptr = 1
int_append_test.vhdl:72:13:@0ms:(report note): ptr = 2
int_append_test.vhdl:72:13:@0ms:(report note): ptr = 3
int_append_test.vhdl:72:13:@0ms:(report note): ptr = 4
int_append_test.vhdl:72:13:@0ms:(report note): ptr = 5
int_append_test.vhdl:72:13:@0ms:(report note): ptr = 6
int_append_test.vhdl:72:13:@0ms:(report note): ptr = 7
int_append_test.vhdl:72:13:@0ms:(report note): ptr = 8
int_append_test.vhdl:72:13:@0ms:(report note): ptr = 9
int_append_test.vhdl:72:13:@0ms:(report note): ptr = 10

where the null value causing the error is reported instead of causing the error.

How is the value of ptr a NULL?

This can be seen across the start of the third loop statement:

        ptr_ptr := new int_vector_ptr_t;

        for i in 0 to 10 loop

            ptr := ptr_ptr.all;

Where ptr_ptr is an access value of access type int_vector_ptr_ptr_t which is itself an access type of type int_vector_ptr_t. We see thatptr_ptris allocated but the value hasn't been initialized, it's aNULLvalue which is subsequently assigned toptr`.

The value doesn't become non NULL until the following if statement:

            if i = 0 then
                ptr_ptr.all := new integer_vector'( (0 => i) );
            else
                ptr_ptr.all := new integer_vector'(ptr.all & i);
            end if;

What happens here is the classical depending-on-a-value-before-it's-assigned, guaranteed to produce an error for a value of an access type.

If you move the assignment to ptr you'll also find the deallocate call is the wrong place, causing a NULL access on i = 1. Moving the deallocate call clears that up:

        for i in 0 to 10 loop

            -- ptr := ptr_ptr.all;  -- REMOVED

            if i = 0 then
                ptr_ptr.all := new integer_vector'( (0 => i) );
            else
                ptr_ptr.all := new integer_vector'(ptr.all & i);
            end if;

            ptr := ptr_ptr.all;  -- MOVED HERE

            -- if ptr = NULL then   -- test for NULL ptr ADDED
            --     report "ptr = NULL for i = " & integer'image(i); -- ADDED
            -- else
                report "ptr_data = " & to_string(ptr.all); -- MOVED HERE
            -- end if;

            -- DEALLOCATE(ptr);  -- REMOVED

        end loop;

        DEALLOCATE(ptr); -- MOVED HERE

The corrected code runs without error:

ghdl -r --std=08 int_append_test
int_append_test.vhdl:46:13:@0ms:(report note): ptr = 0
int_append_test.vhdl:46:13:@0ms:(report note): ptr = 1
int_append_test.vhdl:46:13:@0ms:(report note): ptr = 2
int_append_test.vhdl:46:13:@0ms:(report note): ptr = 3
int_append_test.vhdl:46:13:@0ms:(report note): ptr = 4
int_append_test.vhdl:46:13:@0ms:(report note): ptr = 5
int_append_test.vhdl:46:13:@0ms:(report note): ptr = 6
int_append_test.vhdl:46:13:@0ms:(report note): ptr = 7
int_append_test.vhdl:46:13:@0ms:(report note): ptr = 8
int_append_test.vhdl:46:13:@0ms:(report note): ptr = 9
int_append_test.vhdl:46:13:@0ms:(report note): ptr = 10
int_append_test.vhdl:66:17:@0ms:(report note): ptr_data = 0
int_append_test.vhdl:66:17:@0ms:(report note): ptr_data = 0 1
int_append_test.vhdl:66:17:@0ms:(report note): ptr_data = 0 1 2
int_append_test.vhdl:66:17:@0ms:(report note): ptr_data = 0 1 2 3
int_append_test.vhdl:66:17:@0ms:(report note): ptr_data = 0 1 2 3 4
int_append_test.vhdl:66:17:@0ms:(report note): ptr_data = 0 1 2 3 4 5
int_append_test.vhdl:66:17:@0ms:(report note): ptr_data = 0 1 2 3 4 5 6
int_append_test.vhdl:66:17:@0ms:(report note): ptr_data = 0 1 2 3 4 5 6 7
int_append_test.vhdl:66:17:@0ms:(report note): ptr_data = 0 1 2 3 4 5 6 7 8
int_append_test.vhdl:66:17:@0ms:(report note): ptr_data = 0 1 2 3 4 5 6 7 8 9
int_append_test.vhdl:66:17:@0ms:(report note): ptr_data = 0 1 2 3 4 5 6 7 8 9 10
int_append_test.vhdl:76:13:@0ms:(report note): ptr = 0
int_append_test.vhdl:76:13:@0ms:(report note): ptr = 1
int_append_test.vhdl:76:13:@0ms:(report note): ptr = 2
int_append_test.vhdl:76:13:@0ms:(report note): ptr = 3
int_append_test.vhdl:76:13:@0ms:(report note): ptr = 4
int_append_test.vhdl:76:13:@0ms:(report note): ptr = 5
int_append_test.vhdl:76:13:@0ms:(report note): ptr = 6
int_append_test.vhdl:76:13:@0ms:(report note): ptr = 7
int_append_test.vhdl:76:13:@0ms:(report note): ptr = 8
int_append_test.vhdl:76:13:@0ms:(report note): ptr = 9
int_append_test.vhdl:76:13:@0ms:(report note): ptr = 10

Upvotes: 4

Related Questions