Tcl Source Code

A bad format specifier in Tcl_ObjPrintf() makes the application crash. Specifically, "%s" format specifier with a double argument. Man page says "If the value of format contains internal inconsistencies or invalid specifier formats, the formatted string result produced by Tcl_ObjPrintf will be an error message describing the error.". This is not the case.
Code to reproduce the issue:

file: crash.c
============================================
#include <tcl.h>

static int Crash_Cmd (
         ClientData cdata,
         Tcl_Interp *interp,
         int objc,
         Tcl_Obj * CONST objv[] )
{
    const char *fmt;
    double value;
    if (3 != objc) {
        Tcl_WrongNumArgs(interp, 1, objv, "format value");
        return TCL_ERROR;
    }
    fmt = Tcl_GetString(objv[1]);
    if (TCL_ERROR == Tcl_GetDoubleFromObj(interp, objv[2], &value))
        return TCL_ERROR;

    Tcl_SetObjResult(interp, Tcl_ObjPrintf(fmt, value));
    return TCL_OK;
}

int DLLEXPORT
    Crash_Init(Tcl_Interp *interp)
{
    if (Tcl_InitStubs(interp, "8.5", 0) == NULL) {
        return TCL_ERROR;
    }

    if (Tcl_PkgProvide(interp, "Crash", "0.1") == TCL_ERROR) {
        return TCL_ERROR;
    }

    Tcl_CreateObjCommand(interp, "crashMe", Crash_Cmd, NULL, NULL);

    return TCL_OK;
}
============================================

$ gcc -g -fPIC -shared -I/usr/local/include crash.c -o libcrash.so -DUSE_TCL_STUBS -L/usr/local/lib -ltclstub8.6
$ gdb tclsh8.6
GNU gdb 6.8
Copyright (C) 2008 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "i486-slackware-linux"...
(gdb) run
Starting program: /usr/local/bin/tclsh8.6 
[Thread debugging using libthread_db enabled]
[New Thread 0xb7cb96c0 (LWP 3729)]
[New Thread 0xb7c82b90 (LWP 3732)]
% load ./libcrash.so
% crashMe %.3f [expr {acos(-1)}]
3.142
% crashMe %s [expr {acos(-1)}]

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0xb7cb96c0 (LWP 3729)]
0xb7f79fa5 in AppendPrintfToObjVA (objPtr=0x80b0618, format=0x8096178 "%s", 
    argList=0xbf9fd188 "û!\t@\211cç·")
    at /home/emiliano/src/tcl/generic/tclStringObj.c:2419
2419while ((!gotPrecision || lastNum--) && (*end != '\0')) {
(gdb) p end
$1 = 0x54442d18 <Address 0x54442d18 out of bounds>
(gdb) bt
#0  0xb7f79fa5 in AppendPrintfToObjVA (objPtr=0x80b0618, 
    format=0x8096178 "%s", argList=0xbf9fd188 "û!\t@\211cç·")
    at /home/emiliano/src/tcl/generic/tclStringObj.c:2419
#1  0xb7f7a296 in Tcl_ObjPrintf (format=0x8096178 "%s")
    at /home/emiliano/src/tcl/generic/tclStringObj.c:2563
#2  0xb7e76446 in Crash_Cmd (cdata=0x0, interp=0x806d990, objc=3, 
    objv=0x8074aac) at crash.c:20
#3  0xb7ea22ac in NRRunObjProc (data=0x80b21c4, interp=0x806d990, result=0)
    at /home/emiliano/src/tcl/generic/tclBasic.c:4385
#4  0xb7ea206c in TclNRRunCallbacks (interp=0x806d990, result=0, rootPtr=0x0)
    at /home/emiliano/src/tcl/generic/tclBasic.c:4332
#5  0xb7ea42fe in TclEvalObjEx (interp=0x806d990, objPtr=0xbf9fd184, 
    flags=131072, invoker=0x0, word=0)
    at /home/emiliano/src/tcl/generic/tclBasic.c:5907
#6  0xb7ea429e in Tcl_EvalObjEx (interp=0x806d990, objPtr=0x54442d18, 
    flags=131072) at /home/emiliano/src/tcl/generic/tclBasic.c:5888
#7  0xb7f2cb66 in Tcl_RecordAndEvalObj (interp=0x806d990, cmdPtr=0x80b05a0, 
    flags=131072) at /home/emiliano/src/tcl/generic/tclHistory.c:192
#8  0xb7f567f4 in Tcl_MainEx (argc=-1, argv=0xbf9fd608, 
    appInitProc=0x8048615 <Tcl_AppInit>, interp=0x806d990)
    at /home/emiliano/src/tcl/generic/tclMain.c:518
#9  0xb7f56c48 in Tcl_Main (argc=1, argv=0xbf9fd604, 
    appInitProc=0x8048615 <Tcl_AppInit>)
---Type <return> to continue, or q <return> to quit---
    at /home/emiliano/src/tcl/generic/tclMain.c:667
#10 0x08048603 in main (argc=1, argv=0x8063bf0)
    at /home/emiliano/src/tcl/unix/tclAppInit.c:80
(gdb)


Tcl compiled with --enable-symbols. Platform is Linux i386

Yes. The kind of "internal inconsistencies" that make code!=TCL_OK are like "bad XPG positional index", or "forbidden modifier for the type" (unsigned bignum), or various overflows. They are, indeed, internal, so there's no real implication that "consistency with the varargs" is checked. But the clarification seems useful anyway.

I believe the docs are referring to:

    if (code != TCL_OK) {
        Tcl_AppendPrintfToObj(objPtr,
                "Unable to format \"%s\" with supplied arguments: %s",
                format, Tcl_GetString(list));
    }

Doc fix committed to 8.6 HEAD, thanks.

Per discussion in #tcl on IRC, here is some material to add to documentation for StringObj(3) (to be inserted in paragraph after "will be an error message describing the error.": 

It is impossible  however to provide runtime protection against 
mismatches between the format and any subsequent arguments.
Compile-time protection may be provided by some compilers.

allow_comments - 0

Reopening after clarification from Emiliano on the chat: there is still a very misleading sentence in the manpage (StringObj.3):

"  If the value of format contains  internal  inconsistencies  or  invalid
"  specifier   formats,   the   formatted   string   result   produced  by
"  Tcl_ObjPrintf will be an error message describing the error.

I wonder what kind of "internal inconsistencies" can be detected at runtime (we're not talking about compile-time warning, well addressed by Jan's fixes)...

allow_comments - 1

Fixed in HEAD.

This cannot be backported because extensions using Tcl_Panic()
might suddenly start to generate warnings which were previously
undetected. Although those warnings are valid, this is not
expected in a bug-fix release.

Closing

data_type - 360894

Reclassified accordingly.

File Added - 399074: 3159920.patch

Agreed with ferrieux. This is not a bug, but a feature request.
Here is a patch, introducing __attribute__((__printf__ .....))
varargs checking for gcc. There are 3 functions with printf-like
arguments.

Using this patch, I discovered various varargs problems in
Tcl_Panic calls, that alone shows this is a good idea.

Comments???

That's a generic compiler issue: vararg type checking is only available as specific compiler support for built-ins (e.g. sscanf in gcc). So basically you're requesting printf("%s",(char *)1) to _not_ raise a SIGBUS ? That's impossible.