Undefined behavior detected by ubsan #7

New Issue

claunia · 2026-01-29T20:28:38Z

claunia commented

2026-01-29 20:28:38 +00:00

Originally created by @nemequ on GitHub (Apr 13, 2015).

ubsan detects a lot of unaligned stores/loads, as well as a signed integer overflow, and a left shift of a negative value:

/home/nemequ/local/src/squash/plugins/brotli/brotli/enc/./././static_dict.h:57:19: runtime error: left shift of negative value -32
/home/nemequ/local/src/squash/plugins/brotli/brotli/enc/./././static_dict.h:57:29: runtime error: signed integer overflow: -3096880 + -2147483648 cannot be represented in type 'int [25]'
/home/nemequ/local/src/squash/plugins/brotli/brotli/dec/./bit_reader.h:150:38: runtime error: store to misaligned address 0x7fff9b36051d for type 'uint64_t', which requires 8 byte alignment
0x7fff9b36051d: note: pointer points here
 59 92 ba 00 00 00 00  f0 4f 22 02 00 00 00 00  f8 4f 22 02 00 00 00 00  70 05 36 9b ff 7f 00 00  22
             ^ 
/home/nemequ/local/src/squash/plugins/brotli/brotli/dec/./bit_reader.h:151:42: runtime error: store to misaligned address 0x7fff9b360525 for type 'uint8_t', which requires 8 byte alignment
0x7fff9b360525: note: pointer points here
 00 00 00 00 00 00 00  f8 4f 22 02 00 00 00 00  70 05 36 9b ff 7f 00 00  22 5d 54 8f 98 7f 00 00  80
             ^ 
/home/nemequ/local/src/squash/plugins/brotli/brotli/dec/./bit_reader.h:152:43: runtime error: store to misaligned address 0x7fff9b36052d for type 'uint8_t', which requires 8 byte alignment
0x7fff9b36052d: note: pointer points here
 00 00 00 00 00 00 00  70 05 36 9b ff 7f 00 00  22 5d 54 8f 98 7f 00 00  80 05 36 9b ff 7f 00 00  f8
             ^ 
/home/nemequ/local/src/squash/plugins/brotli/brotli/dec/./bit_reader.h:153:43: runtime error: store to misaligned address 0x7fff9b360535 for type 'uint8_t', which requires 8 byte alignment
0x7fff9b360535: note: pointer points here
 00 00 00 00 7f 00 00  22 5d 54 8f 98 7f 00 00  80 05 36 9b ff 7f 00 00  f8 4f 22 02 00 00 00 00  b8
             ^ 
/home/nemequ/local/src/squash/plugins/brotli/brotli/dec/./bit_reader.h:188:17: runtime error: load of misaligned address 0x7fff9b3600d5 for type 'const uint64_t', which requires 8 byte alignment
0x7fff9b3600d5: note: pointer points here
 ab b8 8f 65 ed 9c e7  0f 83 d2 75 fa fc b0 e8  37 25 3e 42 c1 d4 85 c5  8f a0 11 8f 16 90 87 8f  90
             ^

Originally created by @nemequ on GitHub (Apr 13, 2015). ubsan detects a lot of unaligned stores/loads, as well as a signed integer overflow, and a left shift of a negative value: ``` /home/nemequ/local/src/squash/plugins/brotli/brotli/enc/./././static_dict.h:57:19: runtime error: left shift of negative value -32 /home/nemequ/local/src/squash/plugins/brotli/brotli/enc/./././static_dict.h:57:29: runtime error: signed integer overflow: -3096880 + -2147483648 cannot be represented in type 'int [25]' /home/nemequ/local/src/squash/plugins/brotli/brotli/dec/./bit_reader.h:150:38: runtime error: store to misaligned address 0x7fff9b36051d for type 'uint64_t', which requires 8 byte alignment 0x7fff9b36051d: note: pointer points here 59 92 ba 00 00 00 00 f0 4f 22 02 00 00 00 00 f8 4f 22 02 00 00 00 00 70 05 36 9b ff 7f 00 00 22 ^ /home/nemequ/local/src/squash/plugins/brotli/brotli/dec/./bit_reader.h:151:42: runtime error: store to misaligned address 0x7fff9b360525 for type 'uint8_t', which requires 8 byte alignment 0x7fff9b360525: note: pointer points here 00 00 00 00 00 00 00 f8 4f 22 02 00 00 00 00 70 05 36 9b ff 7f 00 00 22 5d 54 8f 98 7f 00 00 80 ^ /home/nemequ/local/src/squash/plugins/brotli/brotli/dec/./bit_reader.h:152:43: runtime error: store to misaligned address 0x7fff9b36052d for type 'uint8_t', which requires 8 byte alignment 0x7fff9b36052d: note: pointer points here 00 00 00 00 00 00 00 70 05 36 9b ff 7f 00 00 22 5d 54 8f 98 7f 00 00 80 05 36 9b ff 7f 00 00 f8 ^ /home/nemequ/local/src/squash/plugins/brotli/brotli/dec/./bit_reader.h:153:43: runtime error: store to misaligned address 0x7fff9b360535 for type 'uint8_t', which requires 8 byte alignment 0x7fff9b360535: note: pointer points here 00 00 00 00 7f 00 00 22 5d 54 8f 98 7f 00 00 80 05 36 9b ff 7f 00 00 f8 4f 22 02 00 00 00 00 b8 ^ /home/nemequ/local/src/squash/plugins/brotli/brotli/dec/./bit_reader.h:188:17: runtime error: load of misaligned address 0x7fff9b3600d5 for type 'const uint64_t', which requires 8 byte alignment 0x7fff9b3600d5: note: pointer points here ab b8 8f 65 ed 9c e7 0f 83 d2 75 fa fc b0 e8 37 25 3e 42 c1 d4 85 c5 8f a0 11 8f 16 90 87 8f 90 ^ ```

claunia closed this issue

2026-01-29 20:28:39 +00:00

claunia commented

2026-01-29 20:28:39 +00:00

@nemequ commented on GitHub (Jun 13, 2015):

Probably related (warnings from clang):

/home/nemequ/local/src/squash/plugins/brotli/brotli/dec/./bit_reader.h:151:8: error: cast from 'uint8_t *' (aka 'unsigned char *') to 'uint64_t *'
      (aka 'unsigned long *') increases required alignment from 1 to 8 [-Werror,-Wcast-align]
      *(uint64_t*)(dst + bytes_read) = 0;
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/nemequ/local/src/squash/plugins/brotli/brotli/dec/./bit_reader.h:152:8: error: cast from 'uint8_t *' (aka 'unsigned char *') to 'uint64_t *'
      (aka 'unsigned long *') increases required alignment from 1 to 8 [-Werror,-Wcast-align]
      *(uint64_t*)(dst + bytes_read + 8) = 0;
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/nemequ/local/src/squash/plugins/brotli/brotli/dec/./bit_reader.h:153:8: error: cast from 'uint8_t *' (aka 'unsigned char *') to 'uint64_t *'
      (aka 'unsigned long *') increases required alignment from 1 to 8 [-Werror,-Wcast-align]
      *(uint64_t*)(dst + bytes_read + 16) = 0;
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/nemequ/local/src/squash/plugins/brotli/brotli/dec/./bit_reader.h:154:8: error: cast from 'uint8_t *' (aka 'unsigned char *') to 'uint64_t *'
      (aka 'unsigned long *') increases required alignment from 1 to 8 [-Werror,-Wcast-align]
      *(uint64_t*)(dst + bytes_read + 24) = 0;
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/nemequ/local/src/squash/plugins/brotli/brotli/dec/./bit_reader.h:189:30: error: cast from 'uint8_t *' (aka 'unsigned char *') to
      'const uint32_t *' (aka 'const unsigned int *') increases required alignment from 1 to 4 [-Werror,-Wcast-align]
    br->val_ |= ((uint64_t)(*(const uint32_t*)(
                             ^~~~~~~~~~~~~~~~~~

@nemequ commented on GitHub (Jun 13, 2015): Probably related (warnings from clang): ``` /home/nemequ/local/src/squash/plugins/brotli/brotli/dec/./bit_reader.h:151:8: error: cast from 'uint8_t *' (aka 'unsigned char *') to 'uint64_t *' (aka 'unsigned long *') increases required alignment from 1 to 8 [-Werror,-Wcast-align] *(uint64_t*)(dst + bytes_read) = 0; ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/nemequ/local/src/squash/plugins/brotli/brotli/dec/./bit_reader.h:152:8: error: cast from 'uint8_t *' (aka 'unsigned char *') to 'uint64_t *' (aka 'unsigned long *') increases required alignment from 1 to 8 [-Werror,-Wcast-align] *(uint64_t*)(dst + bytes_read + 8) = 0; ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/nemequ/local/src/squash/plugins/brotli/brotli/dec/./bit_reader.h:153:8: error: cast from 'uint8_t *' (aka 'unsigned char *') to 'uint64_t *' (aka 'unsigned long *') increases required alignment from 1 to 8 [-Werror,-Wcast-align] *(uint64_t*)(dst + bytes_read + 16) = 0; ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/nemequ/local/src/squash/plugins/brotli/brotli/dec/./bit_reader.h:154:8: error: cast from 'uint8_t *' (aka 'unsigned char *') to 'uint64_t *' (aka 'unsigned long *') increases required alignment from 1 to 8 [-Werror,-Wcast-align] *(uint64_t*)(dst + bytes_read + 24) = 0; ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/nemequ/local/src/squash/plugins/brotli/brotli/dec/./bit_reader.h:189:30: error: cast from 'uint8_t *' (aka 'unsigned char *') to 'const uint32_t *' (aka 'const unsigned int *') increases required alignment from 1 to 4 [-Werror,-Wcast-align] br->val_ |= ((uint64_t)(*(const uint32_t*)( ^~~~~~~~~~~~~~~~~~ ```

claunia commented

2026-01-29 20:28:40 +00:00

@eustas commented on GitHub (Sep 1, 2015):

Unaligned loads/stores in bit_reader are intentional. We carefully profiled and decided to use them for better performance.

I'm going to take a look at overflow/shifting issues soon.

@eustas commented on GitHub (Sep 1, 2015): Unaligned loads/stores in bit_reader are intentional. We carefully profiled and decided to use them for better performance. I'm going to take a look at overflow/shifting issues soon.

claunia commented

2026-01-29 20:28:40 +00:00

@nemequ commented on GitHub (Sep 1, 2015):

FWIW at least clang and, to a lesser extent, gcc are pretty good about generating fast code for memcpy with fixed sizes. Newer versions of GCC are better, but still not optimal. It might be worth it to keep an eye on that GCC bug and revisit the issue once it is resolved… unaligned accesses can cause problems down the road when the CC starts optimizing things better (this happened to LZ4 recently).

@nemequ commented on GitHub (Sep 1, 2015): FWIW at least clang and, to a lesser extent, gcc are pretty good about [generating fast code for memcpy with fixed sizes](http://fastcompression.blogspot.fr/2015/08/accessing-unaligned-memory.html). Newer versions of GCC are better, [but still not optimal](https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67366). It might be worth it to keep an eye on that GCC bug and revisit the issue once it is resolved… unaligned accesses can cause problems down the road when the CC starts optimizing things better (this happened to LZ4 recently).

claunia commented

2026-01-29 20:28:40 +00:00

@rygorous commented on GitHub (Sep 22, 2015):

Unaligned loads/stores: the "official"/safe way to do an unaligned 32-bit load from "p" is:

uint32_t tmp;
memcpy(&tmp, p, sizeof(uint32_t));
// use tmp

Store likewise via memcpy. I know this is really gross, but I've run into several compilers that get really aggressive about assuming that loads/stores are aligned, and all major C/C++ compilers actually detect this type of use of memcpy and turn it into the unaligned loads/stores you want. I would prefer not to have to do this kind of thing, but compilers tend to get more pedantic about spec-lawyering all the time, and especially ARM compilers like to combine adjacent LDRs (which do not require alignment on ARMv6+) into LDRDs (which do), which has caused "fun" portability problems for me in the past.

@rygorous commented on GitHub (Sep 22, 2015): Unaligned loads/stores: the "official"/safe way to do an unaligned 32-bit load from "p" is: ``` uint32_t tmp; memcpy(&tmp, p, sizeof(uint32_t)); // use tmp ``` Store likewise via memcpy. I know this is really gross, but I've run into several compilers that get really aggressive about assuming that loads/stores are aligned, and all major C/C++ compilers actually detect this type of use of memcpy and turn it into the unaligned loads/stores you want. I would prefer not to have to do this kind of thing, but compilers tend to get more pedantic about spec-lawyering all the time, and especially ARM compilers like to combine adjacent LDRs (which do not require alignment on ARMv6+) into LDRDs (which do), which has caused "fun" portability problems for me in the past.

claunia commented

2026-01-29 20:28:41 +00:00

@eustas commented on GitHub (Sep 23, 2015):

We're going to make unaligned safer and more portable soon. So, stay tuned.

@eustas commented on GitHub (Sep 23, 2015): We're going to make unaligned safer and more portable soon. So, stay tuned.

claunia commented

2026-01-29 20:28:41 +00:00

@eustas commented on GitHub (Sep 28, 2015):

Fixed with PR 183.
Compiling with -DBROTLI_BUILD_PORTABLE avoids overlapping memcpy and unaligned reads.

@eustas commented on GitHub (Sep 28, 2015): Fixed with PR 183. Compiling with -DBROTLI_BUILD_PORTABLE avoids overlapping memcpy and unaligned reads.

claunia referenced this issue

2026-01-29 20:40:25 +00:00