1294627 - Enhance gdb pretty printer for nsTHashtable to pierce alias templates and handle when it's used as a hashset, like for ManagedTemplate

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Andrew Sutherland [:asuth] (he/him)]

While printing an mozilla:dom::ContentParent instance I noticed an error message like the following:
  AttributeError: 'NoneType' object has no attribute 'startswith'

This was due to the use of the "ManagedContainer" type within the generated PContentParent.h file:
  ManagedContainer<PAPZParent> mManagedPAPZParent;

ManagedContainer is an alias template defined like so:
  template<typename Protocol>
  using ManagedContainer = nsTHashtable<nsPtrHashKey<Protocol>>;

This was confusing the "walk_template_to_given_base" helper, so I added a strip_typedefs() there to fix that.

The other problem was that the pretty printer was assuming nsTHashtable<EntryType> would always be handed an nsBaseHashtableET<...> as its EntryType.  But ManagedContainer was just directly using a key type.  I believe this is the idiom I have heard talk of on IRC where nsTHashtable can be used like a set rather than a map.  Accordingly I've also taught it to recognize the difference and claim to be an array when it's a set.

Patch coming in a sec once I get the bug number.

Comment 1

[Andrew Sutherland [:asuth] (he/him)]

Attachment: Pierce alias templates, understand set usage. v1

The result is that the previously broken cases now look like:
    mManagedPSendStreamParent = mozilla::ManagedContainer, 
    mManagedPScreenManagerParent = mozilla::ManagedContainer = {0x7fc4962dcb40}, 
    mManagedPSmsParent = mozilla::ManagedContainer, 

Regrettably, however the type is being modeled by gdb, we're getting "mozilla::ManagedContainer" instead of "mozilla::ManagedContainer<PScreenManagerParent>".  According to "ptype", the field has type:
  mozilla::ManagedContainer mManagedPScreenManagerParent;
Although maybe if we dug around inside the Type hierarchy we could find it.

Normal maps still look like we expect:
  mControlledDocuments = nsRefPtrHashtable<nsISupportsHashKey, mozilla::dom::workers::ServiceWorkerRegistrationInfo>, 

  mRegisteringDocuments = nsClassHashtable<nsCStringHashKey, nsTArray<nsCOMPtr<nsIWeakReference> > >, 

  mRegistrationInfos = nsClassHashtable<nsCStringHashKey, mozilla::dom::workers::ServiceWorkerManager::RegistrationDataPerPrincipal> = {
    ["http://localhost:8000"] = [(mozilla::dom::workers::ServiceWorkerManager::RegistrationDataPerPrincipal *) 0x7fc4af1aee00],
    ["https://www.pokedex.org^userContextId=2"] = [(mozilla::dom::workers::ServiceWorkerManager::RegistrationDataPerPrincipal *) 0x7fc4af1ae980],
    ["https://googlechrome.github.io"] = [(mozilla::dom::workers::ServiceWorkerManager::RegistrationDataPerPrincipal *) 0x7fc4af1aea80]
  },

Comment 2

[Tom Tromey :tromey]

Comment on attachment 8780395 [details] [diff] [review]
Pierce alias templates, understand set usage. v1

Review of attachment 8780395 [details] [diff] [review]:
-----------------------------------------------------------------

Thank you for doing this.

I actually don't know the nsTHashtable code well enough to say whether this is correct.
However I think it's low risk to accept it.

I did find one nit in the to_string method, which I think should be fixed before landing.
r+ with that caveat.

::: python/gdbpp/gdbpp/thashtable.py
@@ +119,5 @@
>      def to_string(self):
> +        table = self.value['mTable']
> +        # Number of entries
> +        entryCount = table['mEntryCount']
> +

|table| and |entryCount| don't seem to be used here.

Comment 3

[Andrew Sutherland [:asuth] (he/him)]

(In reply to Tom Tromey :tromey from comment #2)
> ::: python/gdbpp/gdbpp/thashtable.py
> @@ +119,5 @@
> >      def to_string(self):
> > +        table = self.value['mTable']
> > +        # Number of entries
> > +        entryCount = table['mEntryCount']
> > +
> 
> |table| and |entryCount| don't seem to be used here.

Whoops, thanks for catching!  I had temporarily added a " (%d entries)" suffix to the to_string() for debugging which used these, but I clearly was too quick in my patch cleanup.  I took it back out because the number of potentially empty buckets seemed noisy/confusing and knowing the number of occupied buckets requires traversing the buckets using the logic in children() which didn't actually seem all that valuable.  In the mManaged* list in ContentParent, for example, most of the sets had 0 entries or 1 empty entry, which seemed more distracting than useful when skimming the object.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e002e37dff5b
Enhance gdb pretty printer for nsTHashtable to pierce alias templates and handle when it's used as a hashset, like for ManagedTemplate. r=tromey

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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