Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

invalid memory address or nil pointer dereference in expression.newFunctionImpl->expression.BuildCastFunctionWithCheck #55886

Closed
ycybfhb opened this issue Sep 5, 2024 · 9 comments · Fixed by #57403
Labels
affects-8.5 This bug affects the 8.5.x(LTS) versions. impact/panic severity/major sig/execution SIG execution type/bug The issue is confirmed as a bug.

Comments

@ycybfhb
Copy link

ycybfhb commented Sep 5, 2024

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

SQL to init database
create table t_dci ( 
c_dph7 int ,
c_l1t tinyint ,
c_d3wokzls77 int ,
c_bywfl tinyint unique ,
c_kzre text ,
c_fzqupuma int not null ,
c_w9qyk_fpj double not null ,
c_ib1xsf3c8d tinyint ,
primary key(c_d3wokzls77, c_dph7) CLUSTERED) pre_split_regions=6;

create table t_jg8o ( 
c_otj13 int unique ,
c_foveoe text ,
c_jbb text ,
c_s int not null unique ,
c_ovz0 double ,
c_m0qqv_cl4x text ,
c_cz text not null ,
c__qy double ,
c_z int not null ,
c_a90ol text not null ,
primary key(c_z) CLUSTERED) pre_split_regions=2;

alter table t_jg8o add column c_tazb9 int;

create table t_rc ( 
c_m2y int ,
c_yu tinyint not null ,
primary key(c_m2y) CLUSTERED) pre_split_regions=2;

create table t__9r63 ( 
c_g7eofzlxn int ,
c_r58lkh double ,
c_x2erxo10w int ,
c_wsr tinyint ,
c_hd2v4v0 double ,
c_tb3u text ,
c_onfeptr2q tinyint unique ,
primary key(c_g7eofzlxn, c_x2erxo10w) CLUSTERED) pre_split_regions=7;

alter table t__9r63 add column c_o8tsf double;

alter table t_jg8o add column c_mgjb text;

insert into t__9r63 (c_g7eofzlxn, c_r58lkh, c_x2erxo10w, c_wsr, c_hd2v4v0, c_tb3u, c_onfeptr2q, c_o8tsf) values 
  (1275540443, 9223372036854775806.5, -1407328577, (NOT NOT(cast((-945767894 <= -8247562720541611887) as unsigned))), 35.15, 'l7ngfavtf2', (NOT NOT(cast((cast(cast(null as signed) as signed) > cast(-1581230237 as signed)) as unsigned))), 22.55), 
  (0, 6.4, -23, (-4 is not NULL), -256.4, 'nrqgupym', (NOT NOT(cast((cast(null as char) < cast(null as char)) as unsigned))), 49.2), 
  (-2005512947, 14.81, 1215057043, ((NOT NOT(cast((cast(-4255 as signed) <=> cast(-9223372036854775807.2 as double)) as unsigned)))) 
    or ((1928377790 is NULL)), 93.95, 'ycra', (NOT NOT(cast((-4902061839617880638 < cast(null as signed)) as unsigned))), 53.76), 
  (-865548105, 27.38, -932309610, 1=1, 65.29, cast(null as char), (NOT NOT(cast((cast(cast(null as char) as char) != cast(cast(null as char) as char)) as unsigned))), 2.71);

create table t_glzh3lb0ro ( 
c_kffac5e63 int not null unique ,
c_p int ,
c_i3ml int ,
c_c7njaqnyv7 int not null ,
c_wd3x double ,
c__n3bhft5z int not null ,
c_pf8vmpo8 int not null ,
primary key(c__n3bhft5z, c_p) NONCLUSTERED) shard_row_id_bits=9 pre_split_regions=7;

alter table t_rc add column c_b48gd04utl text;


SQL that causes error
WITH
cte_0 AS (
SELECT
  (select c_i3ml from t_glzh3lb0ro order by c_i3ml limit 1 offset 5)
       as c0,
  ref_0.c_a90ol as c4,
  case when (cast((select stddev_pop(c_r58lkh) from t__9r63)
           as double) = ( 
        select distinct 
              cast(ref_1.c_wd3x as double) as c0
            from 
              t_jg8o as ref_7
            where (NOT NOT(cast((cast((ref_1.c_wd3x < ( 
                  select distinct 
                        ref_7.c_ovz0 as c0
                      from 
                        t_jg8o as ref_8
                      where (subq_0.c6 not in (
                        (ref_4.c_jbb not like 'j_e2u'), (NOT NOT(cast((cast(-6801295627085852521 as signed) != cast(-7092484920321976545 as signed)) as unsigned))), (NOT NOT(cast((cast(ref_7.c_s as signed) != cast((NOT NOT(cast((cast(ref_8.c_tazb9 as signed) >= cast(ref_1.c_pf8vmpo8 as signed)) as unsigned))) as unsigned)) as unsigned)))))
                    union
                    (
                    select distinct 
                        cast((select min(c_l1t) from t_dci)
                           as unsigned) as c0
                      from 
                        t_glzh3lb0ro as ref_9
                      where (EXISTS (
                        select  
                            ref_10.c_a90ol as c0, 
                            ref_10.c_z as c1, 
                            ref_10.c_foveoe as c2
                          from 
                            t_jg8o as ref_10
                          where (ref_9.c_kffac5e63 in (
                            select  
                                  ref_11.c_dph7 as c0
                                from 
                                  t_dci as ref_11
                                where 1=1
                              union
                              (
                              select  
                                  ref_12.c_m2y as c0
                                from 
                                  t_rc as ref_12
                                where (NOT NOT(cast((ref_12.c_b48gd04utl <=> ref_12.c_b48gd04utl) as unsigned)))
                              )))))
                    )
                     limit 1)) as unsigned) > cast(ref_0.c__qy as double)) as unsigned)))
          union
          (
          select  
              ref_5.c_r58lkh as c0
            from 
              t_dci as ref_13
            where 0<>0
          )
           limit 1)) then inet6_aton(
        cast(ref_0.c_foveoe as char)) else ref_4.c_cz end
       as c5,
  atan2(
      cast(((NOT NOT(cast((cast(7549699 as signed) <> cast(ref_5.c_onfeptr2q as decimal)) as unsigned)))) 
        and (((ref_0.c_mgjb not in (
            ref_2.c_tb3u, ref_4.c_foveoe, ref_0.c_m0qqv_cl4x, ref_0.c_a90ol))) 
          or ((NOT NOT(cast((subq_0.c0 >= ref_4.c_jbb) as unsigned))))) as unsigned), 
      cast((-1843462142 < ( 
        select  
            ref_0.c_tazb9 as c0
          from 
            t_rc as ref_14
          where ((ref_1.c_i3ml is NULL)) 
            and ((NOT NOT(cast((cast(ref_2.c_x2erxo10w as signed) && cast(ref_2.c_r58lkh as double)) as unsigned))))
          limit 1)) as unsigned)) as c7,
  last_value(
        cast(ref_4.c_m0qqv_cl4x as char)) over (partition by ref_1.c_pf8vmpo8 order by ref_1.c_kffac5e63) as c8,
  subq_1.c0 as c9,
  ref_1.c_c7njaqnyv7 as c10
FROM
  ((((t_jg8o as ref_0
            left outer join (t_glzh3lb0ro as ref_1
              cross join t__9r63 as ref_2
              )
            on (((NOT NOT(cast((cast(ref_0.c_jbb as char) > cast(ref_0.c_cz as char)) as unsigned)))) 
                and (0<>0)))
          right outer join (select  
                count(
                cast(cast(null as char) as char)) as c0, 
                count(
                cast((select c_b48gd04utl from t_rc order by c_b48gd04utl limit 1 offset 3)
                   as char)) as c1, 
                count(
                cast(ref_3.c_kzre as char)) as c2, 
                count(
                cast(ref_3.c_l1t as unsigned)) as c3, 
                count(*) as c4, 
                count(
                cast(ref_3.c_kzre as char)) as c5, 
                count(
                cast(ref_3.c_dph7 as signed)) as c6, 
                ref_3.c_fzqupuma as c7
              from 
                t_dci as ref_3
              where 0<>0
              group by ref_3.c_fzqupuma) as subq_0
          on ((ref_0.c_jbb like 'r%1an%6xy')))
        right outer join (t_jg8o as ref_4
          right outer join t__9r63 as ref_5
          on ((NOT NOT(cast((cast(ref_5.c_g7eofzlxn as signed) != cast(ref_5.c_g7eofzlxn as signed)) as unsigned)))))
        on ((NOT NOT(cast((cast(ref_0.c_mgjb as char) < cast(ref_2.c_tb3u as char)) as unsigned)))))
      inner join (select  
            ref_6.c_yu as c0
          from 
            t_rc as ref_6
          where (ref_6.c_m2y between ref_6.c_m2y and ref_6.c_m2y)
          group by ref_6.c_yu) as subq_1
      on (1=1))
ORDER BY
  c0, c4, c5, c7, c8, c9, c10 desc
),
cte_4 AS (
SELECT
  (select c_r58lkh from t__9r63 order by c_r58lkh limit 1 offset 2)
       as c0,
  subq_4.c0 as c1,
  subq_4.c2 as c2
FROM
  (select  
          ref_26.c_o8tsf as c0, 
          ref_28.c_b48gd04utl as c1, 
          ref_29.c_wd3x as c2
        from 
          (((t__9r63 as ref_26
                left outer join t_dci as ref_27
                on ((NOT NOT(cast((cast(ref_27.c_dph7 as signed) >= cast(ref_26.c_hd2v4v0 as double)) as unsigned)))))
              cross join t_rc as ref_28
              )
            left outer join t_glzh3lb0ro as ref_29
            on (ref_28.c_m2y = ref_29.c_kffac5e63 ))
        where (NOT NOT(cast((cast(ref_27.c_bywfl as signed) = cast(ref_29.c_pf8vmpo8 as signed)) as unsigned)))
        order by c0, c1, c2 desc
        limit 141) as subq_4
WHERE
  0<>0
ORDER BY
  c0, c1, c2 asc
LIMIT 128
)
SELECT
  ref_34.c10 as c5
FROM
  cte_0 as ref_34
WHERE
  (EXISTS (
    select  
        ref_35.c0 as c0, 
        ref_35.c2 as c1, 
        ref_34.c4 as c2, 
        ref_35.c0 as c3, 
        ref_34.c7 as c4, 
        ref_35.c2 as c5, 
        ref_34.c9 as c6, 
        ref_35.c2 as c7, 
        log(
          cast((select c_yu from t_rc order by c_yu limit 1 offset 5)
             as unsigned), 
          cast(-8271925302915805343 as signed)) as c8
      from 
        cte_4 as ref_35
      where (NOT NOT(cast((cast(ref_34.c8 as char) <= cast(case when (EXISTS (
              select  
                  ref_36.c1 as c0, 
                  ref_36.c1 as c1, 
                  ref_36.c0 as c2, 
                  ref_36.c0 as c3, 
                  ref_34.c0 as c4, 
                  ref_36.c2 as c5, 
                  ref_36.c1 as c6
                from 
                  cte_4 as ref_36
                where (NOT NOT(cast((cast(ref_35.c1 as double) XOR cast(ref_36.c2 as double)) as unsigned))))) then cast(ref_34.c8 as char) else ref_34.c5 end
             as char)) as unsigned)))
      limit 76));

2. What did you expect to see? (Required)

Expect no crashes

3. What did you see instead (Required)

runtime error: invalid memory address or nil pointer dereference
github.com/pingcap/tidb/pkg/server.(*clientConn).Run.func1
	/workspace/source/tidb/pkg/server/conn.go:1044
runtime.gopanic
	/usr/local/go/src/runtime/panic.go:914
github.com/pingcap/tidb/pkg/executor.(*Compiler).Compile.func1
	/workspace/source/tidb/pkg/executor/compiler.go:58
runtime.gopanic
	/usr/local/go/src/runtime/panic.go:914
runtime.panicmem
	/usr/local/go/src/runtime/panic.go:261
runtime.sigpanic
	/usr/local/go/src/runtime/signal_unix.go:861
github.com/pingcap/tidb/pkg/expression.BuildCastFunctionWithCheck
	/workspace/source/tidb/pkg/expression/builtin_cast.go:2312
github.com/pingcap/tidb/pkg/expression.BuildCastFunction
	/workspace/source/tidb/pkg/expression/builtin_cast.go:2305
github.com/pingcap/tidb/pkg/expression.newFunctionImpl
	/workspace/source/tidb/pkg/expression/scalar_function.go:206
github.com/pingcap/tidb/pkg/expression.NewFunction
	/workspace/source/tidb/pkg/expression/scalar_function.go:306
github.com/pingcap/tidb/pkg/expression.NewFunctionInternal
	/workspace/source/tidb/pkg/expression/scalar_function.go:326
github.com/pingcap/tidb/pkg/expression.evaluateExprWithNullInNullRejectCheck
	/workspace/source/tidb/pkg/expression/expression.go:990
github.com/pingcap/tidb/pkg/expression.evaluateExprWithNullInNullRejectCheck
	/workspace/source/tidb/pkg/expression/expression.go:953
github.com/pingcap/tidb/pkg/expression.evaluateExprWithNullInNullRejectCheck
	/workspace/source/tidb/pkg/expression/expression.go:953
github.com/pingcap/tidb/pkg/expression.evaluateExprWithNullInNullRejectCheck
	/workspace/source/tidb/pkg/expression/expression.go:953
github.com/pingcap/tidb/pkg/expression.EvaluateExprWithNull
	/workspace/source/tidb/pkg/expression/expression.go:916
github.com/pingcap/tidb/pkg/planner/util.isNullRejectedSimpleExpr
	/workspace/source/tidb/pkg/planner/util/null_misc.go:109
github.com/pingcap/tidb/pkg/planner/util.IsNullRejected
	/workspace/source/tidb/pkg/planner/util/null_misc.go:91
github.com/pingcap/tidb/pkg/planner/core/operator/logicalop.(*LogicalJoin).ConvertOuterToInnerJoin
	/workspace/source/tidb/pkg/planner/core/operator/logicalop/logical_join.go:666
github.com/pingcap/tidb/pkg/planner/core/operator/logicalop.(*LogicalJoin).ConvertOuterToInnerJoin
	/workspace/source/tidb/pkg/planner/core/operator/logicalop/logical_join.go:682
github.com/pingcap/tidb/pkg/planner/core/operator/logicalop.(*LogicalJoin).ConvertOuterToInnerJoin
	/workspace/source/tidb/pkg/planner/core/operator/logicalop/logical_join.go:684
github.com/pingcap/tidb/pkg/planner/core/operator/logicalop.(*LogicalSelection).ConvertOuterToInnerJoin
	/workspace/source/tidb/pkg/planner/core/operator/logicalop/logical_selection.go:297
github.com/pingcap/tidb/pkg/planner/core/operator/logicalop.(*LogicalJoin).ConvertOuterToInnerJoin
	/workspace/source/tidb/pkg/planner/core/operator/logicalop/logical_join.go:682
github.com/pingcap/tidb/pkg/planner/core/operator/logicalop.(*LogicalProjection).ConvertOuterToInnerJoin
	/workspace/source/tidb/pkg/planner/core/operator/logicalop/logical_projection.go:532
github.com/pingcap/tidb/pkg/planner/core/operator/logicalop.(*BaseLogicalPlan).ConvertOuterToInnerJoin
	/workspace/source/tidb/pkg/planner/core/operator/logicalop/base_logical_plan.go:338
github.com/pingcap/tidb/pkg/planner/core/operator/logicalop.(*LogicalJoin).ConvertOuterToInnerJoin
	/workspace/source/tidb/pkg/planner/core/operator/logicalop/logical_join.go:684
github.com/pingcap/tidb/pkg/planner/core/operator/logicalop.(*LogicalProjection).ConvertOuterToInnerJoin
	/workspace/source/tidb/pkg/planner/core/operator/logicalop/logical_projection.go:532
github.com/pingcap/tidb/pkg/planner/core.(*ConvertOuterToInnerJoin).Optimize
	/workspace/source/tidb/pkg/planner/core/rule_outer_to_inner_join.go:42
github.com/pingcap/tidb/pkg/planner/core.logicalOptimize
	/workspace/source/tidb/pkg/planner/core/optimizer.go:977
github.com/pingcap/tidb/pkg/planner/core.doOptimize
	/workspace/source/tidb/pkg/planner/core/optimizer.go:258
github.com/pingcap/tidb/pkg/planner/core.DoOptimize
	/workspace/source/tidb/pkg/planner/core/optimizer.go:317
github.com/pingcap/tidb/pkg/planner.optimize
	/workspace/source/tidb/pkg/planner/optimize.go:526
github.com/pingcap/tidb/pkg/planner.Optimize
	/workspace/source/tidb/pkg/planner/optimize.go:357
github.com/pingcap/tidb/pkg/executor.(*Compiler).Compile
	/workspace/source/tidb/pkg/executor/compiler.go:101
github.com/pingcap/tidb/pkg/session.(*session).ExecuteStmt
	/workspace/source/tidb/pkg/session/session.go:2093
github.com/pingcap/tidb/pkg/server.(*TiDBContext).ExecuteStmt
	/workspace/source/tidb/pkg/server/driver_tidb.go:291
github.com/pingcap/tidb/pkg/server.(*clientConn).handleStmt
	/workspace/source/tidb/pkg/server/conn.go:2058
github.com/pingcap/tidb/pkg/server.(*clientConn).handleQuery
	/workspace/source/tidb/pkg/server/conn.go:1811
github.com/pingcap/tidb/pkg/server.(*clientConn).dispatch
	/workspace/source/tidb/pkg/server/conn.go:1385
github.com/pingcap/tidb/pkg/server.(*clientConn).Run
	/workspace/source/tidb/pkg/server/conn.go:1147
github.com/pingcap/tidb/pkg/server.(*Server).onConn
	/workspace/source/tidb/pkg/server/server.go:741

4. What is your TiDB version? (Required)

Release Version: v8.4.0-alpha-66-g1167e0c
Edition: Community
Git Commit Hash: 1167e0c06fc8e2dd066bd974315284bc0e884acc
Git Branch: HEAD
UTC Build Time: 2024-09-03 01:15:19
GoVersion: go1.21.13
Race Enabled: false
Check Table Before Drop: false
Store: tikv

We are the BASS team from the School of Cyber Science and Technology at Beihang University. Our main focus is on system software security, operating systems, and program analysis research, as well as the development of automated program testing frameworks for detecting software defects. Using our self-developed database vulnerability testing tool, we have identified the above-mentioned vulnerabilities in TiDB that may lead to database crashes.

@ycybfhb ycybfhb added the type/bug The issue is confirmed as a bug. label Sep 5, 2024
@ycybfhb ycybfhb changed the title invalid memory address or nil pointer dereference in expression.BuildCastFunction->expression.BuildCastFunctionWithCheck invalid memory address or nil pointer dereference in expression.newFunctionImpl->expression.BuildCastFunctionWithCheck Sep 5, 2024
@windtalker
Copy link
Contributor

Hi @ycybfhb I can not reproduce the issue using the provided schema and sql, do you use some special settings in your cluster?

@ycybfhb
Copy link
Author

ycybfhb commented Sep 12, 2024

Hi @ycybfhb I can not reproduce the issue using the provided schema and sql, do you use some special settings in your cluster?

Hello, please run this before execution. (This is the default behaviour of MySQL ODBC ANSI driver)

set collation_connection='latin1_bin';

@windtalker
Copy link
Contributor

Hi @ycybfhb I can not reproduce the issue using the provided schema and sql, do you use some special settings in your cluster?

Hello, please run this before execution. (This is the default behaviour of MySQL ODBC ANSI driver)

set collation_connection='latin1_bin';

Thanks, I can reproduce it now.

@windtalker
Copy link
Contributor

The root cause of this issue is there is "Illegal mix of collations (%s,%s) and (%s,%s) for operation '%s'" error when build Function.
But in evaluateExprWithNullInNullRejectCheck:

c := NewFunctionInternal(ctx, x.FuncName.L, x.RetType.Clone(), args...)

it uses NewFunctionInternal to construct the new function, and inside NewFunctionInternal, it will just ignore the error and return nil instead.
I would like label this issue as a planner bug because the callstack is from Optimize, and there is no error handle logical in the whole ConvertOuterToInnerJoin process.
Screenshot 2024-09-14 at 09 08 12

@windtalker windtalker added sig/planner SIG: Planner and removed sig/execution SIG execution labels Sep 14, 2024
@ti-chi-bot ti-chi-bot bot added the affects-8.5 This bug affects the 8.5.x(LTS) versions. label Nov 1, 2024
@ghazalfamilyusa
Copy link
Contributor

ghazalfamilyusa commented Nov 12, 2024

@windtalker : what is the expected behaviour of the expression logic when there is a mix of collation? Is there a graceful way of returning to the caller? I do not see a way of fixing this from the consumer side (optimizer). One way is to make expression.EvaluateExprWithNull return result and a boolean for the success/failure of the evaluation.

@windtalker
Copy link
Contributor

@windtalker : what is the expected behaviour of the expression logic when there is a mix of collation? Is there a graceful way of returning to the caller? I do not see a way of fixing this from the consumer side (optimizer). One way is to make expression.EvaluateExprWithNull return result and a boolean for the success/failure of the evaluation.

I think if there is a mix of collation, TiDB will try to call deriveCollation to get a collation that can be used, but sometimes deriveCollation return error, in this case, TiDB should return this error to the client/user.
For how to fix this issue, I think as the comment of NewFunctionInternal said:

// NewFunctionInternal is similar to NewFunction, but do not return error, should only be used internally.
// Deprecated: use NewFunction instead, old logic here is for the convenience of go linter error check.
// while for the new function creation, some errors can also be thrown out, for example, args verification
// error, collation derivation error, special function with meta doesn't be initialized error and so on.
// only threw the these internal error out, then we can debug and dig it out quickly rather than in a confusion
// of index out of range / nil pointer error / function execution error.
func NewFunctionInternal(ctx BuildContext, funcName string, retType *types.FieldType, args ...Expression) Expression {

evaluateExprWithNullInNullRejectCheck should use NewFunction and handle the error explicitly.

@ghazalfamilyusa
Copy link
Contributor

ghazalfamilyusa commented Nov 13, 2024

@windtalker OK and looked at this but the error is actally a panice and not an error. In any case, the consumer is an optimization which is OK if the check is true (nullreject) or false (not null reject or unknown which covers error). Seems the fix need to be applied in NewFunctionInternal (may be use NewFunction or similiar logic) to return result and bool for success/failure. Do you agree and if so who can fix it? The other way to fix it is to make the consumer not make the call for such cases which is hard and may cause false positives.

@ghazalfamilyusa
Copy link
Contributor

We can bring back the old logic of returning an bool based on context (optimization path) and keep the rest the same.

@ghazalfamilyusa ghazalfamilyusa removed their assignment Nov 14, 2024
@ghazalfamilyusa
Copy link
Contributor

Thanks @windtalker for taking care of the fix as we discussed. The expression logic returns an error to the caller if it is an optimization "ctx.IsInNullRejectCheck()"

@fixdb fixdb added sig/execution SIG execution and removed sig/planner SIG: Planner labels Nov 14, 2024
@windtalker windtalker removed may-affects-5.4 This bug maybe affects 5.4.x versions. may-affects-6.1 labels Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects-8.5 This bug affects the 8.5.x(LTS) versions. impact/panic severity/major sig/execution SIG execution type/bug The issue is confirmed as a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants