<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Nov 26, 2013 at 4:01 PM, Sean Silva <span dir="ltr"><<a href="mailto:silvas@purdue.edu" target="_blank">silvas@purdue.edu</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="im"><span style="font-size:13px;font-family:arial,sans-serif">+#define TEST_SECTION(testname, arg, expect) \</span><br style="font-size:13px;font-family:arial,sans-serif">
<span style="font-size:13px;font-family:arial,sans-serif">+ TEST_F(WinLinkParserTest, testname) { \</span><br style="font-size:13px;font-family:arial,sans-serif">
<span style="font-size:13px;font-family:arial,sans-serif">+ EXPECT_TRUE(parse("link.exe", "/section:.text," arg, "a.obj", nullptr)); \</span><br style="font-size:13px;font-family:arial,sans-serif">
<span style="font-size:13px;font-family:arial,sans-serif">+ llvm::Optional<uint32_t> val = _context.getSectionAttributes(</span><span style="font-size:13px;font-family:arial,sans-serif">".text"); \</span><br style="font-size:13px;font-family:arial,sans-serif">
<span style="font-size:13px;font-family:arial,sans-serif">+ EXPECT_TRUE(val.hasValue()); \</span><br style="font-size:13px;font-family:arial,sans-serif">
<span style="font-size:13px;font-family:arial,sans-serif">+ EXPECT_EQ(expect, *val); \</span><br style="font-size:13px;font-family:arial,sans-serif">
<span style="font-size:13px;font-family:arial,sans-serif">+ }</span><br style="font-size:13px;font-family:arial,sans-serif"><span style="font-size:13px;font-family:arial,sans-serif">+</span><br style="font-size:13px;font-family:arial,sans-serif">
<span style="font-size:13px;font-family:arial,sans-serif">+TEST_SECTION(SectionD, "d", llvm::COFF::IMAGE_SCN_MEM_</span><span style="font-size:13px;font-family:arial,sans-serif">DISCARDABLE);</span><br style="font-size:13px;font-family:arial,sans-serif">
<span style="font-size:13px;font-family:arial,sans-serif">+TEST_SECTION(SectionE, "e", llvm::COFF::IMAGE_SCN_MEM_</span><span style="font-size:13px;font-family:arial,sans-serif">EXECUTE);</span><br style="font-size:13px;font-family:arial,sans-serif">
<span style="font-size:13px;font-family:arial,sans-serif">+TEST_SECTION(SectionK, "k", llvm::COFF::IMAGE_SCN_MEM_NOT_</span><span style="font-size:13px;font-family:arial,sans-serif">CACHED);</span><br style="font-size:13px;font-family:arial,sans-serif">
<span style="font-size:13px;font-family:arial,sans-serif">+TEST_SECTION(SectionP, "p", llvm::COFF::IMAGE_SCN_MEM_NOT_</span><span style="font-size:13px;font-family:arial,sans-serif">PAGED);</span><br style="font-size:13px;font-family:arial,sans-serif">
<span style="font-size:13px;font-family:arial,sans-serif">+TEST_SECTION(SectionR, "r", llvm::COFF::IMAGE_SCN_MEM_</span><span style="font-size:13px;font-family:arial,sans-serif">READ);</span><br style="font-size:13px;font-family:arial,sans-serif">
<span style="font-size:13px;font-family:arial,sans-serif">+TEST_SECTION(SectionS, "s", llvm::COFF::IMAGE_SCN_MEM_</span><span style="font-size:13px;font-family:arial,sans-serif">SHARED);</span><br style="font-size:13px;font-family:arial,sans-serif">
<span style="font-size:13px;font-family:arial,sans-serif">+TEST_SECTION(SectionW, "w", llvm::COFF::IMAGE_SCN_MEM_</span><span style="font-size:13px;font-family:arial,sans-serif">WRITE);</span><br>
<div><span style="font-size:13px;font-family:arial,sans-serif"><br></span></div></div><div><span style="font-size:13px;font-family:arial,sans-serif">Is there a better way to test this? It seems like all it's doing is checking is whether you typed (pasted?) the same thing in two different files; this same sort of test doesn't seem like it would have caught the bug that you are fixing in this commit.</span></div>
</div></blockquote><div><br></div><div>It should eventually be tested using llvm-readobj by checking the section attributes. The problem is that the parser is already implemented in the driver but the writer is not. So we can write tests only for the drivers, which ended up tests like the above.</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><span class="HOEnZb"><font color="#888888"><div><span style="font-size:13px;font-family:arial,sans-serif"></span></div>
<div><span style="font-size:13px;font-family:arial,sans-serif">-- Sean Silva</span></div></font></span></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra">
<br><br><div class="gmail_quote">On Tue, Nov 26, 2013 at 12:57 PM, Rui Ueyama <span dir="ltr"><<a href="mailto:ruiu@google.com" target="_blank">ruiu@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Author: ruiu<br>
Date: Tue Nov 26 11:57:05 2013<br>
New Revision: 195774<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=195774&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=195774&view=rev</a><br>
Log:<br>
[PECOFF] Fix parameter mapping for /section.<br>
<br>
The current mapping for /section one character options is really bogus.<br>
Map to the correct flags.<br>
<br>
Modified:<br>
lld/trunk/lib/Driver/WinLinkDriver.cpp<br>
lld/trunk/unittests/DriverTests/WinLinkDriverTest.cpp<br>
<br>
Modified: lld/trunk/lib/Driver/WinLinkDriver.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/lib/Driver/WinLinkDriver.cpp?rev=195774&r1=195773&r2=195774&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/lib/Driver/WinLinkDriver.cpp?rev=195774&r1=195773&r2=195774&view=diff</a><br>
==============================================================================<br>
--- lld/trunk/lib/Driver/WinLinkDriver.cpp (original)<br>
+++ lld/trunk/lib/Driver/WinLinkDriver.cpp Tue Nov 26 11:57:05 2013<br>
@@ -156,7 +156,7 @@ llvm::COFF::MachineTypes stringToMachine<br>
//<br>
// /section option is to set non-default bits in the Characteristics fields of<br>
// the section header. D, E, K, P, R, S, and W represent discardable,<br>
-// not_cachable, not_pageable, shared, execute, read, and write bits,<br>
+// execute, not_cachable, not_pageable, read, shared, and write bits,<br>
// respectively. You can specify multiple flags in one /section option.<br>
//<br>
// If the flag starts with "!", the flags represent a mask that should be turned<br>
@@ -185,11 +185,11 @@ bool parseSection(StringRef option, std:<br>
attribs |= flag; \<br>
break<br>
CASE('d', llvm::COFF::IMAGE_SCN_MEM_DISCARDABLE);<br>
- CASE('e', llvm::COFF::IMAGE_SCN_MEM_NOT_CACHED);<br>
- CASE('k', llvm::COFF::IMAGE_SCN_MEM_NOT_PAGED);<br>
- CASE('p', llvm::COFF::IMAGE_SCN_MEM_SHARED);<br>
- CASE('r', llvm::COFF::IMAGE_SCN_MEM_EXECUTE);<br>
- CASE('s', llvm::COFF::IMAGE_SCN_MEM_READ);<br>
+ CASE('e', llvm::COFF::IMAGE_SCN_MEM_EXECUTE);<br>
+ CASE('k', llvm::COFF::IMAGE_SCN_MEM_NOT_CACHED);<br>
+ CASE('p', llvm::COFF::IMAGE_SCN_MEM_NOT_PAGED);<br>
+ CASE('r', llvm::COFF::IMAGE_SCN_MEM_READ);<br>
+ CASE('s', llvm::COFF::IMAGE_SCN_MEM_SHARED);<br>
CASE('w', llvm::COFF::IMAGE_SCN_MEM_WRITE);<br>
#undef CASE<br>
default:<br>
<br>
Modified: lld/trunk/unittests/DriverTests/WinLinkDriverTest.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/unittests/DriverTests/WinLinkDriverTest.cpp?rev=195774&r1=195773&r2=195774&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/unittests/DriverTests/WinLinkDriverTest.cpp?rev=195774&r1=195773&r2=195774&view=diff</a><br>
==============================================================================<br>
--- lld/trunk/unittests/DriverTests/WinLinkDriverTest.cpp (original)<br>
+++ lld/trunk/unittests/DriverTests/WinLinkDriverTest.cpp Tue Nov 26 11:57:05 2013<br>
@@ -236,6 +236,36 @@ TEST_F(WinLinkParserTest, SectionAlignme<br>
EXPECT_EQ(8192U, _context.getSectionDefaultAlignment());<br>
}<br>
<br>
+TEST_F(WinLinkParserTest, InvalidAlignment) {<br>
+ EXPECT_FALSE(parse("link.exe", "/align:1000", "a.obj", nullptr));<br>
+ EXPECT_EQ("Section alignment must be a power of 2, but got 1000\n",<br>
+ errorMessage());<br>
+}<br>
+<br>
+TEST_F(WinLinkParserTest, Include) {<br>
+ EXPECT_TRUE(parse("link.exe", "/include:foo", "a.out", nullptr));<br>
+ auto symbols = _context.initialUndefinedSymbols();<br>
+ EXPECT_FALSE(symbols.empty());<br>
+ EXPECT_EQ("foo", symbols[0]);<br>
+}<br>
+<br>
+TEST_F(WinLinkParserTest, Merge) {<br>
+ EXPECT_TRUE(parse("link.exe", "/merge:.foo=.bar", "/merge:.bar=.baz",<br>
+ "a.out", nullptr));<br>
+ EXPECT_EQ(".baz", _context.getFinalSectionName(".foo"));<br>
+ EXPECT_EQ(".baz", _context.getFinalSectionName(".bar"));<br>
+ EXPECT_EQ(".abc", _context.getFinalSectionName(".abc"));<br>
+}<br>
+<br>
+TEST_F(WinLinkParserTest, Merge_Circular) {<br>
+ EXPECT_FALSE(parse("link.exe", "/merge:.foo=.bar", "/merge:.bar=.foo",<br>
+ "a.out", nullptr));<br>
+}<br>
+<br>
+//<br>
+// Tests for /section<br>
+//<br>
+<br>
TEST_F(WinLinkParserTest, Section) {<br>
EXPECT_TRUE(parse("link.exe", "/section:.teXT,dekpRSW", "a.obj", nullptr));<br>
uint32_t expect = llvm::COFF::IMAGE_SCN_MEM_DISCARDABLE |<br>
@@ -267,31 +297,23 @@ TEST_F(WinLinkParserTest, SectionNegativ<br>
EXPECT_EQ(expect, _context.getSectionAttributeMask(".teXT"));<br>
}<br>
<br>
-TEST_F(WinLinkParserTest, InvalidAlignment) {<br>
- EXPECT_FALSE(parse("link.exe", "/align:1000", "a.obj", nullptr));<br>
- EXPECT_EQ("Section alignment must be a power of 2, but got 1000\n",<br>
- errorMessage());<br>
-}<br>
-<br>
-TEST_F(WinLinkParserTest, Include) {<br>
- EXPECT_TRUE(parse("link.exe", "/include:foo", "a.out", nullptr));<br>
- auto symbols = _context.initialUndefinedSymbols();<br>
- EXPECT_FALSE(symbols.empty());<br>
- EXPECT_EQ("foo", symbols[0]);<br>
-}<br>
-<br>
-TEST_F(WinLinkParserTest, Merge) {<br>
- EXPECT_TRUE(parse("link.exe", "/merge:.foo=.bar", "/merge:.bar=.baz",<br>
- "a.out", nullptr));<br>
- EXPECT_EQ(".baz", _context.getFinalSectionName(".foo"));<br>
- EXPECT_EQ(".baz", _context.getFinalSectionName(".bar"));<br>
- EXPECT_EQ(".abc", _context.getFinalSectionName(".abc"));<br>
-}<br>
+#define TEST_SECTION(testname, arg, expect) \<br>
+ TEST_F(WinLinkParserTest, testname) { \<br>
+ EXPECT_TRUE(parse("link.exe", "/section:.text," arg, "a.obj", nullptr)); \<br>
+ llvm::Optional<uint32_t> val = _context.getSectionAttributes(".text"); \<br>
+ EXPECT_TRUE(val.hasValue()); \<br>
+ EXPECT_EQ(expect, *val); \<br>
+ }<br>
+<br>
+TEST_SECTION(SectionD, "d", llvm::COFF::IMAGE_SCN_MEM_DISCARDABLE);<br>
+TEST_SECTION(SectionE, "e", llvm::COFF::IMAGE_SCN_MEM_EXECUTE);<br>
+TEST_SECTION(SectionK, "k", llvm::COFF::IMAGE_SCN_MEM_NOT_CACHED);<br>
+TEST_SECTION(SectionP, "p", llvm::COFF::IMAGE_SCN_MEM_NOT_PAGED);<br>
+TEST_SECTION(SectionR, "r", llvm::COFF::IMAGE_SCN_MEM_READ);<br>
+TEST_SECTION(SectionS, "s", llvm::COFF::IMAGE_SCN_MEM_SHARED);<br>
+TEST_SECTION(SectionW, "w", llvm::COFF::IMAGE_SCN_MEM_WRITE);<br>
<br>
-TEST_F(WinLinkParserTest, Merge_Circular) {<br>
- EXPECT_FALSE(parse("link.exe", "/merge:.foo=.bar", "/merge:.bar=.foo",<br>
- "a.out", nullptr));<br>
-}<br>
+#undef TEST_SECTION<br>
<br>
//<br>
// Tests for /defaultlib and /nodefaultlib.<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div></div>