Page MenuHomePhabricator

[lldb] Make symbol list output from `image dump symtab` not depend on internal ordering of DenseMap
ClosedPublic

Authored by teemperor on Sep 2 2020, 8:47 AM.

Details

Summary

image dump symtab seems to output the symbols in whatever order they appear in the DenseMap
that is used to filter out symbols with non-unique addresses. As DenseMap is a hash map this order
can change at any time so the output of this command is pretty unstable. This
also causes the Breakpad/symtab.test to fail with enabled reverse iteration (which reverses the DenseMap
order to find issues like this).

This patch makes the DenseMap a std::vector and uses a separate DenseSet to do the address filtering.
The output order is now dependent on the order in which the symbols are read (which should be
deterministic). It might also avoid a bit of work as all the work for creating the Symbol constructor
parameters is only done when we can actually emplace a new Symbol.

Diff Detail

Event Timeline

teemperor requested review of this revision.Sep 2 2020, 8:47 AM
teemperor created this revision.

This should also avoid changing the test all the time when someone touches one of the symbols (as in D68533 )

labath accepted this revision.Sep 3 2020, 1:22 AM

Looks good.

This should also avoid changing the test all the time when someone touches one of the symbols (as in D68533 )

AFAICT, that patch modified the test because it actually introduced a new symbol -- the order of the existing symbols was not changed.

This revision is now accepted and ready to land.Sep 3 2020, 1:22 AM

Looks good.

This should also avoid changing the test all the time when someone touches one of the symbols (as in D68533 )

AFAICT, that patch modified the test because it actually introduced a new symbol -- the order of the existing symbols was not changed.

Oh true, somehow phab is just highlighting the function names as if they have changed. My bad!

Herald added a project: Restricted Project. · View Herald TranscriptSep 3 2020, 1:28 AM