[xscreensaver] gcc49 sanitizer array elements oversize fixes
Mamoru TASAKA
mtasaka at fedoraproject.org
Wed Sep 3 16:02:17 UTC 2014
commit 1e7a622a70045520a4986decf273b633913d97e1
Author: Mamoru TASAKA <mtasaka at fedoraproject.org>
Date: Thu Sep 4 01:01:54 2014 +0900
gcc49 sanitizer array elements oversize fixes
- Make parallel build actually work
...-don-t-pass-negative-value-to-the-index-o.patch | 36 ++++++++++++
...gine-display-fix-count-size-for-sin_table.patch | 31 +++++++++++
...w_histogram-avoid-one-byte-ahead-access-f.patch | 42 ++++++++++++++
...ewline-make-it-sure-that-color-is-set-pos.patch | 58 ++++++++++++++++++++
...-forallinbank-evaluate-bankc-beforehand-c.patch | 46 ++++++++++++++++
...t_braid-limit-braid-braidlength-correctly.patch | 36 ++++++++++++
...oingies-RenderSproingie-limit-shift-value.patch | 35 ++++++++++++
xscreensaver-5.29-abstractile.patch | 20 +++++++
xscreensaver.spec | 40 +++++++++++---
9 files changed, 337 insertions(+), 7 deletions(-)
---
diff --git a/xscreensaver-5.29-1001-maze-backup-don-t-pass-negative-value-to-the-index-o.patch b/xscreensaver-5.29-1001-maze-backup-don-t-pass-negative-value-to-the-index-o.patch
new file mode 100644
index 0000000..05f50b8
--- /dev/null
+++ b/xscreensaver-5.29-1001-maze-backup-don-t-pass-negative-value-to-the-index-o.patch
@@ -0,0 +1,36 @@
+From dc7cc26e7c7442c2181b6f1d9b1fe88ea331a89c Mon Sep 17 00:00:00 2001
+From: Mamoru TASAKA <mtasaka at fedoraproject.org>
+Date: Sun, 31 Aug 2014 03:37:33 +0900
+Subject: [PATCH 1001/1003] maze/backup: don't pass negative value to the index
+ of array
+
+gcc49 sanitizer detected the following error.
+../../hacks/maze.c:874:31: runtime error: index -1 out of bounds for type 'move_list_struct [1000000]'
+../../hacks/maze.c:875:31: runtime error: index -1 out of bounds for type 'move_list_struct [1000000]'
+
+The line 751 in create_maze() suggests that st->sqnum can be -1.
+Don't pass this value to the index of array.
+---
+ hacks/maze.c | 6 ++++--
+ 1 file changed, 4 insertions(+), 2 deletions(-)
+
+diff --git a/hacks/maze.c b/hacks/maze.c
+index 10704fa..4e73151 100644
+--- a/hacks/maze.c
++++ b/hacks/maze.c
+@@ -871,8 +871,10 @@ static int
+ backup (struct state *st) /* back up a move */
+ {
+ st->sqnum--;
+- st->cur_sq_x = st->move_list[st->sqnum].x;
+- st->cur_sq_y = st->move_list[st->sqnum].y;
++ if (st->sqnum >= 0) {
++ st->cur_sq_x = st->move_list[st->sqnum].x;
++ st->cur_sq_y = st->move_list[st->sqnum].y;
++ }
+ return ( st->sqnum );
+ }
+
+--
+2.1.0
+
diff --git a/xscreensaver-5.29-1002-engine-display-fix-count-size-for-sin_table.patch b/xscreensaver-5.29-1002-engine-display-fix-count-size-for-sin_table.patch
new file mode 100644
index 0000000..f3c5e00
--- /dev/null
+++ b/xscreensaver-5.29-1002-engine-display-fix-count-size-for-sin_table.patch
@@ -0,0 +1,31 @@
+From 94688a6abee76f135062ed67d4d34f34e1b28045 Mon Sep 17 00:00:00 2001
+From: Mamoru TASAKA <mtasaka at fedoraproject.org>
+Date: Sun, 31 Aug 2014 03:50:10 +0900
+Subject: [PATCH 1002/1003] engine/display: fix count size for sin_table
+
+gcc49 sanitizer found the following error:
+../../../hacks/glx/engine.c:659:24: runtime error: index 720 out of bounds for type 'float [720]'
+../../../hacks/glx/engine.c:660:24: runtime error: index 720 out of bounds for type 'float [720]'
+
+Since countof() is defined on the head of this source, use this
+to count the size of sin_table array correctly.
+---
+ hacks/glx/engine.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/hacks/glx/engine.c b/hacks/glx/engine.c
+index 9c37c8b..7e7442f 100644
+--- a/hacks/glx/engine.c
++++ b/hacks/glx/engine.c
+@@ -655,7 +655,7 @@ static int display(Engine *e)
+
+ /* init the ln[] matrix for speed */
+ if (e->ln_init == 0) {
+- for (e->ln_init = 0 ; e->ln_init < 730 ; e->ln_init++) {
++ for (e->ln_init = 0 ; e->ln_init < countof(e->sin_table) ; e->ln_init++) {
+ zb = e->sin_table[e->ln_init];
+ yb = e->cos_table[e->ln_init];
+ /* y ordinate of piston */
+--
+2.1.0
+
diff --git a/xscreensaver-5.29-1003-tronbit-draw_histogram-avoid-one-byte-ahead-access-f.patch b/xscreensaver-5.29-1003-tronbit-draw_histogram-avoid-one-byte-ahead-access-f.patch
new file mode 100644
index 0000000..d0a80cd
--- /dev/null
+++ b/xscreensaver-5.29-1003-tronbit-draw_histogram-avoid-one-byte-ahead-access-f.patch
@@ -0,0 +1,42 @@
+From 056ec8d29ccee04a39503892b3b6b05738677e08 Mon Sep 17 00:00:00 2001
+From: Mamoru TASAKA <mtasaka at fedoraproject.org>
+Date: Sun, 31 Aug 2014 04:13:04 +0900
+Subject: [PATCH 1003/1003] tronbit/draw_histogram: avoid one byte ahead access
+ for histogram
+
+gcc49 sanitizer found the following error:
+../../../hacks/glx/tronbit.c:320:38: runtime error: index 512 out of bounds for type 'unsigned char [512]'
+
+tick_bit() suggests that bp->histogram_fp can be raised up to
+countof(bp->history) -1 = countof(bp->histogram) -1.
+The variable j has the value with bp->histogram_fp + 1, by the line
+316, so j can be countof(bp->histogram). With this value,
+bp->histogram[j] can raise one byte ahead access error.
+---
+ hacks/glx/tronbit.c | 3 ++-
+ 1 file changed, 2 insertions(+), 1 deletion(-)
+
+diff --git a/hacks/glx/tronbit.c b/hacks/glx/tronbit.c
+index 3ab69f1..7fe0a2e 100644
+--- a/hacks/glx/tronbit.c
++++ b/hacks/glx/tronbit.c
+@@ -317,6 +317,7 @@ draw_histogram (ModeInfo *mi, GLfloat ratio)
+ for (i = 0; i < samples; i++)
+ {
+ GLfloat x = i;
++ if (j >= samples) j = 0;
+ GLfloat y = bp->histogram[j];
+ GLfloat z = 0;
+
+@@ -327,7 +328,7 @@ draw_histogram (ModeInfo *mi, GLfloat ratio)
+ y *= scaley;
+
+ glVertex3f (x, y, z);
+- if (++j >= samples) j = 0;
++ ++j;
+ polys++;
+ }
+ glEnd();
+--
+2.1.0
+
diff --git a/xscreensaver-5.29-1004-abstract-_newline-make-it-sure-that-color-is-set-pos.patch b/xscreensaver-5.29-1004-abstract-_newline-make-it-sure-that-color-is-set-pos.patch
new file mode 100644
index 0000000..c7c6862
--- /dev/null
+++ b/xscreensaver-5.29-1004-abstract-_newline-make-it-sure-that-color-is-set-pos.patch
@@ -0,0 +1,58 @@
+From d15c60ddb956fea5cfd283f63d062aff2cfe3998 Mon Sep 17 00:00:00 2001
+From: XScreenSaver owners <xscreensaver-owner at fedoraproject.org>
+Date: Mon, 1 Sep 2014 22:08:53 +0900
+Subject: [PATCH] abstract/_newline: make it sure that color is set positive
+
+gcc49 sanitizer detected the following error:
+../../hacks/abstractile.c:564:29: runtime error: index -2 out of bounds for type 'int [40]'
+../../hacks/abstractile.c:564:29: runtime error: index -3 out of bounds for type 'int [40]'
+
+Well, the function _getcolor() is rather complex, however the
+above error suggests that _getcolor() can return negative value,
+and as the result the following sequence can happen:
+
+- In _newline(), (_getcolor(st,x,y))%st->ncolors sometimes gets
+ negative value.
+ Note that on C89, the result of a%b where a is negative is implementation
+ defined, on C99 the result of a%b where a is negative is 0 or negative.
+- Then st->dline[st->li].color can have very large value (because
+ this is unsigned int)
+- And in the folloing line (in _newline()), _getdeo() is called.
+ In _getdeo(), cr = (de) ? st->dline[st->li].color : st->eline[st->li].color
+ can have negative value (cr is defined as int) and the following line
+ negative value is passed as the element of the array.
+
+Let's kill this error. Note that in _newline():
+-------------------------------------
+ st->dline[st->li].color = (lt==LINE_NEW) ?
+ (_getcolor(st,x,y))%st->ncolors : st->dline[bl].color;
+ if (st->dline[st->li].color) < 0 st->dline[st->li].color += st->ncolors;
+ -------------------------------------
+does not work, because st->dline[st->li].color is declared as unsigned int.
+---
+ hacks/abstractile.c | 9 +++++++--
+ 1 file changed, 7 insertions(+), 2 deletions(-)
+
+diff --git a/hacks/abstractile.c b/hacks/abstractile.c
+index aaac029..9ac84cf 100644
+--- a/hacks/abstractile.c
++++ b/hacks/abstractile.c
+@@ -1079,8 +1079,13 @@ _newline(struct state *st)
+ True : False;
+ st->dline[st->li].obj = (lt==LINE_NEW) ? st->oi :
+ st->dline[bl].obj;
+- st->dline[st->li].color = (lt==LINE_NEW) ?
+- (_getcolor(st,x,y))%st->ncolors : st->dline[bl].color;
++ if (lt==LINE_NEW) {
++ int color = (_getcolor(st,x,y))%st->ncolors;
++ if (color < 0) color += st->ncolors;
++ st->dline[st->li].color = color;
++ } else {
++ st->dline[st->li].color = st->dline[bl].color;
++ }
+ st->dline[st->li].deo=(_getdeo(st,x,y,st->dmap,1) +
+ (random()%st->dvar) + (random()%st->dvar))*st->ddir;
+ st->dline[st->li].ndol=0;
+--
+2.1.0
+
diff --git a/xscreensaver-5.29-1005-vermiculate-forallinbank-evaluate-bankc-beforehand-c.patch b/xscreensaver-5.29-1005-vermiculate-forallinbank-evaluate-bankc-beforehand-c.patch
new file mode 100644
index 0000000..98fab81
--- /dev/null
+++ b/xscreensaver-5.29-1005-vermiculate-forallinbank-evaluate-bankc-beforehand-c.patch
@@ -0,0 +1,46 @@
+From ea62d0a8349c7a144816cc0368d913efa9e68264 Mon Sep 17 00:00:00 2001
+From: Mamoru TASAKA <mtasaka at fedoraproject.org>
+Date: Tue, 2 Sep 2014 12:57:51 +0900
+Subject: [PATCH 1005/1006] vermiculate/forallinbank: evaluate bankc beforehand
+ correctly
+
+gcc49 sanitizer detected errors like below:
+../../hacks/vermiculate.c:998:8: runtime error: index -1 out of bounds for type 'linedata [120]'
+../../hacks/vermiculate.c:1004:8: runtime error: index 120 out of bounds for type 'unsigned char [120]'
+../../hacks/vermiculate.c:1029:4: runtime error: index 120 out of bounds for type 'unsigned char [120]'
+
+These errors happen in "forallinbank" macro, defined at around the line 883.
+Looking at the lines around 985 - 987, it reads:
+---------------------------------------------------------------
+ 982 int fbnkt = st->bnkt;
+ 985 for (bankc = 1; bankc <= fbnkt; bankc++)
+ 986 {
+ 987 linedata *L = &st->thread[fbank[bankc - 1] - 1];
+---------------------------------------------------------------
+Judging from this code, in the macro "forallinbank" it is expected that
+at the code "LDP = &st->thread[st->bank[bankc - 1] - 1],", bankc should be
+within 1 and st->bnkt, so the part "bankc <= st->bnkt" should be evaluated
+before "LDP = &st->thread[st->bank[bankc - 1] - 1]".
+---
+ hacks/vermiculate.c | 5 +++--
+ 1 file changed, 3 insertions(+), 2 deletions(-)
+
+diff --git a/hacks/vermiculate.c b/hacks/vermiculate.c
+index c50785a..4a93541 100644
+--- a/hacks/vermiculate.c
++++ b/hacks/vermiculate.c
+@@ -882,8 +882,9 @@ consume_instring(struct state *st)
+ block in which it's invoked, since it declares variables: */
+ #define forallinbank(LDP) linedata *LDP; int bankc; \
+ for (bankc = 1; \
+- (LDP = &st->thread[st->bank[bankc - 1] - 1], \
+- bankc <= st->bnkt); bankc++)
++ ((bankc <= st->bnkt) ? ( \
++ (LDP = &st->thread[st->bank[bankc - 1] - 1], 1) \
++ ) : 0) ; bankc++)
+ {
+ forallinbank (L) L->slice = degs / (st->ch - '0');
+ }
+--
+1.9.3
+
diff --git a/xscreensaver-5.29-1006-braid-init_braid-limit-braid-braidlength-correctly.patch b/xscreensaver-5.29-1006-braid-init_braid-limit-braid-braidlength-correctly.patch
new file mode 100644
index 0000000..4358045
--- /dev/null
+++ b/xscreensaver-5.29-1006-braid-init_braid-limit-braid-braidlength-correctly.patch
@@ -0,0 +1,36 @@
+From 9fb3aad7e886601e32aa922cf5be69c98c504662 Mon Sep 17 00:00:00 2001
+From: Mamoru TASAKA <mtasaka at fedoraproject.org>
+Date: Tue, 2 Sep 2014 13:54:10 +0900
+Subject: [PATCH 1006/1006] braid/init_braid: limit braid->braidlength
+ correctly
+
+gcc49 sanitizer detected errors like below:
+../../hacks/braid.c:225:20: runtime error: index 50 out of bounds for type 'int [50]'
+../../hacks/braid.c:227:27: runtime error: index 50 out of bounds for type 'int [50]'
+../../hacks/braid.c:118:12: runtime error: index 50 out of bounds for type 'int [50]'
+../../hacks/braid.c:120:17: runtime error: index 50 out of bounds for type 'int [50]'
+
+In init_braid(), currently at the line 203 braid->braidlength can have the value
+MAXLENGTH at the maximum, and at the line 232 braid->braidlength may be incremented
+by 1, so braid->braidlength can be MAXLENGTH + 1. So the part "braid->braidword[i]"
+appearing in braid.c can do one-byte ahead access.
+---
+ hacks/braid.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/hacks/braid.c b/hacks/braid.c
+index c73b89d..d134e63 100644
+--- a/hacks/braid.c
++++ b/hacks/braid.c
+@@ -200,7 +200,7 @@ init_braid(ModeInfo * mi)
+ braid->nstrands = INTRAND(MINSTRANDS,
+ MAX(MIN(MIN(MAXSTRANDS, MI_COUNT(mi)),
+ (int) ((braid->max_radius - braid->min_radius) / 5.0)), MINSTRANDS));
+- braid->braidlength = INTRAND(MINLENGTH, MIN(MAXLENGTH, braid->nstrands * 6));
++ braid->braidlength = INTRAND(MINLENGTH, MIN(MAXLENGTH -1, braid->nstrands * 6));
+
+ for (i = 0; i < braid->braidlength; i++) {
+ braid->braidword[i] =
+--
+1.9.3
+
diff --git a/xscreensaver-5.29-1007-sproingies-RenderSproingie-limit-shift-value.patch b/xscreensaver-5.29-1007-sproingies-RenderSproingie-limit-shift-value.patch
new file mode 100644
index 0000000..ae1e7df
--- /dev/null
+++ b/xscreensaver-5.29-1007-sproingies-RenderSproingie-limit-shift-value.patch
@@ -0,0 +1,35 @@
+From 95756dbf034f6df8869ef933a8c3cc4b8a6ce162 Mon Sep 17 00:00:00 2001
+From: Mamoru TASAKA <mtasaka at fedorapeoject.org>
+Date: Wed, 3 Sep 2014 12:46:43 +0900
+Subject: [PATCH] sproingies/RenderSproingie: limit shift value
+
+gcc49 sanitizer detected the following error:
+../../../hacks/glx/sproingies.c:554:24: runtime error: shift exponent 32 is too large for 32-bit type 'int'
+
+This means that thisSproingie->frame - BOOM_FRAME can be no less than
+32. So just clipping this value to 31 to make it sure that shift exponent
+is in the range for int.
+---
+ hacks/glx/sproingies.c | 6 +++++-
+ 1 file changed, 5 insertions(+), 1 deletion(-)
+
+diff --git a/hacks/glx/sproingies.c b/hacks/glx/sproingies.c
+index 0892eff..42ccf2b 100644
+--- a/hacks/glx/sproingies.c
++++ b/hacks/glx/sproingies.c
+@@ -551,7 +551,11 @@ RenderSproingie(int t, sp_instance * si)
+ glTranslatef((GLfloat) (thisSproingie->x) + 0.5,
+ (GLfloat) (thisSproingie->y) + 0.5,
+ (GLfloat) (thisSproingie->z) - 0.5);
+- scale = (GLfloat) (1 << (thisSproingie->frame - BOOM_FRAME));
++ {
++ int boom_scale = thisSproingie->frame - BOOM_FRAME;
++ if (boom_scale >= 31) boom_scale = 31;
++ scale = (GLfloat) (1 << boom_scale);
++ }
+ glScalef(scale, scale, scale);
+ if (!si->wireframe) {
+ if (!si->mono)
+--
+1.9.3
+
diff --git a/xscreensaver-5.29-abstractile.patch b/xscreensaver-5.29-abstractile.patch
new file mode 100644
index 0000000..a566a90
--- /dev/null
+++ b/xscreensaver-5.29-abstractile.patch
@@ -0,0 +1,20 @@
+diff --git a/hacks/abstractile.c b/hacks/abstractile.c
+index aaac029..9ac84cf 100644
+--- a/hacks/abstractile.c
++++ b/hacks/abstractile.c
+@@ -1079,8 +1079,13 @@ _newline(struct state *st)
+ True : False;
+ st->dline[st->li].obj = (lt==LINE_NEW) ? st->oi :
+ st->dline[bl].obj;
+- st->dline[st->li].color = (lt==LINE_NEW) ?
+- (_getcolor(st,x,y))%st->ncolors : st->dline[bl].color;
++ if (lt==LINE_NEW) {
++ int color = (_getcolor(st,x,y))%st->ncolors;
++ if (color < 0) color += st->ncolors;
++ st->dline[st->li].color = color;
++ } else {
++ st->dline[st->li].color = st->dline[bl].color;
++ }
+ st->dline[st->li].deo=(_getdeo(st,x,y,st->dmap,1) +
+ (random()%st->dvar) + (random()%st->dvar))*st->ddir;
+ st->dline[st->li].ndol=0;
diff --git a/xscreensaver.spec b/xscreensaver.spec
index ffc920b..20b7ae8 100644
--- a/xscreensaver.spec
+++ b/xscreensaver.spec
@@ -10,13 +10,13 @@
%define split_getimage 1
%endif
-%define fedora_rel 1
+%define fedora_rel 2
%global use_clang_as_cc 0
%global use_clang_analyze 0
%global use_cppcheck 0
-%global use_gcc_strict_sanitize 0
+%global use_gcc_strict_sanitize 0
%undefine extrarel
%if 0%{?fedora}
@@ -37,7 +37,7 @@ Buildroot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
Summary: X screen saver and locker
Name: %{name}
Version: %{mainversion}
-Release: %{fedora_rel}%{?dist}%{?extrarel}.1
+Release: %{fedora_rel}%{?dist}%{?extrarel}
Epoch: 1
License: MIT
Group: Amusements/Graphics
@@ -71,8 +71,14 @@ Patch51: xscreensaver-5.12-test-passwd-segv-tty.patch
Patch52: xscreensaver-5.12-tests-miscfix.patch
#
# Patch with git format-patch format
-# Shut down cppcheck error / warning messages
-# Shutdown gcc fsanitize error
+# gcc49 sanitizer array element fixes
+Patch1001: xscreensaver-5.29-1001-maze-backup-don-t-pass-negative-value-to-the-index-o.patch
+Patch1002: xscreensaver-5.29-1002-engine-display-fix-count-size-for-sin_table.patch
+Patch1003: xscreensaver-5.29-1003-tronbit-draw_histogram-avoid-one-byte-ahead-access-f.patch
+Patch1004: xscreensaver-5.29-1004-abstract-_newline-make-it-sure-that-color-is-set-pos.patch
+Patch1005: xscreensaver-5.29-1005-vermiculate-forallinbank-evaluate-bankc-beforehand-c.patch
+Patch1006: xscreensaver-5.29-1006-braid-init_braid-limit-braid-braidlength-correctly.patch
+Patch1007: xscreensaver-5.29-1007-sproingies-RenderSproingie-limit-shift-value.patch
# Patches end
Requires: xscreensaver-base = %{epoch}:%{version}-%{release}
Requires: xscreensaver-extras = %{epoch}:%{version}-%{release}
@@ -127,7 +133,7 @@ BuildRequires: libXrandr-devel
BuildRequires: libXt-devel
BuildRequires: libXxf86misc-devel
BuildRequires: libXxf86vm-devel
-BuildRequires: gtk2-devel
+BuildRequires: gtk2-devel
BuildRequires: libjpeg-devel
BuildRequires: libglade2-devel
%if 0%{?fedora}
@@ -317,6 +323,14 @@ rm -f driver/XScreenSaver_ad.h
%__git commit -m "Base patches committed" -a
+%__cat %PATCH1001 | %__git am
+%__cat %PATCH1002 | %__git am
+%__cat %PATCH1003 | %__git am
+%__cat %PATCH1004 | %__git am
+%__cat %PATCH1005 | %__git am
+%__cat %PATCH1006 | %__git am
+%__cat %PATCH1007 | %__git am
+
change_option(){
set +x
ADFILE=$1
@@ -525,6 +539,7 @@ rm -f configure
#( cd po ; make generate_potfiles_in update-po )
# ???
( cd po ; make generate_potfiles_in ; cp -p POTFILES.in .. ; export srcdir=.. ; make update-po ; rm -f ../POTFILES_in )
+( ( cd ../po ; git add *.po ; git commit -m "po regenerated" ) || true )
%endif
%if 0%{?use_clang_analyze} >= 1
@@ -532,8 +547,15 @@ rm -f configure
mkdir clang-analyze
%endif
-%__make %{?_smp_mflags} -k \
+for dir in \
+ utils driver hacks hacks/glx po
+do
+ %__make %{?_smp_mflags} -k \
+ -C $dir \
GMSGFMT="msgfmt --statistics"
+done
+# Again
+%__make %{?_smp_mflags} -k
%if %{modular_conf}
# Make XScreenSavar.ad modular (bug 200881)
@@ -929,6 +951,10 @@ exit 0
%endif
%changelog
+* Thu Sep 4 2014 Mamoru TASAKA <mtasaka at fedoraproject.org> - 1:5.29-2
+- gcc49 sanitizer array elements oversize fixes
+- Make parallel build actually work
+
* Mon Aug 18 2014 Fedora Release Engineering <rel-eng at lists.fedoraproject.org> - 1:5.29-1.1
- Rebuilt for https://fedoraproject.org/wiki/Fedora_21_22_Mass_Rebuild
More information about the scm-commits
mailing list