[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:29:56 PDT 2019


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/e0f2427a/attachment.html>


More information about the llvm-commits mailing list