I'm sending three smaller patches for review, which try to fix icc
and pathscale (mis)compilation problems described in my last email.
Not sure if we need to fix the overflow check x + 1 <= 0 (x > 0) at
src/backend/executor/execQual.c:3088, which icc optimizes away, too.
Fix x + y < x overflow checks
Fix x + 1 < x overflow checks
Fix overflow checking in repeat()
src/backend/utils/adt/float.c | 8 ++++----
src/backend/utils/adt/oracle_compat.c | 18 +++++++-----------
src/backend/utils/adt/varbit.c | 7 +++++--
src/backend/utils/adt/varlena.c | 10 ++++++----
src/pl/plpgsql/src/pl_exec.c | 4 ++--
5 files changed, 24 insertions(+), 23 deletions(-)
On 1/20/13 2:58 AM, Xi Wang wrote:
> Intel's icc and PathScale's pathcc compilers optimize away several
> overflow checks, since they consider signed integer overflow as
> undefined behavior. This leads to a vulnerable binary.
>
> Currently we use -fwrapv to disable such (mis)optimizations in gcc,
> but not in other compilers.
>
>
> Examples
> ========
>
> 1) x + 1 <= 0 (assuming x > 0).
>
> src/backend/executor/execQual.c:3088
>
> Below is the simplified code.
>
> -----------------------------------------
> void bar(void);
> void foo(int this_ndims)
> {
> if (this_ndims <= 0)
> return;
> int elem_ndims = this_ndims;
> int ndims = elem_ndims + 1;
> if (ndims <= 0)
> bar();
> }
> -----------------------------------------
>
> $ icc -S -o - sadd1.c
> ...
> foo:
> # parameter 1: %edi
> ..B1.1:
> ..___tag_value_foo.1:
> ..B1.2:
> ret
>
> 2) x + 1 < x
>
> src/backend/utils/adt/float.c:2769
> src/backend/utils/adt/float.c:2785
> src/backend/utils/adt/oracle_compat.c:1045 (x + C < x)
>
> Below is the simplified code.
>
> -----------------------------------------
> void bar(void);
> void foo(int count)
> {
> int result = count + 1;
> if (result < count)
> bar();
> }
> -----------------------------------------
>
> $ icc -S -o - sadd2.c
> ...
> foo:
> # parameter 1: %edi
> ..B1.1:
> ..___tag_value_foo.1:
> ret
> 3) x + y <= x (assuming y > 0)
>
> src/backend/utils/adt/varbit.c:1142
> src/backend/utils/adt/varlena.c:1001
> src/backend/utils/adt/varlena.c:2024
> src/pl/plpgsql/src/pl_exec.c:1975
> src/pl/plpgsql/src/pl_exec.c:1981
>
> Below is the simplified code.
>
> -----------------------------------------
> void bar(void);
> void foo(int sp, int sl)
> {
> if (sp <= 0)
> return;
> int sp_pl_sl = sp + sl;
> if (sp_pl_sl <= sl)
> bar();
> }
> -----------------------------------------
>
> $ icc -S -o - sadd3.c
> foo:
> # parameter 1: %edi
> # parameter 2: %esi
> ..B1.1:
> ..___tag_value_foo.1:
> ..B1.2:
> ret
>
> Possible fixes
> ==============
>
> * Recent versions of icc and pathcc support gcc's workaround option,
> -fno-strict-overflow, to disable some optimizations based on signed
> integer overflow. It's better to add this option to configure.
> They don't support gcc's -fwrapv yet.
>
> * This -fno-strict-overflow option cannot help in all cases: it cannot
> prevent the latest icc from (mis)compiling the 1st case. We could also
> fix the source code by avoiding signed integer overflows, as follows.
>
> x + y <= 0 (assuming x > 0, y > 0)
> --> x > INT_MAX - y
>
> x + y <= x (assuming y > 0)
> --> x > INT_MAX - y
>
> I'd suggest to fix the code rather than trying to work around the
> compilers since the fix seems simple and portable.
>
> See two recent compiler bugs of -fwrapv/-fno-strict-overflow as well.
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55883
> http://software.intel.com/en-us/forums/topic/358200
>
> * I don't have access to IBM's xlc compiler. Not sure how it works for
> the above cases.
>
> - xi
>
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
