[llvm] r370032 - [yaml2obj] - Don't allow setting StOther and Other/Visibility at the same time.
Vlad Tsyrklevich via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 28 06:14:46 PDT 2019
I've reverted this change, it was causing check-llvm failures on the MSan
bot:
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/34675/steps/check-lld%20msan/logs/stdio
On Tue, Aug 27, 2019 at 2:57 AM George Rimar via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> Author: grimar
> Date: Tue Aug 27 02:58:39 2019
> New Revision: 370032
>
> URL: http://llvm.org/viewvc/llvm-project?rev=370032&view=rev
> Log:
> [yaml2obj] - Don't allow setting StOther and Other/Visibility at the same
> time.
>
> This is a follow up discussed in the comments of D66583.
>
> Currently, if for example, we have both StOther and Other set in YAML
> document for a symbol,
> then yaml2obj reports an "unknown key 'Other'" error.
> It happens because 'mapOptional()' is never called for 'Other/Visibility'
> in this case,
> leaving those unhandled.
>
> This message does not describe the reason of the error well. This patch
> fixes it.
>
> Differential revision: https://reviews.llvm.org/D66642
>
> Modified:
> llvm/trunk/include/llvm/ObjectYAML/ELFYAML.h
> llvm/trunk/lib/ObjectYAML/ELFEmitter.cpp
> llvm/trunk/lib/ObjectYAML/ELFYAML.cpp
> llvm/trunk/test/tools/yaml2obj/elf-symbol-stother.yaml
>
> Modified: llvm/trunk/include/llvm/ObjectYAML/ELFYAML.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ObjectYAML/ELFYAML.h?rev=370032&r1=370031&r2=370032&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/ObjectYAML/ELFYAML.h (original)
> +++ llvm/trunk/include/llvm/ObjectYAML/ELFYAML.h Tue Aug 27 02:58:39 2019
> @@ -107,7 +107,12 @@ struct Symbol {
> ELF_STB Binding;
> llvm::yaml::Hex64 Value;
> llvm::yaml::Hex64 Size;
> - uint8_t Other;
> + Optional<uint8_t> Other;
> +
> + // This can be used to set any custom value for the st_other field
> + // when it is not possible to do so using the "Other" field, which only
> takes
> + // specific named constants.
> + Optional<uint8_t> StOther;
> };
>
> struct SectionOrType {
>
> Modified: llvm/trunk/lib/ObjectYAML/ELFEmitter.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ObjectYAML/ELFEmitter.cpp?rev=370032&r1=370031&r2=370032&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/ObjectYAML/ELFEmitter.cpp (original)
> +++ llvm/trunk/lib/ObjectYAML/ELFEmitter.cpp Tue Aug 27 02:58:39 2019
> @@ -464,7 +464,12 @@ toELFSymbols(NameToIdxMap &SN2I, ArrayRe
> }
> // else Symbol.st_shndex == SHN_UNDEF (== 0), since it was zero'd
> earlier.
> Symbol.st_value = Sym.Value;
> - Symbol.st_other = Sym.Other;
> +
> + if (Sym.Other)
> + Symbol.st_other = *Sym.Other;
> + else if (Sym.StOther)
> + Symbol.st_other = *Sym.StOther;
> +
> Symbol.st_size = Sym.Size;
> }
>
>
> Modified: llvm/trunk/lib/ObjectYAML/ELFYAML.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ObjectYAML/ELFYAML.cpp?rev=370032&r1=370031&r2=370032&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/ObjectYAML/ELFYAML.cpp (original)
> +++ llvm/trunk/lib/ObjectYAML/ELFYAML.cpp Tue Aug 27 02:58:39 2019
> @@ -866,15 +866,28 @@ void MappingTraits<ELFYAML::ProgramHeade
> namespace {
>
> struct NormalizedOther {
> - NormalizedOther(IO &)
> - : Visibility(ELFYAML::ELF_STV(0)), Other(ELFYAML::ELF_STO(0)) {}
> - NormalizedOther(IO &, uint8_t Original)
> - : Visibility(Original & 0x3), Other(Original & ~0x3) {}
> + NormalizedOther(IO &) {}
> + NormalizedOther(IO &, Optional<uint8_t> Original) {
> + if (uint8_t Val = *Original & 0x3)
> + Visibility = Val;
> + if (uint8_t Val = *Original & ~0x3)
> + Other = Val;
> + }
>
> - uint8_t denormalize(IO &) { return Visibility | Other; }
> + Optional<uint8_t> denormalize(IO &) {
> + if (!Visibility && !Other)
> + return None;
> +
> + uint8_t Ret = 0;
> + if (Visibility)
> + Ret |= *Visibility;
> + if (Other)
> + Ret |= *Other;
> + return Ret;
> + }
>
> - ELFYAML::ELF_STV Visibility;
> - ELFYAML::ELF_STO Other;
> + Optional<ELFYAML::ELF_STV> Visibility;
> + Optional<ELFYAML::ELF_STO> Other;
> };
>
> } // end anonymous namespace
> @@ -896,17 +909,16 @@ void MappingTraits<ELFYAML::Symbol>::map
> // targets (e.g. MIPS) may use it to specify the named bits to set (e.g.
> // STO_MIPS_OPTIONAL). For producing broken objects we want to allow
> writing
> // any value to st_other. To do this we allow one more field called
> "StOther".
> - // If it is present in a YAML document, we set st_other to that integer,
> - // ignoring the other fields.
> - Optional<llvm::yaml::Hex64> Other;
> - IO.mapOptional("StOther", Other);
> - if (Other) {
> - Symbol.Other = *Other;
> - } else {
> - MappingNormalization<NormalizedOther, uint8_t> Keys(IO, Symbol.Other);
> - IO.mapOptional("Visibility", Keys->Visibility, ELFYAML::ELF_STV(0));
> - IO.mapOptional("Other", Keys->Other, ELFYAML::ELF_STO(0));
> - }
> + // If it is present in a YAML document, we set st_other to its integer
> value
> + // whatever it is.
> + // obj2yaml should not print 'StOther', it should print 'Visibility' and
> + // 'Other' fields instead.
> + assert(!IO.outputting() || !Symbol.StOther.hasValue());
> + IO.mapOptional("StOther", Symbol.StOther);
> + MappingNormalization<NormalizedOther, Optional<uint8_t>> Keys(IO,
> +
> Symbol.Other);
> + IO.mapOptional("Visibility", Keys->Visibility);
> + IO.mapOptional("Other", Keys->Other);
> }
>
> StringRef MappingTraits<ELFYAML::Symbol>::validate(IO &IO,
> @@ -915,6 +927,8 @@ StringRef MappingTraits<ELFYAML::Symbol>
> return "Index and Section cannot both be specified for Symbol";
> if (Symbol.NameIndex && !Symbol.Name.empty())
> return "Name and NameIndex cannot both be specified for Symbol";
> + if (Symbol.StOther && Symbol.Other)
> + return "StOther cannot be specified for Symbol with either Visibility
> or Other";
> return StringRef();
> }
>
>
> Modified: llvm/trunk/test/tools/yaml2obj/elf-symbol-stother.yaml
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/yaml2obj/elf-symbol-stother.yaml?rev=370032&r1=370031&r2=370032&view=diff
>
> ==============================================================================
> --- llvm/trunk/test/tools/yaml2obj/elf-symbol-stother.yaml (original)
> +++ llvm/trunk/test/tools/yaml2obj/elf-symbol-stother.yaml Tue Aug 27
> 02:58:39 2019
> @@ -77,3 +77,32 @@ Symbols:
> StOther: 4
> - Name: bar
> StOther: 0xff
> +
> +## Check we can't set StOther for a symbol if Visibility or Other is also
> specified.
> +
> +# RUN: not yaml2obj --docnum=5 2>&1 %s | FileCheck %s --check-prefix=ERR2
> +# RUN: not yaml2obj --docnum=6 2>&1 %s | FileCheck %s --check-prefix=ERR2
> +
> +# ERR2: error: StOther cannot be specified for Symbol with either
> Visibility or Other
> +
> +--- !ELF
> +FileHeader:
> + Class: ELFCLASS32
> + Data: ELFDATA2LSB
> + Type: ET_REL
> + Machine: EM_MIPS
> +Symbols:
> + - Name: foo
> + StOther: 0
> + Other: [ STO_MIPS_OPTIONAL ]
> +
> +--- !ELF
> +FileHeader:
> + Class: ELFCLASS32
> + Data: ELFDATA2LSB
> + Type: ET_REL
> + Machine: EM_MIPS
> +Symbols:
> + - Name: foo
> + StOther: 0
> + Visibility: STV_DEFAULT
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190828/9243d9e0/attachment.html>
More information about the llvm-commits
mailing list