<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman",serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
span.EmailStyle17
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:#1F497D;}
.MsoChpDefault
{mso-style-type:export-only;
font-family:"Calibri",sans-serif;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal"><a name="_MailEndCompose"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">I’ve added Elizabeth, who is the original patch author. Hopefully she can help out here. Additionally, Eli did the original review,
so hopefully he can chime in as well.<o:p></o:p></span></a></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"><br>
I believe the necessity for this warning came out of the discussion on fixing the section behavior. The issue here is that redeclaring with a different ‘section’ can cause some pretty nasty issues, since it isn’t terribly clear what happens if the variable
is used in the meantime.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">There is 1 change that I can think of that Elizabeth and I discussed, which was to only warn in the case where there was a USAGE between these two redeclarations.
I’m not sure that will allow na_cl to compile, however it is my belief that if there IS a usage between link.h:64 and nacl_bootstrap.c:434, that this is a bug in nacl that is simply being uncovered thanks to this new warning.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">Is there a good/reasonable reason the source of nacl wants to redeclare this with a different ‘section’?<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">From:</span></b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif"> thakis@google.com [mailto:thakis@google.com]
<b>On Behalf Of </b>Nico Weber<br>
<b>Sent:</b> Wednesday, October 4, 2017 12:59 PM<br>
<b>To:</b> Keane, Erich <erich.keane@intel.com><br>
<b>Cc:</b> cfe-commits <cfe-commits@lists.llvm.org><br>
<b>Subject:</b> Re: r314262 - Emit section information for extern variables.<o:p></o:p></span></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal">Hi Erich,<o:p></o:p></p>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">this breaks existing code. NaCl does this:<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<div>
<p class="MsoNormal">#include <link.h><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal">struct r_debug _r_debug __attribute__((nocommon, section(".r_debug")));<o:p></o:p></p>
</div>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">(There's a lengthy-ish comment for why in <a href="https://cs.chromium.org/chromium/src/native_client/src/trusted/service_runtime/linux/nacl_bootstrap.c?q=nacl_bootstrap.c&sq=package:chromium&dr&l=424">
https://cs.chromium.org/chromium/src/native_client/src/trusted/service_runtime/linux/nacl_bootstrap.c?q=nacl_bootstrap.c&sq=package:chromium&dr&l=424</a>)<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">link.h in e.g. the debian jessie sysroot says:<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">extern struct r_debug _r_debug;<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">After this change, clang complains:<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<div>
<p class="MsoNormal">../../native_client/src/trusted/service_runtime/linux/nacl_bootstrap.c:434:16: error: section attribute is specified on redeclared variable [-Werror,-Wsection]<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">struct r_debug _r_debug __attribute__((nocommon, section(".r_debug")));<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"> ^<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">../../build/linux/debian_jessie_amd64-sysroot/usr/include/link.h:67:23: note: previous declaration is here<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">extern struct r_debug _r_debug;<o:p></o:p></p>
</div>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">This code used to work in clang, and continues to work in gcc. So this patch probably isn't quite the right approach. Ideas?<o:p></o:p></p>
</div>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal">On Tue, Sep 26, 2017 at 7:42 PM, Erich Keane via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<o:p></o:p></p>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<p class="MsoNormal">Author: erichkeane<br>
Date: Tue Sep 26 16:42:34 2017<br>
New Revision: 314262<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=314262&view=rev" target="_blank">
http://llvm.org/viewvc/llvm-project?rev=314262&view=rev</a><br>
Log:<br>
Emit section information for extern variables.<br>
<br>
Currently, if _attribute_((section())) is used for extern variables,<br>
section information is not emitted in generated IR when the variables are used.<br>
This is expected since sections are not generated for external linkage objects.<br>
However NiosII requires this information as it uses special GP-relative accesses<br>
for any objects that use attribute section (.sdata). GCC keeps this attribute in<br>
middle-end.<br>
<br>
This change emits the section information for all targets.<br>
<br>
Patch By: Elizabeth Andrews<br>
<br>
Differential Revision:<a href="https://reviews.llvm.org/D36487" target="_blank">https://reviews.llvm.org/D36487</a><br>
<br>
<br>
Modified:<br>
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
cfe/trunk/lib/CodeGen/CodeGenModule.cpp<br>
cfe/trunk/lib/Sema/SemaDecl.cpp<br>
cfe/trunk/test/Sema/attr-section.c<br>
<br>
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=314262&r1=314261&r2=314262&view=diff" target="_blank">
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=314262&r1=314261&r2=314262&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)<br>
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Sep 26 16:42:34 2017<br>
@@ -2620,6 +2620,8 @@ def err_attribute_section_invalid_for_ta<br>
"argument to 'section' attribute is not valid for this target: %0">;<br>
def warn_mismatched_section : Warning<<br>
"section does not match previous declaration">, InGroup<Section>;<br>
+def warn_attribute_section_on_redeclaration : Warning<<br>
+ "section attribute is specified on redeclared variable">, InGroup<Section>;<br>
<br>
def err_anonymous_property: Error<<br>
"anonymous property is not supported">;<br>
<br>
Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=314262&r1=314261&r2=314262&view=diff" target="_blank">
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=314262&r1=314261&r2=314262&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)<br>
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Tue Sep 26 16:42:34 2017<br>
@@ -2432,6 +2432,12 @@ CodeGenModule::GetOrCreateLLVMGlobal(Str<br>
EmitGlobalVarDefinition(D);<br>
}<br>
<br>
+ // Emit section information for extern variables.<br>
+ if (D->hasExternalStorage()) {<br>
+ if (const SectionAttr *SA = D->getAttr<SectionAttr>())<br>
+ GV->setSection(SA->getName());<br>
+ }<br>
+<br>
// Handle XCore specific ABI requirements.<br>
if (getTriple().getArch() == llvm::Triple::xcore &&<br>
D->getLanguageLinkage() == CLanguageLinkage &&<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaDecl.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=314262&r1=314261&r2=314262&view=diff" target="_blank">
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=314262&r1=314261&r2=314262&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue Sep 26 16:42:34 2017<br>
@@ -2607,6 +2607,16 @@ void Sema::mergeDeclAttributes(NamedDecl<br>
}<br>
}<br>
<br>
+ // This redeclaration adds a section attribute.<br>
+ if (New->hasAttr<SectionAttr>() && !Old->hasAttr<SectionAttr>()) {<br>
+ if (auto *VD = dyn_cast<VarDecl>(New)) {<br>
+ if (VD->isThisDeclarationADefinition() != VarDecl::Definition) {<br>
+ Diag(New->getLocation(), diag::warn_attribute_section_on_redeclaration);<br>
+ Diag(Old->getLocation(), diag::note_previous_declaration);<br>
+ }<br>
+ }<br>
+ }<br>
+<br>
if (!Old->hasAttrs())<br>
return;<br>
<br>
<br>
Modified: cfe/trunk/test/Sema/attr-section.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/attr-section.c?rev=314262&r1=314261&r2=314262&view=diff" target="_blank">
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/attr-section.c?rev=314262&r1=314261&r2=314262&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/Sema/attr-section.c (original)<br>
+++ cfe/trunk/test/Sema/attr-section.c Tue Sep 26 16:42:34 2017<br>
@@ -19,3 +19,7 @@ void __attribute__((section("foo,zed")))<br>
void __attribute__((section("bar,zed"))) test2(void) {} // expected-warning {{section does not match previous declaration}}<br>
<br>
enum __attribute__((section("NEAR,x"))) e { one }; // expected-error {{'section' attribute only applies to functions, methods, properties, and global variables}}<br>
+<br>
+extern int a; // expected-note {{previous declaration is here}}<br>
+int *b = &a;<br>
+extern int a __attribute__((section("foo,zed"))); // expected-warning {{section attribute is specified on redeclared variable}}<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><o:p></o:p></p>
</blockquote>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</div>
</body>
</html>