[PATCH 0/3] Work around icc miscompilation

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
>