Page MenuHomePhabricator

[ThinLTO] return error instead of crashing on invalid input
Needs ReviewPublic

Authored by trofi on Apr 29 2020, 4:04 PM.

Details

Summary

In https://bugs.llvm.org/show_bug.cgi?id=45636 firefox was linked
against stale object files that don't match profiled build.

Before the change ld.lld was crashing with a backtrace as:

LLVM ERROR: Function Import: link error:
  linking module flags 'ProfileSummary':
    IDs have conflicting values in 'Mutex_posix.o' and 'nsBrowserApp.o'
  PLEASE submit a bug report to https://bugs.llvm.org/ and include the
  crash backtrace.

 #0 0x000055b58915b86a llvm::sys::PrintStackTrace(llvm::raw_ostream&)
   llvm/lib/Support/Unix/Signals.inc:564:22
 ...
 #11 0x000055b589111f89 llvm::report_fatal_error(llvm::Twine const&, bool)
   llvm/lib/Support/ErrorHandling.cpp:113:27
 #12 0x000055b5891120e1 llvm/lib/Support/ErrorHandling.cpp:87:21
 #13 0x000055b58a22611f llvm::FunctionImporter::importFunctions(...)
   llvm/lib/Transforms/IPO/FunctionImport.cpp:1250:25

After the change ld.lld collects all errors during ThinLTO and reports
them as usual:

ld.lld: error: Function Import: link error:
  linking module flags 'ProfileSummary':
    IDs have conflicting values in 'Mutex_posix.o' and 'nsBrowserApp.o'

Diff Detail

Event Timeline

trofi created this revision.Apr 29 2020, 4:04 PM

Could you add a test case for this?

(@tejohnson - might be the suitable reviewer here)

jdoerfert added a subscriber: jdoerfert.

Having a backtrace and the bug filing message seems excessive. Maybe you can even mention stale obj files in the message.
I'll add some reviewers that might want to chime in, otherwise I'll revisit this.

dmajor added a subscriber: dmajor.May 18 2020, 3:54 PM
lattner resigned from this revision.May 18 2020, 9:06 PM

Please address the earlier suggestions (needs a test case, and also agree it might be nice to have the message suggest stale objects as possible culprit).

trofi added a comment.Thu, Nov 12, 3:42 PM

Please address the earlier suggestions (needs a test case

The failure looks like a low-level profile ID mismatch when merging profiles obtained when running tests against slightly different sources. I'll need some help on how I can craft the equivalent using lit mechanism. Original 40MB compressed input to lld is at https://bugs.llvm.org/show_bug.cgi?id=45636#c1 if it's of any help.

and also agree it might be nice to have the message suggest stale objects as possible culprit).

DR description provides new error message as:

"""
ld.lld: error: Function Import: link error:

linking module flags 'ProfileSummary':
  IDs have conflicting values in 'Mutex_posix.o' and 'nsBrowserApp.o'

"""

can you suggest how it should be amended instead?

Please address the earlier suggestions (needs a test case

The failure looks like a low-level profile ID mismatch when merging profiles obtained when running tests against slightly different sources. I'll need some help on how I can craft the equivalent using lit mechanism. Original 40MB compressed input to lld is at https://bugs.llvm.org/show_bug.cgi?id=45636#c1 if it's of any help.

There is already a test for this particular error message in llvm/test/Linker/module-flags-6-a.ll. It uses llvm-link. Presumably you can use the same input files but invoke via lld to test?

and also agree it might be nice to have the message suggest stale objects as possible culprit).

DR description provides new error message as:

"""
ld.lld: error: Function Import: link error:

linking module flags 'ProfileSummary':
  IDs have conflicting values in 'Mutex_posix.o' and 'nsBrowserApp.o'

"""

can you suggest how it should be amended instead?

The error message that should be changed ("IDs have conflicting values") is coming from in llvm/lib/Linker/IRMover.cpp. Maybe change it to something like "IDs have conflicting values in 'Mutex_posix.o' and 'nsBrowserApp.o' (possible use of stale object files)"? Outside of development it seems as though this would only happen in the stale object case.