[llvm] r370032 - [yaml2obj] - Don't allow setting StOther and Other/Visibility at the same time.
George Rimar via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 28 06:28:01 PDT 2019
Ok. Sorry for breakage. I had no email from the bot :\
(The correct link seems is http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap-msan/builds/14395/steps/check-llvm%20msan/logs/stdio)
Best regards,
George | Developer | Access Softek, Inc
________________________________
От: Vlad Tsyrklevich <vlad at tsyrklevich.net>
Отправлено: 28 августа 2019 г. 16:14
Кому: George Rimar
Копия: Bbbbb
Тема: Re: [llvm] r370032 - [yaml2obj] - Don't allow setting StOther and Other/Visibility at the same time.
CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. If you suspect potential phishing or spam email, report it to ReportSpam at accesssoftek.com
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<mailto: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<mailto: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/bee9d037/attachment-0001.html>
More information about the llvm-commits
mailing list