Repository: dotnet/runtime
File: src/coreclr/jit/flowgraph.cpp (C++)

Code before:

//------------------------------------------------------------------------
// fgGetExcptnTarget: finds the block to jump to that will throw a given kind of exception
//
// Arguments:
//    kind -- kind of exception to throw
//    fromBlock -- block that will jump to the throw helper
//    createIfNeeded -- create the block if it does not already exist
//
// Return Value:
//    Code descriptor for the appropriate throw helper block, or nullptr if no such
//    descriptor exists (only when createIfNeeded is false).
//
Compiler::AddCodeDsc* Compiler::fgGetExcptnTarget(SpecialCodeKind kind, BasicBlock* fromBlock, bool createIfNeeded)
{
    assert(fgUseThrowHelperBlocks() || (kind == SCK_FAIL_FAST));
    AddCodeDsc*          add = nullptr;
    AddCodeDscMap* const map = fgGetAddCodeDscMap();
    AddCodeDscKey        key(kind, fromBlock, this);
    map->Lookup(key, &add);

    if (add == nullptr)
    {
        add = fgCreateAddCodeDsc(fromBlock, kind);

        // Create the block...
        //
        if (createIfNeeded)
        {
            fgCreateThrowHelperBlock(add);
        }
    }

    return add;
}

//------------------------------------------------------------------------
// bbThrowIndex: find acd map key for a given block
//
// Arguments:
//    blk -- block that may eventually throw an exception
//    dsg [out] -- designator for which region controls throw block placement
//
// Return Value:
//    encoded region value to use in acd key formation
//
unsigned Compiler::bbThrowIndex(BasicBlock* blk, AcdKeyDesignator* dsg)
{
    const unsigned tryIndex = blk->bbTryIndex;
    const unsigned hndIndex = blk->bbHndIndex;
    const bool     inTry    = tryIndex > 0;

Review comment (bug):
`fgGetExcptnTarget` currently creates a new `AddCodeDsc` whenever the lookup misses, even when `createIfNeeded` is false. This contradicts the function comment (which says it can return nullptr when not creating) and can return a partially-initialized descriptor (`acdDstBlk == nullptr`) to callers that expect an existing helper block, risking null dereferences in non-assert builds. Consider returning nullptr (and not allocating the map) when `createIfNeeded` is false and no entry exists.

Code after:

//------------------------------------------------------------------------
// fgGetExcptnTarget: finds the block to jump to that will throw a given kind of exception
//
// Arguments:
//    kind -- kind of exception to throw
//    fromBlock -- block that will jump to the throw helper
//    createIfNeeded -- create the block if it does not already exist
//
// Return Value:
//    Code descriptor for the appropriate throw helper block.
//    When createIfNeeded is false, this can return an AddCodeDsc where acdDstBlk is nullptr.
//    If createIfNeeded is true, acdDstBlk will be non-null.
//
Compiler::AddCodeDsc* Compiler::fgGetExcptnTarget(SpecialCodeKind kind, BasicBlock* fromBlock, bool createIfNeeded)
{
    assert(fgUseThrowHelperBlocks() || (kind == SCK_FAIL_FAST));
    AddCodeDsc*          add = nullptr;
    AddCodeDscMap* const map = fgGetAddCodeDscMap();
    AddCodeDscKey        key(kind, fromBlock, this);
    map->Lookup(key, &add);

    if (add == nullptr)
    {
        add = fgCreateAddCodeDsc(fromBlock, kind);

        // Create the block...
        //
        if (createIfNeeded)
        {
            fgCreateThrowHelperBlock(add);
        }
    }

    return add;
}

//------------------------------------------------------------------------
// bbThrowIndex: find acd map key for a given block
//
// Arguments:
//    blk -- block that may eventually throw an exception
//    dsg [out] -- designator for which region controls throw block placement
//
// Return Value:
//    encoded region value to use in acd key formation
//
unsigned Compiler::bbThrowIndex(BasicBlock* blk, AcdKeyDesignator* dsg)
{
    const unsigned tryIndex = blk->bbTryIndex;
    const unsigned hndIndex = blk->bbHndIndex;