Ticket UUID: | 3bc0f44ef3a8ef3d83c64e55ac155d58719f487a | |||
Title: | UBSan complains about body.chars[] usage | |||
Type: | Bug | Version: | core-8-6-branch | |
Submitter: | chrstphrchvz | Created on: | 2020-09-08 23:54:47 | |
Subsystem: | 18. [text] | Assigned To: | jan.nijtmans | |
Priority: | 5 Medium | Severity: | Cosmetic | |
Status: | Closed | Last Modified: | 2024-06-13 20:24:17 | |
Resolution: | Fixed | Closed By: | fvogel | |
Closed on: | 2024-06-13 20:24:17 | |||
Description: |
Probably the biggest complaint UBSan (-fsanitize=array-bounds) has so far about Tk is usage of the body.chars[] member of a TkTextSegment. Some examples which appear immediately when running the widget demo: tk/unix/../generic/tkTextBTree.c:1074:2: runtime error: index 25 out of bounds for type 'char [2]' SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior tk/unix/../generic/tkTextBTree.c:1074:2 in tk/unix/../generic/tkTextBTree.c:4613:5: runtime error: index 63 out of bounds for type 'char [2]' SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior tk/unix/../generic/tkTextBTree.c:4613:5 in tk/unix/../generic/tkTextBTree.c:4613:5: runtime error: index 63 out of bounds for type 'char [2]' SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior tk/unix/../generic/tkTextBTree.c:4613:5 in tk/unix/../generic/tkTextBTree.c:4614:5: runtime error: index 64 out of bounds for type 'char [2]' SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior tk/unix/../generic/tkTextBTree.c:4614:5 in tk/unix/../generic/tkTextDisp.c:1502:31: runtime error: index 105 out of bounds for type 'char [2]' SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior tk/unix/../generic/tkTextDisp.c:1502:31 in tk/unix/../generic/tkTextDisp.c:7650:28: runtime error: index 105 out of bounds for type 'char [2]' SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior tk/unix/../generic/tkTextDisp.c:7650:28 in I don't think I have any good suggestions about how to address this complaint. Feel free to close as wontfix, and whoever wants to use UBSan gets to ignore this. It can be silenced by defining any involved functions as __attribute__((no_sanitize("array-bounds"))), e.g. static Tcl_Obj * __attribute__((no_sanitize("array-bounds"))) TextGetText(…) { … } | |||
User Comments: |
fvogel added on 2024-06-13 20:24:17:
Fix provided in ticket [dacd18294b], follow-up there. fvogel added on 2024-06-13 06:03:19: Duplicate of [dacd18294b] that I have just re-found. Declaring 'char chars[TCL_UTF_MAX]' in the TkTextSegment struct makes no sense IMO: This array has nothing to do with TCL_UTF_MAX at all. I see there were several successive changes here: first [bed8018384], then [8847c81c47] and finally [6067cc55df] which is the current state. Jan, would you be so kind to have another look please? This clutters the output of UBSan. Thanks! chrstphrchvz added on 2023-04-02 16:45:11: The only suggestion I can think of is to either revert [8847c81c4797], or introduce yet another macro like pre-[8847c81c4797] TKFLEXARRAY (i.e. with only the GNU syntax alternative and not the C99 syntax alternative) which is usable in a union. chrstphrchvz added on 2023-04-02 16:34:12: Reopening: these errors have returned due to changes in [8847c81c4797] and [6067cc55dfa5] (so the error now says …for type 'char [3]'). Given the comment in [8847c81c4797], maybe this is won’t-fix. jan.nijtmans added on 2020-09-11 11:01:29: Merged to core-8-6-branch and trunk now. Warnings on VC++ 6.0 below, as reported by Harald, eliminated too (Thanks!) oehhar added on 2020-09-11 07:36:15: Sorry, saw the question for me only today. It builds well. There are some warnings. Here is a build log: fossil open ..\fossil\tk.fossil bug-3bc0f44ef3 cd win nmake -f makefile.vc TCLDIR=c:\test\tcl8.7a3 ...snipped all usual stuff, only warnings left C:\test\tk8.7-bug-3bc0f44ef3_2020-09-11\win\..\generic\tkPointer.c(302) : warning C4018: '==' : signed/unsigned mismatch C:\test\tk8.7-bug-3bc0f44ef3_2020-09-11\win\..\win\tkWinFont.c(1500) : warning C4761: integral size mismatch in argument; conversion supplied C:\test\tk8.7-bug-3bc0f44ef3_2020-09-11\win\..\win\tkWinFont.c(1500) : warning C4761: integral size mismatch in argument; conversion supplied C:\test\tk8.7-bug-3bc0f44ef3_2020-09-11\win\..\win\tkWinFont.c(1522) : warning C4761: integral size mismatch in argument; conversion supplied C:\test\tk8.7-bug-3bc0f44ef3_2020-09-11\win\..\win\tkWinFont.c(1522) : warning C4761: integral size mismatch in argument; conversion supplied Test are without any failure. Thank you, Harald fvogel added on 2020-09-09 17:06:18: I can't test with VC++6.0. AFAIK only Harald Oehlmann still builds with this compiler. Harald, could you confirm branch bug-3bc0f44ef3 builds with VC++6.0 ? Thanks in advance! chrstphrchvz added on 2020-09-09 16:38:01: Yes, UBSan does not warn for chars[0]. jan.nijtmans added on 2020-09-09 15:40:57: @chrstphrchvz: Does this commit help for UBSan??? (If not, then it doesn't make sense to do this at all ...). If it helps for UBSan but doesn't build for VC++ 6.0, we can use a workaround, e.g. a #define TCL_FLEXARRAY which has value 1 for VC++ 6.0 (or other older microsoft compilers) and 0 for all other compilers. fvogel added on 2020-09-09 07:12:36: For me it builds with MSVC 2008 Express Edition. jan.nijtmans added on 2020-09-09 06:20:58: How about [da241cf9ce1cf74e] ? Don't know if this works for older compilers .... |