[PATCH] D48792: [ARM] Set execute-only flags in .text.

[Ivan Lozano via Phabricator via llvm-commits]

ivanlozano added inline comments.


================
Comment at: lib/MC/ELFObjectWriter.cpp:1123
+      }
+    }
   }
----------------
echristo wrote:
> ivanlozano wrote:
> > eugenis wrote:
> > > My understanding of MC infrastructure is a bit fuzzy, but this looks like a strange fixup where in most cases (but not always) the section would have correct flags set up already.
> > > 
> > > Also, is hasInstructions() correct predicate? What if there are data fragments in .text?
> > > 
> > > Would it be possible (and would it solve the issue?) to track "contains non-instructions" property on any section in a target-independent way, and then set SHF_ARM_PURECODE later in ARM-specific code when we are ready to emit the binary? The way we would not need to access the target when an empty text section is created.
> > > 
> > You're probably right that we need to search for data fragments as well. The empty .text contains an empty MCDataFragment and MCAlignFragment, so just looking for the absence of these is insufficient.
> > 
> > If we go the route of a tracking property, what would be the best place to set this value? MCObjectStreamer::EmitBytes looks like a good candidate, though I'm not sure if this is the only place it needs to be set. Alternatively, we could define hasData() as a function that iterates through the fragment list and checks if any of the data fragments have a non-zero size.
> > 
> > There's some other architecture-specific code already in this file (EM_MIPS), so I initially figured it'd be alright to include this here. I'm not opposed to moving this to ARM-specific code, but I could use some pointers as to where that might be. Alternatively we could skip emitting empty .text sections on every architecture, though I'm not sure if this would cause other issues.
> > 
> > 
> Avoiding the empty .text sections could cause a bit of a compatibility issue, but it might be worth doing.
> 
> I added a comment above as to where you could add the target specific code.
> 
> I think the idea of tracking pure code at some level is a great idea. I don't know if it's worth tracking in the general MC layer versus additions in the ARM side of things. Reduce the general complexity and make the ARM backend handle it.
> 
> Thoughts?
If we avoid emitting empty .text sections, it causes ~100 tests to fail (at least when I tried this quickly). It's probably best to handle it in a separate patch-set to ensure the impact can be properly considered.


https://reviews.llvm.org/D48792