Bug 175812 - We are using valueProfileForBytecodeOffset when there may not be a value profile
Summary: We are using valueProfileForBytecodeOffset when there may not be a value profile
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-08-21 22:31 PDT by Saam Barati
Modified: 2017-08-22 09:29 PDT (History)
12 users (show)

See Also:


Attachments
patch (15.34 KB, patch)
2017-08-21 23:40 PDT, Saam Barati
msaboff: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Saam Barati 2017-08-21 22:31:16 PDT
Currently, switching to this crashes on stress/inlined-tail-call-in-inlined-setter-should-not-crash-when-getting-value-profile.js

ValueProfile* CodeBlock::valueProfileForBytecodeOffset(int bytecodeOffset)
{
    OpcodeID opcodeID = Interpreter::getOpcodeID(instructions()[bytecodeOffset]);
    unsigned length = opcodeLength(opcodeID);
    ValueProfile* result = instructions()[bytecodeOffset + length - 1].u.profile;
#if !ASSERT_DISABLED
    bool found = false;
    for (unsigned i = 0; i < numberOfValueProfiles(); ++i) {
        ValueProfile* profile = valueProfile(i);
        if (profile->m_bytecodeOffset == bytecodeOffset) {
            ASSERT(profile == result);
            found = true;
            break;
        }
    }
    ASSERT(found);
#endif
    return result;
}

I'll fix and land this change
Comment 1 Saam Barati 2017-08-21 23:09:27 PDT
I'm moving to two functions:
ValueProfile& valueProfileForBytecodeOffset(int);
ValueProfile* tryGetValueProfileForBytecodeOffset(int);
Comment 2 Saam Barati 2017-08-21 23:40:42 PDT
Created attachment 318740 [details]
patch
Comment 3 Michael Saboff 2017-08-22 07:26:19 PDT
Comment on attachment 318740 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=318740&action=review

r=me with the suggested build fix.

> Source/JavaScriptCore/jit/JITInlines.h:974
>      ASSERT(valueProfile);

Looks like this line needs to be removed to fix the Debug build.
Comment 4 Saam Barati 2017-08-22 09:28:33 PDT
landed in:
https://trac.webkit.org/changeset/221018/webkit
Comment 5 Radar WebKit Bug Importer 2017-08-22 09:29:04 PDT
<rdar://problem/34014145>