This is an archive of the discontinued LLVM Phabricator instance.

[ELF] - Reveal more information in -Map file about assignments.
ClosedPublic

Authored by grimar on Mar 26 2018, 6:05 AM.

Details

Summary

Currently, LLD print symbol assignment commands to the map file,
but it does not do that for assignments that are outside of the section
descriptions. Such assignments can affect the layout though.

The patch implements the following:

  1. Teaches LLD to print symbol assignments outside of section declaration.
  2. Teaches LLD to print PROVIDE/HIDDEN/PROVIDE hidden commands.

In case when symbol is not provided, nothing will be printed.

Second part slightly differs from gold. It prints information about all
symbols to the map file (both about provided and not provided):

.bar3       0x0000000000000030        0x8 test.o
               [!provide]                        PROVIDE (sym4 = 0x2a)
               0x000000000000002a     PROVIDE (sym3 = 0x2a)

I think it probably does not make much sense to print not-provided
symbols, so patch ignores them. (Though it would be easy to print them too).

Diff Detail

Repository
rL LLVM

Event Timeline

grimar created this revision.Mar 26 2018, 6:05 AM
ruiu added inline comments.Mar 27 2018, 2:41 PM
ELF/LinkerScript.h
113–114 ↗(On Diff #139780)

I think the original comment makes sense. I can understand what "before doing the assignment" means, but it feels easier to understand if you just say it's an address of an assignment.

118 ↗(On Diff #139780)

Remove "

202–207 ↗(On Diff #139780)

Why do you need to add these field to two different places? We generally should avoid doing this.

ELF/MapFile.cpp
153 ↗(On Diff #139780)

Let's not generalize. Just pass a boolean value instead of a string.

OK, so you are using this function only twice. I'd inline this function.

grimar updated this revision to Diff 140066.Mar 28 2018, 5:12 AM
grimar marked 3 inline comments as done.
  • Addressed review comments.
ELF/LinkerScript.h
202–207 ↗(On Diff #139780)

I am not adding Size in this patch, just adjusting the comments.

Now we have SymbolAssignment, ByteCommand and OutputSection users of field Size.
I thought about the possibility to move this field to BaseCommand but my concern is
that for each of these commands, Size has the different meaning. Size of the output section is obvious,
as well as the size of the data command, but the size of the assignment is very different.
We do not have Size in AssertCommand (and it probably makes no sense to have it).
Currently, all of the instances has its own comments and it looks good to me.

Fine by me, but wait for a final OK from Rui.

Some parts of this could be an independent cleanup. Replacing Offset with Addr for example. Please commit that first.

grimar updated this revision to Diff 140918.Apr 4 2018, 2:49 AM
  • Rebased after cleanup committed (rL329162).
espindola accepted this revision.Apr 4 2018, 8:04 AM

Fine by me, but do wait for a second OK from Rui.

This revision is now accepted and ready to land.Apr 4 2018, 8:04 AM
ruiu accepted this revision.Apr 4 2018, 2:28 PM

LGTM

This revision was automatically updated to reflect the committed changes.