1693184 - Add marker/magic number in stencil XDR, in between each field

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P3)

Description

[Tooru Fujisawa [:arai]]

For crash like bug 1693137,
if we encode/decode some marker/magic number in between each field/section inside the XDR data,
we can verify that until which point the XDR data was correct.

we already have XDRState::codeMarker, and legacy XDR was using it.

bug 1687095 will remove delazification chunks, and we can have such markers inside CompilationStencil fields, without so much size regression.

Comment 1

[Tooru Fujisawa [:arai]]

The structure after bug 1687095 will be the following,
and we could add marker between each section,
or maybe skip for some fixed-length section.

If we use 4 bytes marker, it will take at most 14 x 4 bytes = 56 bytes.

# ======== Version Check ========
uint32_t                buildIdLength;
uint8_t                 buildId[buildIdLength];

# ======== CompiltionStencil.source_ ========
uint8_t                 dataType;
if dataType == DataType::Compressed*NotRetrievable {
   uint32_t             uncompressedLength;
   uint32_t             compressedLength;
   uint8_t              bytes[compressedLength];
}
if dataType == DataType::Uncompressed*NotRetrievable {
   uint32_t             uncompressedLength;
   CharT                sourceUnits[uncompressedLength];
}

uint8_t                 haveSourceMap;
if haveSourceMap {
  uint32_t              sourceMapURL_length;
  CharT                 sourceMapURL[sourceMapURL_length];
}
uint8_t                 haveDisplayURL;
if haveDisplayURL {
  uint32_t              displayURL_length;
  CharT                 displayURL[displayURL_length];
}
uint8_t                 haveFilename;
if haveFilename {
  uint32_t              filename_length;
  CharT                 filename[filename_length];
}

uint8_t                 PADDING[];  # <= unnecessary

# ======== CompilationStencil.parserAtomData ========
uint32_t                atomVectorLength;
uint32_t                atomCount;
loop 0..atomCount {
  uint32_t              index;
  uint8_t               PADDING[];
  ParserAtom            header;
  CharT                 data[header.length()];
}

# ======== CompilationStencil.functionKey ========
uint32_t                functionKey;

# ======== CompilationStencilSpanSize ========
uint8_t                 sizeKind;
if sizeKind == XDRSpanSizeKind::Base8Kind {
  uint8_t               scriptSize;
  uint8_t               gcThingSize;
  uint8_t               scopeSize;
}
if sizeKind == XDRSpanSizeKind::All8Kind {
  uint8_t               scriptSize;
  uint8_t               gcThingSize;
  uint8_t               scopeSize;
  uint8_t               scriptExtraSize;
  uint8_t               regExpSize;
  uint8_t               bigIntSize;
  uint8_t               objLiteralSize;
}
if sizeKind == XDRSpanSizeKind::All32Kind {
  uint32_t              scriptSize;
  uint32_t              gcThingSize;
  uint32_t              scopeSize;
  uint32_t              scriptExtraSize;
  uint32_t              regExpSize;
  uint32_t              bigIntSize;
  uint32_t              objLiteralSize;
}

# ======== CompilationStencil.scopeData ========
uint8_t                 PADDING[];
ScopeStencil            scopeData[scopeSize];

# ======== CompilationStencil.scopeNames ========
loop 0..scopeSize {
  uint8_t               PADDING[];
  ScopeDataT::SlotInfo  slotInfo;
  TaggedParserAtomIndex names[slotInfo.length];
}

# ======== CompilationStencil.regExpData ========
uint8_t                 PADDING[];
RegExpStencil           regExpData[regExpSize];

# ======== CompilationStencil.bigIntData ========
loop 0..bigIntSize {
  uint32_t              size
  uint8_t               PADDING[];
  uint8_t               source[size];
}

# ======== CompilationStencil.objLiteralData ========
loop 0..objLiteralSize {
  uint8_t               flags;
  uint32_t              propertyCount;
  uint32_t              length;
  uint8_t               PADDING[];
  uint8_t               code[length];
}

# ======== CompilationStencil.sharedData ========
uint8_t                   kind;
if kind == Kind::Single {
  uint32_t              size;
  uint8_t               immutableScriptData[size];
}
if kind == Kind::Vector {
  uint32_t              length;
  loop 0..length {
    uint8_t             exists;
    if exists {
      uint32_t          size;
      uint8_t           immutableScriptData[size];
    }
  }
}
if kind == Kind::Map {
  uint32_t              count;
  loop 0..count {
    uint32_t            index;
    uint32_t            size;
    uint8_t             immutableScriptData[size];
  }
}

# ======== CompilationStencil.gcThingData ========
uint8_t                 PADDING[];
TaggedScriptThingIndex  gcThingData[gcThingSize];

# ======== CompilationStencil.scriptData ========
uint8_t                 PADDING[];
ScriptStencil           scriptData[scriptSize];

# ======== CompilationStencil.scriptExtra ========
uint8_t                 PADDING[];
Scriptextrasize         scriptExtra[scriptExtraSize];

# ======== CompilationStencil.moduleMetadata ========
if scriptExtra[CompilationStencil::TopLevelIndex].isModule() {
  uint32_t              requestedModules_length;
  uint8_t               PADDING[];
  StencilModuleEntry    requestedModules[requestedModules_length];

  uint32_t              importEntries_length;
  uint8_t               PADDING[];
  StencilModuleEntry    importEntries[importEntries_length];

  uint32_t              localExportEntries_length;
  uint8_t               PADDING[];
  StencilModuleEntry    localExportEntries[localExportEntries_length];

  uint32_t              indirectExportEntries_length;
  uint8_t               PADDING[];
  StencilModuleEntry    indirectExportEntries[indirectExportEntries_length];

  uint32_t              starExportEntries_length;
  uint8_t               PADDING[];
  StencilModuleEntry    starExportEntries[starExportEntries_length];

  uint32_t              functionDecls_length;
  uint8_t               PADDING[];
  TaggedParserAtomIndex functionDecls[functionDecls_length];

  uint8_t               isAsync;
}

Comment 2

[Tooru Fujisawa [:arai]]

Attachment: Bug 1693184 - Add marker between each CompilationStencil field in XDR. r=mgaudet

Depends on D105924

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6f384df0be5c
Add marker between each CompilationStencil field in XDR. r=mgaudet

Comment 4

[Atila Butkovits]

Backed out for causing failures on test_script_loader_js_cache.html.

Backout link: https://hg.mozilla.org/integration/autoland/rev/813a75a0d4f83eaefa7f4351272ba73734cc396d

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&searchStr=android%2C7.0%2Cx86-64%2Cwebrender%2Copt%2Cmochitests%2Ctest-android-em-7.0-x86_64-qr%2Fopt-geckoview-mochitest-plain-e10s%2C3&revision=6f384df0be5c2dde836a3e6762dfe9f3edfd50ae&selectedTaskRun=Tkd2M0TEQgOSXjZSkN51xQ.0

First failure log: https://treeherder.mozilla.org/logviewer?job_id=332466454&repo=autoland&lineNumber=2788
Second failure log: https://treeherder.mozilla.org/logviewer?job_id=332472957&repo=autoland&lineNumber=3007

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b6898dad63a3
Add marker between each CompilationStencil field in XDR. r=mgaudet

Comment 6

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/b6898dad63a3