Page MenuHomePhabricator

[yaml2elf] - Treat the SHN_UNDEF section as kind of regular section.
ClosedPublic

Authored by grimar on Mon, Jul 22, 6:23 AM.

Details

Summary

We have a logic that adds a few sections implicitly.
Though the SHT_NULL section with section number 0
is an exception.

In D64913 I want to teach yaml2obj to redefine the null section.
And in this patch I want to add it to the sections list,
to make it kind of regular section.

Diff Detail

Repository
rL LLVM

Event Timeline

grimar created this revision.Mon, Jul 22, 6:23 AM
grimar updated this revision to Diff 211077.Mon, Jul 22, 6:26 AM
  • A minor simplification.

[yaml2elf] - Treat the SHT_NULL section as kind of regular section.

Suggest referring to this section with SHN_UNDEF. Because a section of type SHT_NULL may exist in a different place.

Thought SHT_NULL section which is placed at index 0 is still kind of exception.

Though the SHT_NULL section with section number 0 is an exception. (Though other reviewers may have better recommendation:)

tools/yaml2obj/yaml2elf.cpp
199 ↗(On Diff #211077)

/*IsImplicit=*/true may be a more popular choice (clang-format uses that form if you place a space in between).

328 ↗(On Diff #211077)

You can circumvent the violation of https://llvm.org/docs/CodingStandards.html#don-t-evaluate-end-every-time-through-a-loop by assigning Doc.Sections.size() to a variable above (it is also used above)

grimar marked an inline comment as done.Mon, Jul 22, 7:33 AM
grimar added inline comments.
tools/yaml2obj/yaml2elf.cpp
328 ↗(On Diff #211077)

Doc.Sections is a vector. size() has a trivial implementation, for me it is:

	_NODISCARD size_type size() const noexcept
		{	// return length of sequence
		return (static_cast<size_type>(this->_Mylast() - this->_Myfirst()));
		}

I do not think it is very common to introduce a new variable to store size() value
returned by a vector in LLVM (well, sometimes we use (for I = 0, E =..., but not always).

And it doesn't seem worth to do. I think that coding standart assumes that re-evaluting might
be expensive. But this is not the case.

grimar marked an inline comment as done.Mon, Jul 22, 7:34 AM
grimar added inline comments.
tools/yaml2obj/yaml2elf.cpp
328 ↗(On Diff #211077)

Just search for ".size();" :)

jhenderson accepted this revision.Mon, Jul 22, 9:43 AM

LGTM, once MaskRay's comments have been addressed/agreed not to do.

This revision is now accepted and ready to land.Mon, Jul 22, 9:43 AM
MaskRay accepted this revision.Mon, Jul 22, 5:40 PM

LGTM once the title/description is fixed :)

grimar retitled this revision from [yaml2elf] - Treat the SHT_NULL section as kind of regular section. to [yaml2elf] - Treat the SHN_UNDEF section as kind of regular section..Tue, Jul 23, 12:03 AM
grimar edited the summary of this revision. (Show Details)
This revision was automatically updated to reflect the committed changes.
grimar marked an inline comment as done.
Herald added a project: Restricted Project. · View Herald TranscriptTue, Jul 23, 12:39 AM