Page MenuHomePhabricator

[llvm-readelf]Print filename for multiple inputs and fix formatting regression
ClosedPublic

Authored by jhenderson on Aug 8 2019, 7:55 AM.

Details

Summary

This patch addresses two closely related bugs: https://bugs.llvm.org/show_bug.cgi?id=42930 and https://bugs.llvm.org/show_bug.cgi?id=42931. GNU readelf prints the file name for every input unless there is only one input and that input is not an archive. This patch adds the printing for multiple inputs. A previous change did it for archives, but introduced a regression with GNU compatibility for single-output formatting, resulting in a spurious initial blank line. This is fixed in this patch too.

Diff Detail

Repository
rL LLVM

Event Timeline

jhenderson created this revision.Aug 8 2019, 7:55 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 8 2019, 7:55 AM
MaskRay accepted this revision.Aug 8 2019, 6:15 PM
MaskRay added inline comments.
test/tools/llvm-readobj/file-name.test
28 ↗(On Diff #214154)

To test the first line is blank: # NAME1: {{^$}}

This is what readelf does (since its initial import in 1999):

/* Process the file.  */
if (show_name)
  printf (_("\nFile: %s\n"), filedata->file_name);

I think the blank line is pretty arbitrary. It is there just because it was easy to implement...

This revision is now accepted and ready to land.Aug 8 2019, 6:15 PM
grimar accepted this revision.Aug 9 2019, 12:56 AM

LGTM.

jhenderson marked an inline comment as done.Aug 9 2019, 2:31 AM
jhenderson added inline comments.
test/tools/llvm-readobj/file-name.test
28 ↗(On Diff #214154)

I agree that the line is arbitrary. Unfortunately, I've noticed a local parsing script that relies on it in some cases, so I'd be surprised if there aren't others out there doing the same thing.

To test the first line is blank: # NAME1: {{^$}}

I knew that somebody would comment on this :)

I was just typing out this response, when I realised that we don't need the added complexity:

All your suggested pattern does is test that there is an empty line somewhere in the input (and that later checks follow it). It doesn't guarantee it's the first line. Just as we want one empty line at the start, we also don't want 2+

I realised however that FileCheck ensures that the first check matches the first line, and then the CHECK-NEXT matches the next, so it can't be an arbitrary location. I'll update the test before committing.

This revision was automatically updated to reflect the committed changes.