Tcl Source Code

View Ticket
Login
Bounty program for improvements to Tcl and certain Tcl packages.
2021-12-05
08:38 Ticket [206e58994f] tcl builds with clang but fails to run correctly status still Open with 3 other changes artifact: 309347677f user: chrstphrchvz
2021-05-04
07:31 Ticket [24b9181478] Fix unsafe buffer lifetime status still Closed with 5 other changes artifact: f66017aece user: dkf
2021-05-03
19:55 Closed ticket [24b9181478]. artifact: 01b85557b9 user: jan.nijtmans
19:42
Fix [24b9181478]: Fix unsafe buffer lifetime check-in: f6468f7763 user: jan.nijtmans tags: core-8-6-branch
19:36
Fix [24b9181478]: Fix unsafe buffer lifetime check-in: 98af80f133 user: jan.nijtmans tags: core-8-5-branch
18:40 New ticket [24b9181478] Fix unsafe buffer lifetime. artifact: ecdbee36b1 user: rupprecht

Ticket UUID: 24b918147808e33e9bdb31a516fd29daaddd7149
Title: Fix unsafe buffer lifetime
Type: Patch Version: 8.5
Submitter: rupprecht Created on: 2021-05-03 18:40:04
Subsystem: 25. Channel System Assigned To: jan.nijtmans
Priority: 5 Medium Severity: Minor
Status: Closed Last Modified: 2021-05-04 07:31:13
Resolution: Fixed Closed By: dkf
    Closed on: 2021-05-04 07:31:13
Description:
tl;dr this patch fixes a lifetime bug detected in trunk versions of Clang:

$ git diff generic/tclIO.c
diff --git a/generic/tclIO.c b/generic/tclIO.c
index dc028b522..a23fff80f 100644
--- a/generic/tclIO.c
+++ b/generic/tclIO.c
@@ -4296,9 +4296,10 @@ Write(
        nextNewLine = (char *)memchr(src, '\n', srcLen);
     }
 
+    char safe[BUFFER_PADDING];
     while (srcLen + saved + endEncoding > 0) {
        ChannelBuffer *bufPtr;
-       char *dst, safe[BUFFER_PADDING];
+       char *dst;
        int result, srcRead, dstLen, dstWrote, srcLimit = srcLen;
 
        if (nextNewLine) {

Longer summary: a recent update to clang improves optimizations related to memcpy:
https://github.com/llvm/llvm-project/commit/f5446b769a7929806f72256fccd4826d66502e59.

When we upgraded to a version of clang that includes this optimization, we
encountered a build error from a step that was consuming the output of a tcl/tk
script, and found that the tcl script was producing junk output. Upon further
inspection, this is because the tcl interpreter built by clang was entirely
optimizing away the "safe" buffer in the patch above. This appears to be valid,
as the lifetime of the "safe" buffer ends at the closing of the for loop, and
is not guaranteed to be preserved on the next loop iteration. Moving the buffer
outside of the for loop will preserve the "safe" buffer for the necessary
lifetime.
User Comments: dkf added on 2021-05-04 07:31:13:

Good catch; that code was wrong.


jan.nijtmans added on 2021-05-03 19:55:07:

Well, this but can be traced back to Tcl 8.5 (and probably much further back in history). Thanks for this detailed explanation and for the fix.

Fixed now in all active branches (8.5 and up)