This is an archive of the discontinued LLVM Phabricator instance.

[llvm-objdump] Replace array_pod_sort with llvm::stable_sort
ClosedPublic

Authored by MaskRay on Mar 24 2020, 4:00 PM.

Details

Summary

llvm-objdump.test has 3 array_pod_sort() used for symbolization.
array_pod_start() calls qsort() internally and can have different
behaviors across different libcs. Use llvm::stable_sort instead.

Diff Detail

Event Timeline

MaskRay created this revision.Mar 24 2020, 4:00 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 24 2020, 4:00 PM
thopre accepted this revision.Mar 25 2020, 6:52 AM

LGTM. This is what I think is causing tools/llvm-objdump/X86/disassemble-functions.test to fail in some settings (see my comment https://reviews.llvm.org/D75631#1940245)

llvm/tools/llvm-objdump/llvm-objdump.cpp
1205–1207

Also non allocatable sections have 0 for start address. Dunno if it's worth mentioning.

This revision is now accepted and ready to land.Mar 25 2020, 6:52 AM
davidb accepted this revision.Mar 25 2020, 7:10 AM

Thanks for this!

davidb added inline comments.Mar 25 2020, 7:16 AM
llvm/tools/llvm-objdump/llvm-objdump.cpp
1205–1207

Might be worth considering this as a future change. I think excluding sections that do not have SHF_ALLOC could cause issues in some cases where one might forget to set the appropriate section flags in assembly.

This revision was automatically updated to reflect the committed changes.
jhenderson added inline comments.Mar 26 2020, 1:55 AM
llvm/tools/llvm-objdump/llvm-objdump.cpp
1207

Here and below: stabalize -> stabilize.

MaskRay marked 2 inline comments as done.Mar 26 2020, 9:11 AM
MaskRay added inline comments.
llvm/tools/llvm-objdump/llvm-objdump.cpp
1207