Обсуждение: address sanitizer crash

AddressSanitizer (http://clang.llvm.org/docs/AddressSanitizer.html)
(also available through gcc, but I used clang), reports one bug, which
I traced down to this code in ruleutils.c:
   [elsewhere:]   #define ViewSelectRuleName "_RETURN"

   /*    * Get the pg_rewrite tuple for the view's SELECT rule    */   args[0] = ObjectIdGetDatum(viewoid);   args[1] =
PointerGetDatum(ViewSelectRuleName);  nulls[0] = ' ';   nulls[1] = ' ';   spirc = SPI_execute_plan(plan_getviewrule,
args,nulls, true, 2);
 

[I also think that the 2 here is wrong, probably thinking it means 2
arguments, but it means 2 result rows, but only one is needed.  But
that's unrelated to the bug.]

The datums end up in datumCopy(), which tries to copy 64 bytes of
"name" data, overrunning the end of the string.

I think the correct code is something like
   args[1] = DirectFunctionCall1(namein, CStringGetDatum(ViewSelectRuleName));

which indeed makes the crash go away.  (A more explicit string-to-name
function might be useful and could also be used in other places.)

This is the only remaining issue found by AddressSanitizer that is
clearly attributable to the PostgreSQL source code (after the recently
fixed issue with memcpy with identical arguments).  There are crashes
in PL/Perl and PL/Python, but it's not clear to me yet whose fault
they are.

Peter Eisentraut <peter_e@gmx.net> writes:
> AddressSanitizer (http://clang.llvm.org/docs/AddressSanitizer.html)
> (also available through gcc, but I used clang), reports one bug, which
> I traced down to this code in ruleutils.c:

>     [elsewhere:]
>     #define ViewSelectRuleName "_RETURN"

>     /*
>      * Get the pg_rewrite tuple for the view's SELECT rule
>      */
>     args[0] = ObjectIdGetDatum(viewoid);
>     args[1] = PointerGetDatum(ViewSelectRuleName);
>     nulls[0] = ' ';
>     nulls[1] = ' ';
>     spirc = SPI_execute_plan(plan_getviewrule, args, nulls, true, 2);

Yes, the plan clearly is built expecting $2 to be of type "name",
so this has been wrong since day 1.  Amazing that no actual bug reports
have surfaced from it.

> I think the correct code is something like
>     args[1] = DirectFunctionCall1(namein, CStringGetDatum(ViewSelectRuleName));

That would be OK.  The more usual coding pattern is to declare a local
NameData variable, namestrcpy into that, and then PointerGetDatum on
the variable's address is actually correct.  However, that's just
micro-optimization that I don't think we care about here.

> [I also think that the 2 here is wrong, probably thinking it means 2
> arguments, but it means 2 result rows, but only one is needed.  But
> that's unrelated to the bug.]

Yes, while harmless that's clearly in error, should be zero.  The other
call of SPI_execute_plan in ruleutils.c has the same thinko.

Please fix and back-patch.
        regards, tom lane