[llvm-objcopy] --set-section-flags: allow "large" to add SHF_X86_64_LARGE

[tkoeppe]

Summary

Currently, objcopy cannot set the new flag SHF_X86_64_LARGE. This change introduces the named flag "large" which translates to that section flag.

An "invalid argument" error is produced if a user attempts to set the flag on an architecture other than X86_64.

[aeubanks]

can you include a diff with more context? https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

[aeubanks]

yeah we should check if Config.OutputArch is ELF::EM_X86_64, in line with https://reviews.llvm.org/D148358

[rnk]

llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp

86

I think the thing to do is to:

• Pass in the elf machine
• Make NewFlags an output parameter (out parameters seem more consistent with local code than ErrorOr)
• Return an Error instead
• While handling the generic SecLarge flag, check the ELF machine, produce an error for non x86_64 machine values
• Propagate that up out of setSectionFlagsAndType, whose caller can return Error

[tkoeppe]

In D153262#4444479, @aeubanks wrote:

can you include a diff with more context? https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

Yes, sorry, I forgot about that. Will do that for the next revision for sure!

[tkoeppe]

In D153262#4444492, @aeubanks wrote:

yeah we should check if Config.OutputArch is ELF::EM_X86_64, in line with https://reviews.llvm.org/D148358

And what do we do if the flag is requested but we're on the wrong architecture? A fatal error? That might be better than silently ignoring an nonsensical request.

I see rnk provided a comprehensive strategy, thanks!

[aeubanks]

In D153262#4444522, @tkoeppe wrote:

In D153262#4444492, @aeubanks wrote:

yeah we should check if Config.OutputArch is ELF::EM_X86_64, in line with https://reviews.llvm.org/D148358

And what do we do if the flag is requested but we're on the wrong architecture? A fatal error? That might be better than silently ignoring an nonsensical request.

yes and we should say that it's only supported on x86-64 in the error message

[tkoeppe]

Made it an error to try to set the "large" flag when not on an x86_64 architecture.

[tkoeppe]

Fixed return type of setSectionFlagsAndType.

[jhenderson]

Could you add some tests please?

llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp

74

EM isn't a particularly self-descriptive name. Let's call it EMachine like it's called in other places. Same goes throughout the changes.

86

I would prefer this to return an Expected<uint64_t> with the new flags. It's easier to work with than an output parameter + Error return. Otherwise, I agree with the rest of these comments (which I see you've already addressed).

96

I'm not convinced this is the right error value - you could argue that the input flag type is invalid, rather than the file type, so this would end up being errc::invalid_argument, I think.

97

I don't have a good solution to this currently, but the use of the command-line term in a libary function is a little concerning. A future tool might use a different name for the SHF_X86_64_LARGE flag, which would then make this error non-sensical. We should avoid referring to the command-line term this deep. For now, you could probably get away with referring directly to the flag name "SHF_X86_64_LARGE", and we can then change it to something else (e.g. SHF_*_LARGE) if another machine gains support.

705–707

I don't think this is the correct way of finding the machine architecture at this point in the code. If Config.OutputArch isn't set, the result will be a value that doesn't support EM_X86_64_LARGE, even though the actual object being created might be an X86_64 object (because it was already one). Take a look at how OutputElfType is created in executeObjcopyOnBinary for example.

[tkoeppe]

In D153262#4448043, @jhenderson wrote:

Could you add some tests please?

Could you please advise how to add an objcpy test that only runs under x86_64?

llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp

86

Sure thing, changed to Expected.

96

Yes, makes sense, changed!

97

I see, makes sense. I was thinking of giving the user a clue what they did wrong, and both the command line flag and this code are particular to objcpy, but I see your point. Changed.

[tkoeppe]

Addressed some review comments.

[tkoeppe]

Minor fixes.

[tkoeppe]

Changed how EMachine is determined; now only using Config.OutputArch if that is available, otherwise Obj.Machine.

llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp

705–707

Aha, I'm now using Config.OutputArch ? Config.OutputArch->EMachine : Obj.Machine, PTAL.

[tkoeppe]

Added test, updated PreserveMask to include ~SHF_X86_64_LARGE.

[tkoeppe]

In D153262#4448104, @tkoeppe wrote:

In D153262#4448043, @jhenderson wrote:

Could you add some tests please?

Could you please advise how to add an objcpy test that only runs under x86_64?

I just added it unconditionally to llvm/test/tools/llvm-objcopy/ELF/set-section-flags.test for now. PTAL.

[jhenderson]

One more thing: you need to add the new flag value to the list of supported flags and their meaning in the llvm-objcopy command guide (llvm/docs/CommandGuide).

llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp

97

Yeah, I get what your point is, but as this is a library, we can't assume all future users will have the same interface. You could do some more significant changes, if you wanted, which would allow us to get the "textual" version of the flag reported. One idea would be to simply pass the map of SectionFlag enum values to their textual versions into this method. Alternatively, a callback that takes a SectionFlag and converts it to the relevant form. I don't think it's necessary for this patch, but it's an option, if you want to improve the message.

104

Please make sure there is testing for this change.

There's also a potential issue here, if I'm not mistaken: on other architectures, there might be a flag with the same value as SHF_X86_64_LARGE, but which should be handled independently. You'll want a test case for this to show what should happen.

118–121

I'll be honest, I don't consider this code readable. I'd prefer the NewFlags initialization to be outside the if, as per the inline edit. Also, the use of auto hdies that this is an Expected return, which doesn't seem desirable.

705–707

Looks better. Please make sure you have testing that shows the behaviour with and without Config.OutputArch set (both to a supporting architecture and not) and with and without an Obj.Machine that supports the new flag. I believe that means you need 6 tests?

• Config.OutputArch not set, Obj.Machine is EM_X86_64.
• Config.OutputArch not set, Obj.Machine is not EM_X86_64.
• Config.OutputArch set to x86-64, Obj.Machine is EM_X86_64.
• Config.OutputArch set to x86-64, Obj.Machine is not EM_X86_64.
• Config.OutputArch set to other machine, Obj.Machine is EM_X86_64.
• Config.OutputArch set to other machine, Obj.Machine is not EM_X86_64.

725–726

Rather than repeat this code, would it make more sense to pass in the final value from outside the function?

[tkoeppe]

Review comments.

llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp

104

Hm, an interesting wrinke:

According to https://github.com/llvm/llvm-project/commit/d67c1e129bfd6afa756fed7e3988110bf3d70260#diff-38b4803de5adf661f3d9c7e5e1602a1f097bb79a7828cf31b9c629eb3af94fd1R18, it seems that we actually want to _preserve_ the flag, and not allow it to be changed by the user. That seems unfortunate.

What do you think, can we reverse that decision?

118–121

I can replace the auto with the proper type, which is definitely an improvement.

I'd say keeping the scope small is generally a win, since there's less one has to keep in mind for longer than necessary. But if you strongly prefer your style, we can of course use that.

725–726

Wait, yes, we don't need to compute this at all, since Obj.Machine is already set to the right value on L624, so we can just use that directly. PTAL.

[tkoeppe]

llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp

104

And you're right that the same numeric value is used for other flags in other architectures!

I think I need some overall guidance here:

• We currently _preserve_ some so-called "OS flags".
• Several different archs use the same numeric value for one of their OS flags.
• But now we actually _don't_ want to preserve the "LARGE" flag but make it configurable.

Is that even a workable idea? It'd potentially break existing users who have been silently propagating the flag (even if that's unlikely). But we really need a way to set this flag on our own custom sections that we assemble by other means.

There seems to be a limitation of the current objcpy interface: it doesn't explicitly allow distinguishing "set", "clear", and "leave as is"; rather the set of volatile/preserved flags is somehow nebulously hardcoded, and there doesn't seem an evolutionary path to promote a flag from preserved to volatile.

What do you reckon?

[jhenderson]

llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp

104

First up, and most importantly, we should aim for compatibility with GNU objcopy. As this is a new feature (at least, I'm assuming it's not available in GNU objcopy yet), you should discuss with them a) the flag name in the command-line (i.e. "large") and b) how this should/should not be preserved.

Asssuming we implement this feature as you are planning, I think it's reasonable to do the following:

1. Flags that are processor specific should be "preserved" when the machine doesn't have that numeric flag value as a known OS flag. It's unclear what should happen if you are trying to change the machine type in this case (in other words, is it the new or old OS that's important?).
2. Flags that are processor specific that are known for the relevant machine should NOT be preserved. However, we would need to consider backwards compatibility here: if a user is currently modifying the flags of a section that has the SHF_X86_64_LARGE flag, if we no longer preserve it by default, they will suddenly find the flag is lost when they update their version of llvm-objcopy. This seems a little unfriendly. The alternative is to preserve the flag if it exists, or add it if it doesn't and is requested, but this means users won't have the ability to drop the flag. Again, it may be worth discussing with the GNU developers. A migration path might be to introduce a new command-line option to specifically remove flags, and then leave some legacy behaviour in --set-section-flags to maintain the current behaviour there (whilst still allowing that option to add the flag, as suggested).

@MaskRay, any thoughts on this?

118–121

I'm not convinced you have to keep anything in mind. Here's my rationale:

1. A variable that isn't used has no impact on the later code (obviously), so it doesn't matter if it is still in scope.
2. A variable that is used later needs to be available, so it obviously still needs to be in scope.
3. A variable that is redeclared later on will result in a compiler error. This will suggest that you either need to start using the existing variable (i.e. transition it from 1) to 2)) or to give the new one (or the old one) a different name.

The only cases I can think of where an existing variable could trip you up are:
a) when you create a new variable in the same scope, but forget to give it a type, and therefore accidentally reuse an existing variable.
b) when you create a new variable in an inner scope, and therefore start hiding the existing variable.
In the first of these two cases either it doesn't matter - if the variable wasn't used after this point, then overwriting it isn't an issue, whilst if the variable was used after this point, we're not in a position where limiting scope would be relevant. In the second case, at least with Visual Studio you can have warning levels turned up to warn you about this, but regardless, you don't impact the outer variables behaviour at all.

(Sorry, that ended up being a longer thought process than I intended!)

[MaskRay]

The GNU objcopy feature request is https://sourceware.org/bugzilla/show_bug.cgi?id=30592

llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp

104

I think GNU objcopy's current behavior is to preserve all SHF_MASKOS | SHF_MASKPROC bits. For EM_X86_64 objects, it will make sense to not preserve SHF_X86_64_LARGE (if "large" is not specified with --set-section-flags, drop the section flag).

I think our behavior should behave like this as well. If a non-x86-64 object has the flag, we should preserve it, e.g. SHF_MIPS_GPREL == SHF_X86_64_LARGE.

I don't worry about compatibility here. The default -mlarge-data-threshold= is 65536 and it's difficult to get SHF_X86_64_LARGE sections with GCC (Clang doesn't emit this yet). I do not find real use cases of -mlarge-data-threshold= on https://sourcegraph.com/search .

llvm/test/tools/llvm-objcopy/ELF/set-section-flags.test

97 ↗

(On Diff #534952)

Consider adding a section with SHF_X86_64_FLAG and calling objcopy --set-section-flags on it but without specifying "large". Test that the flag is cleared.

[MaskRay]

A more descriptive subject may be: [llvm-objcopy] --set-section-flags: allow "large" to add SHF_X86_64_LARGE

[tkoeppe]

Many thanks, @MaskRay!

I believe GNU objcopy doesn't have a "large" flag at the moment, but it does preserve any existing SHF_X86_64_LARGE sectopm flags. I also agree that it seems OK to change this, but I don't have a good industry-wise overview.

@jhenderson: Do you think this is acceptable?

[tkoeppe]

Made SHF_X86_64_LARGE flag non-preserved only on x86_64; added tests that the section flag is unset when the corresponding command line flag is absent.

llvm/test/tools/llvm-objcopy/ELF/set-section-flags.test

97 ↗

(On Diff #534952)

Done, I'm just using the previous file and calling --set-section-flags again without large.

[jhenderson]

clang-format is complaining in the pre-merge checks. Please make sure you have run it on all your new code.

llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp

104

All sounds good to me, thanks @MaskRay.

705–707

Pinging this comment about testing.

llvm/test/tools/llvm-objcopy/ELF/rename-section-flag-osproc-mask.test

30 ↗

(On Diff #536757)

Probably should have a blank line between the two blocks, for readability.

[tkoeppe]

In D153262#4470305, @jhenderson wrote:

clang-format is complaining in the pre-merge checks. Please make sure you have run it on all your new code.

Sorrry, I thought I had done that, but I must have missed a final pass -- fixing in the next revision!

llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp

705–707

Yes, will do, I just wanted to get agreement on the overall plan first!

[MaskRay]

llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp

89

This can be tested when you add a non-EM_X86_64 test.

section flag 'large' ... may be more appropriate since for a non-EM_X86_64 machine, the flag is not called SHF_X86_64_LARGE.

[jhenderson]

llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp

89

At the moment, the "large" option is only valid for X86_64, hence there's a one-to-one mapping of the option name to the flag name, and it's therefore safe to use the term on non-X86-64 machines (because "large" means "set the SHF_X86_64_LARGE" flag specifically).

As stated earlier, I don't think mentioning a command-line term in a library is a great idea, because another tool might use another mechanism to configure SecLarge. We could pass in a mapping of flag names (e.g. SecLarge) to their textual representation (e.g. large) so that the calling code could configure it appropriately, but that seems a bit overkill at the moment.

[tkoeppe]

Updated both the "guide" manual and the usage help text.

Added a new "multiarch" test file that tests converting from x86_64 to MIPS and back.

[tkoeppe]

In D153262#4451491, @jhenderson wrote:

One more thing: you need to add the new flag value to the list of supported flags and their meaning in the llvm-objcopy command guide (llvm/docs/CommandGuide).

Done.

llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp

705–707

I've added a new file with a set of tests now. Notes:

• When the target is non-EM_X86_64, it is an error to set the "large" flag (this is tested).
• When not setting a target, the preservation or lack thereof of the "large" flag is tested by rename-section-flag-osproc-mask.test.
• When converting from EM_X86_64 to non-EM_X86_64, any present "large" flags are reinterpreted to the target's interpretation of that flag value.
• When converting from non-EM_X86_64 to EM_X86_64 and --set-section-flag is used, the "large" flag is set based on whether it is specified on the command line. When --set-section-flag is not used, existing flags are reinterpreted.

[MaskRay]

llvm/test/tools/llvm-objcopy/ELF/set-section-flags-large-multiarch.test

4 ↗

(On Diff #540193)

The long option --output-target is fine, but the short -O is much more common and can make the RUN line easier to read.

37 ↗

(On Diff #540193)

We may need two SHF_X86_64_LARGE sections, one operated on by --set-section-flags. This way we can demonstrate that the untouched section keeps having the SHF_X86_64_LARGE flag.

llvm/tools/llvm-objcopy/ObjcopyOpts.td

88 ↗

(On Diff #540193)

While here, remove the trailing period. I think for TableGen help messages the convention is to omit the period. Some options may be inconsistent, just ignore them:)

[MaskRay]

llvm/docs/CommandGuide/llvm-objcopy.rst

155 ↗

(On Diff #540193)

architecture is not x86_64.

I think the "target format" term is somewhat ambiguous. Sometimes it refers specifically to the object file format (LLVM may call it BinaryFormat): ELF, COFF, etc. Here we just mean the architecture (sometimes called processor or machine, but I find "architecture" better).

[tkoeppe]

llvm/docs/CommandGuide/llvm-objcopy.rst

155 ↗

(On Diff #540193)

Done, changed to "architecture".

[tkoeppe]

Wording/punctuation tweaks in the help text. Added another, untouched section to the new unit test.

[aeubanks]

llvm/docs/CommandGuide/llvm-objcopy.rst

155 ↗

(On Diff #540205)

[jhenderson]

llvm/docs/CommandGuide/llvm-objcopy.rst

141 ↗

(On Diff #540205)

Perhaps worth extending this to say "all object file formats or target architectures"?

155 ↗

(On Diff #540205)

I found "this" somewhat awkward in the comment (the semi-colon is still needed though). How about "rejected if target architecture is not x86_64"?

llvm/test/tools/llvm-objcopy/ELF/set-section-flags-large-multiarch.test

10 ↗

(On Diff #540205)

As you don't have many different variant test cases which have different expected outputs, you don't need so many check-prefixes. I suggest reducing them down to just the REINTERPRET* prefixes and one other (e.g. CHECK).

21 ↗

(On Diff #540205)

As this is only used in one case, I recommend moving it to immediately after that case's RUN line. Also, I suggest including the "error: " prefix, as it helps ensure we've captured the whole message.

30–32 ↗

(On Diff #540205)

Is it worth having a version of this section (i.e. one without SHF_X86_64_LARGE) that isn't operated on by --set-section-flags, just to complete the test matrix? (Possibly not)

[tkoeppe]

I cannot figure out why clang-format rejects llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp: when I run the formatter script (clang/tools/clang-format/git-clang-format llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp) it doesn't make any changes.

llvm/test/tools/llvm-objcopy/ELF/set-section-flags-large-multiarch.test

30–32 ↗

(On Diff #540205)

Yes, because there's a difference in behaviour: if you don't operate on the section, the flag is reinterpreserved when you change architectures, but if you do operate on it, then it might get dropped. I initially had a separate objcopy run for this, but maskray@ suggested just rolling that into a section that's not operated on.

37 ↗

(On Diff #540193)

@jhenderson (See this comment.)

[tkoeppe]

llvm/test/tools/llvm-objcopy/ELF/set-section-flags-large-multiarch.test

10 ↗

(On Diff #540205)

Oh yes, I see, nice. Done.

[tkoeppe]

Review comments: Simplify tests, improve documentation

[jhenderson]

In D153262#4518480, @tkoeppe wrote:

I cannot figure out why clang-format rejects llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp: when I run the formatter script (clang/tools/clang-format/git-clang-format llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp) it doesn't make any changes.

Is it possible there's a difference in versions between the clang-format being run by the pre-merge checks and the version you're using? Anyway, see my inline edit. Hopefully that solves the pre-merge check issue.

llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp

97–98

When I run my local clang-format on this code, I get the inline edit.

llvm/test/tools/llvm-objcopy/ELF/set-section-flags-large-multiarch.test

30–32 ↗

(On Diff #540205)

I think you misunderstood my comment. At the moment, you have the following three cases (one case per section):

section without flag, specified by --set-section-flags (.foo)
section with flag, specified by --set-section-flags (.bar)
section with flag, not specified by --set-section-flags (.untouched)

I'm suggesting you add the missing case:
section without flag, not specified by --set-section-flags

Hopefully that case is trivial, but given you've got the rest of the test matrix, I think having coverage for that final combination would make sense.

[tkoeppe]

llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp

97–98

Aha, I see! With my original format, clang-format makes no changes. But if I make a local edit to put everything on a single line and _then_ run clang-format, then it produces your version.

It seems like both versions are acceptable to clang-format, but if it needs to make a change, it prefers your version?

[tkoeppe]

llvm/test/tools/llvm-objcopy/ELF/set-section-flags-large-multiarch.test

30–32 ↗

(On Diff #540205)

Ah yes, of course, I did indeed misread your message. Sorry! Adding the test.

[tkoeppe]

Add another test case; fix formatting.

[jhenderson]

LGTM, thanks!

[tkoeppe]

In D153262#4531443, @jhenderson wrote:

LGTM, thanks!

Likewise, thank _you_ for the review! This is going to be great help for our large binaries, much appreciated.

llvm/include/llvm/ObjCopy/CommonConfig.h

enum SectionFlag {
   SecCode = 1 << 5,
   SecData = 1 << 6,
   SecRom = 1 << 7,
   SecMerge = 1 << 8,
   SecStrings = 1 << 9,
   SecContents = 1 << 10,
   SecShare = 1 << 11,
   SecExclude = 1 << 12,
-  LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/SecExclude)
+  SecLarge = 1 << 13,
+  LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/SecLarge)
 };
 
 struct SectionRename {
   StringRef OriginalName;
   StringRef NewName;
   std::optional<SectionFlag> NewFlags;
 };
 

llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp

static bool onlyKeepDWOPred(const Object &Obj, const SectionBase &Sec) {
   // We can't remove the section header string table.
   if (&Sec == Obj.SectionNames)
     return false;
   // Short of keeping the string table we want to keep everything that is a DWO
   // section and remove everything else.
   return !isDWOSection(Sec);
 }
 
-static uint64_t getNewShfFlags(SectionFlag AllFlags) {
+static Expected<uint64_t> getNewShfFlags(SectionFlag AllFlags,
+                                         uint16_t EMachine) {
   uint64_t NewFlags = 0;
   if (AllFlags & SectionFlag::SecAlloc)
     NewFlags |= ELF::SHF_ALLOC;
   if (!(AllFlags & SectionFlag::SecReadonly))
     NewFlags |= ELF::SHF_WRITE;
   if (AllFlags & SectionFlag::SecCode)
     NewFlags |= ELF::SHF_EXECINSTR;
   if (AllFlags & SectionFlag::SecMerge)
     NewFlags |= ELF::SHF_MERGE;
   if (AllFlags & SectionFlag::SecStrings)
     NewFlags |= ELF::SHF_STRINGS;
   if (AllFlags & SectionFlag::SecExclude)
     NewFlags |= ELF::SHF_EXCLUDE;
+  if (AllFlags & SectionFlag::SecLarge) {
+    if (EMachine != EM_X86_64)
+      return createStringError(errc::invalid_argument,
+                               "section flag SHF_X86_64_LARGE can only be used "
+                               "with x86_64 architecture");
+    NewFlags |= ELF::SHF_X86_64_LARGE;
+  }
   return NewFlags;
 }
 
 static uint64_t getSectionFlagsPreserveMask(uint64_t OldFlags,
                                             uint64_t NewFlags) {
   // Preserve some flags which should not be dropped when setting flags.
   // Also, preserve anything OS/processor dependant.
   const uint64_t PreserveMask =
       (ELF::SHF_COMPRESSED | ELF::SHF_GROUP | ELF::SHF_LINK_ORDER |
        ELF::SHF_MASKOS | ELF::SHF_MASKPROC | ELF::SHF_TLS |
        ELF::SHF_INFO_LINK) &
       ~ELF::SHF_EXCLUDE;
   return (OldFlags & PreserveMask) | (NewFlags & ~PreserveMask);
 }
 
 static void setSectionType(SectionBase &Sec, uint64_t Type) {
   // If Sec's type is changed from SHT_NOBITS due to --set-section-flags,
   // Offset may not be aligned. Align it to max(Align, 1).
   if (Sec.Type == ELF::SHT_NOBITS && Type != ELF::SHT_NOBITS)
     Sec.Offset = alignTo(Sec.Offset, std::max(Sec.Align, uint64_t(1)));
   Sec.Type = Type;
 }
 
-static void setSectionFlagsAndType(SectionBase &Sec, SectionFlag Flags) {
-  Sec.Flags = getSectionFlagsPreserveMask(Sec.Flags, getNewShfFlags(Flags));
+static Error setSectionFlagsAndType(SectionBase &Sec, SectionFlag Flags,
+                                    uint16_t EMachine) {
+  if (auto NewFlags = getNewShfFlags(Flags, EMachine); !NewFlags)
+    return NewFlags.takeError();
+  else
+    Sec.Flags = getSectionFlagsPreserveMask(Sec.Flags, *NewFlags);
 
   // In GNU objcopy, certain flags promote SHT_NOBITS to SHT_PROGBITS. This rule
   // may promote more non-ALLOC sections than GNU objcopy, but it is fine as
   // non-ALLOC SHT_NOBITS sections do not make much sense.
   if (Sec.Type == SHT_NOBITS &&
       (!(Sec.Flags & ELF::SHF_ALLOC) ||
        Flags & (SectionFlag::SecContents | SectionFlag::SecLoad)))
     setSectionType(Sec, ELF::SHT_PROGBITS);
+
+  return Error::success();
 }
 
 static ElfType getOutputElfType(const Binary &Bin) {
   // Infer output ELF type from the input ELF object
   if (isa<ELFObjectFile<ELF32LE>>(Bin))
     return ELFT_ELF32LE;
   if (isa<ELFObjectFile<ELF64LE>>(Bin))
     return ELFT_ELF64LE;
for (const NewSymbolInfo &SI : Config.SymbolsToAdd)
     addSymbol(Obj, SI, ELFConfig.NewSymbolVisibility);
 
   // --set-section-{flags,type} work with sections added by --add-section.
   if (!Config.SetSectionFlags.empty() || !Config.SetSectionType.empty()) {
     for (auto &Sec : Obj.sections()) {
       const auto Iter = Config.SetSectionFlags.find(Sec.Name);
       if (Iter != Config.SetSectionFlags.end()) {
         const SectionFlagsUpdate &SFU = Iter->second;
-        setSectionFlagsAndType(Sec, SFU.NewFlags);
+        uint16_t EMachine = Config.OutputArch.value_or(MachineInfo()).EMachine;
+        if (Error E = setSectionFlagsAndType(Sec, SFU.NewFlags, EMachine))
+          return E;
       }
       auto It2 = Config.SetSectionType.find(Sec.Name);
       if (It2 != Config.SetSectionType.end())
         setSectionType(Sec, It2->second);
     }
   }
 
   if (!Config.SectionsToRename.empty()) {
     std::vector<RelocationSectionBase *> RelocSections;
     DenseSet<SectionBase *> RenamedSections;
     for (SectionBase &Sec : Obj.sections()) {
       auto *RelocSec = dyn_cast<RelocationSectionBase>(&Sec);
       const auto Iter = Config.SectionsToRename.find(Sec.Name);
       if (Iter != Config.SectionsToRename.end()) {
         const SectionRename &SR = Iter->second;
         Sec.Name = std::string(SR.NewName);
-        if (SR.NewFlags)
-          setSectionFlagsAndType(Sec, *SR.NewFlags);
+        if (SR.NewFlags) {
+          uint16_t EMachine =
+              Config.OutputArch.value_or(MachineInfo()).EMachine;
+          if (Error E = setSectionFlagsAndType(Sec, *SR.NewFlags, EMachine))
+            return E;
+        }
         RenamedSections.insert(&Sec);
       } else if (RelocSec && !(Sec.Flags & SHF_ALLOC))
         // Postpone processing relocation sections which are not specified in
         // their explicit '--rename-section' commands until after their target
         // sections are renamed.
         // Dynamic relocation sections (i.e. ones with SHF_ALLOC) should be
         // renamed only explicitly. Otherwise, renaming, for example, '.got.plt'
         // would affect '.rela.plt', which is not desirable.

llvm/tools/llvm-objcopy/ObjcopyOptions.cpp

return llvm::StringSwitch<SectionFlag>(SectionName)
       .CaseLower("code", SectionFlag::SecCode)
       .CaseLower("data", SectionFlag::SecData)
       .CaseLower("rom", SectionFlag::SecRom)
       .CaseLower("merge", SectionFlag::SecMerge)
       .CaseLower("strings", SectionFlag::SecStrings)
       .CaseLower("contents", SectionFlag::SecContents)
       .CaseLower("share", SectionFlag::SecShare)
       .CaseLower("exclude", SectionFlag::SecExclude)
+      .CaseLower("large", SectionFlag::SecLarge)
       .Default(SectionFlag::SecNone);
 }
 
 static Expected<SectionFlag>
 parseSectionFlagSet(ArrayRef<StringRef> SectionFlags) {
   SectionFlag ParsedFlags = SectionFlag::SecNone;
   for (StringRef Flag : SectionFlags) {
     SectionFlag ParsedFlag = parseSectionRenameFlag(Flag);
     if (ParsedFlag == SectionFlag::SecNone)
       return createStringError(
           errc::invalid_argument,
           "unrecognized section flag '%s'. Flags supported for GNU "
           "compatibility: alloc, load, noload, readonly, exclude, debug, "
-          "code, data, rom, share, contents, merge, strings",
+          "code, data, rom, share, contents, merge, strings, large",
           Flag.str().c_str());
     ParsedFlags |= ParsedFlag;
   }
 
   return ParsedFlags;
 }
 
 static Expected<SectionRename> parseRenameSectionValue(StringRef FlagValue) {