[PATCH] D58047: [LLD][ELF][ARM] Synthesise missing .ARM.exidx sections.

[Peter Smith via Phabricator via llvm-commits]

peter.smith marked an inline comment as done.
peter.smith added a comment.

I think what I have there is correct but it obviously could be clearer. I've added some alternatives inline.



================
Comment at: ELF/SyntheticSections.cpp:3055
+      LinkSec(Link) {
+  IsSentinel = Link == nullptr;
+  RawData = Data;
----------------
ruiu wrote:
> You are assigning a `nullptr` to a boolean member.
> 
> Who sets IsSentinel to true? Looks like there's no code doing that.
I don't think I am as the == will have higher precedence than the =. However I should find another way of expressing it. The simplest way is to put some parentheses around so that it is IsSentinel = (Link == nullptr);

What I need is a way of distinguishing between the Sentinel (so I can write the address of the end of the section). We do know that at at construction time as the Sentinel won't have a Section to link to at construction time as we don't know what it will be yet, in all other cases we do know. Alternatives:
- 2 constructors, One with no argument (IsSentinel = True), One with an InputSection * argument (IsSentinel = False). This does duplicate quite a bit of code though.
- Set IsSentinel = true when we assign the highest InputSection to be the Sentinel Section Link.

Any preferences?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58047/new/

https://reviews.llvm.org/D58047