False positive in C/C++ dead code detection

[Faycal572]

Description of the false positive

The query for unreachable code (BasicBlock where not bb1.isReachable()) incorrectly detects live code as dead code in C/C++.
Specifically, in the function Tcl_TranslateFileName, it incorrectly flags a for loop that is clearly reachable and executed in normal control flow.
tclPlatform is a global variable, so the condition if (tclPlatform == TCL) can be true at runtime, making the code reachable.

This leads to several false positives in real codebases.

Code samples or links to source code

Here is an example:

char *Tcl_TranslateFileName(Tcl_Interp *interp, CONST char *name, Tcl_DString *bufferPtr)
{
    Tcl_Obj *path = Tcl_NewStringObj(name, -1);
    Tcl_Obj *transPtr;

    Tcl_IncrRefCount(path);
    transPtr = Tcl_FSGetTranslatedPath(interp, path);
    if (transPtr == NULL)
    {
        Tcl_DecrRefCount(path);
        return NULL;
    }

    Tcl_DStringInit(bufferPtr);
    Tcl_DStringAppend(bufferPtr, Tcl_GetString(transPtr), -1);
    Tcl_DecrRefCount(path);
    Tcl_DecrRefCount(transPtr);

    if (tclPlatform == TCL)
    {
        char *p;
        for (p = Tcl_DStringValue(bufferPtr); *p != '\0'; p++)  // <-- this block is reported as unreachable, but it is reachable
        {
            if (*p == '/')
            {
                *p = '\\';
            }
        }
    }
    return Tcl_DStringValue(bufferPtr);
}

Query used to detect unreachable blocks:

import cpp

predicate isUserFile(File f) {
  not f.getFile().getAbsolutePath().matches("%/include/%") and 
  not (f.toString().matches("%file://:0:0:0:0%")) 
}

from BasicBlock bb1
where
  isUserFile(bb1.getEnclosingFunction().getFile()) and
  not bb1.isReachable()
select 
  bb1.getEnclosingFunction().getName(),
  bb1.getEnclosingFunction().getLocation(),
  bb1.getStart().getLocation().getStartLine(),
  bb1.getEnd().getLocation().getStartLine()

The block:

{
    char *p;
    for (p = Tcl_DStringValue(bufferPtr); *p != '\0'; p++)

is wrongly reported as unreachable (dead code), but it is active and necessary to correctly translate file paths.

This false positive happens in several similar real-world cases, and makes the dead code detection query unreliable.

[jketema]

Hi @Faycal572,

Unfortunately I cannot reproduce the problem you're reporting. I've tried is to create a database as follows:

git clone https://github.com/tcltk/tcl
cd tcl/unix
./configure
codeql database create -l cpp -c make ../db

When I then run your query on that database, I do not see the problem you're reporting. Could you provide instructions on how to reproduce the issue?

[Faycal572]

Hi, thanks for your answer.

The code where I observe this false positive comes from an internal tool developed by my company, and unfortunately I cannot share the full project for confidentiality reasons.

However, the root cause seems clear:
In my code, tclPlatform is a global variable whose value is only determined at runtime, not at compile time.
Thus, the condition if (tclPlatform == TCL) can be true at runtime, and the for loop inside is reachable.
But currently, the dead code query (BasicBlock not reachable) incorrectly considers it unreachable.

I have seen this false positive in several similar cases.

Thanks again!

[jketema]

Thanks for your answer,

In my code, tclPlatform is a global variable whose value is only determined at runtime, not at compile time.
Thus, the condition if (tclPlatform == TCL) can be true at runtime, and the for loop inside is reachable.
But currently, the dead code query (BasicBlock not reachable) incorrectly considers it unreachable.

That's an incorrect diagnosis. First, isReachable does not look at the values of global variables. Second, if you look at the code from https://github.com/tcltk/tcl you'll see that is very similar https://github.com/tcltk/tcl/blob/4fa72e4d115f34198b849b5354a75ae458aeb981/generic/tclFileName.c#L1026, and also depends on a global variable https://github.com/tcltk/tcl/blob/4fa72e4d115f34198b849b5354a75ae458aeb981/generic/tclFileName.c#L23 Yet, the problem you report does not occur.

I suspect there's some deeper problem with the quality of your database, but given that you cannot share any reproducer, this is impossible for me to diagnose any further here. However, given our license and the fact that you cannot share any further details here will mean that your company is a paying customer, the way forward here would be to open a support ticket with GitHub support as explained here https://docs.github.com/en/support/contacting-github-support/creating-a-support-ticket. That will allow for a more private way of sharing data.

[github-actions]

This issue is stale because it has been open 14 days with no activity. Comment or remove the Stale label in order to avoid having this issue closed in 7 days.

[github-actions]

This issue was closed because it has been inactive for 7 days.