Page MenuHomePhabricator

[obj2yaml] - Match ".stack_size" with the original section name, and not the uniquified name.
ClosedPublic

Authored by rahmanl on Tue, Sep 15, 3:12 PM.

Details

Summary

Without this patch, obj2yaml decodes the content of only one ".stack_size" section. Other sections are dumped with their full contents.

Diff Detail

Event Timeline

rahmanl created this revision.Tue, Sep 15, 3:12 PM
Herald added a project: Restricted Project. · View Herald Transcript
rahmanl requested review of this revision.Tue, Sep 15, 3:13 PM
rahmanl edited reviewers, added: grimar, MaskRay; removed: espindola.Tue, Sep 15, 3:14 PM
grimar accepted this revision.Wed, Sep 16, 2:02 AM
grimar added a reviewer: jhenderson.

LGTM, thanks!

This revision is now accepted and ready to land.Wed, Sep 16, 2:02 AM

nit: add a "[obj2yaml]" prefix to the patch name:

[obj2yaml] - Match ".stack_size" with the original section name, and not the uniquified name.
jhenderson added inline comments.Wed, Sep 16, 2:04 AM
llvm/test/tools/obj2yaml/ELF/stack-sizes.yaml
122

Can we use Entries: instead of raw content for the input too? (Same below)

rahmanl updated this revision to Diff 292279.Wed, Sep 16, 11:12 AM
rahmanl marked an inline comment as done.

Use "Entries:" instead of "Content:" in test.

rahmanl retitled this revision from Match ".stack_size" with the original section name, and not the uniquified name. to [obj2yaml] - Match ".stack_size" with the original section name, and not the uniquified name..Wed, Sep 16, 11:15 AM
MaskRay accepted this revision.Wed, Sep 16, 11:17 AM
rahmanl updated this revision to Diff 292284.Wed, Sep 16, 11:30 AM
  • Rebase with upstream.
This revision was landed with ongoing or failed builds.Wed, Sep 16, 11:33 AM
This revision was automatically updated to reflect the committed changes.

@rahmanl This conflicts with D87385. Please re-check before committing.

MaskRay reopened this revision.Wed, Sep 16, 12:10 PM
This revision is now accepted and ready to land.Wed, Sep 16, 12:10 PM
MaskRay requested changes to this revision.Wed, Sep 16, 12:10 PM
This revision now requires changes to proceed.Wed, Sep 16, 12:10 PM
rahmanl updated this revision to Diff 292304.Wed, Sep 16, 12:10 PM
  • Change the getSectionName call to pass pointer.
MaskRay added a comment.EditedWed, Sep 16, 12:11 PM

For a reverted revision, you can click "Add Action" - "Reopen Revision" followed by "Request Changes". There is usually no need creating a new one unless the new patch undergoes a very large refactoring.

Make sure you commit has Differential Revision: https://reviews.llvm.org/D87727 and arc diff 'HEAD^' can upload a new diff.

Sorry about the headache @MaskRay I should've built after rebasing.

MaskRay added a comment.EditedWed, Sep 16, 12:19 PM

Sorry about the headache @MaskRay I should've built after rebasing.

No problem! There are 100 commits every day and a previous test can easily get stale after, say, 6 hours.
And for a revert (e.g. f80f2516a2697218eeb7af80de3b13c38f342987), please mention the reason why it was reverted (it caused compilation issues. Even better it could have mentioned that it conflicted with a previous commit) (And a revert should be tested as well, sometimes a revert can cause new problems)

MaskRay accepted this revision.Wed, Sep 16, 1:40 PM

LGTM.

This revision is now accepted and ready to land.Wed, Sep 16, 1:40 PM
This revision was landed with ongoing or failed builds.Wed, Sep 16, 2:24 PM
This revision was automatically updated to reflect the committed changes.