Re: address sanitizer crash - Mailing list pgsql-hackers

From Tom Lane
Subject Re: address sanitizer crash
Date
Msg-id 12508.1385131258@sss.pgh.pa.us
Whole thread Raw
In response to address sanitizer crash  (Peter Eisentraut <peter_e@gmx.net>)
List pgsql-hackers
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



pgsql-hackers by date:

Previous
From: Pavel Stehule
Date:
Subject: Re: new unicode table border styles for psql
Next
From: Alvaro Herrera
Date:
Subject: Re: new unicode table border styles for psql