[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:57:16 PDT 2019
Oof, you're right. It was the only ObjectYAML change I saw from yesterday
and I only verified it using manual inspection. My apology for the false
alarm, I'll reland your change and note why.
On Wed, Aug 28, 2019 at 6:49 AM George Rimar <grimar at accesssoftek.com>
wrote:
> One more question. Are you sure it was my commit?
>
>
> Bots are saying that something is wrong for MarchO:
>
> ********************
> Testing: 0 .. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..
> Testing Time: 135.85s
> ********************
> Failing Tests (3):
> LLVM :: ObjectYAML/MachO/DWARF-debug_info.yaml
> LLVM :: ObjectYAML/MachO/DWARF-debug_line.yaml
> LLVM :: ObjectYAML/MachO/DWARF5-debug_info.yaml
>
> My changes were for ELF and I think the code I changed is never called
> during these tests...
>
>
> Best regards,
> George | Developer | Access Softek, Inc
> ------------------------------
> *От:* Vlad Tsyrklevich <vlad at tsyrklevich.net>
> *Отправлено:* 28 августа 2019 г. 16:29
> *Кому:* George Rimar
> *Копия:* Bbbbb
> *Тема:* Re: [llvm] r370032 - [yaml2obj] - Don't allow setting StOther and
> Other/Visibility at the same time.
>
> Oops, included a link for a different breakage! Sorry about that.
>
> The reason you didn't get emails is because svn was down a chunk of
> yesterday so the sanitizer bots were functionally disabled.
>
> On Wed, Aug 28, 2019 at 6:28 AM George Rimar <grimar at accesssoftek.com>
> wrote:
>
>> 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> 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/9a9acca3/attachment-0001.html>
More information about the llvm-commits
mailing list