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.

Sign up for GitHub

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

Jump to bottom

[improve](ub) fix some runtime error of ubsan when downcast #35343

Merged
merged 2 commits into from
May 27, 2024
Merged

[improve](ub) fix some runtime error of ubsan when downcast #35343

merged 2 commits into from
May 27, 2024

Conversation

zhangstar333
Copy link
Contributor

@zhangstar333 zhangstar333 commented May 24, 2024 •
edited
Loading

Proposed changes

those code could work well, but it will be report some runtime error under UBSAN,
so refactor it to let's ubsan could running happy.

the mainly change is those two function, use const IColumn* as column type rather than const ColumnString*

virtual void deserialize_and_merge_vec(const AggregateDataPtr* places, size_t offset,
                                       AggregateDataPtr rhs, const IColumn* column,
                                       Arena* arena, const size_t num_rows) const = 0;

virtual void deserialize_and_merge_vec_selected(const AggregateDataPtr* places, size_t offset,
                                                AggregateDataPtr rhs, const IColumn* column,
                                                Arena* arena, const size_t num_rows) const = 0;

/root/doris/be/src/pipeline/exec/aggregation_sink_operator.cpp:307:37: runtime error: downcast of address 0x55900dcc7980 which does not point to an object of type 'vectorized::ColumnString' (aka 'ColumnStr<unsigned int>')
/root/doris/be/src/pipeline/exec/aggregation_sink_operator.cpp:352:37: runtime error: downcast of address 0x5590525fcd80 which does not point to an object of type 'vectorized::ColumnString' (aka 'ColumnStr<unsigned int>')
/root/doris/be/src/pipeline/exec/aggregation_sink_operator.cpp:352:37: runtime error: downcast of address 0x55eb507287c0 which does not point to an object of type 'vectorized::ColumnString' (aka 'ColumnStr<unsigned int>')
/root/doris/be/src/vec/exec/vaggregation_node.h:676:61: runtime error: downcast of address 0x55906dd8f740 which does not point to an object of type 'ColumnString' (aka 'ColumnStr<unsigned int>')

Issue Number: close #xxx

Further comments

If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...

@doris-robot
Copy link

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR

Since 2024-03-18, the Document has been moved to doris-website.
See Doris Document.

@zhangstar333
Copy link
Contributor Author

run buildall

@github-actions GitHub Actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@github-actions GitHub Actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@zhangstar333
Copy link
Contributor Author

run buildall

@github-actions GitHub Actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@doris-robot
Copy link

TPC-H: Total hot run time: 41265 ms 
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 90bcb472b0e34c4b817c5b5a787f74d215d2c4c8, data reload: false

------ Round 1 ----------------------------------
q1	17617	4376	4243	4243
q2	2018	189	202	189
q3	10473	1230	1216	1216
q4	10211	786	850	786
q5	7478	2726	2698	2698
q6	223	141	136	136
q7	965	608	625	608
q8	9223	2194	2144	2144
q9	9440	6684	6760	6684
q10	9596	3915	3875	3875
q11	459	238	265	238
q12	510	242	227	227
q13	17576	3244	3095	3095
q14	267	225	231	225
q15	512	473	494	473
q16	535	411	395	395
q17	1000	701	749	701
q18	8566	7927	7917	7917
q19	5565	1573	1509	1509
q20	672	328	328	328
q21	5216	3951	3301	3301
q22	334	277	278	277
Total cold run time: 118456 ms
Total hot run time: 41265 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4626	4460	4412	4412
q2	374	266	262	262
q3	3195	2939	2889	2889
q4	1939	1637	1629	1629
q5	5515	5552	5517	5517
q6	211	124	129	124
q7	2196	1806	1825	1806
q8	3271	3443	3423	3423
q9	8739	8736	8769	8736
q10	3931	3794	3879	3794
q11	582	500	507	500
q12	789	624	626	624
q13	17000	3143	3253	3143
q14	298	274	270	270
q15	533	482	478	478
q16	455	419	418	418
q17	1815	1530	1479	1479
q18	7983	7675	7412	7412
q19	1667	1527	1588	1527
q20	1993	1763	1818	1763
q21	9817	4730	4741	4730
q22	577	488	497	488
Total cold run time: 77506 ms
Total hot run time: 55424 ms

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 35.65% (9019/25297)
Line Coverage: 27.31% (74589/273077)
Region Coverage: 26.54% (38605/145448)
Branch Coverage: 23.41% (19696/84148)
Coverage Report: http://coverage.selectdb-in.cc/coverage/90bcb472b0e34c4b817c5b5a787f74d215d2c4c8_90bcb472b0e34c4b817c5b5a787f74d215d2c4c8/report/index.html

@doris-robot
Copy link

TPC-DS: Total hot run time: 168477 ms 
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 90bcb472b0e34c4b817c5b5a787f74d215d2c4c8, data reload: false

query1	903	379	377	377
query2	6452	2294	2413	2294
query3	6640	203	203	203
query4	19557	17406	17305	17305
query5	4144	417	417	417
query6	240	157	152	152
query7	4572	300	296	296
query8	235	198	176	176
query9	8481	2415	2397	2397
query10	452	282	271	271
query11	10548	10054	10030	10030
query12	143	90	91	90
query13	1640	387	364	364
query14	10175	6673	6650	6650
query15	214	169	169	169
query16	7189	267	260	260
query17	1352	523	511	511
query18	1899	278	266	266
query19	193	155	156	155
query20	93	82	86	82
query21	194	141	133	133
query22	4146	3887	3945	3887
query23	33430	33617	32880	32880
query24	7184	2822	2769	2769
query25	523	357	360	357
query26	698	156	154	154
query27	2226	315	321	315
query28	5058	2080	2077	2077
query29	844	606	597	597
query30	260	170	174	170
query31	957	760	750	750
query32	89	50	55	50
query33	524	272	260	260
query34	857	477	479	477
query35	723	601	597	597
query36	1064	903	889	889
query37	111	70	72	70
query38	2917	2778	2720	2720
query39	883	780	792	780
query40	193	128	126	126
query41	82	44	43	43
query42	104	95	100	95
query43	571	544	533	533
query44	1098	739	749	739
query45	185	168	160	160
query46	1060	721	747	721
query47	1859	1762	1777	1762
query48	371	293	299	293
query49	840	383	385	383
query50	770	382	401	382
query51	6712	6648	6725	6648
query52	100	90	89	89
query53	356	295	289	289
query54	551	449	430	430
query55	72	72	78	72
query56	258	252	252	252
query57	1097	1005	1014	1005
query58	255	213	217	213
query59	3151	3260	3023	3023
query60	279	251	254	251
query61	88	94	87	87
query62	592	455	441	441
query63	316	288	283	283
query64	8520	2242	1708	1708
query65	3166	3068	3101	3068
query66	906	328	335	328
query67	15357	14896	14748	14748
query68	4624	539	530	530
query69	484	267	264	264
query70	1206	1047	1111	1047
query71	398	273	273	273
query72	7761	5198	2691	2691
query73	711	321	322	321
query74	6051	5583	5642	5583
query75	3380	2618	2686	2618
query76	2957	1026	986	986
query77	594	266	271	266
query78	10173	9707	9713	9707
query79	2396	526	523	523
query80	1250	449	459	449
query81	530	242	242	242
query82	1291	95	93	93
query83	239	173	172	172
query84	246	87	83	83
query85	1424	263	264	263
query86	465	320	284	284
query87	3223	3142	3114	3114
query88	3890	2342	2355	2342
query89	471	390	379	379
query90	2007	187	190	187
query91	129	97	99	97
query92	62	54	52	52
query93	2525	519	505	505
query94	1280	210	196	196
query95	419	328	320	320
query96	593	269	266	266
query97	3181	2988	3023	2988
query98	253	222	221	221
query99	1153	846	866	846
Total cold run time: 262085 ms
Total hot run time: 168477 ms

@doris-robot
Copy link

ClickBench: Total hot run time: 30.86 s 
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 90bcb472b0e34c4b817c5b5a787f74d215d2c4c8, data reload: false

query1	0.04	0.04	0.04
query2	0.08	0.04	0.04
query3	0.23	0.04	0.05
query4	1.68	0.07	0.07
query5	0.50	0.49	0.51
query6	1.13	0.72	0.72
query7	0.02	0.01	0.01
query8	0.05	0.04	0.04
query9	0.54	0.49	0.49
query10	0.54	0.55	0.54
query11	0.16	0.10	0.11
query12	0.15	0.13	0.12
query13	0.60	0.58	0.58
query14	0.75	0.80	0.79
query15	0.83	0.81	0.81
query16	0.36	0.36	0.36
query17	0.92	0.94	0.95
query18	0.24	0.24	0.24
query19	1.75	1.68	1.66
query20	0.02	0.01	0.01
query21	15.41	0.70	0.68
query22	4.52	6.11	2.32
query23	18.32	1.36	1.24
query24	1.57	0.34	0.24
query25	0.14	0.09	0.08
query26	0.26	0.17	0.17
query27	0.08	0.08	0.08
query28	13.29	1.03	0.99
query29	12.62	3.38	3.25
query30	0.24	0.06	0.06
query31	2.88	0.37	0.38
query32	3.29	0.47	0.47
query33	2.88	2.88	2.89
query34	17.14	4.42	4.40
query35	4.52	4.51	4.56
query36	0.69	0.50	0.46
query37	0.17	0.16	0.15
query38	0.15	0.14	0.15
query39	0.04	0.03	0.04
query40	0.17	0.15	0.14
query41	0.09	0.04	0.04
query42	0.05	0.04	0.04
query43	0.03	0.03	0.04
Total cold run time: 109.14 s
Total hot run time: 30.86 s

@zhangstar333
Copy link
Contributor Author

run buildall

@doris-robot
Copy link

TPC-H: Total hot run time: 41959 ms 
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 90bcb472b0e34c4b817c5b5a787f74d215d2c4c8, data reload: false

------ Round 1 ----------------------------------
q1	17609	4385	4256	4256
q2	2016	183	190	183
q3	10529	1262	1164	1164
q4	10191	829	862	829
q5	7475	2759	2773	2759
q6	222	134	135	134
q7	981	605	610	605
q8	9229	2163	2119	2119
q9	10118	6680	6663	6663
q10	9798	3865	3903	3865
q11	444	246	244	244
q12	455	222	243	222
q13	17551	3267	3121	3121
q14	256	224	223	223
q15	500	472	489	472
q16	524	397	401	397
q17	979	721	699	699
q18	8435	7886	7832	7832
q19	2371	1578	1556	1556
q20	649	314	324	314
q21	5281	4075	4016	4016
q22	366	286	301	286
Total cold run time: 115979 ms
Total hot run time: 41959 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4544	4413	4377	4377
q2	386	275	278	275
q3	3126	2952	2750	2750
q4	1862	1628	1669	1628
q5	5457	5525	5493	5493
q6	219	124	123	123
q7	2155	1830	1835	1830
q8	3224	3392	3345	3345
q9	8692	8643	8747	8643
q10	3903	3775	3820	3775
q11	593	500	487	487
q12	796	642	629	629
q13	17185	3141	3154	3141
q14	316	276	258	258
q15	538	476	490	476
q16	498	421	408	408
q17	1778	1466	1461	1461
q18	7751	7646	7516	7516
q19	2180	1584	1556	1556
q20	2002	1779	1801	1779
q21	4878	4634	4756	4634
q22	570	493	492	492
Total cold run time: 72653 ms
Total hot run time: 55076 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 168844 ms 
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 90bcb472b0e34c4b817c5b5a787f74d215d2c4c8, data reload: false

query1	903	376	381	376
query2	6456	2366	2267	2267
query3	6650	205	206	205
query4	19123	17261	17264	17261
query5	4103	414	415	414
query6	247	158	151	151
query7	4593	297	301	297
query8	243	185	178	178
query9	8643	2409	2371	2371
query10	463	304	270	270
query11	10505	9986	10040	9986
query12	140	94	90	90
query13	1637	378	366	366
query14	10241	6859	6646	6646
query15	210	178	172	172
query16	7738	261	257	257
query17	1663	527	535	527
query18	1937	266	275	266
query19	192	155	150	150
query20	96	84	82	82
query21	201	128	126	126
query22	4146	4020	4095	4020
query23	33464	32910	32993	32910
query24	7314	2758	2820	2758
query25	563	348	358	348
query26	716	155	160	155
query27	2192	318	312	312
query28	5267	2057	2068	2057
query29	879	615	599	599
query30	259	169	177	169
query31	948	769	746	746
query32	94	52	55	52
query33	508	262	259	259
query34	875	478	475	475
query35	680	606	607	606
query36	1048	910	924	910
query37	109	69	72	69
query38	2881	2781	2735	2735
query39	816	804	786	786
query40	209	121	124	121
query41	45	44	43	43
query42	105	101	99	99
query43	599	544	531	531
query44	1076	751	749	749
query45	180	167	166	166
query46	1062	721	740	721
query47	1866	1739	1761	1739
query48	367	302	299	299
query49	857	382	385	382
query50	775	379	387	379
query51	6718	6622	6555	6555
query52	106	99	92	92
query53	349	285	288	285
query54	546	440	441	440
query55	74	72	72	72
query56	280	236	246	236
query57	1130	1019	1066	1019
query58	232	206	212	206
query59	3461	3129	3235	3129
query60	285	250	262	250
query61	91	88	88	88
query62	597	462	440	440
query63	312	283	289	283
query64	8494	2343	1717	1717
query65	3159	3124	3103	3103
query66	786	323	321	321
query67	15117	15115	15078	15078
query68	4532	534	549	534
query69	445	270	275	270
query70	1110	1142	1103	1103
query71	375	266	269	266
query72	7502	2713	2558	2558
query73	711	320	320	320
query74	6027	5576	5619	5576
query75	3306	2627	2639	2627
query76	2346	1006	1039	1006
query77	414	270	272	270
query78	10267	9854	9744	9744
query79	2254	520	523	520
query80	1015	451	425	425
query81	550	244	245	244
query82	957	97	92	92
query83	247	170	174	170
query84	256	87	84	84
query85	1012	273	264	264
query86	476	328	300	300
query87	3320	3137	3167	3137
query88	3559	2366	2370	2366
query89	461	392	376	376
query90	2037	189	185	185
query91	125	102	97	97
query92	68	46	48	46
query93	1907	517	508	508
query94	1137	208	188	188
query95	397	319	307	307
query96	587	263	269	263
query97	3146	2972	2993	2972
query98	235	216	216	216
query99	1133	881	843	843
Total cold run time: 259535 ms
Total hot run time: 168844 ms

@doris-robot
Copy link

ClickBench: Total hot run time: 30.09 s 
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 90bcb472b0e34c4b817c5b5a787f74d215d2c4c8, data reload: false

query1	0.04	0.03	0.03
query2	0.08	0.03	0.04
query3	0.22	0.05	0.05
query4	1.68	0.07	0.06
query5	0.50	0.50	0.51
query6	1.13	0.74	0.71
query7	0.02	0.01	0.02
query8	0.05	0.05	0.04
query9	0.53	0.47	0.50
query10	0.54	0.57	0.53
query11	0.15	0.12	0.11
query12	0.14	0.12	0.11
query13	0.59	0.59	0.59
query14	0.80	0.76	0.77
query15	0.82	0.80	0.80
query16	0.35	0.36	0.37
query17	0.94	1.02	0.99
query18	0.19	0.28	0.23
query19	1.84	1.71	1.68
query20	0.01	0.01	0.01
query21	15.45	0.69	0.68
query22	5.02	6.54	1.52
query23	18.37	1.34	1.28
query24	2.02	0.23	0.21
query25	0.15	0.09	0.08
query26	0.27	0.17	0.18
query27	0.09	0.07	0.08
query28	13.19	1.03	0.99
query29	12.90	3.36	3.28
query30	0.24	0.06	0.05
query31	2.87	0.38	0.40
query32	3.28	0.46	0.47
query33	2.89	2.93	2.91
query34	17.03	4.47	4.43
query35	4.53	4.47	4.56
query36	0.68	0.48	0.46
query37	0.17	0.16	0.15
query38	0.14	0.14	0.15
query39	0.05	0.03	0.03
query40	0.16	0.14	0.14
query41	0.09	0.04	0.04
query42	0.06	0.04	0.04
query43	0.04	0.04	0.04
Total cold run time: 110.31 s
Total hot run time: 30.09 s

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 35.65% (9019/25297)
Line Coverage: 27.31% (74592/273086)
Region Coverage: 26.55% (38618/145460)
Branch Coverage: 23.41% (19694/84144)
Coverage Report: http://coverage.selectdb-in.cc/coverage/90bcb472b0e34c4b817c5b5a787f74d215d2c4c8_90bcb472b0e34c4b817c5b5a787f74d215d2c4c8/report/index.html

@github-actions GitHub Actions
Copy link
Contributor

PR approved by anyone and no changes requested.

HappenLee
HappenLee approved these changes May 27, 2024
View reviewed changes
Copy link
Contributor

@HappenLee HappenLee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@github-actions GitHub Actions
Copy link
Contributor

PR approved by at least one committer and no changes requested.

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label May 27, 2024
@zhangstar333 zhangstar333 merged commit 6e7ec66 into apache:master May 27, 2024
27 of 29 checks passed
yiguolei pushed a commit that referenced this pull request May 27, 2024
@zhangstar333 @Doris-Extras
[improve](ub) fix some runtime error of ubsan when downcast (#35343)
596fb6f 
those code could work well, but it will be report some runtime error under UBSAN,
so refactor it to let's ubsan could running happy.
seawinde pushed a commit to seawinde/doris that referenced this pull request May 27, 2024
@zhangstar333 @seawinde
[improve](ub) fix some runtime error of ubsan when downcast (apache#3…
cb1f751 
…5343)

those code could work well, but it will be report some runtime error under UBSAN,
so refactor it to let's ubsan could running happy.
dataroaring pushed a commit that referenced this pull request May 28, 2024
@zhangstar333 @dataroaring
[improve](ub) fix some runtime error of ubsan when downcast (#35343)
fe46cad 
those code could work well, but it will be report some runtime error under UBSAN,
so refactor it to let's ubsan could running happy.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mryange Mryange Mryange approved these changes

@HappenLee HappenLee HappenLee approved these changes

Assignees
No one assigned
Labels
approved Indicates a PR has been approved by one committer. dev/3.0.0-merged reviewed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@zhangstar333 @doris-robot @HappenLee @Mryange @dataroaring