[task 2019-12-04T19:59:12.017Z] 15:16.18 /builds/worker/checkouts/gecko/obj-analyzed/dist/include/nsIChannel.h: In member function 'bool nsIChannel::IsDocument()':
[task 2019-12-04T19:59:12.017Z] 15:16.18 /builds/worker/checkouts/gecko/obj-analyzed/dist/include/nsIChannel.h:124:3: internal compiler error: in XIL_GetFunctionFields, at type.c:382

Reduced test case (also in attachment):
https://hg.mozilla.org/try/rev/6dfe5f42cb26080bc0cdcf70502e508b9c466968

Steve, I don't know if you already had a bug for this.
Let me know if there is anything I can do to help with fixing this.

Thanks for filing this. I hadn't quite gotten around to it. :-)

In brief: GetFunctionFields(nsIRequest) processes GetLoadGroup() method that processes class nsILoadGroup that processes its base class nsIRequest and calls GetFunctionFields(nsIRequest) for it.

This isn't supposed to happen, because the methods should have already been processed at this point.

An added wrinkle that I'm not sure matters, is that class nsIRequest is processed early, including GetFunctionFields(nsIRequest), and then later some more compilation happens and prepends the declaration of nsIRequest::~nsIRequest to its list of function field declarations. The information gathered in GetFunctionFields is cached on the first declaration of a type, not the type itself -- but I think this is behaving as expected; if the set of known decls changes, the type needs to be reprocessed, and it is.

I notice in the "good" case (without the triggering patch above), that the first time nsIRequest is processed, it also processes nsILoadGroup. In the "bad" case, it does not. I'm trying to figure out why.

I guess the patch above somehow manages to trigger the problem because GetTRRMode() calls GetLoadFlags() which... is somehow known to access nsILoadGroup? Or maybe it happens when processing nsILoadGroup and gathering up its base classes' methods? (Weird because it inherits that one from its base, so this would be type nsIRequest there, but... oh well, I probably don't need to figure that out. Unless I wanted to construct a minimal test case from scratch. Currently I'm using a 200k line preprocessed source file that I don't have an easy way to cut down.)

I have a potential fix. I pushed it to try at https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=280402978&revision=c1ae6118ca861d7299f34b62ae17793a3d6a3d36 (which is red because it hits some fatal warning.)

I'll write up what's going on soon so I can put it up for review. I'm not 100% confident that it's the right fix, but it makes sense.

Copied from the patch comment:

The basic structure is:

XIL_TranslateRecordType(csu):
  r = lookup_or_create(csu.name)
  if csu is incomplete:
    return r

  // first generate types for all virtual methods in this class.
  // the function fields need to be able to get types for these methods
  // without triggering a reentrant generation for a record type.
  for each virtual method in csu:
    XIL_TranslateType(method)

  XIL_GetFunctionFields(csu)

  return r

XIL_GetFunctionFields(csu):
  decl = csu.first_decl
  pworking = VTableWork[decl]
  assert(!*pworking)
  pvirt = VTable[decl]
  if *pvirt:
    return *pvirt

  *pworking = true

  for each decl in csu that is a base class:
    XIL_GetFunctionFields(decl.type)

  for each virtual method defined directly in csu:
    XIL_TranslateField(method)

  *pworking = false

The scenario:

XIL_TranslateRecordType(nsIRequest)
  nsIRequest is complete, so it continues to generate
  while translating virtual methods, it does XIL_TranslateRecordType(nsILoadGroup)
    nsILoadGroup is incomplete
  XIL_GetFunctionFields(nsIRequest)
    VTable[decl1] is filled in, where decl1 is the first decl in nsIRequest
More compilation happens
  this compilation prepends nsIRequest::~nsIRequest to the set of virtual methods on nsIRequest
XIL_TranslateRecordType(nsIChannel)
  nsIChannel is complete, so it continues to generate
  while translating virtual methods, neither nsIRequest nor nsILoadGroup is observed
  XIL_GetFunctionFields(nsIChannel)
    VTableWorking[nsIChannel] is set to true
    the base class nsIRequest is encountered
    XIL_GetFunctionFields(nsIRequest)
      TYPE_FIELDS(nsIRequest) is now decl2 (aka ~nsIRequest()), not decl1
        VTableWork[decl2] is unset, so proceed to generation
      while processing nsIRequest virtual methods, nsILoadRequest is encountered
      XIL_TranslateRecordType(nsILoadRequest)
        nsILoadRequest is now complete! generation begins
        every nsILoadRequest virtual method is translated
        XIL_GetFunctionFields(nsILoadRequest)
          base class is nsIRequest, so...
          XIL_GetFunctionFields(nsIRequest)
            VTableWork[decl2] is already set, reentry detected, kaboom!

My conclusion is that the first part of XIL_GetFunctionFields() is incomplete, when it translates the types of all virtual methods in order to prevent later reentry. Because you can still get reentry via a base class's methods, and that part happens unconditionally at the start of XIL_GetFunctionFields and is not prevented from reentering.

My fix is to not only preprocess all virtual methods (via XIL_TranslateType), but also all virtual methods of base classes. This is O(n^2), which is unfortunate, but it's just O(n^2) lookups for something that will pretty much always be found. A more narrow fix might be to only translate virtual methods for bases whose first decl is not found in VTable[]?

https://hg.mozilla.org/mozilla-central/rev/470e720cf9ed