<div dir="ltr">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.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Aug 28, 2019 at 6:49 AM George Rimar <<a href="mailto:grimar@accesssoftek.com">grimar@accesssoftek.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div dir="ltr" style="font-size:12pt;color:rgb(0,0,0);background-color:rgb(255,255,255);font-family:Calibri,Arial,Helvetica,sans-serif">
<p>One more question. Are you sure it was my commit?<br>
</p>
<p><br>
</p>
<p>Bots are saying that something is wrong for MarchO:<br>
</p>
<pre style="font-family:"Courier New",courier,monotype,monospace"><span class="gmail-m_7189488183778529535stdout">********************
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</span></pre>
<p>My changes were for ELF and I think the code I changed is never called during these tests...<br>
</p>
<p><br>
</p>
<div id="gmail-m_7189488183778529535Signature">
<div name="divtagdefaultwrapper">
<div class="gmail-m_7189488183778529535BodyFragment"><font size="2">
<div class="gmail-m_7189488183778529535PlainText">Best regards,<br>
George | Developer | <span style="background-color:rgb(255,255,255);color:rgb(33,33,33);font-family:Calibri,sans-serif;font-size:13.3333px">Access Softek, Inc</span></div>
</font></div>
</div>
</div>
<div style="color:rgb(33,33,33)">
<hr style="display:inline-block;width:98%">
<div id="gmail-m_7189488183778529535divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>От:</b> Vlad Tsyrklevich <<a href="mailto:vlad@tsyrklevich.net" target="_blank">vlad@tsyrklevich.net</a>><br>
<b>Отправлено:</b> 28 августа 2019 г. 16:29<br>
<b>Кому:</b> George Rimar<br>
<b>Копия:</b> Bbbbb<br>
<b>Тема:</b> Re: [llvm] r370032 - [yaml2obj] - Don't allow setting StOther and Other/Visibility at the same time.</font>
<div> </div>
</div>
<div>
<div dir="ltr">Oops, included a link for a different breakage! Sorry about that.
<div><br>
</div>
<div>The reason you didn't get emails is because svn was down a chunk of yesterday so the sanitizer bots were functionally disabled.</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Wed, Aug 28, 2019 at 6:28 AM George Rimar <<a href="mailto:grimar@accesssoftek.com" target="_blank">grimar@accesssoftek.com</a>> wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div dir="ltr" style="font-size:12pt;color:rgb(0,0,0);background-color:rgb(255,255,255);font-family:Calibri,Arial,Helvetica,sans-serif">
<p>Ok. Sorry for breakage. I had no email from the bot :\<br>
</p>
<p><br>
</p>
<p>(The correct link seems is <a href="http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap-msan/builds/14395/steps/check-llvm%20msan/logs/stdio" target="_blank">http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap-msan/builds/14395/steps/check-llvm%20msan/logs/stdio</a>)<br>
</p>
<p><br>
</p>
<div id="gmail-m_7189488183778529535gmail-m_-9114016494626773977Signature">
<div name="divtagdefaultwrapper">
<div class="gmail-m_7189488183778529535gmail-m_-9114016494626773977BodyFragment"><font size="2">
<div class="gmail-m_7189488183778529535gmail-m_-9114016494626773977PlainText">Best regards,<br>
George | Developer | <span style="background-color:rgb(255,255,255);color:rgb(33,33,33);font-family:Calibri,sans-serif;font-size:13.3333px">Access Softek, Inc</span></div>
</font></div>
</div>
</div>
<div style="color:rgb(33,33,33)">
<hr style="display:inline-block;width:98%">
<div id="gmail-m_7189488183778529535gmail-m_-9114016494626773977divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>От:</b> Vlad Tsyrklevich <<a href="mailto:vlad@tsyrklevich.net" target="_blank">vlad@tsyrklevich.net</a>><br>
<b>Отправлено:</b> 28 августа 2019 г. 16:14<br>
<b>Кому:</b> George Rimar<br>
<b>Копия:</b> Bbbbb<br>
<b>Тема:</b> Re: [llvm] r370032 - [yaml2obj] - Don't allow setting StOther and Other/Visibility at the same time.</font>
<div> </div>
</div>
<div>
<div style="font-size:9pt;font-family:Calibri,sans-serif">
<h3 style="background-color:rgb(255,255,255);font-size:10pt;border:1px dotted rgb(0,51,51);padding:0.8em">
<span style="color:rgb(255,102,0)">CAUTION:<strong> </strong></span>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 <a href="mailto:ReportSpam@accesssoftek.com" target="_blank">ReportSpam@accesssoftek.com</a></h3>
</div>
<div>
<div dir="ltr">I've reverted this change, it was causing check-llvm failures on the MSan bot: <a href="http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/34675/steps/check-lld%20msan/logs/stdio" target="_blank">http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/34675/steps/check-lld%20msan/logs/stdio</a><br>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Tue, Aug 27, 2019 at 2:57 AM George Rimar via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Author: grimar<br>
Date: Tue Aug 27 02:58:39 2019<br>
New Revision: 370032<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=370032&view=rev" rel="noreferrer" target="_blank">
http://llvm.org/viewvc/llvm-project?rev=370032&view=rev</a><br>
Log:<br>
[yaml2obj] - Don't allow setting StOther and Other/Visibility at the same time.<br>
<br>
This is a follow up discussed in the comments of D66583.<br>
<br>
Currently, if for example, we have both StOther and Other set in YAML document for a symbol,<br>
then yaml2obj reports an "unknown key 'Other'" error.<br>
It happens because 'mapOptional()' is never called for 'Other/Visibility' in this case,<br>
leaving those unhandled.<br>
<br>
This message does not describe the reason of the error well. This patch fixes it.<br>
<br>
Differential revision: <a href="https://reviews.llvm.org/D66642" rel="noreferrer" target="_blank">
https://reviews.llvm.org/D66642</a><br>
<br>
Modified:<br>
llvm/trunk/include/llvm/ObjectYAML/ELFYAML.h<br>
llvm/trunk/lib/ObjectYAML/ELFEmitter.cpp<br>
llvm/trunk/lib/ObjectYAML/ELFYAML.cpp<br>
llvm/trunk/test/tools/yaml2obj/elf-symbol-stother.yaml<br>
<br>
Modified: llvm/trunk/include/llvm/ObjectYAML/ELFYAML.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ObjectYAML/ELFYAML.h?rev=370032&r1=370031&r2=370032&view=diff" rel="noreferrer" target="_blank">
http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ObjectYAML/ELFYAML.h?rev=370032&r1=370031&r2=370032&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/ObjectYAML/ELFYAML.h (original)<br>
+++ llvm/trunk/include/llvm/ObjectYAML/ELFYAML.h Tue Aug 27 02:58:39 2019<br>
@@ -107,7 +107,12 @@ struct Symbol {<br>
ELF_STB Binding;<br>
llvm::yaml::Hex64 Value;<br>
llvm::yaml::Hex64 Size;<br>
- uint8_t Other;<br>
+ Optional<uint8_t> Other;<br>
+<br>
+ // This can be used to set any custom value for the st_other field<br>
+ // when it is not possible to do so using the "Other" field, which only takes<br>
+ // specific named constants.<br>
+ Optional<uint8_t> StOther;<br>
};<br>
<br>
struct SectionOrType {<br>
<br>
Modified: llvm/trunk/lib/ObjectYAML/ELFEmitter.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ObjectYAML/ELFEmitter.cpp?rev=370032&r1=370031&r2=370032&view=diff" rel="noreferrer" target="_blank">
http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ObjectYAML/ELFEmitter.cpp?rev=370032&r1=370031&r2=370032&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/ObjectYAML/ELFEmitter.cpp (original)<br>
+++ llvm/trunk/lib/ObjectYAML/ELFEmitter.cpp Tue Aug 27 02:58:39 2019<br>
@@ -464,7 +464,12 @@ toELFSymbols(NameToIdxMap &SN2I, ArrayRe<br>
}<br>
// else Symbol.st_shndex == SHN_UNDEF (== 0), since it was zero'd earlier.<br>
Symbol.st_value = Sym.Value;<br>
- Symbol.st_other = Sym.Other;<br>
+<br>
+ if (Sym.Other)<br>
+ Symbol.st_other = *Sym.Other;<br>
+ else if (Sym.StOther)<br>
+ Symbol.st_other = *Sym.StOther;<br>
+<br>
Symbol.st_size = Sym.Size;<br>
}<br>
<br>
<br>
Modified: llvm/trunk/lib/ObjectYAML/ELFYAML.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ObjectYAML/ELFYAML.cpp?rev=370032&r1=370031&r2=370032&view=diff" rel="noreferrer" target="_blank">
http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ObjectYAML/ELFYAML.cpp?rev=370032&r1=370031&r2=370032&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/ObjectYAML/ELFYAML.cpp (original)<br>
+++ llvm/trunk/lib/ObjectYAML/ELFYAML.cpp Tue Aug 27 02:58:39 2019<br>
@@ -866,15 +866,28 @@ void MappingTraits<ELFYAML::ProgramHeade<br>
namespace {<br>
<br>
struct NormalizedOther {<br>
- NormalizedOther(IO &)<br>
- : Visibility(ELFYAML::ELF_STV(0)), Other(ELFYAML::ELF_STO(0)) {}<br>
- NormalizedOther(IO &, uint8_t Original)<br>
- : Visibility(Original & 0x3), Other(Original & ~0x3) {}<br>
+ NormalizedOther(IO &) {}<br>
+ NormalizedOther(IO &, Optional<uint8_t> Original) {<br>
+ if (uint8_t Val = *Original & 0x3)<br>
+ Visibility = Val;<br>
+ if (uint8_t Val = *Original & ~0x3)<br>
+ Other = Val;<br>
+ }<br>
<br>
- uint8_t denormalize(IO &) { return Visibility | Other; }<br>
+ Optional<uint8_t> denormalize(IO &) {<br>
+ if (!Visibility && !Other)<br>
+ return None;<br>
+<br>
+ uint8_t Ret = 0;<br>
+ if (Visibility)<br>
+ Ret |= *Visibility;<br>
+ if (Other)<br>
+ Ret |= *Other;<br>
+ return Ret;<br>
+ }<br>
<br>
- ELFYAML::ELF_STV Visibility;<br>
- ELFYAML::ELF_STO Other;<br>
+ Optional<ELFYAML::ELF_STV> Visibility;<br>
+ Optional<ELFYAML::ELF_STO> Other;<br>
};<br>
<br>
} // end anonymous namespace<br>
@@ -896,17 +909,16 @@ void MappingTraits<ELFYAML::Symbol>::map<br>
// targets (e.g. MIPS) may use it to specify the named bits to set (e.g.<br>
// STO_MIPS_OPTIONAL). For producing broken objects we want to allow writing<br>
// any value to st_other. To do this we allow one more field called "StOther".<br>
- // If it is present in a YAML document, we set st_other to that integer,<br>
- // ignoring the other fields.<br>
- Optional<llvm::yaml::Hex64> Other;<br>
- IO.mapOptional("StOther", Other);<br>
- if (Other) {<br>
- Symbol.Other = *Other;<br>
- } else {<br>
- MappingNormalization<NormalizedOther, uint8_t> Keys(IO, Symbol.Other);<br>
- IO.mapOptional("Visibility", Keys->Visibility, ELFYAML::ELF_STV(0));<br>
- IO.mapOptional("Other", Keys->Other, ELFYAML::ELF_STO(0));<br>
- }<br>
+ // If it is present in a YAML document, we set st_other to its integer value<br>
+ // whatever it is.<br>
+ // obj2yaml should not print 'StOther', it should print 'Visibility' and<br>
+ // 'Other' fields instead.<br>
+ assert(!IO.outputting() || !Symbol.StOther.hasValue());<br>
+ IO.mapOptional("StOther", Symbol.StOther);<br>
+ MappingNormalization<NormalizedOther, Optional<uint8_t>> Keys(IO,<br>
+ Symbol.Other);<br>
+ IO.mapOptional("Visibility", Keys->Visibility);<br>
+ IO.mapOptional("Other", Keys->Other);<br>
}<br>
<br>
StringRef MappingTraits<ELFYAML::Symbol>::validate(IO &IO,<br>
@@ -915,6 +927,8 @@ StringRef MappingTraits<ELFYAML::Symbol><br>
return "Index and Section cannot both be specified for Symbol";<br>
if (Symbol.NameIndex && !Symbol.Name.empty())<br>
return "Name and NameIndex cannot both be specified for Symbol";<br>
+ if (Symbol.StOther && Symbol.Other)<br>
+ return "StOther cannot be specified for Symbol with either Visibility or Other";<br>
return StringRef();<br>
}<br>
<br>
<br>
Modified: llvm/trunk/test/tools/yaml2obj/elf-symbol-stother.yaml<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/yaml2obj/elf-symbol-stother.yaml?rev=370032&r1=370031&r2=370032&view=diff" rel="noreferrer" target="_blank">
http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/yaml2obj/elf-symbol-stother.yaml?rev=370032&r1=370031&r2=370032&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/tools/yaml2obj/elf-symbol-stother.yaml (original)<br>
+++ llvm/trunk/test/tools/yaml2obj/elf-symbol-stother.yaml Tue Aug 27 02:58:39 2019<br>
@@ -77,3 +77,32 @@ Symbols:<br>
StOther: 4<br>
- Name: bar<br>
StOther: 0xff<br>
+<br>
+## Check we can't set StOther for a symbol if Visibility or Other is also specified.<br>
+<br>
+# RUN: not yaml2obj --docnum=5 2>&1 %s | FileCheck %s --check-prefix=ERR2<br>
+# RUN: not yaml2obj --docnum=6 2>&1 %s | FileCheck %s --check-prefix=ERR2<br>
+<br>
+# ERR2: error: StOther cannot be specified for Symbol with either Visibility or Other<br>
+<br>
+--- !ELF<br>
+FileHeader:<br>
+ Class: ELFCLASS32<br>
+ Data: ELFDATA2LSB<br>
+ Type: ET_REL<br>
+ Machine: EM_MIPS<br>
+Symbols:<br>
+ - Name: foo<br>
+ StOther: 0<br>
+ Other: [ STO_MIPS_OPTIONAL ]<br>
+<br>
+--- !ELF<br>
+FileHeader:<br>
+ Class: ELFCLASS32<br>
+ Data: ELFDATA2LSB<br>
+ Type: ET_REL<br>
+ Machine: EM_MIPS<br>
+Symbols:<br>
+ - Name: foo<br>
+ StOther: 0<br>
+ Visibility: STV_DEFAULT<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote>
</div>
</div>
</div>
</div>
</div>
</blockquote>
</div>
</div>
</div>
</div>
</blockquote></div>