Em sex., 2 de set. de 2022 às 09:01, Justin Pryzby <pry...@telsasoft.com> escreveu:
> On Mon, Aug 29, 2022 at 10:06:55AM +0900, Kyotaro Horiguchi wrote: > > At Fri, 26 Aug 2022 10:28:50 -0300, Ranier Vilela <ranier...@gmail.com> > wrote in > > > At function has_matching_range, if variable ranges->nranges == 0, > > > we exit quickly with a result equal to false. > > > > > > This means that nranges can be zero. > > > It occurs then that it is possible then to occur an array out of > bonds, in > > > the initialization of the variable maxvalue. > > > So if nranges is equal to zero, there is no need to initialize > minvalue and > > > maxvalue. > > > > > > The patch tries to fix it, avoiding possible errors by using maxvalue. > > > > However it seems that nranges will never be zero, still the fix looks > > good to me since it is generally allowed to be zero. I don't find a > > similar mistake related to Range.nranges. > > Actually, the nranges==0 branch is hit during regression tests: > > https://coverage.postgresql.org/src/backend/access/brin/brin_minmax_multi.c.gcov.html > > I'm not sure, but I *suspect* that compilers usually check > ranges->nranges==0 > before reading ranges->values[2 * ranges->nranges - 1]; > > Especially since it's a static function. > > Even if they didn't (say, under -O0), values[-1] would probably point to > a palloc header, which would be enough to "not crash" before returning > one line later. > > But +1 to fix this and other issues even if they would never crash. > Thanks Justin. Based on comments by David, I made a new patch. To simplify I've included the 0001 in the 0002 patch. Summary: 1. Once that ranges->nranges is invariant, avoid the loop if ranges->nranges <= 0. This matches the current behavior. 2. Once that ranges->nsorted is invariant, avoid the loop if ranges->nsorted <= 0. This matches the current behavior. 3. Remove the invariant cxt from ranges->nsorted loop. 4. Avoid possible overflows when using int to store length strings. 5. Avoid possible out-of-bounds when ranges->nranges == 0. 6. Avoid overhead when using unnecessary StringInfoData to convert Datum a to Text b. Attached is 0002. regards, Ranier Vilela
0002-avoid-small-issues-brin_minmax_multi.patch
Description: Binary data
