Re: [HACKERS] Getting server crash on Windows when using ICU collation

PFA patch that fixes the issue described in above thread. As mentioned
in the above thread, the crash is basically happening in varstr_cmp()
function and  it's  only happening on Windows because in varstr_cmp(),
if the collation provider is ICU, we don't even think of calling ICU
functions to compare the string. Infact, we directly attempt to call
the OS function wsccoll*() which is not expected. Thanks.

--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com

On Sat, Jun 10, 2017 at 3:25 PM, Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
> Hi All,
>
> I am seeing a server crash when running queries using ICU collations on
> Windows. Following are the steps to reproduce the crash with the help of
> patch to enable icu feature on Windows - [1],
>
> 1) psql -d postgres
>
> 2) CREATE DATABASE icu_win_test
>      TEMPLATE template0
>      ENCODING 'UTF8'
>     LC_CTYPE 'C'
>    LC_COLLATE 'C';
>
> 3) \c icu_win_test;
>
> 4) icu_win_test=# select 'B' > 'a' COLLATE "de-u-co-standard-x-icu";
> server closed the connection unexpectedly
>         This probably means the server terminated abnormally
>         before or while processing the request.
> The connection to the server was lost. Attempting reset: Failed.
> !>
>
> The backtrace observed in the core dump is,
>
>> postgres.exe!varstr_cmp(char * arg1, int len1, char * arg2, int len2,
>> unsigned int collid)  Line 1494 C
>   postgres.exe!text_cmp(varlena * arg1, varlena * arg2, unsigned int collid)
> Line 1627 C
>   postgres.exe!text_gt(FunctionCallInfoData * fcinfo)  Line 1738 + 0x12
> bytes C
>   postgres.exe!ExecInterpExpr(ExprState * state, ExprContext * econtext,
> char * isnull)  Line 650 + 0xa bytes C
>   postgres.exe!evaluate_expr(Expr * expr, unsigned int result_type, int
> result_typmod, unsigned int result_collation)  Line 4719 C
>   postgres.exe!evaluate_function(unsigned int funcid, unsigned int
> result_type, int result_typmod, unsigned int result_collid, unsigned int
> input_collid, List * args, char funcvariadic, HeapTupleData * func_tuple,
> eval_const_expressions_context * context)  Line 4272 + 0x50 bytes C
>   postgres.exe!simplify_function(unsigned int funcid, unsigned int
> result_type, int result_typmod, unsigned int result_collid, unsigned int
> input_collid, List * * args_p, char funcvariadic, char process_args, char
> allow_non_const, eval_const_expressions_context * context)  Line 3914 + 0x44
> bytes C
>   postgres.exe!eval_const_expressions_mutator(Node * node,
> eval_const_expressions_context * context)  Line 2627 C
>   postgres.exe!expression_tree_mutator(Node * node, Node * (void)* mutator,
> void * context)  Line 2735 + 0x37 bytes C
>   postgres.exe!expression_tree_mutator(Node * node, Node * (void)* mutator,
> void * context)  Line 2932 + 0x8 bytes C
>   postgres.exe!eval_const_expressions(PlannerInfo * root, Node * node)  Line
> 2413 C
>   postgres.exe!subquery_planner(PlannerGlobal * glob, Query * parse,
> PlannerInfo * parent_root, char hasRecursion, double tuple_fraction)  Line
> 623 + 0x2d bytes C
>
> RCA:
> ====
> As seen in the backtrace, the crash is basically happening in varstr_cmp()
> function. AFAICU, this crash is only happening on Windows because in
> varstr_cmp(), if the encoding type is UTF8, we don't even think of calling
> ICU functions to compare the string. Infact, we directly attempt to call the
> OS function wsccoll*().
>
> The point is, if collation provider is ICU, then we should tryto call ICU
> functions for collation support instead of calling OS functions. This thing
> is being taken care inside varstr_cmp() for Linux but surprisingly it's not
> handled for Windows. Please have a look at below code snippet in
> varstr_cmp() to know on how it is being taken care for linux,
>
> #endif   /* WIN32 */
> ........
> ........
>
> #ifdef USE_ICU
> #ifdef HAVE_UCOL_STRCOLLUTF8
> if (GetDatabaseEncoding() == PG_UTF8)
> {
>    UErrorCode status;
>
>    status = U_ZERO_ERROR;
>    result = ucol_strcollUTF8(mylocale->info.icu.ucol,
>                                           arg1, len1,
>                                           arg2, len2,
>                                          &status);
>      if (U_FAILURE(status))
>          ereport(ERROR,
>                      (errmsg("collation failed: %s", u_errorName(status))));
>  }
> else
> #endif
> {
>       int32_t ulen1,
>       ulen2;
>      UChar   *uchar1, *uchar2;
>
>     ulen1 = icu_to_uchar(&uchar1, arg1, len1);
>     ulen2 = icu_to_uchar(&uchar2, arg2, len2);
>
>     result = ucol_strcoll(mylocale->info.icu.ucol,
>                                   uchar1, ulen1,
>                                   uchar2, ulen2);
>  }
> #else /* not USE_ICU */
> /* shouldn't happen */
>  elog(ERROR, "unsupported collprovider: %c", mylocale->provider);
>  #endif   /* not USE_ICU */
> }
> else
> {
> #ifdef HAVE_LOCALE_T
>      result = strcoll_l(a1p, a2p, mylocale->info.lt);
> #else
>     /* shouldn't happen */
>      elog(ERROR, "unsupported collprovider: %c", mylocale->provider);
> #endif
> }
>
> Fix:
> ====
> I am currently working on this and will try to come up with the fix asap.
>
> [1] -
> https://www.postgresql.org/message-id/CAE9k0P%3DQRjtS1a0rgTyKag_%2Bs6XCs7vovV%2BgSkUfYVASog0P8w%40mail.gmail.com
>
>
> --
> With Regards,
> Ashutosh Sharma
> EnterpriseDB:http://www.enterprisedb.com

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers