Package Constant: Check missing dsc_address instead of dsc_dtype and restore impure after using existing request

[Noremos]

Original report: https://groups.google.com/g/firebird-devel/c/uPpRhaAKT0M

In the makeValue function, the value is compiled only once and then cached. Previously, the dsc_address was always present, but now it is not required for calculating the hash, so it may be missing. This leads to null pointer read/write.

The second issue is related with request impure space. First, the calls to pass1 and pass2 were missing, leading to invalid addresses being used. Second, using an existing request, its impure will be corrupt. So, it should be restored after the node is executed.

SQL to reproduce:

shell rm -rf /tmp/test.fdb;
create database ' /tmp/test.fdb';

set bail on;
set list on;
set autoterm on;
set autoddl off;
set keep_tran on;


recreate package pg_test as
begin
    procedure sp_test() returns(o smallint);
end;

recreate package body pg_test as
begin
    constant PG_BODY_CONST smallint = -10;
    procedure sp_test() returns(o smallint) as
    begin
        o = pg_body_const;
        suspend;
    end
end
;
commit;

select p.o as get_pkg_head_const_1  from pg_test.sp_test as p rows 1; -- segfault

[aafemt]

Execution nodes are supposed to reserve space in impure area using CompilerScratch::allocImpure during compilation, remember the returned offset and during execution get the address for usage with Request::getImpure.

[asfernandes]

When is this function called with null request and when is it called with not null request?

[Noremos]

With request: when executing ReferenceNode.
Without request: when performing a packet scan (without minican) in metacache.
I will add a comment to clearify it

[AlexPeshkoff]

BTW - why not save descriptor for getDesc() in a cache? I.e. move almost all code from getDesc() to scan().

[Noremos]

Added a comment about null request

BTW - why not save descriptor for getDesc() in a cache? I.e. move almost all code from getDesc() to scan().

Good point: getDesc queries the descriptor by name, but there's nothing stopping one from getting it via package->findConstant. Especially considering the function is already called when checking for the constant's existence. I'll create a separate PR with this optimization.

[Noremos]

The PR with the improveemnt: #9069

[asfernandes]

This whole thing is looking as a complete hack for me.

[Noremos]

Got it. I just wanted to save time. I'll do it the right way

[Noremos]

Fixed

[asfernandes]

Why are you not building a statement/request which is standard way to execute code?
There are many things allowed in constants. How you do ensure that all that code and new code is prepared to take that shortcut?

[Noremos]

There are many things allowed in constants. How you do ensure that all that code and new code is prepared to take that shortcut?

You are right. Now the handmade requset will be used