Re: address sanitizer crash

Peter Eisentraut <peter_e(at)gmx(dot)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