Issue 262 in webp: SIGBUS in HistogramAddEval() on IRIX while encoding lossless images

[we...@googlecode.com]

Status: New
Owner: ----
Labels: Type-Defect Priority-Medium

New issue 262 by rainer.c...@sevenval.com: SIGBUS in HistogramAddEval() on
IRIX while encoding lossless images
https://code.google.com/p/webp/issues/detail?id=262

Using libwebp 0.4.2 and 0.4.3 on IRIX, compiled as n32 binaries using
MIPSPro c99 7.4.4, I get BUS errors when trying to use
WebPEncodeLosslessRGB(). WebPEncodeRGB works flawlessly. Changing the
assignment in histogram.c:336 from out->bit_cost_ = cost; into a memcpy()
fixes the problem and correct webp files can be created. I have not
encountered any problems writing lossy files so far. My quick attempt to
re-order or pad the members of VP8LHistogram has been unsuccessful.

Thread 0x10000
> 0 HistogramAddEval(a = 0x1066b0e0, b = 0x1066c208, out = 0x1026850c,
> cost_threshold = 924.255)
> ["/usr/people/canavan/src/libwebp/libwebp-0.4.3/src/enc/histogram.c":336,
> 0x44ee64]
1 HistogramCombineEntropyBin(image_histo = 0x10649ae0, histos =
0x1026850c, bin_map = 0x107e2348, bin_depth = 136, combine_cost_factor =
0.16)
["/usr/people/canavan/src/libwebp/libwebp-0.4.3/src/enc/histogram.c":542,
0x44fbfc]
2 VP8LGetHistoImageSymbols(xsize = 480, ysize = 270, refs = 0x7ffb7150,
quality = 70, histo_bits = 5, cache_bits = 0, image_histo = 0x10649ae0,
histogram_symbols = 0x1026cf10)
["/usr/people/canavan/src/libwebp/libwebp-0.4.3/src/enc/histogram.c":729,
0x4506c0]
3 EncodeImageInternal(bw = 0x7ffb7278, argb = 0x105abae8, hash_chain =
0x1042d1d0, refs_array = 0x1042d1a0, width = 480, height = 270, quality =
70, cache_bits = 0, histogram_bits = 5)
["/usr/people/canavan/src/libwebp/libwebp-0.4.3/src/enc/vp8l.c":600,
0x467b38]
4 VP8LEncodeStream(config = 0x7ffb7358, picture = 0x7ffb73d8, bw =
0x7ffb7278)
["/usr/people/canavan/src/libwebp/libwebp-0.4.3/src/enc/vp8l.c":1126,
0x469aa0]
5 VP8LEncodeImage(config = 0x7ffb7358, picture = 0x7ffb73d8)
["/usr/people/canavan/src/libwebp/libwebp-0.4.3/src/enc/vp8l.c":1211,
0x469e80]
6 WebPEncode(config = 0x7ffb7358, pic = 0x7ffb73d8)
["/usr/people/canavan/src/libwebp/libwebp-0.4.3/src/enc/webpenc.c":378,
0x46b2c0]
7 Encode(rgba = 0x1030fd90
= "\226\226\377\226\226\377\226\226\377\226\226\377\226\226\377\226\226\377\226\226\377\226\226\377\226\226\377\226\226\377\226\226\377\226\226\377\226\226\377\226\226\377\341\226\377\341\226\377\341\226\377\341\226\377\341\226\377\223\223\374\223\223\374\223\223\374\223\223\374\223\223\374\223\223\374\223\223\374\223\223\374\223\223\374\223\223\374\223\223\374\223\223\374\223\223\374\220\220\370\220\220\370\220\220\370\220\220\370\220\220\370\220\220\370\220\220\370\220\220\370\220\220\370\220\220\370\220\220\...",
width = 480, height = 270, stride = 1440, import = 0x459b30, quality_factor
= 70.0, lossless = 1, output = 0x7ffb7568)
["/usr/people/canavan/src/libwebp/libwebp-0.4.3/src/enc/picture.c":252,
0x453024]
8 WebPEncodeLosslessRGB(in = 0x1030fd90
= "\226\226\377\226\226\377\226\226\377\226\226\377\226\226\377\226\226\377\226\226\377\226\226\377\226\226\377\226\226\377\226\226\377\226\226\377\226\226\377\226\226\377\341\226\377\341\226\377\341\226\377\341\226\377\341\226\377\223\223\374\223\223\374\223\223\374\223\223\374\223\223\374\223\223\374\223\223\374\223\223\374\223\223\374\223\223\374\223\223\374\223\223\374\223\223\374\220\220\370\220\220\370\220\220\370\220\220\370\220\220\370\220\220\370\220\220\370\220\220\370\220\220\370\220\220\370\220\220\...",
w = 480, h = 270, bps = 1440, out = 0x7ffb7568)
["/usr/people/canavan/src/libwebp/libwebp-0.4.3/src/enc/picture.c":282,
0x45331c]
9 WriteWEBP(fp = 0xfb4f7f8, pic = 0x1030fd90
= "\226\226\377\226\226\377\226\226\377\226\226\377\226\226\377\226\226\377\226\226\377\226\226\377\226\226\377\226\226\377\226\226\377\226\226\377\226\226\377\226\226\377\341\226\377\341\226\377\341\226\377\341\226\377\341\226\377\223\223\374\223\223\374\223\223\374\223\223\374\223\223\374\223\223\374\223\223\374\223\223\374\223\223\374\223\223\374\223\223\374\223\223\374\223\223\374\220\220\370\220\220\370\220\220\370\220\220\370\220\220\370\220\220\370\220\220\370\220\220\370\220\220\370\220\220\370\220\220\...",
ptype = 0, w = 480, h = 270, rmap = 0x10111a70
= "\341\326\313\277\264\251\235\222\207{peYNC7\226\223\220\215\212\207\203\200}zwspmjgc`]ZWSPMJGD@=:740-*\'$
\035\032\027\024\020\r\n\a\004", gmap = 0x10111b70
= "\226\214\202xndZPF<2(\036\024\n", bmap = 0x10111c70
= "\377\364\350\334\320\304\270\254\241\225\211}qeYM\377\374\370\364\360\355\351\345\341\335\332\326\322\316\312\307\303\277\273\270\264\260\254\250\245\241\235\231\2
25\222\216\212\206\203\177{wsplhd`]YUQM\031")
["/usr/people/canavan/src/xv/xv-3.10a/xvwebp.c":444, 0x100f37d4]
10 writeWEBP() ["/usr/people/canavan/src/xv/xv-3.10a/xvwebp.c":371,
0x100f34b8]
11 doCmd(cmd = 0) ["/usr/people/canavan/src/xv/xv-3.10a/xvwebp.c":268,
0x100f309c]
12 clickWEBPD(x = 135, y = 156)
["/usr/people/canavan/src/xv/xv-3.10a/xvwebp.c":240, 0x100f2fb4]
13 WEBPCheckEvent(xev = 0x7ffb7b20)
["/usr/people/canavan/src/xv/xv-3.10a/xvwebp.c":127, 0x100f294c]
14 handleButtonEvent(event = 0x7ffb7b20, donep = 0x7ffb77e0, retvalp =
0x7ffb77e4) ["/usr/people/canavan/src/xv/xv-3.10a/xvevent.c":1359,
0x10021dac]
15 HandleEvent(event = 0x7ffb7b20, donep = 0x7ffb7abc)
["/usr/people/canavan/src/xv/xv-3.10a/xvevent.c":241, 0x1001e7e8]
16 EventLoop() ["/usr/people/canavan/src/xv/xv-3.10a/xvevent.c":143,
0x1001e564]
17 mainLoop() ["/usr/people/canavan/src/xv/xv-3.10a/xv.c":3813,
0x1001b18c]
18 main(argc = 1, argv = 0x7ffb7ee4)
["/usr/people/canavan/src/xv/xv-3.10a/xv.c":1046, 0x10010ce0]
19 __start()
["/xlv55/kudzu-apr12/work/irix/lib/libc/libc_n32_M4/csu/crt1text.s":177,
0x1000e018]

The image data shown in the backtrace above is from
http://www.gstatic.com/webp/gallery/1.webp

--
You received this message because this project is configured to send all
issue notifications to this address.
You may adjust your notification preferences at:
https://code.google.com/hosting/settings

[we...@googlecode.com]

Comment #1 on issue 262 by pascal.m...@gmail.com: SIGBUS in

HistogramAddEval() on IRIX while encoding lossless images
https://code.google.com/p/webp/issues/detail?id=262

i'm curious: why this field in particular?

Does it happen with this cost_ field when re-ordering? Or is it triggered
by another field then, once reordered?

[we...@googlecode.com]

Comment #2 on issue 262 by rainer.c...@sevenval.com: SIGBUS in

HistogramAddEval() on IRIX while encoding lossless images
https://code.google.com/p/webp/issues/detail?id=262

Turns out I overlooked the fact that VP8LHistogram isn't packed - the
members in there are always aligned "suitably" no matter where in the
struct they are defined. Things change of course if I #PRAGMA pack the
whole thing, but that doesn't change the BUS errors. They are always
triggered at the first line of code that touches a double in a histogram
that is incorrectly aligned. Depending on the input image, it's either the
location above or (line number should be 398)

[UpdateHistogramCost +0x4,0x439440]
h->red_cost_ = PopulationCost(h->red_, NUM_LITERAL_CODES);

> 0 UpdateHistogramCost()
1 HistogramCopyAndAnalyze()
2 VP8LGetHistoImageSymbols()
3 EncodeImageInternal()
4 VP8LEncodeStream()

The actual culprit is VP8AllocateHistogramSet(). While malloc() always
returns a pointer that is aligned % 8 (as is necessary for the
double ...cost_), memory* is aligned only % 4 when the histogram[i] are
initialized if size is even and pointers are 32 bit.

For example, when saving 5.webp (the fire breathing man from the sample
gallery), the patch below leads to the output below:

initmem: @1028f028
mem: @1028f03c
realign: @1028f040
h: 0 @1028f040
h: 1 @10290178
initmem: @1028f028
mem: @1028f038
h: 0 @1028f038
initmem: @1028f028
mem: @1028f038
h: 0 @1028f038
initmem: @116ddfc8
mem: @116debd4
realign: @116debd8
h: 0 @116debd8
h: 1 @116dfd10
h: 2 @116e0e48
h: 3 @116e1f80
h: 4 @116e30b8
h: 5 @116e41f0
h: 6 @116e5328
h: 7 @116e6460
h: 8 @116e7598
h: 9 @116e86d0
[...]


As an alternative to the uintptr_t arithmetic, one can just make sure that
there's an even number of pointers before the histograms (and +8 for the
size could be reduced to +sizeof(*set)):

if (size % 2 == 0) {
memory += sizeof(*set);

Maybe add && (sizeof(*set) != sizeof(double)).

The patch below fixes all SIGBUS errors for me, I haven't managed to cause
any even after saving dozens of random images with xv and ~300 more with
cwebp -lossless. No memcpy required.

--- src/libwebp-0.4.3/src/enc/histogram.c Wed Mar 11 07:06:09 CET 2015
+++ histogram_debug.c Mon Sep 14 23:46:19 CEST 2015
@@ -103,9 +103,11 @@
VP8LHistogramSet* set;
const size_t total_size = sizeof(*set)
+ sizeof(*set->histograms) * size
- + (size_t)VP8LGetHistogramSize(cache_bits) *
size;
+ + (size_t)VP8LGetHistogramSize(cache_bits) *
size
+ + 8;
uint8_t* memory = (uint8_t*)WebPSafeMalloc(total_size, sizeof(*memory));
if (memory == NULL) return NULL;
+fprintf(stderr, "initmem: @%x\n", memory);

set = (VP8LHistogramSet*)memory;
memory += sizeof(*set);
@@ -113,8 +115,20 @@
memory += size * sizeof(*set->histograms);
set->max_size = size;
set->size = size;
+
+#ifdef WEBP_FORCE_ALIGNED
+fprintf(stderr, "mem: @%x\n", memory);
+if ((uintptr_t)(memory) % 8 != 0) {
+ memory += 8 - ((uintptr_t)memory%8);
+fprintf(stderr, "realign: @%x\n", memory);
+}
+#endif
+
for (i = 0; i < size; ++i) {
set->histograms[i] = (VP8LHistogram*)memory;
+#ifdef WEBP_FORCE_ALIGNED
+fprintf(stderr, "h: %i @%x\n", i, set->histograms[i]);
+#endif
// literal_ won't necessary be aligned.
set->histograms[i]->literal_ = (uint32_t*)(memory +
sizeof(VP8LHistogram));
VP8LHistogramInit(set->histograms[i], cache_bits);

[we...@googlecode.com]

Comment #3 on issue 262 by s...@google.com: SIGBUS in HistogramAddEval() on

IRIX while encoding lossless images
https://code.google.com/p/webp/issues/detail?id=262

thanks for the analysis!
There's indeed a need for some cleanup on the packed malloc's, to add some
proper ALIGN.
Patch to follow...

[we...@googlecode.com]

Updates:
Status: Started

Comment #4 on issue 262 by jz...@google.com: SIGBUS in HistogramAddEval()

on IRIX while encoding lossless images
https://code.google.com/p/webp/issues/detail?id=262

https://chromium-review.googlesource.com/#/c/300289/ has been merged:
cd82440 VP8LAllocateHistogramSet: align histogram[] entries

it should address the issue similarly to your patch. let us know if there's
any further breakage and thanks again for the report.

[we...@googlecode.com]

Comment #5 on issue 262 by rainer.c...@sevenval.com: SIGBUS in

HistogramAddEval() on IRIX while encoding lossless images
https://code.google.com/p/webp/issues/detail?id=262

The patch in cd82440 on top of libwebp 0.4.3 works for me on IRIX.

[we...@googlecode.com]

Comment #6 on issue 262 by jz...@google.com: SIGBUS in HistogramAddEval()

on IRIX while encoding lossless images
https://code.google.com/p/webp/issues/detail?id=262

Great, thanks for trying it out. I'll pull this into the 0.4.4 release once
I branch for that.

[we...@googlecode.com]

Updates:
Status: Fixed

Comment #7 on issue 262 by pascal.m...@gmail.com: SIGBUS in

HistogramAddEval() on IRIX while encoding lossless images
https://code.google.com/p/webp/issues/detail?id=262

[closing as fixed]