• Alexey Kopytov's avatar
    Bug#55077: Assertion failed: width > 0 && to != ((void *)0), · 04ae1aa9
    Alexey Kopytov authored
               file .\dtoa.c
    
    The assertion failure was correct because the 'width' argument
    of my_gcvt() has the signed integer type, whereas the unsigned
    value UINT_MAX32 was being passed by the caller
    (Field_double::val_str()) leading to a negative width in
    my_gcvt().
    
    The following chain of problems was found by further analysis:
    
    1. The display width for a floating point number is calculated
    in Field_double::val_str() as either field_length or the
    maximum possible length of string representation of a floating
    point number, whichever is greater. Since in the bug's test
    case field_length is UINT_MAX32, we get the same value as the
    display width. This does not make any sense because for numeric
    values field_length only matters for ZEROFILL columns,
    otherwise it does not make sense to allocate that much memory
    just to print a number. Field_float::val_str() has a similar
    problem.
    
    2. Even if the above wasn't the case, we would still get a
    crash on a slightly different test case when trying to allocate
    UINT_MAX32 bytes with String::alloc() because the latter does
    not handle such large input values correctly due to alignment
    overflows.
    
    3. Even when String::alloc() is fixed to return an error when
    an alignment overflow occurs, there is still a problem because
    almost no callers check its return value, and
    Field_double::val_str() is not an exception (same for
    Field_float::val_str()).
    
    4. Even if all of the above wasn't the case, creating a
    Field_double object with UINT_MAX32 as its field_length does
    not make much sense either, since the .frm code limits it to
    MAX_FIELD_CHARLENGTH (255) bytes. Such a beast can only be
    created by create_tmp_field_from_item() from an Item with
    REAL_RESULT as its result_type() and UINT_MAX32 as its
    max_length.
    
    5. For the bug's test case, the above condition (REAL_RESULT
    Item with max_length = UINT_MAX32) was a result of
    Item_func_if::fix_length_and_dec() "shortcutting" aggregation
    of argument types when one of the arguments was a constant
    NULL. In this case, the attributes of the aggregated type were
    simply copied from the other, non-NULL argument, but max_length
    was still calculated as per the general, non-shortcut case, by
    choosing the greatest of argument's max_length, which is
    obviously not correct.
    
    The patch addresses all of the above problems, even though
    fixing the assertion failure for the particular test case would
    require only a subset of the above problems to be solved.
    
    
    client/sql_string.cc:
      Return an error in case of uint32 overflow in alignment.
      Also assert there was no overflow to help find such conditions
      in debug builds, since almost no callers check the return value
      of String::alloc().
    mysql-test/r/func_if.result:
      Add a test case for bug #55077.
    mysql-test/t/func_if.test:
      Add a test case for bug #55077.
    sql/field.cc:
      - Assert we don't operate with fields wider than 255 
      (MAX_FIELD_CHARLENGTH) bytes in both Field_float and  
      Field_double. 
      - Don't take field_length into account when calculating the 
      output buffer length.
      - Check the return value of String::alloc()
    sql/item_cmpfunc.cc:
      When shortcutting type aggregation, don't take the NULL 
      argument's max_length into account.
    sql/sql_string.cc:
      Return an error in case of uint32 overflow in alignment.
      Also assert there was no overflow to help find such conditions
      in debug builds, since almost no callers check the return value
      of String::alloc().
    04ae1aa9
item_cmpfunc.cc 142 KB