Re: [bug-libunistring] Unistring 0.9.10 and Undefined Behavior sanitizer findings

[Jeffrey Walton]

On Thu, Mar 7, 2019 at 7:28 PM Jeffrey Walton <address@hidden> wrote:
>
> On Thu, Mar 7, 2019 at 2:10 AM Jeffrey Walton <address@hidden> wrote:
> >
> > I used CFLAGS="-DNDEBUG -g2 -O1 -fsanitize=undefined". I built the
> > library, then ran a 'make test V=1'. The output presented to the user
> > looks OK. However, it looks like UBsan found some undefined behavior:
> >
> > libunistring-0.9.10$ grep -IR 'runtime error'
> > tests/test-u8-width-linebreaks.log:unilbrk/u8-possible-linebreaks.c:49:3:
> > runtime error: null pointer passed as argument 1, which is declared to
> > never be null
> > tests/test-u32-to-u8.log:unistr/u8-cmp.c:38:10: runtime error: null
> > pointer passed as argument 2, which is declared to never be null
> > tests/test-numeric.log:unictype/numeric.c:52:54: runtime error: left
> > shift of 34927 by 16 places cannot be represented in type 'int'
> > tests/test-nfkc.log:unistr/u8-cmp.c:38:10: runtime error: null pointer
> > passed as argument 2, which is declared to never be null
> > tests/test-u8-tolower.log:unistr/u8-cmp.c:38:10: runtime error: null
> > pointer passed as argument 2, which is declared to never be null
> > tests/test-u16-to-u8.log:unistr/u8-cmp.c:38:10: runtime error: null
> > pointer passed as argument 2, which is declared to never be null
> > tests/test-nfkd.log:unistr/u8-cmp.c:38:10: runtime error: null pointer
> > passed as argument 2, which is declared to never be null
> > tests/test-u8-casefold.log:unistr/u8-cmp.c:38:10: runtime error: null
> > pointer passed as argument 2, which is declared to never be null
> > tests/unicase/test-ulc-casecmp2.sh.log:uniconv/u8-conv-from-enc.c:89:7:
> > runtime error: null pointer passed as argument 2, which is declared to
> > never be null
> > tests/unicase/test-ulc-casecoll2.sh.log:uniconv/u8-conv-from-enc.c:89:7:
> > runtime error: null pointer passed as argument 2, which is declared to
> > never be null
> > tests/test-nfc.log:unistr/u8-cmp.c:38:10: runtime error: null pointer
> > passed as argument 2, which is declared to never be null
> > tests/test-u8-totitle.log:unistr/u8-cmp.c:38:10: runtime error: null
> > pointer passed as argument 2, which is declared to never be null
> > tests/test-nfd.log:unistr/u8-cmp.c:38:10: runtime error: null pointer
> > passed as argument 2, which is declared to never be null
> > tests/test-u16-possible-linebreaks.log:unilbrk/u16-possible-linebreaks.c:49:3:
> > runtime error: null pointer passed as argument 1, which is declared to
> > never be null
> > tests/test-u16-width-linebreaks.log:unilbrk/u16-possible-linebreaks.c:49:3:
> > runtime error: null pointer passed as argument 1, which is declared to
> > never be null
> > tests/test-u8-toupper.log:unistr/u8-cmp.c:38:10: runtime error: null
> > pointer passed as argument 2, which is declared to never be null
> > tests/test-u8-possible-linebreaks.log:unilbrk/u8-possible-linebreaks.c:49:3:
> > runtime error: null pointer passed as argument 1, which is declared to
> > never be null
>
> This may be helpful as a starting point:
> https://github.com/noloader/Build-Scripts/blob/master/patch/unistring.patch
>
> However, when I remove the UB it results in 10 crashes of the test suite:

The failures were tracked down to u8_cmp. This fixes the function:

u8_cmp (const uint8_t *s1, const uint8_t *s2, size_t n)
{
    /* Use the fact that the UTF-8 encoding preserves lexicographic order. */
+   if (s1 == NULL || s2 == NULL) return 0;
  return memcmp ((const char *) s1, (const char *) s2, n);
}

I am not clear how a NULL string compared to another non-NULL string
is considered equal. But it fixes the crash and self tests.

Jeff