Hi,
I searched the mailing list but found nothing. Any reason why
TupleDescAttr is a macro and not a static inline?
Rather than adding an Assert attached POC replace TupleDescAttr macro
by a static inline function with AssertArg.
It:
- Factorize Assert.
- Trigger an Assert in JIT_deform if natts is wrong.
- Currently In HEAD
src/backend/access/common/tupdesc.c:TupleDescCopyEntry() compiler can
optimize out AssertArg(PointerIsValid(...)), no idea
if compiling with both cassert and -O2 make sense though).
- Remove two UB in memcpy when natts is zero.
Note:
Comment line 1480 in ../contrib/tablefunc/tablefunc.c is wrong it's
the fourth column.
Regards
Didier
On Thu, Jun 13, 2019 at 8:35 PM Andres Freund <[email protected]> wrote:
>
> Hi,
>
> On June 13, 2019 11:08:15 AM PDT, didier <[email protected]> wrote:
> >Extensions can do it, timescaledb in this case with:
> >INSERT INTO ... RETURNING *;
> >
> >Or replacing the test in llvm_compile_expr with an Assert in
> >slot_compile_deform ?
>
> In that case we ought to never generate a deform expression step - core code
> doesn't afair. That's only done I'd there's actually something to deform. I'm
> fine with adding an assert tough
>
> Andres
> --
> Sent from my Android device with K-9 Mail. Please excuse my brevity.
diff --git a/src/backend/access/common/heaptuple.c b/src/backend/access/common/heaptuple.c
index a48a6cd757..23ef9a6a75 100644
--- a/src/backend/access/common/heaptuple.c
+++ b/src/backend/access/common/heaptuple.c
@@ -86,9 +86,6 @@ getmissingattr(TupleDesc tupleDesc,
{
Form_pg_attribute att;
- Assert(attnum <= tupleDesc->natts);
- Assert(attnum > 0);
-
att = TupleDescAttr(tupleDesc, attnum - 1);
if (att->atthasmissing)
diff --git a/src/backend/access/common/tupdesc.c b/src/backend/access/common/tupdesc.c
index 6bc4e4c036..850771acf5 100644
--- a/src/backend/access/common/tupdesc.c
+++ b/src/backend/access/common/tupdesc.c
@@ -115,7 +115,8 @@ CreateTupleDescCopy(TupleDesc tupdesc)
desc = CreateTemplateTupleDesc(tupdesc->natts);
/* Flat-copy the attribute array */
- memcpy(TupleDescAttr(desc, 0),
+ if (desc->natts)
+ memcpy(TupleDescAttr(desc, 0),
TupleDescAttr(tupdesc, 0),
desc->natts * sizeof(FormData_pg_attribute));
@@ -156,7 +157,8 @@ CreateTupleDescCopyConstr(TupleDesc tupdesc)
desc = CreateTemplateTupleDesc(tupdesc->natts);
/* Flat-copy the attribute array */
- memcpy(TupleDescAttr(desc, 0),
+ if (desc->natts)
+ memcpy(TupleDescAttr(desc, 0),
TupleDescAttr(tupdesc, 0),
desc->natts * sizeof(FormData_pg_attribute));
@@ -274,16 +276,6 @@ TupleDescCopyEntry(TupleDesc dst, AttrNumber dstAttno,
Form_pg_attribute dstAtt = TupleDescAttr(dst, dstAttno - 1);
Form_pg_attribute srcAtt = TupleDescAttr(src, srcAttno - 1);
- /*
- * sanity checks
- */
- AssertArg(PointerIsValid(src));
- AssertArg(PointerIsValid(dst));
- AssertArg(srcAttno >= 1);
- AssertArg(srcAttno <= src->natts);
- AssertArg(dstAttno >= 1);
- AssertArg(dstAttno <= dst->natts);
-
memcpy(dstAtt, srcAtt, ATTRIBUTE_FIXED_PART_SIZE);
/*
@@ -611,13 +603,6 @@ TupleDescInitEntry(TupleDesc desc,
Form_pg_type typeForm;
Form_pg_attribute att;
- /*
- * sanity checks
- */
- AssertArg(PointerIsValid(desc));
- AssertArg(attributeNumber >= 1);
- AssertArg(attributeNumber <= desc->natts);
-
/*
* initialize the attribute fields
*/
@@ -682,11 +667,6 @@ TupleDescInitBuiltinEntry(TupleDesc desc,
{
Form_pg_attribute att;
- /* sanity checks */
- AssertArg(PointerIsValid(desc));
- AssertArg(attributeNumber >= 1);
- AssertArg(attributeNumber <= desc->natts);
-
/* initialize the attribute fields */
att = TupleDescAttr(desc, attributeNumber - 1);
att->attrelid = 0; /* dummy value */
@@ -770,13 +750,6 @@ TupleDescInitEntryCollation(TupleDesc desc,
AttrNumber attributeNumber,
Oid collationid)
{
- /*
- * sanity checks
- */
- AssertArg(PointerIsValid(desc));
- AssertArg(attributeNumber >= 1);
- AssertArg(attributeNumber <= desc->natts);
-
TupleDescAttr(desc, attributeNumber - 1)->attcollation = collationid;
}
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index d768b9b061..2fbd2ef207 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -3842,7 +3842,6 @@ heap_tuple_attr_equals(TupleDesc tupdesc, int attrnum,
}
else
{
- Assert(attrnum <= tupdesc->natts);
att = TupleDescAttr(tupdesc, attrnum - 1);
return datumIsEqual(value1, value2, att->attbyval, att->attlen);
}
diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c
index 77a48b039d..f34334b2a2 100644
--- a/src/backend/parser/parse_relation.c
+++ b/src/backend/parser/parse_relation.c
@@ -2600,7 +2600,6 @@ expandTupleDesc(TupleDesc tupdesc, Alias *eref, int count, int offset,
}
}
- Assert(count <= tupdesc->natts);
for (varattno = 0; varattno < count; varattno++)
{
Form_pg_attribute attr = TupleDescAttr(tupdesc, varattno);
@@ -2837,8 +2836,6 @@ get_rte_attribute_type(RangeTblEntry *rte, AttrNumber attnum,
/* Composite data type, e.g. a table's row type */
Form_pg_attribute att_tup;
- Assert(tupdesc);
- Assert(attnum <= tupdesc->natts);
att_tup = TupleDescAttr(tupdesc, attnum - 1);
/*
@@ -3050,8 +3047,6 @@ get_rte_attribute_is_dropped(RangeTblEntry *rte, AttrNumber attnum)
/* Composite data type, e.g. a table's row type */
Form_pg_attribute att_tup;
- Assert(tupdesc);
- Assert(attnum - atts_done <= tupdesc->natts);
att_tup = TupleDescAttr(tupdesc,
attnum - atts_done - 1);
return att_tup->attisdropped;
diff --git a/src/backend/utils/cache/catcache.c b/src/backend/utils/cache/catcache.c
index 00def27881..8554e42803 100644
--- a/src/backend/utils/cache/catcache.c
+++ b/src/backend/utils/cache/catcache.c
@@ -1918,8 +1918,6 @@ CatCacheFreeKeys(TupleDesc tupdesc, int nkeys, int *attnos, Datum *keys)
Form_pg_attribute att;
/* system attribute are not supported in caches */
- Assert(attnum > 0);
-
att = TupleDescAttr(tupdesc, attnum - 1);
if (!att->attbyval)
diff --git a/src/include/access/tupdesc.h b/src/include/access/tupdesc.h
index a06800555c..4b09c26915 100644
--- a/src/include/access/tupdesc.h
+++ b/src/include/access/tupdesc.h
@@ -89,7 +89,17 @@ typedef struct TupleDescData
typedef struct TupleDescData *TupleDesc;
/* Accessor for the i'th attribute of tupdesc. */
+#if 1
+static inline FormData_pg_attribute *TupleDescAttr(TupleDesc tupdesc, int i)
+{
+ AssertArg(PointerIsValid(tupdesc));
+ AssertArg(i >= 0);
+ AssertArg(i < tupdesc->natts);
+ return &tupdesc->attrs[i];
+}
+#else
#define TupleDescAttr(tupdesc, i) (&(tupdesc)->attrs[(i)])
+#endif
extern TupleDesc CreateTemplateTupleDesc(int natts);
