Re: macOS (Big Sur, Apple Silicon) 'make check' fails in test-crypto-tlscredsx509

[Roman Bolshakov]

On Tue, Feb 02, 2021 at 08:50:24AM -0600, Eric Blake wrote:
> On 2/1/21 11:19 PM, Roman Bolshakov wrote:
> 
> > After a session of debugging I believe there's an issue with Clang 12.
> > Here's a test program (it reproduces unexpected ASN1_VALUE_NOT_VALID
> > from _asn1_time_der() in libtasn1):
> > 
> > #include <stdio.h>
> > 
> > static int func2(char *foo) {
> >         fprintf(stderr, "%s:%d foo: %p\n", __func__, __LINE__, foo);
> >         if (foo == NULL) {
> >                 fprintf(stderr, "%s:%d foo: %p\n", __func__, __LINE__, foo);
> >                 return 1;
> >         }
> >         return 0;
> > }
> > 
> > int func1(char *foo) {
> >         int counter = 0;
> >         if (fprintf(stderr, "IO\n") > 0)
> >                 counter += 10;
> >         fprintf(stderr, "%s:%d foo: %p counter %d\n", __func__, __LINE__, 
> > foo, counter);
> >         if(!func2(foo + counter)) {
> 
> This line has unspecified behavior in the C standard.  Adding an integer
> to a pointer is only well-specified if the pointer is to an array and
> the integer is within the bounds or the slot just past the array.  But
> since you called func1(NULL), foo is NOT pointing to an array, and
> therefore foo+counter points to garbage, and the compiler is free to
> optimize it at will.

Hi Eric,

Thanks a lot for pointing out this. It was surprising to me but
interesting:

http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2012.htm#clarifying-the-c-memory-object-model-out-of-bounds-pointer-arithmetic
http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2222.htm#pointer-arithmetic

As far as I understand wording in the standard, null pointer doesn't
point to an object (6.3.2.3p3). Therefore pointer arithmetic exception
for non-array objects (6.5.6p7) doesn't apply to null pointers but it
does apply to valid object pointers:

"For the purposes of these operators, a pointer to an object that is not
an element of an array behaves the same as a pointer to the first
element of an array of length one with the type of the object as its
element type."

So I was curious how clang would behave if we pass NULL conditionally.
We could do that by changing main() in the example above to:

int main(int argc, char *argv[]) {
        int ret;
        char *foo;

        if (argc > 1)
                foo = malloc(90 * sizeof(char));
        else
                foo = NULL;

        ret = func1(foo);

        if (argc > 1)
                free(foo);

        return ret;
}

And it returns "good" for specified behaviour (if foo points to malloc'd
memory):
 $ ./a.out f
 IO
 func1:17 foo: 0x14be06790 counter 10
 func2:5 foo: 0x14be0679a
 good

The behaviour is different if foo is initialized to NULL.
 $ ./a.out
 IO
 func1:17 foo: 0x0 counter 10
 func2:5 foo: 0xa
 func2:7 foo: 0x0
 broken

> > 
> > So, immediate workaround would be to downgrade optimization level of 
> > libtasn1
> > to -O1 in homebrew.
> > 
> > I've submitted the issue to Apple bugtracker:
> > FB8986815
> 
> Yes, it's annoying that as compilers get smarter, it exposes the
> presence of unspecified code in weird ways.  But I don't see this as a
> bug in clang, but as a bug in libtasn1 for assuming undefined behavior
> produces a sane result.
> 

Yes, strictly speaking the compiler is compliant. Given the example
libtasn1 should likely introduce a second variable for an integer
offset instead of relying on null pointer arithmetic.

It'd also be good if clang would print an error or a warning for null
pointer arithmetic.

Thanks,
Roman