This is an archive of the discontinued LLVM Phabricator instance.

[Assembler] Better error messages for .org directive
ClosedPublic

Authored by olista01 on Dec 5 2016, 6:08 AM.

Details

Summary

Currently, the error messages we emit for the .org directive when the
expression is not absolute or is out of range do not include the line
number of the directive, so it can be hard to track down the problem if
a file contains many .org directives.

This patch stores the source location in the MCOrgFragment, so that it
can be used for diagnostics emitted during layout.

Since layout is an iterative process, and the errors are detected during
each iteration, it would have been possible for errors to be reported
multiple times. To prevent this, I've made the assembler bail out after
each iteration if any errors have been reported. This will still allow
multiple unrelated errors to be reported in the common case where they
are all detected in the first round of layout.

Diff Detail

Repository
rL LLVM

Event Timeline

olista01 updated this revision to Diff 80261.Dec 5 2016, 6:08 AM
olista01 retitled this revision from to [Assembler] Better error messages for .org directive.
olista01 updated this object.
olista01 added reviewers: rengolin, t.p.northover.
olista01 set the repository for this revision to rL LLVM.
olista01 added a subscriber: llvm-commits.
rengolin edited edge metadata.Dec 12 2016, 2:21 AM

One small comment, otherwise, LGTM.

lib/MC/MCAssembler.cpp
925 ↗(On Diff #80261)

why do you need to compute the fragment size? Is it for the FragmentOffset parameter in the error message?

olista01 added inline comments.Dec 14 2016, 2:53 AM
lib/MC/MCAssembler.cpp
925 ↗(On Diff #80261)

The errors are emitted while calculating the fragment size. Previously this just called getFragmentOffset for the last fragment in each section, which caused the size of every fragment except the last to be calculated. I found some cases where the size of the last fragment wasn't evaluated until a later stage. This forces the errors to all be detected at a consistent time.

This revision was automatically updated to reflect the committed changes.

This is awesome! Thank you.