Reputation: 4471
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?
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
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 that
ptr_ptris allocated but the value hasn't been initialized, it's a
NULLvalue which is subsequently assigned to
ptr`.
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